debug: breadcrumbs around LMTP's non-atomic AllocateUID→Save→AppendMessage window#643
Merged
Merged
Conversation
…ssage window report/imap objectid smoketest checks fail intermittently on mdbox/sdbox (never maildir) under concurrent load, with live evidence of a message committing a UID lower than one already committed earlier in the same UIDVALIDITY. internal/lmtp/deliver.go pre-allocates the UID via a separate AllocateUID call, then does a slow storage write (mdbox/sdbox: map lookup + refcount + possible rotation — measurably slower than maildir's flat-file write), then commits via a separate AppendMessage call — unlike every other delivery path (IMAP APPEND, sieve fileinto, MOVE/COPY), which uses the single atomic AllocateAndAppend. Disproved several hypotheses via targeted unit-test reproduction (clean AllocateAndAppend concurrency, ResetFolder's NextUID guard, Recreate's cross-device rename staging, the deliver.go three-step pattern itself all passed 0-collision stress tests), so the exact trigger under real sandbox load is still unconfirmed. These breadcrumbs bracket the suspect window with a per-call correlation id so the next live reproduction can pin the exact interleaving from logs instead of more blind hypothesis testing.
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
Investigating intermittent
report/imap objectidsmoketest failures on mdbox/sdbox (never maildir) under concurrent load. Live sandbox evidence (two independent full imaptest+smoketest rollout runs, same reproduction both times) shows a message committing a UID lower than one already committed earlier in the same UIDVALIDITY — a genuine UID monotonicity violation, not the FTS-checkpoint issue fixed by #640.Root suspect:
internal/lmtp/deliver.gois the only delivery path that does NOT use the single atomicAllocateAndAppend(used by IMAP APPEND, sieve fileinto, and MOVE/COPY). It instead:AllocateUID— separate call, commits and releases the folder lock immediatelybox.Save(...)— for mdbox/sdbox this does map lookup + refcount + possible rotation, measurably slower than maildir's flat-file writeAppendMessage— separate call, re-acquires the lock laterThis leaves a window between steps 1 and 3, open exactly as long as step 2 takes — which is why maildir (fast Save) doesn't reproduce and mdbox/sdbox (slow Save) does.
What's confirmed vs. still open
Confirmed (live sandbox, two independent full-load repro runs): the symptom is real, backend-specific (mdbox/sdbox only), and correlates with concurrent load (imaptest + smoketest running together).
Disproved via targeted unit-test reproduction (all with a real distributed lock server, not a stubbed one):
AllocateAndAppendconcurrency across two independentuserIndexinstances sharing a lock server — 300 concurrent deliveries, 0 collisionsResetFolder'sNextUIDguard (if maxUID >= NextUID) — reads correctly, never regressesmailindex.Recreate's cross-device rename staging — already stages same-directory before the atomic rename, not the bugdeliver.gothree-step pattern itself, replicated with an artificially slow middle step — 5×60 concurrent deliveries, 0 collisionsSo the precise trigger is still unconfirmed — it needs the specific combination of real network/lock latency, real k8s pod scheduling, and many concurrent sub-tests that a Go unit test doesn't reproduce.
This PR
Adds DEBUG-level breadcrumbs bracketing the suspect window with a per-call correlation id (
call_idin deliver.go, matched againstuidin the fileindex-side logs):lmtp: uid allocated(right afterAllocateUID)lmtp: body saved, committing index(right beforeAppendMessage, includessave_ms)lmtp: uid committed/lmtp: index append failed, rolling back savefileindex: uid allocated(insideAllocateUID, includes the folder'sNextUIDstate)fileindex: committing pre-allocated uid(insideAppendMessage, includesnext_uid_before— the smoking-gun field: if a commit'suidis less than the folder'snext_uid_beforeat commit time, that's this bug caught in the act)Purely additive logging behind the existing
LOG_LEVEL=debuggate — no behavior change. Next step is a live repro run under this build to pin the exact interleaving.