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_featurewas not eager loaded, a lazySELECTcall is made to the database. - This
SELECTcall 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