codeant-ai-for-open-source[bot] commented on code in PR #43523:
URL: https://github.com/apache/superset/pull/43523#discussion_r3858608051
##########
superset/dashboards/api.py:
##########
@@ -1996,7 +1996,9 @@ def build_response(status_code: int) -> WerkzeugResponse:
)
if cache_payload.should_trigger_task(
- force, expected_scope=f"dashboard:{dashboard.id}"
+ force,
+ expected_scope=f"dashboard:{dashboard.id}",
+ check_updated_staleness=screenshot_obj.supports_updated_staleness,
Review Comment:
Agreed. The race is valid: `should_trigger_task()` is a read-only check, and
the dashboard endpoint schedules the task without claiming the stale entry. The
worker-side `DistributedLock` prevents duplicate rendering, but not duplicate
queue messages.
The endpoint should perform an atomic claim before calling `.delay()`. A
suitable minimal approach is:
1. Acquire a short-lived per-cache-key enqueue lock (or use an atomic cache
`add` claim key).
2. Re-read the payload while holding that lock.
3. If it is still stale, mark it as claimed (for example,
`PENDING`/`COMPUTING` or a separate refresh marker).
4. Enqueue only after the claim succeeds.
5. Have the worker clear the marker when the refresh completes, with a TTL
fallback for failed tasks.
This must be an atomic cache operation; a plain `get()` followed by `set()`
would leave the same race. The existing worker lock can remain as a second line
of defense, but it cannot solve the queue buildup by itself.
I would insist on this change for high-traffic dashboards, since a burst at
the TTL boundary could otherwise enqueue many redundant Celery tasks.
--
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]
