Harden the O(n) idle-pool reap tests: stable idle window + kept-after-removed coverage - #2249
Merged
hyperxpro merged 1 commit intoJul 18, 2026
Merged
Conversation
Copy link
Copy Markdown
Member
Sorry, something went wrong.
…coverage Follow-up to #2225. Two test-quality fixes for the O(n) iterator-remove reap: cleanerReapsExpiredButKeepsHealthyInSameTick used a 200ms idle window with the fresh channels offered immediately before firing the cleaner, so a GC/scheduling pause on a loaded CI box could age them past the timeout and get them wrongly reaped. Widen the window to 1s (mirrors channelReofferedAfterExpiryIsNotReaped) so only a pause longer than the window can misclassify a fresh channel. Add cleanerContinuesPastRemovedNodesToReachKeptNodes. Because offer() is offerFirst and that mixed test's fresh channels cluster at the front, every reaped node sat at the tail, so the scan never had to unlink a node and then advance to a KEPT node after it. The new test builds an alternating closed/open deque through the remote-close path (channels are closed, not aged, so it is fully deterministic) so each removed node is followed by a kept one, pinning the iterator-remove-then-continue guarantee.
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
1 participant
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.
Motivation:
#2225 replaced DefaultChannelPool's collect-then-removeAll reap with an in-place O(n) iterator removal, but two test gaps remained. First, cleanerReapsExpiredButKeepsHealthyInSameTick used a 200 ms idle timeout, making it susceptible to GC or scheduling delays that could incorrectly classify fresh channels as expired. Second, because offer() inserts at the front of the deque, the test never exercised the iterator-removal case where iteration must continue correctly after removing a node.
Modification:
Increase the idle timeout in the mixed test to 1 second, matching channelReofferedAfterExpiryIsNotReaped, to eliminate the tight timing dependency. Add cleanerContinuesPastRemovedNodesToReachKeptNodes, which deterministically constructs an alternating closed/open deque so every removed node is immediately followed by a retained one, directly validating the iterator-remove-then-continue behavior. Test-only; no production changes.
Result:
The test suite is no longer sensitive to timing jitter and now deterministically verifies the iterator removal logic introduced in #2225.