Improve query performance to retrieve Job Artifacts, LFS Objects, and Uploads with files stored locally
Summary
The following discussion from !25461 (closed) should be addressed:
-
@ahegyi started a discussion: Fixing data doesn't make much sense without adding constraints on the DB side. (unless it's causing application bugs)
Adding NOT NULL constraints will lock the table while the constraint is checked, which is a big problem for high traffic tables. Currently we don't have a way to do this without negatively affecting gitlab.com's stability.
I'd propose the following:
- Wait until !22808 (merged) is finished.
- Create indices to support the background migration !28832 (merged)
- We don't have too much data to migrate, so we could do the data migration "inline". Or keep the BG migration, since it's the safest approach.
- Add
CHECK CONSTRAINT
to the table asNOT VALID
- We need
structure.sql
for this, since rails doesn't support check constraints. - https://www.postgresql.org/docs/9.4/ddl-constraints.html
- A separate migration can do a final update of the potential NULL values (count should be close to 0) and validate constraint without locking the table (only schema change lock is required). (next milestone)
Reasoning about the indices:
Uncached timings are not too great. Let's add a specialized index to make the lookup fast:
add_concurrent_index :ci_job_artifacts, :id, where: 'file_store IS NULL' add_concurrent_index :lfs_objects, :id, where: 'file_store IS NULL' add_concurrent_index :uploads, :id, where: 'store IS NULL'
TO-DO
-
- Create indices to support the background migration - !28832 (merged) (merged) -
- Add CHECK CONSTRAINT to the table as NOT VALID - !28946 (merged) -
- Add migration to do a final update of the potential NULL values (count should be close to 0) - !29537 (closed) -
- Add migration to validate the constraint without locking the table (only schema change lock is required) - !29537 (closed) -
- Remove the set_file_store
callbacks after setting a default value for these columns
Edited by Douglas Barbosa Alexandre