Repository navigation
test: make the concurrency tests independent of the runner's load - #302
Open
CaffeinatedCoder wants to merge 1 commit into
Open
CaffeinatedCoder wants to merge 1 commit into
CaffeinatedCoder wants to merge 1 commit into
Conversation
Seven of the sixteen failed CI runs since 2026-10-13 failed on tests that
check correctness under concurrency but were written against timing: the
500 ms default lock timeout, a fixed Task.Delay, or an upper bound on how
late a timer may fire. On a loaded runner (xunit runs the classes in
parallel on a few cores) those tests fail with a TimeoutException or a
count of zero that says nothing about what they check. Each red check then
has to be opened to see that the PR is not at fault.
- LoadTolerant: a LockTimeout of 30 s (a deadlock still fails, a slow
commit does not), PageFileConfig presets with it, and WaitUntilAsync,
which polls a condition instead of sleeping a fixed time.
- CrossCollectionWriteRaceTests: three writers share the file-wide write
lock and each holds it through its commit; opens with LoadTolerant.
Failed 3 times ("Timed out acquiring collection lock (Insert)").
- ConcurrentConsistencyTests: same timeout for the Server layout; each
reader reads at least once, since on a loaded runner the writers can
finish before a reader task is scheduled ("Reader completed 0 reads").
- IndexPageReleaseTests: the create/drop cycle tests ("Timed out acquiring
commit lock (GroupCommit)") and DropIndex_WaitsForTheCollectionLock,
whose Task.Delay(100) continuation ran after the drop had already timed
out at 500 ms.
- WalLockTimeoutTests: the upper bound on a timed-out wait allows 2 s of
timer lateness (770 ms seen in CI for a 100 ms timeout) and still tells
100 ms from the 5 s default; the cancellation test uses a 10 s timeout
so the 10 ms token wins by a margin.
- CdcScalabilityTests: the slow subscriber blocks on a gate the test opens
instead of sleeping 2 s, and every wait polls for the delivery it
expects; the ordering no longer depends on 200 ms delays.
No library code changes.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This was referenced Oct 17, 2026
mrdevrobot
requested changes
Oct 18, 2026
mrdevrobot
left a comment
Contributor
There was a problem hiding this comment.
Changes requested. The individual changes are sound and the CDC gate is better than the sleeps. But this moves five classes to 30 s locks and leaves the same timeout family in the rest of the suite: under load we still get PageFile write-lock (AllocatePage) timeouts in BTreeSplitTests, BTreeRollbackFreeTests, StringIndexPrefixMatch and others, plus DropIndex_WhileInserting in a class this PR edits. Use one shared load-tolerant timeout for the test suite instead of per-test values, and cover MetricsTests.WatchMetrics_EmitsSnapshots and VacuumTests Spatial_FailedRebuild. Also: the readers' do/while lets "every reader did work" pass on a single read made after the writers finish.
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.
Why
16 of the last 80 runs of the test workflow failed. 7 of them failed on tests that check correctness under concurrency but were written against timing: the 500 ms
LockTimeout.Default, a fixedTask.Delay, or an upper bound on how late a timer may fire. On a loaded runner (xunit runs the classes in parallel on a few cores, other classes block threads, every class commits to the same disk) they fail with aTimeoutExceptionor a count of zero that says nothing about what they check, and every red check has to be opened to see that the PR is not at fault. Today that is the case for #285, #278, #253, #246 and #245.CrossCollectionWriteRaceTests.Concurrent_Inserts_Into_Different_Collections_LoseNothingmain)Timed out acquiring collection lock (Insert): three writers share the file-wide write lock, each holds it through its commitCdcScalabilityTests.Test_Cdc_Slow_Subscriber_Does_Not_Block_Othersmain)Task.DelayIndexPageReleaseTests.DynamicCollection_DropIndex_WaitsForTheCollectionLockTask.Delay(100)continuation ran after the drop had timed out at 500 msIndexPageReleaseTests.CreateDropIndexCycles_{Typed,Dynamic}_DoNotGrowTheFileTimed out acquiring commit lock (GroupCommit)ConcurrentConsistencyTests.ConcurrentInsertAndRead_8Threads_NoCorruptionmain)Reader completed 0 reads: the writers finished before a reader task was scheduledWalLockTimeoutTests.WriteBeginRecord_ThrowsTimeoutException_WhenLockIsHeldmain)< TestWriteTimeoutMs * 5)WalLockTimeoutTests.CancellationToken_AbortsBeforeTimeoutTimeoutExceptioninstead ofOperationCanceledException(The other failed runs were PR-specific or are not timing: an oracle seed on #250,
Spatial_DeleteHalf_OfASplitTree_ReturnsExactlyTheSurvivorsonce on #280, andHighContention_ManySmallTransactions_ConsistentFinalStateonce onmainwith a document whosetfield did not match its collection, which this PR does not touch; see below.)What changes
No library code.
tests/BLite.Tests/LoadTolerant.csholds what the tests share:LoadTolerant.LockTimeout: 30 s read and write. A deadlock still fails the test; a commit waiting for the disk does not.SingleFileandServer(path)are the twoPageFileConfigpresets with it.LoadTolerant.WaitUntilAsync(condition, what): polls every 5 ms for up to 10 s and fails namingwhat, instead of sleeping a fixed time and hoping the background work is done by then.Per test:
CrossCollectionWriteRaceTestsopens withLoadTolerant.SingleFile.ConcurrentConsistencyTestsopens withLoadTolerant.Server; each reader reads at least once (do … while), so the assertion that every reader did work holds whenever the reads were correct.IndexPageReleaseTests: the two cycle tests andDropIndex_WaitsForTheCollectionLockopen withLoadTolerant.SingleFile. The latter still checks that the drop has not completed after 100 ms while the lock is held, which is true however late the continuation runs.WalLockTimeoutTests: the upper bound on a timed-out wait isTestWriteTimeoutMs + 2000, which allows the lateness seen in CI and still tells 100 ms from the 5 s default; the cancellation test configures a 10 s timeout so the 10 ms token wins by a margin.CdcScalabilityTests: the slow subscriber blocks on aManualResetEventSlimthe test opens, instead ofThread.Sleep(2000), so "the fast subscriber received the second event while the slow one is still in its first" is a fact, not a race against 200 ms; every wait polls for the delivery it expects. The 1000-subscriber test polls too.Checked
DOTNET_PROCESSOR_COUNT=2while two full copies of the suite ran alongside as load: 155/155 passed.DOTNET_PROCESSOR_COUNT=2: 3092 passed, 0 failed, 4 skipped, each in about 5 minutes.Connected
TimeoutException, and fix(wal): sync the WAL to disk before a commit is acknowledged #280 makes the sync real. Filed as A commit that waits for another commit's disk sync fails with TimeoutException after 500 ms: LockTimeout.Default is tuned for an unsynced WAL #303; this PR deliberately does not change the default.OracleSeedsTests.Seed_AppliesToTheCurrentTree(a seed patch that no longer applies after the restack), which is that PR's to fix.Spatial_DeleteHalf_OfASplitTree_ReturnsExactlyTheSurvivorsfailed once on fix(wal): sync the WAL to disk before a commit is acknowledged #280 (run 39022045597) with one survivor missing from theWithinresult. 20 runs of that class onmainand 20 on the fix(wal): sync the WAL to disk before a commit is acknowledged #280 branch, both under load, all passed; I have no explanation yet and this PR does not touch it.HighContention_ManySmallTransactions_ConsistentFinalStatefailed once onmain(run 38749678260) at the field check afterFindByIdAsync, not at a timeout. I could not reproduce it in 8 runs under load; it is not a timing assertion and this PR leaves it alone.🤖 Generated with Claude Code