Fix performance of deleting MR approval_rules
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
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
)
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
)
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