Skip to content

WIP: Add Semaphore.refresh_leases retries#4158

Draft
lr4d wants to merge 6 commits into
dask:mainfrom
lr4d:add_semaphore_refresh_retries
Draft

WIP: Add Semaphore.refresh_leases retries#4158
lr4d wants to merge 6 commits into
dask:mainfrom
lr4d:add_semaphore_refresh_retries

Conversation

@lr4d

@lr4d lr4d commented Oct 9, 2020

Copy link
Copy Markdown
Contributor

Builds on top of #4151 , only the last commit is new.

The test added is still flaky.

lr4d added 6 commits October 9, 2020 12:56
This adds retries to the semaphore release method, which must be
configured using "distributed.comm.retry.count". This may come to
use when the scheduler is under high load and some release
calls may fail.
This PR also avoids raising an exception in case a release fails,
instead a warning is logged at ERROR level, and the Semaphore.release
method will return `False`. The lease will eventually be cleaned up
during the semaphore lease validation check, which may be configured using
"distributed.scheduler.locks.lease-validation-interval" and
""distributed.scheduler.locks.lease-timeout".
@lr4d

lr4d commented Oct 9, 2020

Copy link
Copy Markdown
Contributor Author

The rationale here is similar to the issue linked, to increase the robustness of the Semaphore during unstable periods. In case the refresh_leases method fails, we retry the operation. In case all retries fail, we generate an appropriate log message

Base automatically changed from master to main March 8, 2021 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant