aminghadersohi commented on PR #38576: URL: https://github.com/apache/superset/pull/38576#issuecomment-4803262874
Thanks for the detailed review and for verifying against HEAD! **Self-skip bug (dashboards/api.py + thumbnails.py)** Fixed by removing the pre-enqueue COMPUTING write from the API entirely. `compute_and_cache` now wraps the full check → COMPUTING write → screenshot → final cache write inside a `DistributedLock`. The lock TTL matches `THUMBNAIL_COMPUTING_CACHE_TTL` (360 s), so only the first worker to acquire it can proceed. If another task tries to acquire the same lock it raises `AcquireDistributedLockFailedException` and exits immediately, without ever touching the cache. No self-skip possible. **Screenshot path (cache_dashboard_screenshot)** Same fix — `cache_dashboard_screenshot` also flows through `compute_and_cache`, so the DistributedLock covers it too. **Race condition in API check-then-set** The API-level `should_trigger_task` check is now just an optimization (avoid unnecessary enqueues) rather than a correctness gate. If two requests race past it and enqueue two tasks for the same cache key, the DistributedLock in `compute_and_cache` ensures only one runs; the other bails on `AcquireDistributedLockFailedException`. The atomicity guarantee lives in the task, not the API. -- 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]
