Artery test-mode: disarm the unknown-origin gate when the last blackhole heals - #8478
Merged
Merged
Conversation
PassThrough removed a healed pair from its destination set but kept the (now empty) key in the map. AnyBlackholePresent() only checks whether the map is empty, so once any blackhole was ever set it stayed true for the rest of the process, even after every blackhole was healed. That flag gates InboundTestStage: while it is true, the inbound stage drops every envelope from an unknown origin except a HandshakeReq, including a HandshakeRsp. A peer that was never blackholed, or a fresh incarnation of a healed peer, could get silently stuck mid-handshake. RemoveBlackhole now drops the key entirely once its destination set is empty, so AnyBlackholePresent() reflects reality again. Updated the existing SharedTestState and InboundTestStage unit specs that had encoded the stale-residue behavior as expected, and added an end-to-end spec: blackhole A<->B, heal it, then prove a brand-new peer C completes its handshake and exchanges traffic.
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.
Closes #8476.
Healing a blackhole removed the destination from its key's set but kept the (now empty) key in the map, so
AnyBlackholePresent()stayedtruefor the rest of the run. While true,InboundTestStagedrops every unknown-origin envelope exceptHandshakeReq- includingHandshakeRsp- so a fresh incarnation of a peer (new uid = unknown origin) could be unable to complete a handshake after all blackholes were healed. That is precisely the shape of the restart specs in the multi-node suite: blackhole the victim, heal, victim restarts with a new uid.The unknown-origin gate exists to keep mid-handshake traffic from sneaking past an active blackhole. Keeping it armed after the last heal serves no scenario the gate was built for, and breaks one that matters.
Fix:
RemoveBlackholedrops a key once its destination set is empty, soAnyBlackholePresent()reflects whether any blackhole is actually active.Tests: the two existing specs that asserted the residue as intended behavior are inverted to assert the corrected behavior (verified they catch a revert). New end-to-end spec in
ArteryBlackholeEndToEndSpec: blackhole A-B over loopback TCP, heal, then a brand-new system C completes its handshake and exchanges traffic. All 255 Artery unit tests pass. Test-mode only; no production code path changes behavior.