Skip to content

Reduce WORKER_LOCK_MAX_RETRY_INTERVAL to 5 seconds#19755

Merged
MadLittleMods merged 4 commits into
developfrom
madlittlemods/reduce-lock-max-retry-interval
May 7, 2026
Merged

Reduce WORKER_LOCK_MAX_RETRY_INTERVAL to 5 seconds#19755
MadLittleMods merged 4 commits into
developfrom
madlittlemods/reduce-lock-max-retry-interval

Conversation

@MadLittleMods

@MadLittleMods MadLittleMods commented May 5, 2026

Copy link
Copy Markdown
Contributor

Reduce WORKER_LOCK_MAX_RETRY_INTERVAL to 5 seconds.

Better to retry more quickly than have workers wait around. 5 seconds is still a reasonable gap in time to not overwhelm anything.

This matters most in cross-worker scenarios. When locks are on the same worker, when the lock holder releases, we signal to other locks (with the same name/key) that they should try reacquiring the lock immediately. But locks on other workers only re-check based on their retry _timeout_interval.

Updating to 5 seconds to match the previous intentions based on the flawed code. We can assume they were trying to have 5 seconds as the max value to retry.

Spawning from #19394 (comment)

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Code style is correct (run the linters)

Better to retry more quickly than have workers wait around. 5 seconds is still
a reasonable gap in time to no overwhelm anything.

This matters in cross-worker scenarios. When locks are on the same worker,
when the locker holder releases, we signal to other locks that they should
try reacquiring the lock immediately. But locks on other workers for the same
keys only re-check based on their retry `_timeout_interval`.

Updating to 5 seconds to match the previous intentions based on the
[flawed code]
(https://github.com/element-hq/synapse/blob/6100f6e4f7fb0c72f1ae2802683ebc811c0e3a77/synapse/handlers/worker_lock.py#L278).
We can assume they were trying to have 5 seconds as the max value to retry.
@MadLittleMods MadLittleMods added the A-Workers (includes replication) label May 5, 2026
@MadLittleMods MadLittleMods marked this pull request as ready for review May 5, 2026 17:22
@MadLittleMods MadLittleMods requested a review from a team as a code owner May 5, 2026 17:22
@MadLittleMods MadLittleMods merged commit 2829a14 into develop May 7, 2026
78 of 81 checks passed
@MadLittleMods MadLittleMods deleted the madlittlemods/reduce-lock-max-retry-interval branch May 7, 2026 15:36
@MadLittleMods

Copy link
Copy Markdown
Contributor Author

Thanks for the review @erikjohnston 🐕‍🦺

Comment on lines +64 to +67
This matters most in cross-worker scenarios. When locks are on the same worker, when the
lock holder releases, we signal to other locks (with the same name/key) that they
should try reacquiring the lock immediately. But locks on other workers only re-check
based on their retry `_timeout_interval`.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating this reasoning in #19772 (comment)

FrenchGithubUser pushed a commit to famedly/synapse-upstreaming that referenced this pull request Jun 12, 2026
Better to retry more quickly than have workers wait around. 5 seconds is
still a reasonable gap in time to not overwhelm anything.

This matters most in cross-worker scenarios. When locks are on the same
worker, when the lock holder releases, we signal to other locks (with
the same name/key) that they should try reacquiring the lock
immediately. But locks on other workers only re-check based on their
retry `_timeout_interval`.

Updating to 5 seconds to match the previous intentions based on the
[flawed
code](https://github.com/element-hq/synapse/blob/6100f6e4f7fb0c72f1ae2802683ebc811c0e3a77/synapse/handlers/worker_lock.py#L278).
We can assume they were trying to have 5 seconds as the max value to
retry.

Spawning from
element-hq#19394 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Workers (includes replication)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants