Remove N+1 SQL query loading project feature in dashboard
Projects that have a pipeline may need to check whether the user has
permission to read the build (can?(current_user, :read_build, project)
), which requires checking the project_features
table.
This would cause an N+1 SQL query for each project.
This change also has a beneficial side effect that may avoid a race condition. When a user deletes a project, the project is queued for deletion and the user is redirected back to the dashboard page. However, the following may happen:
- The dashboard page may load this deleted project in the list of 20 projects.
- The view will load the project pipeline status from the cache and attempt to show each project.
- When the view encounters the deleted project, it calls
can?(current_user, :read_build, project)
to determine whether to display the pipeline status. - Sidekiq deletes the project from the database.
- However, since the deleted project is still loaded in memory, it will
attempt to call
project.project_feature.access_level
. - Since
project_feature
was not eager loaded, a lazySELECT
call is made to the database. - This
SELECT
call returns nothing, and the user sees a 500 error.
By eager loading project_feature
, we can ensure that we have a
consistent view and avoid records from being deleted later.
Note that the views are cached, so it's possible that eager loads are unnecessary from a performance standpoint.
Edited by Stan Hu