Skip to content

Fix performance of deleting MR approval_rules

Sashi Kumar Kumaresan requested to merge sk/499633-fix-delete-queries into master

What does this MR do and why?

This MR updates the query to delete approval_merge_request_rules that are created from security policies to follow slow iteration pattern.

Also, ProcessScanResultPolicyWorker does these steps:

  • Delete approval_project_rules filtering by project_id + security_orchestration_policy_configuration_id
  • Delete approval_merge_request_rules filtering by merge_requests.project_id + security_orchestration_policy_configuration_id + merge_requests.state
  • Create approval_project_rules
  • For each open MRs in the project, create approval_merge_request_rule from approval_project_rules

We don't need to delete the approval_merge_request_rules collectively upfront as we are anyway iterating through each of the MRs. So we can move the deletion of approval_merge_request_rules to each individual MRs. This will increase the number of delete queries, but the number of rows deleted will be less as we have a limit on the number of policies.

Migration Output

main: == [advisory_lock_connection] object_id: 134700, pg_backend_pid: 57267
main: == 20241018135537 AddIndexApprovalMergeRequestRulesOnMrIdConfigIdAndId: migrating 
main: -- transaction_open?(nil)
main:    -> 0.0000s
main: -- view_exists?(:postgres_partitions)
main:    -> 0.0996s
main: -- index_exists?(:approval_merge_request_rules, [:merge_request_id, :security_orchestration_policy_configuration_id, :id], {:name=>:idx_approval_merge_request_rules_on_mr_id_config_id_and_id, :algorithm=>:concurrently})
main:    -> 0.0211s
main: -- execute("SET statement_timeout TO 0")
main:    -> 0.0022s
main: -- add_index(:approval_merge_request_rules, [:merge_request_id, :security_orchestration_policy_configuration_id, :id], {:name=>:idx_approval_merge_request_rules_on_mr_id_config_id_and_id, :algorithm=>:concurrently})
main:    -> 0.0080s
main: -- execute("RESET statement_timeout")
main:    -> 0.0015s
main: == 20241018135537 AddIndexApprovalMergeRequestRulesOnMrIdConfigIdAndId: migrated (0.1736s)

Database queries

MergeRequest.delete_approval_rules_for_policy_configuration

DELETE 
FROM
    "approval_merge_request_rules" 
WHERE
    "approval_merge_request_rules"."merge_request_id" = 280889987 
    AND "approval_merge_request_rules"."security_orchestration_policy_configuration_id" = 20380

Query Plan

 ModifyTable on public.approval_merge_request_rules  (cost=0.57..3.59 rows=0 width=0) (actual time=2.169..2.170 rows=0 loops=1)
   Buffers: shared hit=20 read=12 dirtied=4
   I/O Timings: read=1.896 write=0.000
   ->  Index Scan using idx_approval_mr_rules_on_mr_id_config_id_and_id on public.approval_merge_request_rules  (cost=0.57..3.59 rows=1 width=6) (actual time=0.487..0.569 rows=4 loops=1)
         Index Cond: ((approval_merge_request_rules.merge_request_id = 280889987) AND (approval_merge_request_rules.security_orchestration_policy_configuration_id = 20380))
         Buffers: shared hit=3 read=8 dirtied=4
         I/O Timings: read=0.494 write=0.000

Time: 71.138 ms
  - planning: 1.862 ms
  - execution: 69.276 ms
    - I/O read: 1.896 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 20 (~160.00 KiB) from the buffer pool
  - reads: 12 (~96.00 KiB) from the OS file cache, including disk I/O
  - dirtied: 4 (~32.00 KiB)
  - writes: 0

delete_merge_request_rules_for_project

DELETE 
FROM
    "approval_merge_request_rules" 
WHERE
    "approval_merge_request_rules"."id" IN (
        SELECT
            "approval_merge_request_rules"."id" 
        FROM
            "approval_merge_request_rules" 
        INNER JOIN
            "merge_requests" 
                ON "merge_requests"."id" = "approval_merge_request_rules"."merge_request_id" 
        WHERE
            "approval_merge_request_rules"."security_orchestration_policy_configuration_id" = 20380 
            AND "approval_merge_request_rules"."id" >= 142679036 
            AND "approval_merge_request_rules"."id" < 157041552
            AND "merge_requests"."state_id" != 3 
            AND "merge_requests"."target_project_id" = 54739485
    )

