Relax timeouts on MOVING rebind without buffered commands - #3894
Merged
Conversation
Contributor
Author
|
run integration tests |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8132ccc3e9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
uglide
approved these changes
Aug 19, 2026
Collaborator
|
run scenario tests |
onRebindStarted was only reported when a command was in flight when the MOVING push arrived, so idle connections never opened the relaxed-timeout window and commands issued during the rebind timed out unrelaxed (recurring RelaxedTimeoutConfigurationTest#timeoutUnrelaxedOnMovingTest failure). Report rebind-started on every MOVING notification, and report completion inline in the empty-stack path where the channel is closed immediately and channelReadComplete() can no longer fire.
a-TODO-rov
force-pushed
the
topic/ggivo/fix-moving-rebind-relaxed-timeouts
branch
from
August 20, 2026 07:55
8132ccc to
1222630
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relax timeouts on MOVING rebind without buffered commands
Summary
MaintenanceAwareConnectionWatchdogonly notifiedonRebindStartedwhen a command was in flight at the moment the MOVING push arrived. On an idle connection the relaxed-timeout window was never opened, so commands issued during the ~rebind window timed out with the normal (unrelaxed) timeout. This surfaced as the recurringRelaxedTimeoutConfigurationTest.timeoutUnrelaxedOnMovingTestscenario failure. Rebind-started is now reported for every MOVING notification, regardless of the command stack.Description
channelReadComplete()never fires again, soonRebindCompletedis reported inline there; the non-empty path keeps reporting completion fromchannelReadComplete()once the stack drains.Changes
onRebindStartednow fires on every MOVING rebind, not only when the command stack is non-empty.onRebindCompleted, so relaxed timeouts are lifted after the grace period instead of never being opened/closed.Testing
MaintenanceAwareConnectionWatchdogUnitTestsupdated: the empty-stack test previously assertedonRebindStartedwas not called — that expectation encoded the bug and now asserts both started and completed notifications; the non-empty-stack tests additionally verify completion is deferred until the stack drains. The scenario testRelaxedTimeoutConfigurationTest#timeoutUnrelaxedOnMovingTestreproduces the bug end-to-end.Note
Medium Risk
Touches maintenance/MOVING rebind lifecycle and command timeout behavior during cluster endpoint moves; scope is narrow but affects timing-sensitive client paths.
Overview
MOVING rebind handling in
MaintenanceAwareConnectionWatchdognow always opens the relaxed-timeout window, including when the connection is idle and no commands were in flight when the push arrived.notifyRebindStartedruns for every rebind (not only when the command stack is non-empty), so commands buffered while the endpoint is disconnected during rebind get the same relaxed timeouts as in-flight work. On the empty-stack path the channel closes immediately andchannelReadComplete()never runs again, soonRebindCompletedis invoked there to end the relaxed window after the grace period; the non-empty path still completes viachannelReadComplete()after the stack drains.Unit tests were updated to match: the idle-connection case now expects both started and completed notifications, and non-empty-stack cases assert completion is not reported until later.
Reviewed by Cursor Bugbot for commit 8132ccc. Bugbot is set up for automated code reviews on this repo. Configure here.