Add index on file_store for pages_deployments table
What does this MR do?
This MR adds an index on the file_store
column for the pages_deployments
table. This needs was discussed as part of the code review for a rake task, which will be executing the following query:
PagesDeployment.where(file_store: ...).find_each(batch_size: batch_size)
Migration Up/Down
etienne@dell ~/src/gdk/gitlab(325285-tmp-index-to-file-store ✗) rails db:migrate:up VERSION=20210401134157
== 20210401134157 AddIndexToPagesDeployments: migrating =======================
-- transaction_open?()
-> 0.0000s
-- index_exists?(:pages_deployments, [:file_store, :id], {:name=>"index_pages_deployments_on_file_store_and_id", :algorithm=>:concurrently})
-> 0.0016s
-- execute("SET statement_timeout TO 0")
-> 0.0004s
-- add_index(:pages_deployments, [:file_store, :id], {:name=>"index_pages_deployments_on_file_store_and_id", :algorithm=>:concurrently})
-> 0.0308s
-- execute("RESET ALL")
-> 0.0005s
== 20210401134157 AddIndexToPagesDeployments: migrated (0.0341s) ==============
etienne@dell ~/src/gdk/gitlab(325285-tmp-index-to-file-store ✗) rails db:migrate:down VERSION=20210401134157
== 20210401134157 AddIndexToPagesDeployments: reverting =======================
-- transaction_open?()
-> 0.0000s
-- indexes(:pages_deployments)
-> 0.0020s
-- execute("SET statement_timeout TO 0")
-> 0.0005s
-- remove_index(:pages_deployments, {:algorithm=>:concurrently, :name=>"index_pages_deployments_on_file_store_and_id"})
-> 0.0343s
-- execute("RESET ALL")
-> 0.0007s
== 20210401134157 AddIndexToPagesDeployments: reverted (0.0383s) ==============
Explain plans
Before index creation
EXPLAIN SELECT id FROM pages_deployments WHERE pages_deployments.file_store = 2
AND id >= 12 AND id <= 47491; # 1000 pages_deployments records in prod
Index Scan using pages_deployments_pkey on public.pages_deployments (cost=0.42..656.97 rows=1001 width=8) (actual time=0.031..129.598 rows=1001 loops=1)
Index Cond: ((pages_deployments.id >= 12) AND (pages_deployments.id <= 47491))
Filter: (pages_deployments.file_store = 2)
Rows Removed by Filter: 0
Buffers: shared hit=188 read=117
I/O Timings: read=126.588
Time: 130.399 ms
- planning: 0.658 ms
- execution: 129.741 ms
- I/O read: 126.588 ms
- I/O write: N/A
Shared buffers:
- hits: 188 (~1.50 MiB) from the buffer pool
- reads: 117 (~936.00 KiB) from the OS file cache, including disk I/O
- dirtied: 0
- writes: 0
After index creation
EXEC CREATE INDEX index_pages_deployments_on_file_store_and_id
ON pages_deployments USING btree (file_store, id);
EXPLAIN SELECT id FROM pages_deployments
WHERE pages_deployments.file_store = 2 AND id >= 12 and id <= 47491;
Index Only Scan using index_pages_deployments_on_file_store_and_id on public.pages_deployments (cost=0.42..62.23 rows=1001 width=8) (actual time=0.224..0.638 rows=1001 loops=1)
Index Cond: ((pages_deployments.file_store = 2) AND (pages_deployments.id >= 12) AND (pages_deployments.id <= 47491))
Heap Fetches: 52
Buffers: shared hit=40 read=6
I/O Timings: read=0.208
Time: 1.571 ms
- planning: 0.865 ms
- execution: 0.706 ms
- I/O read: 0.208 ms
- I/O write: N/A
Shared buffers:
- hits: 40 (~320.00 KiB) from the buffer pool
- reads: 6 (~48.00 KiB) from the OS file cache, including disk I/O
- dirtied: 0
- writes: 0
Does this MR meet the acceptance criteria?
Conformity
-
Does this MR need a changelog?-
I have included a changelog entry. -
I have not included a changelog entry because _____.
-
-
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec
-
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team
Related to #325285 (closed)
Merge request reports
Activity
changed milestone to %13.11
added databasereview pending label
added database label
2 Warnings Please add a merge request type to this merge request. You've made some app changes, but didn't add any tests.
That's OK as long as you're refactoring existing code,
but please consider adding any of the tooling, ~"tooling::pipelines", ~"tooling::workflow", documentation, QA labels.Reviewer roulette
Changes that require review have been detected! A merge request is normally reviewed by both a reviewer and a maintainer in its primary category (e.g. frontend or backend), and by a maintainer in all other categories.
To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.
To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.
Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.
Category Reviewer Maintainer database Alina Mihaila ( @alinamihaila
) (UTC+3, same timezone as@ebaque
)Tiger Watson ( @tigerwnz
) (UTC+12, 9 hours ahead of@ebaque
)~migration No reviewer available No maintainer available If needed, you can retry the
danger-review
job that generated this comment.Generated by
DangerEdited by 🤖 GitLab Bot 🤖added 1 commit
- 5c029bec - Added migration for temporary index on pages deployments
mentioned in merge request !57120 (merged)
- Resolved by Tiger Watson
@furkanayhan
Can you please review this MR?Edited by Etienne Baqué
requested review from @furkanayhan
added workflowin review label and removed workflowin dev label
- Resolved by Tiger Watson
removed review request for @furkanayhan
added 1 commit
- 25bc2e66 - Added migration for temporary index on pages deployments
added 1 commit
- fda016ef - Added migration for temporary index on pages deployments
requested review from @furkanayhan
added databasereviewed label and removed databasereview pending label
assigned to @tigerwnz
requested review from @tigerwnz and removed review request for @furkanayhan
added databaseapproved label and removed databasereviewed label
enabled an automatic merge when the pipeline for f5f4f7c8 succeeds
mentioned in commit 5d3d7a5e
added workflowstaging label and removed workflowin review label
added workflowcanary label and removed workflowstaging label
added workflowproduction label and removed workflowcanary label
added releasedcandidate label