debug: tag reload() breadcrumbs with lock state and NextUID (#647)#648
Merged
Conversation
#645 fixed one trigger of the #644 NextUID-regression class (stale logFD surviving a rename), but the same symptom reproduced again on 2.1.11 under concurrent load with #645's ".log replaced" detector never firing — a second, distinct trigger. withFolderRO (the read path — most importantly OpenFolder's dedup snapshot, hit on every internal/lmtp/deliver.go delivery) calls reload() under only the in-process fs.mu, never acquiring the cross-process distributed lock (u.b.locker) that withFolderLock (the write path) always takes first. An unlocked read can interleave with another process's locked compaction and load a torn view into the shared in-memory folderState, which every subsequent correctly-locked write then trusts as a baseline. reload() now takes a locked bool (true from withFolder/withFolderLock, false from withFolderRO) and logs it plus NextUID before/after on every "fileindex: reload fast-path"/"reload full"/"reload applied" breadcrumb, so the next live reproduction can show an unlocked reload's NextUID diverging from a concurrent locked writer's instead of inferring the race indirectly. Purely additive logging behind LOG_LEVEL=debug — no behavior change.
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
2 participants
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
#645 fixed one trigger of the #644 NextUID-regression class (stale
logFDsurviving a rename), but the same symptom reproduced again on 2.1.11 under concurrent imaptest+smoketest load, with #645's".log replaced"detector never firing — pointing at a second, distinct trigger (filed as #647).withFolderRO(the read path — most importantlyOpenFolder's dedup snapshot, hit on everyinternal/lmtp/deliver.godelivery) callsreload()under only the in-processfs.mu, never acquiring the cross-process distributed lock (u.b.locker) thatwithFolderLock(the write path) always takes first. An unlocked read can interleave with another process's locked compaction and load a torn view into the shared in-memoryfolderState, which every subsequent correctly-locked write then trusts as a baseline.This PR
reload()now takes alocked bool(truefromwithFolder/withFolderLock,falsefromwithFolderRO) and logs it plusNextUIDbefore/after on every"fileindex: reload fast-path"/"reload full"/"reload applied"breadcrumb, so the next live reproduction can directly show an unlocked reload'sNextUIDdiverging from a concurrent locked writer's, instead of inferring the race indirectly from the absence of #645's detector firing.Purely additive logging behind
LOG_LEVEL=debug— no behavior change.go test ./...green, existing #645 regression test (TestConcurrentCompactionNoUIDRegression) updated for the newreload()signature and still passing. appVersion → 2.1.13.