Skip to content

test: make the concurrency tests independent of the runner's load - #302

Open
CaffeinatedCoder wants to merge 1 commit into
EntglDb:mainfrom
CaffeinatedCoder:test/ci-load-flakes
Open

CaffeinatedCoder wants to merge 1 commit into
EntglDb:mainfrom
CaffeinatedCoder:test/ci-load-flakes

Conversation

@CaffeinatedCoder

@CaffeinatedCoder CaffeinatedCoder commented Oct 17, 2026 •

Copy link
Copy Markdown
Collaborator

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 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, other classes block threads, every class commits to the same disk) they fail with a TimeoutException or 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.

Test Failed runs What the runner did
CrossCollectionWriteRaceTests.Concurrent_Inserts_Into_Different_Collections_LoseNothing 3 (2 on main) Timed out acquiring collection lock (Insert): three writers share the file-wide write lock, each holds it through its commit
CdcScalabilityTests.Test_Cdc_Slow_Subscriber_Does_Not_Block_Others 3 (1 on main) the event was not delivered within the 200 ms Task.Delay
IndexPageReleaseTests.DynamicCollection_DropIndex_WaitsForTheCollectionLock 2 the Task.Delay(100) continuation ran after the drop had timed out at 500 ms
IndexPageReleaseTests.CreateDropIndexCycles_{Typed,Dynamic}_DoNotGrowTheFile 1 Timed out acquiring commit lock (GroupCommit)
ConcurrentConsistencyTests.ConcurrentInsertAndRead_8Threads_NoCorruption 2 (main) Reader completed 0 reads: the writers finished before a reader task was scheduled
WalLockTimeoutTests.WriteBeginRecord_ThrowsTimeoutException_WhenLockIsHeld 1 (main) the 100 ms timeout returned after 770 ms (< TestWriteTimeoutMs * 5)
WalLockTimeoutTests.CancellationToken_AbortsBeforeTimeout 1 the 10 ms token lost against the 100 ms timeout: TimeoutException instead of OperationCanceledException

(The other failed runs were PR-specific or are not timing: an oracle seed on #250, Spatial_DeleteHalf_OfASplitTree_ReturnsExactlyTheSurvivors once on #280, and HighContention_ManySmallTransactions_ConsistentFinalState once on main with a document whose t field did not match its collection, which this PR does not touch; see below.)

What changes

No library code. tests/BLite.Tests/LoadTolerant.cs holds 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. SingleFile and Server(path) are the two PageFileConfig presets with it.
  • LoadTolerant.WaitUntilAsync(condition, what): polls every 5 ms for up to 10 s and fails naming what, instead of sleeping a fixed time and hoping the background work is done by then.

Per test:

  • CrossCollectionWriteRaceTests opens with LoadTolerant.SingleFile.
  • ConcurrentConsistencyTests opens with LoadTolerant.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 and DropIndex_WaitsForTheCollectionLock open with LoadTolerant.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 is TestWriteTimeoutMs + 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 a ManualResetEventSlim the test opens, instead of Thread.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

  • The five classes (31 tests), 5 runs in a row with DOTNET_PROCESSOR_COUNT=2 while two full copies of the suite ran alongside as load: 155/155 passed.
  • Full suite with the change, two copies at once with DOTNET_PROCESSOR_COUNT=2: 3092 passed, 0 failed, 4 skipped, each in about 5 minutes.

Connected

🤖 Generated with Claude Code

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>

@mrdevrobot mrdevrobot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

1 active deployment
Production — 2f51fc28 Deployed Oct 17, 2026 by CaffeinatedCoder via build_and_publish #762
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package/core Affects BLite.Core tests Adds or changes tests only (oracles, harnesses); no library code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants