mistercrunch commented on PR #36368: URL: https://github.com/apache/superset/pull/36368#issuecomment-3821446070
Went a bit deeper looking at the indexing strategy to make sure it's covering the workloads/transactions: ## Summary Table | # | Issue | Severity | Type | |---|---|---|---| | 1 | **`scope` column has no index in migration** despite model annotation; private task filter does full scan on `scope` | **High** | Missing index | | 2 | **No index on `ended_at`** — prune query scans full table | **Medium** | Missing index | | 3 | **Redundant `idx_task_subscribers_task_id`** — already covered by unique constraint | **Low** | Unnecessary index | | 4 | **Triple-nested `@transaction()`** — command → DAO → DAO calls cause savepoint overhead | **Low-Medium** | Transaction overhead | | 5 | **Lock acquired inside transaction** — holds DB connection while waiting for distributed lock | **Medium** | Deadlock/contention risk | | 6 | **3 SELECTs per cancel** — validate + re-fetch under lock + refresh after | **Low** | Unnecessary queries | | 7 | **`publish_abort()` fires before transaction commit** — listener might check DB before ABORTING state is visible | **Low** | Race condition | Items 1 and 2 are the most actionable — straightforward index additions in the migration. Item 5 is an architectural concern worth discussing but harder to fix without refactoring the command pattern. -- 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]
