hmds: release the connection when the transport is given up on (ibx#367) - #382
Open
userFRM wants to merge 1 commit into
Open
hmds: release the connection when the transport is given up on (ibx#367)#382userFRM wants to merge 1 commit into
userFRM wants to merge 1 commit into
Conversation
Giving up on the HMDS transport set the disconnected flag and left the dead socket in place. `maybe_spawn_hmds_reconnect` returns early while a connection is present, so it never scheduled an attempt and the transport was stuck for the life of the process. The heartbeat block is guarded on the same pair, so nothing further was sent on it either, and historical requests issued afterwards targeted a connection nothing was going to replace. Both routes there did it: the liveness timeout, and the ordinary receive error — a closed peer, or any other failure out of `try_recv`, which is how the transport usually dies. The flag and the connection now move together in one method that both call, so setting one without the other is not something a caller can express. A reconnect no longer inherits the dead session's outstanding probe. The liveness check sends a TestRequest only when none is pending, so a stale one suppressed every probe on the new connection and it went from silent to declared dead without being asked anything. The warm-up window starts with the new session as well, rather than being counted from the old one and already expired. CCP and the farm gate their reconnects on the flag alone and were unaffected. Closes deepentropy#367.
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.
Problem
Giving up on the HMDS transport set the disconnected flag and left the dead socket in place.
maybe_spawn_hmds_reconnectreturns early while a connection is present, so it never scheduled an attempt — the transport was stuck for the life of the process. The heartbeat block is guarded on the same pair, so nothing further was sent on it either, and historical requests issued afterwards targeted a connection nothing was going to replace. Recovery needed restarting the engine.Both routes there did it: the liveness timeout, and the ordinary receive error — a closed peer, or any other failure out of
try_recv, which is how the transport usually dies.What this changes
The flag and the connection move together in one method that both callers use, so setting one without the other is not something a caller can express.
A reconnect no longer inherits the dead session's outstanding probe. The liveness check sends a TestRequest only when none is pending, so a stale one suppressed every probe on the new connection and it went from silent to declared dead without being asked anything. The warm-up window starts with the new session as well, rather than being counted from the old one and already expired.
CCP and the farm gate their reconnects on the flag alone and were unaffected.
Tests
an_hmds_disconnect_lets_its_reconnect_run— driven through the receive path with the peer gone: the transport is marked dead, the socket is released, and the scheduler arms an attempt instead of skipping forever.an_hmds_reconnect_does_not_inherit_the_dead_session_probe— drivespoll_hmds_reconnectwith a successful result and asserts the probe is cleared and the warm-up restarted.Each fails by name against a compiling reversion of the production line it covers.
Closes #367.
Test plan
an_hmds_disconnect_lets_its_reconnect_runby name — the test seeds a live socket first, or releasing it would be unobservable.an_hmds_reconnect_does_not_inherit_the_dead_session_probe.cargo check --offlineclean on--lib,--lib --features python,--bins,--examples, and each integration target individually.tests/ib_paper_compatcompared against a clean checkout of the base commit — identical sorted diagnostic sets.cargo test --offline --lib— only the two knownconfig::expiry_testsfailures, which fail on the base commit for missing legacy tzdata (fixed separately in config: resolve the legacy timezone names IB states its times in (ibx#335) #336).