Skip to content

Reduce number of database queries to get pages and conditions#2274

Merged
stephencdaly merged 2 commits into
mainfrom
reduce-queries-loading-pages-and-conditions
Oct 15, 2025
Merged

Reduce number of database queries to get pages and conditions#2274
stephencdaly merged 2 commits into
mainfrom
reduce-queries-loading-pages-and-conditions

Conversation

@stephencdaly
Copy link
Copy Markdown
Contributor

What problem does this pull request solve?

Reduce the number of SQL queries to get pages/conditions in a few places to improve performance.

The below images show the load times and number of queries run when adding a condition for a form with 118 pages locally.

Before:

Screenshot 2025-10-09 at 10 46 08

After:

Screenshot 2025-10-09 at 10 45 12

Things to consider when reviewing

  • Ensure that you consider the wider context.
  • Does it work when run on your machine?
  • Is it clear what the code is doing?
  • Do the commit messages explain why the changes were made?
  • Are there all the unit tests needed?
  • Do the end to end tests need updating before these changes will pass?
  • Has all relevant documentation been updated?

@stephencdaly stephencdaly force-pushed the use-page-associations-to-get-routes branch 2 times, most recently from bae0b43 to 2de90a5 Compare October 9, 2025 10:44
Base automatically changed from use-page-associations-to-get-routes to main October 9, 2025 10:55
@stephencdaly stephencdaly force-pushed the reduce-queries-loading-pages-and-conditions branch from 97618fe to a66573e Compare October 9, 2025 11:01
@stephencdaly stephencdaly marked this pull request as ready for review October 9, 2025 11:10
@stephencdaly stephencdaly changed the title Reduce queries loading pages and conditions Reduce number of database queries run to get pages and conditions Oct 9, 2025
@stephencdaly stephencdaly force-pushed the reduce-queries-loading-pages-and-conditions branch from a66573e to 801d819 Compare October 9, 2025 11:13
Comment thread app/models/form.rb Outdated
@stephencdaly stephencdaly force-pushed the reduce-queries-loading-pages-and-conditions branch from 801d819 to 0f7f279 Compare October 9, 2025 12:49
@stephencdaly stephencdaly changed the title Reduce number of database queries run to get pages and conditions Reduce number of database queries to get pages and conditions Oct 9, 2025
Rather than retrieving the conditions per page of a form when we need all conditions for a form, look them up in a single query by joining on the pages table.
@stephencdaly stephencdaly force-pushed the reduce-queries-loading-pages-and-conditions branch 2 times, most recently from ac09163 to dedd9e8 Compare October 14, 2025 14:47
Previously, we would run a SQL query per page in the form to get the
next page, and also to get the conditions for the page.

Use `includes` so that we get the pages and all their conditions by
running fewer queries.

Also accept the `next_page` as an argument to
Page#as_form_document_step as we already know what the next_page is
when we call it from Form#as_form_document so we can avoid an
additional query per page.

Add some additional tests to the request specs so we can be sure that
the form document is updated as expected after updates to pages and
conditions.
@stephencdaly stephencdaly force-pushed the reduce-queries-loading-pages-and-conditions branch from dedd9e8 to 98db76f Compare October 14, 2025 14:57
@sonarqubecloud
Copy link
Copy Markdown

@github-actions
Copy link
Copy Markdown

🎉 A review copy of this PR has been deployed! You can reach it at: https://pr-2274.admin.review.forms.service.gov.uk/

It may take 5 minutes or so for the application to be fully deployed and working. If it still isn't ready
after 5 minutes, there may be something wrong with the ECS task. You will need to go to the integration AWS account
to debug, or otherwise ask an infrastructure person.

For the sign in details and more information, see the review apps wiki page.

@stephencdaly
Copy link
Copy Markdown
Contributor Author

@lfdebrux I've added some more tests to make sure the form_documents are updated as expected after any updates to pages/conditions.

@stephencdaly stephencdaly merged commit 01bc4b7 into main Oct 15, 2025
6 of 7 checks passed
@stephencdaly stephencdaly deleted the reduce-queries-loading-pages-and-conditions branch October 15, 2025 09:06
thomasiles added a commit that referenced this pull request Oct 31, 2025
[Reducing the number of DB queries has made page response times
faster](#2274)

[In Rails 7.2, the default logger started including the number of
database queries in the log](rails/rails#51457).

We already have DB time in the logs. I think it's useful to include the
number of queries as another simple metric.

This is useful for checking performance in local development and I think
probably for production as well. It would enable us to see if the number
of queries is increasing or decreasing over time for specific pages.

Example log entry:

{
  "method": "GET",
  "path": "/forms/1",
  "format": "html",
  "controller": "FormsController",
  "action": "show",
  "status": 200,
  "allocations": 67752,
  "duration": 238.2,
  "view": 23.53,
  "db": 156.87,
  "request_host": "localhost",
  "request_id": "01d5ce3c-7a6c-4d32-9288-cf4350850af3",
  "trace_id": null,
  "form_id": "1",
  "user_ip": null,
  "params": {
    "form_id": "1"
  },
  "queries_count": 26,
  "cached_queries_count": 15
}
thomasiles added a commit that referenced this pull request Oct 31, 2025
[Reducing the number of DB queries has made page response times
faster](#2274)

[In Rails 7.2, the default logger started including the number of
database queries in the log](rails/rails#51457).

We already have DB time in the logs. I think it's useful to include the
number of queries as another simple metric.

This is useful for checking performance in local development and I think
probably for production as well. It would enable us to see if the number
of queries is increasing or decreasing over time for specific pages.

Example log entry:

{
  "method": "GET",
  "path": "/forms/1",
  "format": "html",
  "controller": "FormsController",
  "action": "show",
  "status": 200,
  "allocations": 67752,
  "duration": 238.2,
  "view": 23.53,
  "db": 156.87,
  "request_host": "localhost",
  "request_id": "01d5ce3c-7a6c-4d32-9288-cf4350850af3",
  "trace_id": null,
  "form_id": "1",
  "user_ip": null,
  "params": {
    "form_id": "1"
  },
  "queries_count": 26,
  "cached_queries_count": 15
}
thomasiles added a commit that referenced this pull request Nov 3, 2025
[Reducing the number of DB queries has made page response times
faster](#2274)

[In Rails 7.2, the default logger started including the number of
database queries in the log](rails/rails#51457).

We already have DB time in the logs. I think it's useful to include the
number of queries as another simple metric.

This is useful for checking performance in local development and I think
probably for production as well. It would enable us to see if the number
of queries is increasing or decreasing over time for specific pages.

Example log entry:

{
  "method": "GET",
  "path": "/forms/1",
  "format": "html",
  "controller": "FormsController",
  "action": "show",
  "status": 200,
  "allocations": 67752,
  "duration": 238.2,
  "view": 23.53,
  "db": 156.87,
  "request_host": "localhost",
  "request_id": "01d5ce3c-7a6c-4d32-9288-cf4350850af3",
  "trace_id": null,
  "form_id": "1",
  "user_ip": null,
  "params": {
    "form_id": "1"
  },
  "queries_count": 26,
  "cached_queries_count": 15
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants