Dedup journal.Summary() against retried-shard duplicate records - #72
Merged
Conversation
Journaling is at-least-once: a shard whose lease expires mid-run is requeued and re-runs from scratch, re-emitting every record it already wrote (agent/src/jrn.c: "lease-expiry re-runs gives at-least-once journaling (readers dedup)"). journal.Orphans() already deduped correctly; journal.Summary() did not, so a retried shard inflated `drsync journal cat --summary`, the completion email, and the WebUI journal-summary panel (and via passctrl.recordJournalTypeCounts, the persisted journal_type_counts rollup that /report and the job detail panel read) by exactly its record count. Found live investigating a user report of stale destination ACLs surviving a re-run: a 22k-file/POSIX-ACL reproduction hit a lease expiry mid-pass, inflating counts by 2000. That reproduction did not explain the original symptom (files silently missed with zero journal signal) — this fix addresses the duplication bug it surfaced along the way, not the original report, which remains open pending the user's own larger-scale test. seedVerify/seedDirfix hit the same duplication but deliberately stay un-deduped: both stream the journal in O(batch) memory (TestSeedVerifyMemoryBounded pins this at 1M files), and a whole-pass "seen" set would reintroduce the O(N) memory that design avoids. Safe to leave as-is since a duplicate VerifyEntry/DirMeta only means one file/dir gets re-verified or re-applied, and both operations are idempotent. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PsdNZLfmAFrMX2VUtkLtmm
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.
Summary
journal.Summary()now dedups per(pass, type, rel_path), matching the existing pattern injournal.Orphans()— fixes inflated counts indrsync journal cat --summary, the completion email, the WebUI journal-summary panel, and the persistedjournal_type_countsrollup (/report, job detail panel), all of which route throughSummary().agent/src/jrn.c: "lease-expiry re-runs gives at-least-once journaling (readers dedup)") — a shard whose lease expires mid-run is requeued and re-emits every record it already wrote, not just the remainder.Summary()was one of the readers not honoring that contract.seedVerify/seedDirfix(passctrl.go) hit the identical duplication but are deliberately left un-deduped — both stream the journal in O(batch) memory (TestSeedVerifyMemoryBoundedpins this at 1M files), and a whole-pass "seen" set would reintroduce O(N) memory. Safe to leave as-is: a duplicateVerifyEntry/DirMetaonly causes a redundant re-verify/re-apply, both idempotent.docs/DESIGN-coordinator.md§5 documents the at-least-once contract and which readers honor it, for future readers.Context
Found live while investigating a user report of stale destination ACLs surviving a re-run pass. A 22k-file/POSIX-ACL reproduction triggered a lease expiry mid-pass, which inflated journal-derived counts by exactly the retried shard's record count (2000, in that repro) — a real, independently-reproducible bug, but not an explanation for the original report. Duplicate counting cannot cause files to be silently missed with zero journal signal (a re-run shard re-diffs from scratch and will always re-fix or correctly see nothing to fix). That original symptom remains open; the leading theory is source-filesystem metadata-caching staleness (previously observed with VAST appliances), pending further large-scale testing.
Test plan
gofmt -l .cleango vet ./...cleango test -count=1 ./coordinator/...— all green, includingTestSeedVerifyMemoryBounded(streaming guarantee intact) and new testsTestSummaryDedupsRetriedShard,TestSeedVerifyToleratesRetriedShard,TestSeedDirfixToleratesRetriedShard🤖 Generated with Claude Code
https://claude.ai/code/session_01PsdNZLfmAFrMX2VUtkLtmm