perf(pool): move the claims-journal write off the manager mutex (F1)#6
Merged
Conversation
claimStore.save marshaled and wrote+renamed the whole claim map while the caller held m.mu, so every claim/release/wake/hibernate/reap stalled any concurrent data-plane m.mu acquire for the syscall duration (measured 1.2-1.9ms at 100-1000 live claims). Split it: snapshot() marshals under m.mu and stamps a sequence; commit() writes off the mutex, serialized and coalescing (an older sequence never overwrites a newer one). The six hot sites marshal-under-lock, unlock, commit; rollback paths re-snapshot and re-commit so disk converges to the rolled-back set.
Adversarial review (an agent + codex) found the six-site write split sound (coalescing, lock order, seq/written races all verified) with one real edge: a rolled-back snapshot's best-effort re-commit was discarded, so a double I/O fault with a concurrent successful commit between them could leave disk stuck disagreeing with memory until an unrelated later write. Retry the re-commit so a transient fault converges and a coalesced result counts as done; a persistent failure warns (memory authoritative + Reconcile on start). Also correct save()'s doc: Reconcile holds m.mu (startup, uncontended).
Contributor
Author
Bare-metal F1 proof (.79, Ryzen 9700X, sandbox
|
| live claims | under-lock (old save) |
off-lock (new split) | win |
|---|---|---|---|
| 100 | 38882 ns/acquire | 226.8 ns/acquire | 171× |
| 1000 | 53926 ns/acquire | 796.4 ns/acquire | 68× |
BenchmarkStoreSaveScaling (persist cost): 16.2µs / 65.7µs / 547.7µs at 10 / 100 / 1000 claims. Also re-ran the full bare-metal e2e regression + archive-lifecycle e2e on .79 against sandbox main — both PASS (warm 0.2–0.5ms, clone 39.8ms, archive wake 37ms). The data-plane stall is smaller in absolute terms on bare-metal NVMe than the mac APFS audit estimate, but the before/after on one harness cleanly shows the m.mu contention removed; it grows with claim count and slower substrates. Ready for merge review.
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.
F1 — claims-journal write off
m.muclaimStore.savemarshaled + wrote + renamed the entire claim map while the caller heldm.mu, at every claim/release/wake/hibernate/reap.m.muis the same mutex the data plane takes per exec RTT, so a concurrent claim/wake injected a 1.2–1.9ms stall into every racing data-plane op at 100–1000 live claims (perf audit F1).Change
claimStoresplits the write:snapshot()marshals underm.mu+ stamps a monotonic sequence;commit()does write+rename offm.mu, serialized by its own mutex and coalescing — an older sequence never overwrites a newer one (sequence order == mutation order, so the newest write wins).save()wrapper kept for the one non-hot caller (Reconcile, single-threaded startup).Tests
TestClaimStoreCommitCoalesces(stale snapshot is a no-op) +TestClaimStoreConcurrentCommits(writer off-mutex, -race).go test -race ./...green;golangci-lint0 (linux+darwin).Durability-core change. Pending: (1) an adversarial review pass, (2) a .52/.79 hardware bench confirming the contention win and no regression (the perf-audit acceptance). Opened now for review while .52 is unreachable.