mikebridge opened a new pull request, #44091: URL: https://github.com/apache/superset/pull/44091
### SUMMARY The dataset PUT's conditional-write (`If-Match`) guard compared the client's token against a version-info lookup made with **plain consistent reads**. On MySQL/InnoDB default REPEATABLE READ, those reads are served from the snapshot pinned by the request's *first* read (the auth queries), so a version row committed by a concurrent writer between that first read and this request's row lock stayed invisible: the stale token matched, `raise_for_stale_write` passed, and the second writer overwrote the first — exactly the lost update the lock + `If-Match` pair exists to prevent, surviving on MySQL only (R1 and R2 both open with token v1; R1 locks and commits v2; R2 acquires the lock but its validator read still sees v1). This is the validator-layer sibling of #44027, which fixed the *entity* read one layer down. Postgres (READ COMMITTED, fresh snapshot per statement) and SQLite are unaffected. **Fix**: - `current_live_transaction_id_for_share` (`versioning/queries.py`) — the live version row's `transaction_id` (the token input) read under `FOR SHARE` (`with_for_update(read=True)`): a locking read is exempt from the REPEATABLE READ snapshot and returns committed data, without blocking other readers. Deliberately a plain row query, **not** the existing aggregate — locking clauses and aggregates don't combine reliably across dialects. - `current_entity_version_info` grows `lock_for_stale_check`; when set, the locked transaction id replaces the aggregate's. The displayed version *number* stays a plain aggregate read: a concurrent commit can leave response metadata one behind, but the guard never consults it (documented in the docstring). - The dataset PUT passes `lock_for_stale_check=is_conditional_write()` — only conditional saves pay for the lock; every GET path and unconditional PUT keeps lock-free reads (pinned by test). **Merge-ordering note**: this touches the `datasets/api.py` comment/call block adjacent to #44027's changes; both branches are mine, and whichever lands second takes a trivial rebase. The residual child-collection staleness (lazy `columns`/`metrics` loads) remains tracked separately, as noted in #44027. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF N/A — concurrency fix. Before (MySQL only): a stale `If-Match` token could pass the 412 guard after a concurrent commit. After: the guard's input reflects committed state. ### TESTING INSTRUCTIONS - `pytest tests/unit_tests/versioning/test_version_info_locking.py` — 3 dialect-independent pins: the conditional-path read chains `with_for_update(read=True)` (dropping the flag fails on every backend), `current_entity_version_info` threads the flag and adopts the locked value, and the default path never invokes the locking read at all (GETs stay lock-free). - `pytest tests/integration_tests/versioning/conditional_token_lock_tests.py` — real-backend proof: the `FOR SHARE` read executes on the actual dialect and agrees with the plain read on the quiet path. The MySQL RR staleness itself cannot flip on Postgres and needs an interleaved read view on MySQL, so — per the honest-scope precedent of #44015/#44027 — the statement-shape pin is the load-bearing regression guard. - Manual (MySQL): two sessions PUT the same dataset with the same `If-Match`; before the fix the second could pass the guard and clobber the first; after, it gets 412. ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [ ] 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_01JRLEJS4mUqKBoPjSjviKUW -- 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]
