mikebridge opened a new pull request, #44262: URL: https://github.com/apache/superset/pull/44262
### SUMMARY Follow-up to #43490 (sadpandajoe's lock-hold thread, https://github.com/apache/superset/pull/43490#discussion_r3978079306). Tracked as sc-120493. **Problem.** `_repeats_an_earlier_block` in `superset/commands/deletion_retention/prune_audit.py` was an `EXISTS` nested inside an `EXISTS`, correlated per candidate row — O(history²) per candidate — and the coordination-locked re-check in `_delete_batch` evaluates it for every id in the batch while `write_ahead()` waits. On a 6,000-row multi-reason entity a 500-id batch held the lock **6.4 s on PostgreSQL and ≈50 s on MySQL 8 under REPEATABLE READ** (production's effective level). #43490 shipped the batch-size knob as mitigation; this PR fixes the shape. **Change.** The predicate is reformulated as an *uncorrelated* derived-table select and the row is tested with `id IN (...)`: - `blocked_rows` — blocked, non-NULL-uuid, `created_on <= now` rows (force rows included: a force row may be the predecessor of a later repeat; it is only never a repeat itself); - `blocked_timestamp_groups` — `GROUP BY entity_type, entity_uuid, created_on` with `COUNT(*)`, `COUNT(reason)`, `MIN(reason)`, `MAX(reason)`; - `preceding_groups` — the group table plus `LAG(ts / n / n_coded / min_reason / max_reason) OVER (PARTITION BY entity ORDER BY ts)`: the immediately preceding *distinct* timestamp **P** and its reason statistics, with no group self-join; - `repeat_boundary` — per-entity `MAX(created_on)` of streak-breaking rows. A row repeats iff `trigger <> 'force'`, P exists, `P.ts > boundary` (or no boundary), both timestamp groups are reason-uniform (all-NULL or one shared code — the inclusive tie rule: a differing-reason row tied with either endpoint still breaks the run) and `P.reason IS NOT DISTINCT FROM row.reason`. This is the same rule as before: given an earlier same-reason block E with no reason change in `[E, R]`, `P.ts ∈ [E.ts, R.ts)` so P's group matches and `P > boundary` follows from `E > boundary`; conversely take `E = P`. The equivalence is asserted by execution, not argued: a 40-seed randomised equivalence test keeps the legacy nested-`EXISTS` predicate in the test module and asserts identical candidate sets for all three pruning categories (passes on PostgreSQL and MySQL). All pre-existing behavioural tests are unchanged. **Scope under the lock.** `_delete_batch` reads the batch ids' distinct `(entity_type, entity_uuid)` pairs immediately after `acquire_coordination_lock` and binds them as literal `IN` lists inside the three derived tables. This is scoping only — the columns are immutable for a row, the lookup runs on the fresh post-lock snapshot, and every candidacy predicate is still re-verified in SQL by the re-check — so the sc-118200 survivor invariant is untouched; a unit test pins the order lock → scope lookup → re-check → delete → commit and that pairs ≤ ids ≤ `MAX_BATCH_SIZE`. Literal values are deliberate: a `DISTINCT … JOIN` or `IN (subquery)` scope collapsed PostgreSQL's cardinality estimate of the group table to 1 row and produced a nested loop with `created_on = ts` as a join *filter* (36M row pairs, 5.4 s), and a subquery scope was catastrophic on MySQL (>433 s). The `MAX_BATCH_SIZE` comment records the bind budget (~3,500 positional placeholders at the 500 ceiling; SQLite ≥ 3.32's 32,766 default). **Measurements** — coordination-lock hold, medians of 5, on a 10,805-row seed with one hot entity of 6,000 blocked rows in three reasons (five resolved streaks + one current), `ANALYZE` after seeding, MySQL session forced to `REPEATABLE READ`, a concurrent writer thread timing its own lock wait; the harness performs the scope lookup inside the locked window exactly as `_delete_batch` does. Candidate counts 500/500 and 50/50 on every run. | round | shape | PG @500 | PG @50 | MySQL @500 | MySQL @50 | |---|---|---|---|---|---| | — | #43490 nested EXISTS | 6,423 ms | 149 | ≈49,700 | 1,243 | | 1 | DENSE_RANK CTEs + group self-join, JOIN scope | 6,621 | 6,609 | 68 | 42 | | 2 | LAG over group table, plain subqueries, JOIN scope | 5,392 | 52 | 155 | 122 | | 3 | round 2 + literal entity scope (**this PR**) | **75** | **29** | **160** | **131** | Writer wait ≈ lock-hold − 10 ms throughout. Unlocked discovery: PG 60 ms, MySQL 332 ms at batch 500 (was 1.4 s / 97 s). MySQL's remaining cost is five sequential window passes for the five `LAG` columns (SQLAlchemy Core cannot emit a named `WINDOW`; a single-pass raw prototype ran in 91 ms) — accepted for this PR, tracked as **follow-up sc-120950 for the single MySQL window pass**. Harness, per-round plans and raw outputs live in the spec repo under `reviews/2026-09-14-pr-43490/lock-hold-evidence/` (`sc-120493/`). <details><summary>PostgreSQL 16 — locked re-check plan, batch 500 (abridged: costs, buffers and trivial filters stripped)</summary> ``` === PLAN: locked re-check === Nested Loop Anti Join (actual time=20.347..61.103 rows=500 loops=1) -> Nested Loop (actual time=20.345..60.760 rows=500 loops=1) -> HashAggregate (actual time=20.149..20.251 rows=936 loops=1) -> Merge Right Join (actual time=5.291..20.004 rows=936 loops=1) Merge Cond: ((purge_audit_log_2.entity_uuid)::text = (purge_audit_log_1.entity_uuid)::text) Rows Removed by Filter: 4500 -> GroupAggregate (actual time=0.022..0.023 rows=1 loops=1) -> Sort (actual time=0.019..0.020 rows=5 loops=1) -> Index Scan using ix_purge_audit_log_status_created_on on purge_audit_log purge_audit_log_2 (actual time=0.008..0. -> Materialize (actual time=5.267..19.511 rows=5436 loops=1) -> Nested Loop (actual time=5.266..18.722 rows=5436 loops=1) -> Subquery Scan on preceding_groups (actual time=5.258..11.609 rows=6043 loops=1) Rows Removed by Filter: 5 -> WindowAgg (actual time=5.256..10.858 rows=6048 loops=1) -> Subquery Scan on blocked_timestamp_groups (actual time=5.251..7.501 rows=6048 loops=1) -> GroupAggregate (actual time=5.250..7.116 rows=6048 loops=1) -> Sort (actual time=5.247..5.432 rows=6048 loops=1) -> Seq Scan on purge_audit_log purge_audit_log_3 (actual time=0.019..1.701 rows=6048 l Rows Removed by Filter: 4757 -> Index Scan using ix_purge_audit_log_status_created_on on purge_audit_log purge_audit_log_1 (actual time=0.001..0. -> Index Scan using purge_audit_log_pkey on purge_audit_log (actual time=0.043..0.043 rows=1 loops=936) SubPlan 1 -> Aggregate (actual time=0.002..0.002 rows=1 loops=500) -> Index Only Scan using ix_purge_audit_log_pruning on purge_audit_log streak_break (actual time=0.002..0.002 rows=5 loops=500 Heap Fetches: 464 SubPlan 2 -> Aggregate (actual time=0.002..0.002 rows=1 loops=464) -> Index Only Scan using ix_purge_audit_log_pruning on purge_audit_log streak_break_1 (actual time=0.002..0.002 rows=5 loops=4 Heap Fetches: 464 -> Index Scan using ix_purge_audit_log_status_created_on on purge_audit_log unresolved_attempt (actual time=0.001..0.001 rows=0 loops=500) Execution Time: 61.211 ms RUN 1: discovered=500 valid=500 RUN 2: discovered=500 valid=500 RUN 3: discovered=500 valid=500 RUN 4: discovered=500 valid=500 RUN 5: discovered=500 valid=500 === TIMING [window] PostgreSQL over 5 runs (median / max ms) === ``` </details> <details><summary>MySQL 8.0.44 (REPEATABLE READ) — locked re-check plan, batch 500 (EXPLAIN ANALYZE, abridged)</summary> ``` -> Remove duplicate purge_audit_log rows using temporary table (weedout) (actual time=127..139 rows=500 loops=1) -> Filter: ((repeat_boundary.boundary_ts is null) or (preceding_groups.prev_ts > repeat_boundary.boundary_ts)) (actual time=127..139 rows=500 loop -> Nested loop left join (actual time=127..139 rows=500 loops=1) -> Nested loop antijoin (actual time=127..138 rows=500 loops=1) -> Nested loop inner join (actual time=127..137 rows=500 loops=1) -> Nested loop inner join (actual time=0.0554..9.24 rows=500 loops=1) -> Filter: ((purge_audit_log.`status` = 'blocked') and (purge_audit_log.entity_type = 'measure_slices') and (purge_audit_log.i ? O?t??',' ?\'?M??\\ʊ!-.','`Vo?\Z(CC?-?t???','??V?TJV???q?m?','ͻ??)?O???W lÞ?','???}On?S\'?e??','o:U+\\Fk???s?\'??','+?????G??Қk??\\','3_U ?y@?^Q:??','?Y????N?E?,??0','??? ?A%?Mv??5?Y','v?o?ZN???9f???Z','ܮ??M??Њ???f','Z9??b?D(??? z=?','p???? C?oAY?K','?Z)`??F??[*~??a>','???֟@??W4?g]E','??}??3EU??b\Z?','|?n???@ ??v6Bb???f?L','9vog??C ? ??? ','O?(??`H ?V?qA?r','n% ?k%J??T??w%#','??Z??<I??@L????','Qt????K?????1?+','?5?پJ???B?????','(~H`?JW?:??M?D?',' [?','?_???SF_??9????T','?????yC???C??`?~','?7*z6EN?1?Usz?','as? A????P????','s??hdD???SQ?}L','??3?xWA{??\\?;?l','?R?4qH-?\\?ra?ym','?Oa???Cc??2J??Jt','?gt7x?K??O\'?y? ?','?)"? E??٘?%','? ?wE?B̅??g+ p',' 4???JJ???l&??X','????I?B5? ?.K?? ','ŁP???@???2??(?','?`$Q??G?????9??~','?rŬ*/J5?ŕ^ȰE?','bO{? Aa?]QW?','??x??f@???? ?1R?','h???g c','?SZ ??I%?l??I?','??q ?F&?\r?|9??/','g.J?A¡m-u\0A','???z/?D??8^???f','#?.???LX?t{?#?r','Õ?m?,Gn???m?I??','?3?HK?ME?z)??ɚ?','.???X?Fn??Q?]? ???q??','ז?(iEr??|?????','??>|s E~??c?D???','??"?8BF?a?A ?','?+??{?Je???3ɹ','??Y8D?F"?y(????d','5??0?Lu?fbr?Ԧ','?e???XN?c?i?','?\Z;:?dM?"??v)?','X\'???_H???#??? ','? x??NOr?? T\'??','?t???E??- ','?-Y.?J֍?>?!?Ո','?Q; ]E??j̘??y','?;g??S@????/{?g?','??8?? O{???>M?/','$x??E??? f??','?C?B?N(?:??]??','ߧi~?@̬??=??/?','????@ ?Jx\rJS]m',' ?Vj??Fm????G??\r','iV\\X;K??F?Rؚ)f','??O B&???z?i??','d?څ?LN??&?ʉL','???Y;?H{?;? ','??͇??O??E?*?us?','?fx???O\0????_??','RSk???J?F??(?H?','?3X?H9K2??kb???5','I???@??l?;','tQ???\0H????87B?',' Q??O E??????? ?','Ӎ&???FՆ??s 6?' f????D%?;w??w','??(=?FO???v? H?',' ??GA??>Z???','n???Jm?^?k??Qp','nS???@??????mj','??e?O"BO????5\\$?','?c??C??]{?ɤ??','.}?lN?s?; ??', ????G٭>','?I?a#G???4?6,?','P?0??*M0?!??9??A','??\r@?KIw???G?-??','??&?Hԗ?5)#?','?Ip???I??@6?~??','?OI?ĴA ????Z??','x????iG3?,BN????','cI?T&\0M QM?A???\\???K','?(??`?A_?n??*ڧ','?q?>?J_??#\\?\0G','? ?\'?mC??/L\Z??51','u\r??f1I?W?ztH??','X???sF???J\\??p','Ph?Z[`D???Ƣ???','$ ??U-Fm?BHe/?H RZK','K41??\'M ?B??-F??','?IL??A???[????n','?.?q?Av? ?k?b??','???s??LI???????d','????C]Gl?xB ?x~','????M?O(?17?AX ;','?M?WTVF:??h???','Ī??1Dc?f? !D??','m?(zgAN???;??(',' OP?H?G2???ZF ?',':??ۉ?D?????B','?,㢠?H??BG???f','??? R?LR????A??s','u???M?J??&??','W = C??k??s??','??/?L??@]v??w','eR??pQM@?????ș','?.w?D?M?? ???5B','??4T??Iе???8?0?','??Pu?J????Cg?I','$?\'`Gܝ??????','\0??TnA?#;Y<L','g?? MiLx??po\ru6?','??;\06$Aۿ??? 7?','??I?RLv?\\v?GO??','?)?G?\ ?X?','?d?d8L??`\'X???','F\\?uaAF?V?k(2?','^??fNOi??? ?','?lר5K ?????{?z','\'?|?H?C.?hl?|)V','\0yo4\'JΉ??b???','/h?<?*Kc????u ?','n???5D?? K/d9','G?????B ??U????',',u?GxC%?C6?????','?戄??H???j?(??','t?⪳O???(?#T','?{ ???O|???x?_?E','a??mڗAބ ??k',';.ndbGZ??? *ˆ','??J ?"A??Ή???:' C??Ý#\\???','?\'U??BH??{L??Z\\','?S}XO??g9j?? ','?oV?J????Cм?z',' ?Q)3?D???\0c???','=??\'xM+??[Ϡ?1S','vkj T?O̧?:??ъ?','???)4?@ء?|?5?<?','?)??7 ??CN????? w!p','8d,?$?F???? g+!?','R?8v?FE#????/?','mcP?b<Kw?:I?-??','?5?@1\rD??Kah??N@','??뮱DMȺ?? h*?','y????M??? \Z?ᘥ','6s?1>Cv?u???0Y','?? ? KH???y_?`K','mi?\r?F??IC??6? ','?.<?E???w8h???','5?L9d?Fy???F??$','????R?E<??pug2?`','aϢ M\'A??vΔ\r e','q?-\rk?Og???AA???','ϳ`nGӅ?? ?vo?','? SLO??\r j?{','?, BPEP???ʦ?J','*"??SKt?k?n???','?v?c?AA? ds??"','K~z??Lm?????ґ2','??? \\vCC?]D???OD','M?T??]K??5??A ?','M0??ZE ?`????',' Ҭ? <','O??T1IC??%???SF','?\\?B aA????U???','W?y?\0UL??VQ?p??H','.ʅ?-~Bˬ???@?V?','?*??Fd??NyK? [','(XZ5-NO ? ?|?aF1','[!fl\Z?J&?&?? ͅ?','?,?:??D??%? U?','???>\Z@r??? K?')) and (purge_audit_log.`trigger` <> 'force') and (purge_audit_log.entity_uuid is not -> Index range scan on purge_audit_log using PRIMARY over (id = 0x00796f1034274ace89f89d62d3d47fc3) OR (id = 0x017926c6927 -> Filter: ((purge_audit_log.`status` = 'blocked') and (purge_audit_log.entity_uuid is not null) and (purge_audit_log.created_ -> Single-row index lookup on purge_audit_log using PRIMARY (id=purge_audit_log.id) (actual time=690e-6..709e-6 rows=1 loo -> Select #2 (subquery in condition; dependent) -> Aggregate: max(streak_break.created_on) (actual time=0.00719..0.00721 rows=1 loops=500) -> Filter: ((streak_break.entity_type = purge_audit_log.entity_type) and (streak_break.entity_uuid = purge_audit_l -> Covering index range scan on streak_break using ix_purge_audit_log_pruning over (status = 'confirmed') OR ( -> Select #3 (subquery in condition; dependent) -> Aggregate: max(streak_break.created_on) (actual time=0.00729..0.00731 rows=1 loops=464) -> Filter: ((streak_break.entity_type = purge_audit_log.entity_type) and (streak_break.entity_uuid = purge_audit_l -> Covering index range scan on streak_break using ix_purge_audit_log_pruning over (status = 'confirmed') OR ( -> Filter: ((preceding_groups.entity_type = 'measure_slices') and (preceding_groups.prev_min_reason <=> purge_audit_log.reason) an -> Index lookup on preceding_groups using <auto_key0> (entity_uuid=purge_audit_log.entity_uuid, ts=purge_audit_log.created_on, -> Materialize (actual time=127..127 rows=6048 loops=1) -> Window aggregate with buffering: lag(max_reason) OVER (PARTITION BY blocked_timestamp_groups.entity_type,blocked_ti -> Table scan on <temporary> (actual time=99.3..99.9 rows=6048 loops=1) -> Temporary table (actual time=99.3..99.3 rows=6048 loops=1) -> Window aggregate with buffering: lag(min_reason) OVER (PARTITION BY blocked_timestamp_groups.entity_typ -> Table scan on <temporary> (actual time=78.7..79.4 rows=6048 loops=1) -> Temporary table (actual time=78.7..78.7 rows=6048 loops=1) -> Window aggregate with buffering: lag(n_coded) OVER (PARTITION BY blocked_timestamp_groups.e -> Table scan on <temporary> (actual time=56.6..57.3 rows=6048 loops=1) -> Temporary table (actual time=56.6..56.6 rows=6048 loops=1) -> Window aggregate with buffering: lag(n) OVER (PARTITION BY blocked_timestamp_gr -> Table scan on <temporary> (actual time=36.4..37.1 rows=6048 loops=1) -> Temporary table (actual time=36.4..36.4 rows=6048 loops=1) -> Window aggregate with buffering: lag(purge_audit_log.ts) OVER (PART -> Sort: blocked_timestamp_groups.entity_type, blocked_timestamp_g -> Table scan on blocked_timestamp_groups (actual time=15.6..1 -> Materialize (actual time=15.6..15.6 rows=6048 loops=1) -> Group aggregate: count(0), count(purge_audit_log.re -> Filter: ((purge_audit_log.entity_type = 'measur -> Index scan on purge_audit_log using ix_purg -> Filter: ((unresolved_attempt.entity_uuid = purge_audit_log.entity_uuid) and (unresolved_attempt.entity_type = purge_audit_log.entit -> Index lookup on unresolved_attempt using ix_purge_audit_log_status_created_on (status='pending') (actual time=0.00108..0.00108 -> Filter: ((repeat_boundary.entity_type = purge_audit_log.entity_type) and (repeat_boundary.entity_uuid = purge_audit_log.entity_uuid)) ( -> Covering index lookup on repeat_boundary using <auto_key0> (entity_type=purge_audit_log.entity_type, entity_uuid=purge_audit_log.en -> Materialize (actual time=0.068..0.068 rows=1 loops=1) -> Table scan on <temporary> (actual time=0.0455..0.0455 rows=1 loops=1) -> Aggregate using temporary table (actual time=0.0446..0.0446 rows=1 loops=1) -> Filter: ((purge_audit_log.entity_type = 'measure_slices') and (purge_audit_log.`status` in ('confirmed','target_abs -> Covering index range scan on purge_audit_log using ix_purge_audit_log_pruning over (status = 'confirmed' AND en RUN 1: discovered=500 valid=500 RUN 2: discovered=500 valid=500 RUN 3: discovered=500 valid=500 RUN 4: discovered=500 valid=500 RUN 5: discovered=500 valid=500 ``` </details> ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF N/A (backend query shape; numbers above). ### TESTING INSTRUCTIONS ``` pytest tests/unit_tests/commands/deletion_retention/test_prune_audit.py # 66, incl. lock/scope order pin, bind ceiling on SQLite, lag()-without-CTE guard pytest tests/integration_tests/deletion_retention/prune_audit_tests.py # 35, incl. the 40-seed legacy-vs-new equivalence test (run on PG and MySQL) ``` To reproduce the numbers, run the harness in the evidence directory named above in mode `window` against a PostgreSQL and a MySQL 8 database (`measure_lock_hold.py <uri> window 5 500`). ### ADDITIONAL INFORMATION - [x] Has associated issue: sc-120493 (follow-up sc-120950) - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [ ] Introduces new feature or API - [ ] Removes existing feature or API 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01267VBWbvWTNZUg9GvXKgkC -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
