Repository navigation
Prune the request pool after a reconfig - #699
Merged
Merged
Conversation
After a reconfig the new controller initializes its verification sequence from the verifier, which already reflects the new config, so MaybePruneRevokedRequests never detects the change. The prune in the old controller's decide is usually skipped because the controller is closed by then. As a result, requests revoked by the new config remained in the pool. Prune the pool unconditionally in Consensus.reconfig, after the new components are created and before they are started. Fixes hyperledger#698 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Hagar Meir <hagar.meir@ibm.com>
pfi79
approved these changes
Oct 10, 2026
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.
Fixes #698
Problem
After a reconfig, requests that the new config makes invalid stayed in the request pool:
Consensus.reconfigsets its verification sequence from the verifier inStart. The application has already applied the new config by then, soMaybePruneRevokedRequestsnever sees a change.decideis usually skipped, because the controller is already closed anddecidereturns early.Fix
Prune the pool unconditionally in
Consensus.reconfig, after the new components are created and before they are started. At that point all components are stopped and pool timers are paused, so the prune doesn't race with batching or timers. The verifier already reflects the new config. The existingMaybePruneRevokedRequestscalls are unchanged.Test
TestPoolPrunedAfterReconfig: a follower holds a request in its pool, and a long forward timeout keeps the request from being forwarded to the leader. A reconfig is then committed, and the test app revokes the request's client when it delivers the reconfig. The test checks that the follower's pool ends up empty, and that a new request is still delivered afterwards.The test app gets an opt-in
clientsToRevokelist. When a reconfig is delivered, the listed clients' requests failVerifyRequestand the verification sequence is bumped. Existing tests don't set the list, so their behavior is unchanged.Without the fix the test failed 5/5 runs. With the fix it passed 20/20 runs under
-race.🤖 Generated with Claude Code