fix: recover corrupt distributed records#2376
Merged
Merged
Conversation
📝 WalkthroughWalkthroughDurable atomic file operations, typed corruption errors, corrupt-record removal, and configurable recovery are added to file persistence. Distributed lease and active-run stores now repair corrupt records during upserts and remove stale corrupt records during listing. ChangesCorrupt-record recovery
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Scheduler
participant LeaseStore
participant FileCollection
participant FileUtil
Scheduler->>LeaseStore: ListAll leases
LeaseStore->>FileCollection: Read record
FileCollection-->>LeaseStore: persis.ErrCorrupt
LeaseStore->>FileCollection: RemoveCorrupt(staleBefore)
FileCollection->>FileUtil: RemoveFileDurable
FileUtil-->>FileCollection: Durable removal result
FileCollection-->>LeaseStore: Removed stale record
LeaseStore-->>Scheduler: Active lease count
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Summary
Root cause
The file backend renamed temporary files without syncing the file contents or the containing directory. A host power loss could therefore persist the renamed directory entry while losing the file data, leaving zero-byte or otherwise invalid JSON records.
Distributed lease listing treated any unreadable lease as a global failure. A single corrupt lease could make queue-capacity checks fail closed and leave scheduled runs queued indefinitely. Active-run listing skipped unreadable entries but retained them permanently, causing repeated scans and warnings.
Impact
New writes are durable across process or host crashes when the underlying filesystem honors fsync. Existing corrupt distributed records are removed safely after the configured grace period, and fresh coordinator updates replace corrupt records immediately. Cleanup deletes the damaged file instead of accumulating quarantine artifacts, so collection scans do not degrade over time.
Validation
make fmtmake lintgo test ./internal/cmn/fileutil ./internal/persis/file ./internal/persis/store ./internal/service/scheduler ./internal/service/coordinator -count=1Closes #2375
Summary by cubic
Make file-backed writes crash-durable and recover distributed records from invalid JSON. Coalesce lease heartbeats and safely clean up corrupt files so scheduling and capacity checks continue.
internal/cmn/fileutil: Sync temp file and parent directory for atomic writes; addWriteFileAtomicExclusive; add durable delete; use write-through on Windows.persis: AddErrCorruptand propagate it from the file backend and single-record store.internal/persis/file:CreateusesWriteFileAtomicExclusive;GetreturnsErrCorruptfor invalid JSON; addRemoveCorruptwith durable deletes.internal/persis/store: On upsert, replace corrupt lease/active-run records; on list, remove stale corrupt files after a grace period; lease listing no longer blocks capacity checks; active-run listing skips and cleans corrupt entries; grace period configurable viaWithCorruptRecordGracePeriod.internal/service/coordinator: CoalesceRunHeartbeatlease updates when already fresh; isolate corrupt leases by cancelling only the affected run.Written for commit 605ba9f. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes