Skip to content
Snippets Groups Projects

Add index on file_store for pages_deployments table

Merged Etienne Baqué requested to merge 325285-tmp-index-to-file-store into master
All threads resolved!

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

Availability and Testing

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)

Edited by Etienne Baqué

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Furkan Ayhan removed review request for @furkanayhan

    removed review request for @furkanayhan

  • Etienne Baqué changed the description

    changed the description

  • Etienne Baqué added 1 commit

    added 1 commit

    • 25bc2e66 - Added migration for temporary index on pages deployments

    Compare with previous version

  • Etienne Baqué added 1 commit

    added 1 commit

    • fda016ef - Added migration for temporary index on pages deployments

    Compare with previous version

  • Etienne Baqué changed the description

    changed the description

  • Etienne Baqué requested review from @furkanayhan

    requested review from @furkanayhan

  • Furkan Ayhan approved this merge request

    approved this merge request

  • added databasereviewed label and removed databasereview pending label

  • assigned to @tigerwnz

  • Furkan Ayhan requested review from @tigerwnz and removed review request for @furkanayhan

    requested review from @tigerwnz and removed review request for @furkanayhan

  • Tiger Watson approved this merge request

    approved this merge request

  • added databaseapproved label and removed databasereviewed label

  • Tiger Watson resolved all threads

    resolved all threads

  • Tiger Watson enabled an automatic merge when the pipeline for f5f4f7c8 succeeds

    enabled an automatic merge when the pipeline for f5f4f7c8 succeeds

  • merged

  • Tiger Watson mentioned in commit 5d3d7a5e

    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

  • Please register or sign in to reply
    Loading