Query Plan

 ModifyTable on public.approval_merge_request_rules  (cost=920.06..923.05 rows=0 width=0) (actual time=1376.983..1376.984 rows=0 loops=1)
   Buffers: shared hit=3278 read=1279 dirtied=10
   I/O Timings: read=1362.671 write=0.000
   ->  Nested Loop  (cost=920.06..923.05 rows=1 width=18) (actual time=1376.981..1376.982 rows=0 loops=1)
         Buffers: shared hit=3278 read=1279 dirtied=10
         I/O Timings: read=1362.671 write=0.000
         ->  HashAggregate  (cost=919.49..919.50 rows=1 width=20) (actual time=1376.980..1376.981 rows=0 loops=1)
               Group Key: approval_merge_request_rules_1.id
               Buffers: shared hit=3278 read=1279 dirtied=10
               I/O Timings: read=1362.671 write=0.000
               ->  Nested Loop  (cost=1.14..919.49 rows=1 width=20) (actual time=1376.977..1376.978 rows=0 loops=1)
                     Buffers: shared hit=3278 read=1279 dirtied=10
                     I/O Timings: read=1362.671 write=0.000
                     ->  Index Scan using index_on_merge_requests_for_latest_diffs on public.merge_requests  (cost=0.57..696.13 rows=62 width=10) (actual time=11.027..1357.034 rows=906 loops=1)
                           Index Cond: (merge_requests.target_project_id = 54739485)
                           Filter: (merge_requests.state_id <> 3)
                           Rows Removed by Filter: 0
                           Buffers: shared hit=9 read=921 dirtied=10
                           I/O Timings: read=1351.891 write=0.000
                     ->  Index Scan using idx_approval_mr_rules_on_mr_id_config_id_and_id on public.approval_merge_request_rules approval_merge_request_rules_1  (cost=0.57..3.59 rows=1 width=18) (actual time=0.020..0.020 rows=0 loops=906)
                           Index Cond: ((approval_merge_request_rules_1.merge_request_id = merge_requests.id) AND (approval_merge_request_rules_1.security_orchestration_policy_configuration_id = 20380) AND (approval_merge_request_rules_1.id >= 142679036) AND (approval_merge_request_rules_1.id < 157041552))
                           Buffers: shared hit=3269 read=358
                           I/O Timings: read=10.780 write=0.000
         ->  Index Scan using approval_merge_request_rules_pkey on public.approval_merge_request_rules  (cost=0.57..3.55 rows=1 width=14) (actual time=0.000..0.000 rows=0 loops=0)
               Index Cond: (approval_merge_request_rules.id = approval_merge_request_rules_1.id)
               I/O Timings: read=0.000 write=0.000

Time: 1.382 s
  - planning: 4.928 ms
  - execution: 1.377 s
    - I/O read: 1.363 s
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 3278 (~25.60 MiB) from the buffer pool
  - reads: 1279 (~10.00 MiB) from the OS file cache, including disk I/O
  - dirtied: 10 (~80.00 KiB)
  - writes: 0

delete_merge_request_rules

DELETE 
FROM
    "approval_merge_request_rules" 
WHERE
    "approval_merge_request_rules"."id" IN (
        SELECT
            "approval_merge_request_rules"."id" 
        FROM
            "approval_merge_request_rules" 
        INNER JOIN
            "merge_requests" 
                ON "merge_requests"."id" = "approval_merge_request_rules"."merge_request_id" 
        WHERE
            "approval_merge_request_rules"."security_orchestration_policy_configuration_id" = 20380 
            AND "approval_merge_request_rules"."id" >= 142679036 
            AND "approval_merge_request_rules"."id" < 157041552
            AND "merge_requests"."state_id" != 3 
    )

Query Plan

 ModifyTable on public.approval_merge_request_rules  (cost=66708.18..72450.89 rows=0 width=0) (actual time=3152.409..3152.412 rows=0 loops=1)
   Buffers: shared hit=19162 read=5578 dirtied=7
   I/O Timings: read=3104.512 write=0.000
   ->  Nested Loop  (cost=66708.18..72450.89 rows=1612 width=18) (actual time=3152.408..3152.410 rows=0 loops=1)
         Buffers: shared hit=19162 read=5578 dirtied=7
         I/O Timings: read=3104.512 write=0.000
         ->  HashAggregate  (cost=66707.61..66723.73 rows=1612 width=20) (actual time=3152.408..3152.409 rows=0 loops=1)
               Group Key: approval_merge_request_rules_1.id
               Buffers: shared hit=19162 read=5578 dirtied=7
               I/O Timings: read=3104.512 write=0.000
               ->  Nested Loop  (cost=1.13..66703.58 rows=1612 width=20) (actual time=3152.395..3152.396 rows=0 loops=1)
                     Buffers: shared hit=19162 read=5578 dirtied=7
                     I/O Timings: read=3104.512 write=0.000
                     ->  Index Scan using idx_approval_mr_rules_on_config_id_and_id_and_updated_at on public.approval_merge_request_rules approval_merge_request_rules_1  (cost=0.57..17562.86 rows=14033 width=18) (actual time=2.169..2575.580 rows=5000 loops=1)
                           Index Cond: ((approval_merge_request_rules_1.security_orchestration_policy_configuration_id = 20380) AND (approval_merge_request_rules_1.id >= 142679036) AND (approval_merge_request_rules_1.id < 157041552))
                           Buffers: shared hit=605 read=4135 dirtied=7
                           I/O Timings: read=2558.247 write=0.000
                     ->  Index Scan using idx_merge_requests_on_unmerged_state_id on public.merge_requests  (cost=0.56..3.50 rows=1 width=10) (actual time=0.114..0.114 rows=0 loops=5000)
                           Index Cond: (merge_requests.id = approval_merge_request_rules_1.merge_request_id)
                           Filter: (merge_requests.state_id <> 3)
                           Rows Removed by Filter: 0
                           Buffers: shared hit=18557 read=1443
                           I/O Timings: read=546.265 write=0.000
         ->  Index Scan using approval_merge_request_rules_pkey on public.approval_merge_request_rules  (cost=0.57..3.55 rows=1 width=14) (actual time=0.000..0.000 rows=0 loops=0)
               Index Cond: (approval_merge_request_rules.id = approval_merge_request_rules_1.id)
               I/O Timings: read=0.000 write=0.000

Time: 3.159 s
  - planning: 5.823 ms
  - execution: 3.153 s
    - I/O read: 3.105 s
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 19162 (~149.70 MiB) from the buffer pool
  - reads: 5578 (~43.60 MiB) from the OS file cache, including disk I/O
  - dirtied: 7 (~56.00 KiB)
  - writes: 0

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Addresses https://gitlab.com/gitlab-org/gitlab/-/issues/499633

Edited by Sashi Kumar Kumaresan

Merge request reports