fix(fileindex): serialize withFolderRO's reload against writers via the cross-process lock (#647)#655
Merged
Merged
Conversation
…he cross-process lock (#647) The read path (withFolderRO) reloaded folder state under only the in-process fs.mu, never the cross-process distributed lock the write path (withFolderLock) takes before its own reload. An unlocked reload could interleave with another process's lock-holding compaction (flush + truncateLog) and load a torn view into the shared in-memory folderState — which every subsequent, correctly locked write then trusts as a baseline, regressing the folder's NextUID under concurrent load. This is the second trigger of the #644 class, distinct from the stale-logFD/inode case fixed in #645 (no ".log replaced" WARN fires here — the fd may not have been replaced at all; the poison is in reading base+log content while a writer compacts). The locks API is exclusive-per-key only (no shared/read mode), so the read path takes the same exclusive key as writers — correctness over throughput, as the issue accepts. Re-entrancy is handled by the existing HoldsResource shortcut, so a nested/POP3-QUIT caller that already holds the key does not deadlock. - Extract withDistLock: the HoldsResource-guarded cross-process acquire (plus the existing debug timings), shared by withFolderLock and withFolderRO. - withFolderRO now runs reload() under withDistLock (reload(locked=true)); fn then reads the settled snapshot under fs.mu.RLock WITHOUT holding the exclusive resource, so the lock covers only the reload, not the whole read. Adds TestReadPathSerializesAgainstConcurrentLockHolder: two lock clients (two pods) on one embedded server; client B holds the folder key, a read-only op on the index wired to client A must block until B releases — proving the read reload now serializes against a concurrent compaction.
This was referenced Jul 19, 2026
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.
Closes #647.
Problem
withFolderROreloaded folder state under only the in-processfs.mu, never the cross-process distributed lock (u.b.locker) that the write path (withFolderLock) takes before its own reload. An unlocked reload could interleave with another process's lock-holding compaction (flush+truncateLog) and load a torn view into the shared in-memoryfolderState— which every subsequent, correctly locked write then trusts as a baseline, regressing the folder'sNextUIDunder concurrent load.This is the second trigger of the #644 class, distinct from the stale-
logFD/inode case fixed in #645: no.log replacedWARN fires here — the fd may not have been replaced at all; the poison is in reading base+log content while a writer compacts. It reproduced live on 2.1.11 (after #645) with the same signature (UIDValidityunchanged,NextUID→ 1/2/3), no #645 detector firing.Fix
locksAPI is exclusive-per-key only (no shared/read mode), so the read path takes the same exclusive key as writers — correctness over throughput, as the issue accepts. Re-entrancy is handled by the existingHoldsResourceshortcut, so a nested / POP3-QUIT caller that already holds the key does not deadlock.withDistLock: theHoldsResource-guarded cross-process acquire (plus the existing debug timings), now shared bywithFolderLockandwithFolderRO.withFolderROrunsreload()underwithDistLock(reload(locked=true));fnthen reads the settled snapshot underfs.mu.RLockwithout holding the exclusive resource — the lock covers only the reload, not the whole read, minimizing exclusive hold.Trade-off (deliberate)
Every read on the hot path (LMTP delivery's
OpenFolderdedup, FETCH'sGetMessages) now takes the exclusive per-(user,folder)lock around its reload — an extra Redis round-trip and serialization of reads against writes. The issue accepts this explicitly. A future optimization (shared/read locks) is only possible if the lock backend gains them; not done here.Test
TestReadPathSerializesAgainstConcurrentLockHolder: two lock clients (two pods) on one embedded server; client B holds the folder key, a read-only op on the index wired to client A must block until B releases — proving the read reload now serializes against a concurrent compaction. (Without the fix, the read returns immediately → test fails.)go test ./...+-tags flatcurvebuild +helm lintgreen. appVersion → 2.1.16.