villebro commented on code in PR #43462:
URL: https://github.com/apache/superset/pull/43462#discussion_r3845093394
##########
superset/commands/distributed_lock/release.py:
##########
@@ -37,19 +38,52 @@ class ReleaseDistributedLock(BaseDistributedLockCommand):
"""
Release a distributed lock with automatic backend selection.
- Uses Redis DELETE when DISTRIBUTED_COORDINATION_CONFIG is configured,
- otherwise deletes from KeyValue table.
+ Uses Redis when DISTRIBUTED_COORDINATION_CONFIG is configured, otherwise
the
+ KeyValue table. Release is **ownership-checked**: it only removes the lock
if
+ the stored value still matches this acquisition's ``token``. Without that
+ check, a holder whose TTL expired (letting another holder acquire the same
+ key) would delete the *new* holder's lock on its own release.
"""
+ def __init__(
+ self,
+ namespace: str,
+ params: dict[str, Any] | None = None,
+ token: str | None = None,
+ ) -> None:
+ super().__init__(namespace, params)
+ # The acquisition token to match on release (see
AcquireDistributedLock).
+ # None means "delete unconditionally" — only for callers that did not
+ # acquire via the token-aware path.
+ self.token = token
Review Comment:
Fixed in 075b1c1e8c by threading the acquisition token through the whole
Excel-export flow, so no caller on that path relies on the unconditional
`token=None` delete anymore:
- **Same-process cleanup:** the API now retains `acquire =
AcquireDistributedLock(...)` and the enqueue-failure release passes
`token=acquire.token`, so it only deletes the lock it holds — not one a later
export acquired if this one's TTL already expired.
- **Cross-process handoff:** the token is passed into the Celery task via
`apply_async(kwargs={..., "lock_token": acquire.token})`;
`export_dashboard_excel` takes a `lock_token` param and releases with it. Since
the token is a plain string it serializes cleanly in the task payload and the
ownership check works for both backends (Redis string value / KV `{"token":
...}`).
The `token=None` default stays as a documented escape hatch for callers that
didn't acquire via the token-aware path, but the throttled Excel path no longer
uses it. Extended the existing lock-release tests to assert the token is
threaded through (`token="lock-tok-1"`) and the enqueue test to assert
`apply_async` carries `lock_token`.
--
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]
