Skip to content

test(hosting): fix two races in the row-by-row job output test - #7

Merged
pdevito3 merged 1 commit into
mainfrom
fm/bw-flaky-wake-test
Sep 27, 2026
Merged

pdevito3 merged 1 commit into
mainfrom
fm/bw-flaky-wake-test

Conversation

@pdevito3

Copy link
Copy Markdown
Contributor

Summary

OversizedOutput_OnARowByRowStore_ReAppliesTheBatchOnceAndCountsNoFenceViolation had two races. Both are in the test. Neither is a product defect: Wake-Up Hints are only an earlier poll, and the log order is correct.

Race 1 - the Wake error from CI run 36320674772. The host starts the pump on a thread-pool thread. The pump can therefore subscribe to Wake-Up Hints after app.StartAsync() returns, and FaultableStore.Wake then threw.

 test
   app.StartAsync()          # pump starts on a thread-pool thread
   enqueue small-1, big-1
-  store.Wake("default")     # throws if the pump did not subscribe yet
+  await store.WakeAsync("default")
+    wait for SubscribeAsync (10 s cap)
+    send the hint

Race 2 - found by the stress loop after race 1 was fixed. The pump writes the fence-out log (1208) after the store write that makes both jobs terminal. The test read logs.Entries as soon as both jobs were terminal, so it could miss the log.

 pump cycle                          test
   store.ReportOutcomesAsync  ---->   AwaitTerminalAsync returns
   log 1203 (dead-lettered)
   log 1208 (fenced out)
+                                     WaitForAsync(1208 is logged)
                                      Assert.Single(1208)

The wait uses the same pattern that FailStopTriggerTests already uses for 1208. Log 1203 comes before 1208 on the same thread, so this wait covers it too.

No other test calls FaultableStore.Wake. PollCoalescingTests has its own GatedHintStore, and it already waits on store.Subscribed.

Evidence

Stress loop: 4 parallel lanes of dotnet test --filter JobOutputTooLargeTests (all 3 tests in the class), plus 8 busy-spin processes, on a 10-core Mac.

Build Runs Failures Failure message
Before (main at 4a5514f) 200 28 28 x No pump has subscribed to this store's Wake-Up Hints.
Race 1 fix only 200 1 1 x Assert.Single() Failure at the 1208 assert
Both fixes (this PR) 400 0 -

Forced proofs: I added a temporary Thread.Sleep(500) in WorkerGroupService to make each race happen on every run. These delays are not in the PR.

  • Before: delay before the hint subscription: old test fails 5/5 with the Wake error.
    After: same delay: fixed test passes 5/5.
  • Before: delay before the 1208 log: old test fails 5/5 with the Assert.Single error.
    After: same delay: fixed test passes 5/5.

The full BackWave.Hosting.Tests suite passes: 86/86.

Merge Danger

Door: two-way

Only two test files change. No product code changes.

Blast Radius: tests

FaultableStore.Wake is now WakeAsync. The one caller changed with it.

The host starts the pump on a thread-pool thread, so the pump can subscribe to Wake-Up Hints after StartAsync returns. FaultableStore.Wake then threw because no pump had subscribed yet. WakeAsync now waits for the subscription before it sends the hint.

The pump also writes the fence-out log (1208) after the store write that makes both jobs terminal. The test read the logs as soon as both jobs were terminal, so it could miss that log. The test now waits for the log before it asserts on it.
@pdevito3
pdevito3 merged commit cd6fa22 into main Sep 27, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant