sync: a parked conflict keeps its entity in the round - #271
Merged
Merged
Conversation
A protocol-4 round examines what main changed since the checkpoint and what this box journaled since its base. A run parked over its heartbeat is neither: main's entry is behind the checkpoint and the stamp carries no revision. So once a round parked it, no round ever looked at it again, and the self-heal shipped in 0.44.2 (close the conflict when the entity settles) could never fire: Mady upgraded, synced, was told 'Already in sync', and still had 22 conflicts, each with its first detection time. dirtyIds now includes every entity with an open conflict. Each round re-reconciles them against main's current row: a conflict that has stopped being one (a latest-wins heartbeat, or main coming to agree) merges or adopts and closes; a real one is re-recorded with fresh snapshots, and the round reports it instead of claiming to be in sync. The 0.44.2 test exercised SyncEngine's whole-table round, which production no longer runs. ParkedConflictSyncTest drives a real paged session against SyncRpcServer with the production run push policy, in the field's exact state, and fails on 0.44.2. Also corrects stampActivity's Javadoc, which 0.44.2 wrongly changed to say the next round pushes the live row: an unjournaled stamp is not dirty, so it rides along on the run's next real revision, as first written.
…heartbeat The field's state driven through the command the field runs: a node with a conflict parked over a run's heartbeat, its checkpoint past main's entry and neither side with news, runs sail sync through the fleet harness. Fails with the conflict still pending when StoreReplica.dirtyIds leaves parked conflicts out.
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.
Field report
Mady upgraded to v0.44.2 and synced:
✓ Already in sync with main.— andsail conflictsstill lists all 22last_activity_atconflicts, every one carrying its original detection time. The self-heal from #270 never fired.Root cause
A protocol-4 round examines two sets: entities main changed since the checkpoint, and
dirtyIds()— rows whoserev <> base_rev. A run parked over its heartbeat is in neither: main's entry is already behind the checkpoint, and the stamp is unjournaled so the row is not dirty. Once the first round parked it, no later round ever looked at it again. #270's heal (close the conflict when the entity settles viaadopt) is correct but unreachable.#270's test missed this because it drove
SyncEngine.reconcile— the whole-table round — which production no longer runs. That is on me.Fix
StoreReplica.dirtyIds()now includes every entity with an open conflict (SyncConflicts.pendingIds). Each round re-reconciles them against main's current row (via the existingneedop):adoptcloses it;resolveno longer acts on stale ones), and the round reports it instead of printing "Already in sync".Test
ParkedConflictSyncTestruns a realPagedSyncSessionagainstSyncRpcServerwith the production run push policy, in the field's exact state (conflict parked, checkpoint past main's entry, heartbeat unjournaled, neither side with news). Fails on v0.44.2 with the conflict still pending; passes here withmerged = 1, no conflicts, and main holding the later stamp.Also corrects
stampActivity's Javadoc, which #270 wrongly changed.mvn clean verify: 5,267 tests, all coverage gates met.