Repository navigation
fix(storage): raise LockTimeout.Default to 5 s so writers queued behind slow commits do not time out (#303) - #321
Closed
CaffeinatedCoder wants to merge 1 commit into
Conversation
…nd slow commits do not time out (EntglDb#303) A writer waits for the I/O of every writer queued ahead of it: an auto-committing write holds the collection lock until its commit is in the WAL, and a checkpoint holds the page-file write lock while it syncs the data file. With 500 ms, a healthy but slow disk (and every commit once EntglDb#280 syncs the WAL) turned those waits into TimeoutException and a rolled-back write. LockTimeout.Default is now 5 s read / 5 s write, the same as the WAL's own lock timeout, so a timeout means a stuck writer, not a slow one. Immediate and From() are unchanged; PageFileConfig.LockTimeout already reaches every lock, which the new tests pin. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This was referenced Oct 18, 2026
Closed
Contributor
|
Closing. LockTimeout.Default stays at 500 ms: per-commit fsync will be opt-in (see the review on #280), and with the default unchanged the 500 ms value holds. A longer timeout also delays the detection of a real deadlock tenfold. Applications that opt into full sync can raise the timeout through PageFileConfig.LockTimeout. #322 (an object-initializer config ending up with 0 ms) is a separate bug and stays open. |
This branch was successfully deployed
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.
What was wrong
LockTimeout.Defaultgave every lock 500 ms (#303). A writer does not only wait for other writers' CPU work: an auto-committingInsertAsyncholds the collection lock until its commit is in the WAL (in single-file mode the typed collections share one lock for the whole file,StorageEngine.CreateCollectionWriteLock), and a checkpoint holds the page-file write lock while it syncs the data file (PageFile.Flush). So the second, third, ... writer in the queue waits for one commit's I/O per writer ahead of it. When those add up to more than 500 ms the write fails withTimeoutException("Timed out acquiring collection lock (Insert)", "commit lock (GroupCommit)", "PageFile write lock (AllocatePage)") and is rolled back. That is a healthy database being slow, and the application cannot tell it from a deadlock.On
mainthis already shows on shared CI disks (the flakes #302 works around in the tests). With #280 every commit batch syncs the WAL (4.5 ms mean / 7 ms p99 on Apple Silicon, tens of ms on a consumer SSD under load, hundreds when a cloud disk stalls), and the 500 ms budget becomes routine to exceed.Mechanism
Eight writers on one collection, each commit taking 150 ms to reach the WAL: writer k waits about (k−1) × 150 ms for the collection lock. On
mainwriters 5 to 8 wait more than 500 ms and throw:The async write paths wait with
SemaphoreSlim.WaitAsync, which blocks no thread, so the doc comment's argument for a short timeout (not starving the thread pool) does not apply to them.Fix
Option 1 of the issue:
LockTimeout.Defaultis now 5 s read / 5 s write (MaxConcurrentWritersstays 96). 5 s is what the WAL's own lock already uses (WriteAheadLog,MemoryWriteAheadLog, the WASM WALs all default towriteTimeoutMs = 5_000). The doc comment says why, and that aTimeoutExceptionnow means a stuck writer (a deadlock or a hung disk), not a slow one.Immediateis unchanged for callers who wantSQLITE_BUSYbehaviour, andLockTimeout.From(...)sets anything else.PageFileConfig.LockTimeoutdoc says which presets carry the default.PageFileConfig.LockTimeoutalready reaches every lock that waits with a lock timeout. The test below pins that, so a consumer can rely on raising or lowering it per engine.StorageEngine.Disposethat quoted "WriteTimeoutMs (500 ms)" now says "(then 500 ms)".I left the exception type (
TimeoutException) and the messages as they are: catch sites keep working, and the messages already name the lock. If you would like the messages to also carry the waited time and point atPageFileConfig.LockTimeout, I can do that as a follow-up; it touches ~80 throw sites.Option 2 of the issue (start the waiter's clock after the holder's sync) is more machinery for little gain over a budget no healthy disk reaches.
What still times out, and on what
DocumentCollection,DynamicCollection), commit lock, metadata lock, page-file read/write/async locks, WAL lock, cross-process writer lock (.wal-shm)ReadTimeoutMs, 5 s)PageFileConfig.LockTimeoutWriteTimeoutMs / 16= 312 ms (was 31 ms)PageFileConfig.LockTimeoutWriteTimeoutMs / 16, at least 10 msPageFileConfig.LockTimeoutBeginTransaction(skips the replay, no exception)ReadTimeoutMsPageFileConfig.LockTimeoutStorageEngine(IPageStorage, IWriteAheadLog))PageFileConfig.Defaultand the WAL's constructor defaultWalSharedMemoryopen lockOpenLockTimeoutMsTests
New
LockTimeoutDefaultTests:Default_IsFiveSecondsForReadsAndWrites:LockTimeout.Default == From(5000, 5000, 96).Presets_AndDetectedConfigs_UseTheDefault:Small,Default,Large,Server(path),DetectFromFilefor a Small and a Large file, and the in-memory engine.ConfiguredTimeout_ReachesEveryLock: a Server-layout engine opened withLockTimeout.From(1234, 2345, 7)hands it to the WAL, the main, index and per-collection page files, the admission gate, aDynamicCollectionand aDocumentCollection.CommitLock_WaitsForTheConfiguredWriteTimeout: with the commit lock held and a 150 ms timeout, an insert fails with "commit lock" after 100 ms to 3 s, not after the 5 s default.WritersQueuedBehindSlowCommits_DoNotTimeOut: 8 writers on one collection of an in-memory engine whose WAL flush takes 150 ms (a wrapper aroundMemoryWriteAheadLog). The slowest writer must wait more than 500 ms (so the scenario exceeds the old default), and all 8 inserts succeed.IndexPageReleaseTests.Dispose_WaitsForThePendingFreeCheckpoint_LongerThanTheLockTimeoutasserted that the default write timeout is at most 1 s (it waits 3 × the timeout); it now opens withLockTimeout.From(500, 500)and tests the same thing.On
upstream/main(this PR's tests,LockTimeout.csreverted):Default_IsFiveSecondsForReadsAndWritesandWritersQueuedBehindSlowCommits_DoNotTimeOutfail (the latter with theTimeoutExceptionabove, after 609 ms); the other three pass, as they should, because the setting already flows.Mutations, each caught by the named test:
LockTimeout.Default.WriteTimeoutMson the commit lockCommitLock_WaitsForTheConfiguredWriteTimeout(waited 5 s)DynamicCollection.WriteLockTimeoutMsreturns the defaultConfiguredTimeout_ReachesEveryLockDocumentCollection.WriteLockTimeoutMsreturns the defaultConfiguredTimeout_ReachesEveryLock5_000instead of the configConfiguredTimeout_ReachesEveryLockAsStandaloneConfig(index and collection files) resetsLockTimeoutto the defaultConfiguredTimeout_ReachesEveryLockConfiguredTimeout_ReachesEveryLockFull suite,
dotnet test tests/BLite.Tests(net10.0, macOS / Apple Silicon): 3097 passed, 0 failed, 4 skipped, 2 m 2 s. Two existing tests hold the metadata lock to provoke a timeout (VacuumTests.Vacuum_RebuildIndexes_DocumentCollection_FailedSwap_KeepsIndexAndFreesCopy,IndexBackfillTests.AddedIndex_WhoseMetadataCannotBeSaved_IsNotRegistered_AndItsPagesAreFreed); they now wait 5 s each instead of 0.5 s, included in that time.With #280
#280 makes these waits longer (one WAL sync per commit batch), so this PR should land before it or with it. The two compose:
fix/wal-fsync-on-commitmerges into this branch without conflicts (git auto-mergesPageFile.csandStorageEngine.cs; the two PRs change different lines there). On that throwaway merge, the full suite gives 3103 passed, 0 failed, 4 skipped (this PR's tests plus #280'sWalCommitSyncTests). That run took 12 m 3 s against 2 m 2 s here. The difference is #280's per-commitfsyncon a disk other processes were loading at the time: the test host ran at 23% CPU, and stack samples showed threads inWriteAheadLog.FlushAsync→fsync, a different thread on each sample. Nothing timed out and nothing hung.WritersQueuedBehindSlowCommits_DoNotTimeOutsimulates the sync with a delay, so it tests the slow-disk case on any machine, with or without #280.Compatibility
LockTimeout.Defaultand of every preset that uses it change from 500 ms to 5 s.PageFileConfig.Default with { LockTimeout = LockTimeout.From(500, 500) }restores the old behaviour.ReaderWriterLockSlim, metadata lock, the multi-process catch-up inBeginTransaction) can now hold a thread up to 5 s instead of 0.5 s under contention. Under contention that long, the 500 ms default would have thrown.Fixes #303
🤖 Generated with Claude Code