fix(sync): clear connect state when the remote aborts a dial as already-syncing - #111
Open
vladimirlogachev wants to merge 1 commit into
Open
fix(sync): clear connect state when the remote aborts a dial as already-syncing#111vladimirlogachev wants to merge 1 commit into
vladimirlogachev wants to merge 1 commit into
Conversation
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.
Description
When our dial is rejected by the remote with
AbortReason::AlreadySyncing,on_sync_via_connect_finishedskipson_sync_finished, soNamespaceStates::finishnever runs for that exchange. If no concurrent incoming exchange took the slot over (mutual-dial tie-break), the (namespace, peer) pair staysRunningforever and every later sync trigger for it —NewNeighbor,SyncReport,Resync— is silently dropped instart_connect. The rejection is easy to hit when one side redials right after an exchange while the other side has not yet processed its finish event; downstream we observed pairs permanently stopping to sync, caught by a scenario stress test failing about once in 80 runs.Changes:
engine/state.rs: addNamespaceStates::abort_connect, clearing the running outgoing exchange back toIdle— guarded so it only clears aRunning { origin: Connect(reason) }slot with the sameSyncReason; an accept exchange that took the slot over in the tie-break is left untouched (it finishes through the accept path), as is a newer dial with a different reason.engine/live.rs: call it from theRemoteAbort(AlreadySyncing)arm ofon_sync_via_connect_finished.Breaking Changes
None — additive internal change to the live engine's sync-state tracking; no public API or wire format touched.
Notes & open questions
The guard distinguishes exchanges by
SyncReason, not by session identity — a late abort of an old dial can clear a newer dial started for the same reason. The consequence is an extra idle transition (the next trigger restarts the exchange), not a wedge. If you want it stricter,SyncState::Runningwould need a session id; happy to extend if preferred.Change checklist