EnxDev commented on code in PR #43805:
URL: https://github.com/apache/superset/pull/43805#discussion_r3992222294
##########
superset/dashboards/api.py:
##########
@@ -1811,13 +1858,24 @@ def export_xlsx(self, pk: int) -> WerkzeugResponse:
# otherwise) so the guard works across the web server and workers and
is
# not a no-op under the default cache. The task releases it when it
# settles; the TTL is the backstop if that release is ever lost.
- lock_params = export_lock_params(g.user.id, dashboard.id)
+ # A guest/embedded requester has no DB-backed user id (GuestUser
carries
+ # no ``id`` attribute at all), so all guests share lock slot 0 for the
+ # dashboard; the task reconstructs the guest (with the token's RLS
rules
+ # and resource claims) from the token payload passed alongside.
+ user_id = get_user_id()
+ guest_token_payload = (
+ getattr(g.user, "guest_token", None) if user_id is None else None
+ )
+ lock_params = export_lock_params(
+ user_id or guest_lock_slot(guest_token_payload), dashboard.id
+ )
+ acquire = AcquireDistributedLock(
+ EXPORT_LOCK_NAMESPACE,
+ lock_params,
+ ttl_seconds=EXPORT_LOCK_TTL_SECONDS,
Review Comment:
Nice improvement carrying the acquisition token through; compare-and-delete
closes the stale-release race. One queue-delay gap remains, though: this 720s
TTL starts here in the API, while Celery's 660s hard limit starts only after
worker pickup. If a job waits more than 60s, its lock can expire while it is
still running (and after 720s it starts with no lock), allowing another request
to acquire the key and run concurrently. The frontend explicitly accommodates
broker backlog, so this timing is plausible. Non-blocking for this pass, but
could we follow up with an ownership-checked renewal when the worker starts, or
another queue-aware lock design, plus a timing regression test? A plain
`expire` would need to compare `lock_token` atomically so it cannot extend a
successor's lock.
--
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]
