mikebridge opened a new pull request, #41642: URL: https://github.com/apache/superset/pull/41642
### SUMMARY Fixes an intermittent `RuntimeError: deque mutated during iteration` (surfacing as HTTP 500s and E2E flakiness) caused by an interaction between two changes: - #40194 made `Database.get_sqla_engine` attach a per-call SQLAlchemy `connect` listener when prequeries exist (e.g. `SET search_path` on PostgreSQL), removing it again on context exit — two mutations of the engine's connect-listener deque per call. - #40237 introduced `_ENGINE_CACHE`, making `Engine` objects **shared across threads** per `(database_id, url, engine_kwargs)`. SQLAlchemy stores event listeners in a plain, unlocked `collections.deque`, and dispatch iterates it unguarded (`for fn in self.listeners`). With NullPool, every `raw_connection()` creates a fresh DBAPI connection and fires that dispatch. So one thread's connection checkout can iterate the shared engine's deque while another thread's `get_sqla_engine` enter/exit mutates it — a classic race that reproduces readily under concurrent load (observed as consistent `create-dataset` 500s on one Playwright run and flaked-but-recovered failures on another). **Fix:** compute prequeries *before* fetching the engine, and request a **private, uncached engine** (`cacheable=False`) whenever prequeries are present. The per-call listener add/remove then only ever touches an engine no other thread can see. No-prequery paths keep the shared cache and its #27897 semantics. The listener also closes over per-call, schema-specific prequeries and must not leak to other requests sharing a cached engine — which is why a register-once approach was rejected. This restores the pre-#40237 behaviour for prequery engines only (fresh engine per call); prequery-less databases keep full engine caching. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF N/A (backend concurrency fix) ### TESTING INSTRUCTIONS Unit tests in `tests/unit_tests/models/core_test.py`: - `test_prequery_engine_bypasses_shared_cache` — deterministic regression: with prequeries the yielded engine is never the cached instance and `_ENGINE_CACHE` is untouched; without prequeries caching behaviour is unchanged. - `test_prequeries_execute_on_real_connections` — behavioural guard for #40194: prequeries still execute on every new DBAPI connection (file-backed SQLite, table-creating prequery, observed on the same connection). - `test_concurrent_prequery_connections_do_not_race` — 4 threads × 50 `get_raw_connection` calls with prequeries. **Against the previous code this reliably reproduces the production error** (3/4 threads raised `RuntimeError: deque mutated during iteration` in under a second); with this fix it passes deterministically. ```bash pytest tests/unit_tests/models/core_test.py -q ``` ### 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) -- 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]
