Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove includes course sites on provider courses page #4782

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

tomas-stefano
Copy link
Contributor

Context

There is a performance issue for some providers which has many courses to display in the courses list page.

The line in the controller in the culprit:

def provider
       @provider ||= recruitment_cycle.providers
                                    .includes(courses: %i[sites site_statuses enrichments provider
])
                                      .find_by!(provider_code: params[:provider_code])
     end

This is generating a rails default query where the main nested loop are being higher and making the query takes more than 500ms on average sometimes reaching more than x seconds (it varies).

Explain

Taking a look on the query that the includes generates:

EXPLAIN ANALYZE SELECT
    "course_site".*
FROM
    "course_site"
INNER JOIN
    "site"
    ON "site"."id" = "course_site"."site_id"
INNER JOIN
    "course_site" AS "site_statuses"
    ON "site_statuses"."site_id" = "site"."id"
WHERE
    "site_statuses"."status" IN ('N', 'R')
    AND "course_site"."course_id" IN (
       # a bunch of course ids
    );

If you run the query above you will find some interesting info.

If I can summarise in a statement of my interpretation is:

Somehow that it requires more deep investigations Rails default includes generates the query above and the query above does not use the composite index of course_site.course_id and course_site.site_id.

The details of the explain

  1. The Nested Loop join is iterating over the results of the first part of the join (the outer loop), which is the result of scanning course_site.

For each row in this result, the system performs an inner scan (the inner
loop) to match records from site_statuses and PK_ucas_campus. This can
result in a high number of rows if the tables being joined are large or
if there's a large number of qualifying rows in the tables.

  1. Row Count (rows=5397 / rows=483361) too high!

Rows in the actual result suggests that the query planner was
likely underestimating the number of rows returned from the join.

This can occur if there is high cardinality (many different combinations of records) between the tables
involved in the join.

The high row count suggests that each course_site row is joining with multiple rows in site_statuses.

If each course_site record matches many site_statuses rows, the number of output rows can grow exponentially.

  1. Repeated Access to Site (5775 loops):

For every row returned by the first index scan (course_site), there are
5775 iterations (loops) over the site data and site_statuses,
which increases the overall row count.

Changes proposed in this pull request

Removal the includes for now. Don't merge without testing on the review app

Guidance to review

  1. Visit courses list page for providers with many courses

This is generating a rails default query where the main nested loop
are being higher and making the query takes more than 500ms on average
sometimes reaching more than 2 seconds.

Taking a look on the query:

```
EXPLAIN ANALYZE SELECT
    "course_site".*
FROM
    "course_site"
INNER JOIN
    "site"
    ON "site"."id" = "course_site"."site_id"
INNER JOIN
    "course_site" AS "site_statuses"
    ON "site_statuses"."site_id" = "site"."id"
WHERE
    "site_statuses"."status" IN ('N', 'R')
    AND "course_site"."course_id" IN (
    );
```

If you run the query above you will find some interesting
info.

If I can summarise in a statement of my interpretation is:

Somehow that it requires more deep investigations Rails default
includes generates the query above and the query above does not
use the composite index of course_site.course_id and
course_site.site_id.

The details of the explain

1. The Nested Loop join is iterating over the results of the first part of
the join (the outer loop), which is the result of scanning course_site.

  For each row in this result, the system performs an inner scan (the inner
  loop) to match records from site_statuses and PK_ucas_campus. This can
  result in a high number of rows if the tables being joined are large or
  if there's a large number of qualifying rows in the tables.

2. Row Count (rows=5397 / rows=483361) too high!

  Rows in the actual result suggests that the query planner was
  likely underestimating the number of rows returned from the join.

  This can occur if there is high cardinality (many different combinations of records) between the tables
involved in the join.

   The high row count suggests that each course_site row is joining with
   multiple rows in site_statuses.

   If each course_site record matches many site_statuses rows, the number
   of output rows can grow exponentially.

3. Repeated Access to Site (5775 loops):

  For every row returned by the first index scan (course_site), there are
  5775 iterations (loops) over the site data and site_statuses,
  which increases the overall row count.
@tomas-stefano tomas-stefano added the deploy A Review App will be created for PRs with this label label Dec 17, 2024
@tomas-stefano tomas-stefano requested a review from a team as a code owner December 17, 2024 16:44
@tomas-stefano tomas-stefano merged commit 2d35773 into main Dec 18, 2024
38 checks passed
@tomas-stefano tomas-stefano deleted the td/fix-performance-issue-on-courses-page branch December 18, 2024 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy A Review App will be created for PRs with this label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants