test(hosting): fix two races in the row-by-row job output test - #7
Merged
Merged
Conversation
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.
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.
Summary
OversizedOutput_OnARowByRowStore_ReAppliesTheBatchOnceAndCountsNoFenceViolationhad 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, andFaultableStore.Wakethen threw.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.Entriesas 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
FailStopTriggerTestsalready uses for 1208. Log 1203 comes before 1208 on the same thread, so this wait covers it too.No other test calls
FaultableStore.Wake.PollCoalescingTestshas its ownGatedHintStore, and it already waits onstore.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.mainat 4a5514f)No pump has subscribed to this store's Wake-Up Hints.Assert.Single() Failureat the 1208 assertForced proofs: I added a temporary
Thread.Sleep(500)inWorkerGroupServiceto make each race happen on every run. These delays are not in the PR.After: same delay: fixed test passes 5/5.
Assert.Singleerror.After: same delay: fixed test passes 5/5.
The full
BackWave.Hosting.Testssuite passes: 86/86.Merge Danger
Door: two-way
Only two test files change. No product code changes.
Blast Radius: tests
FaultableStore.Wakeis nowWakeAsync. The one caller changed with it.