test(delivery): fix bus-lag test deadlock via non-blocking observation send (RIG-2514) - #501
Draft
rigel-mintaka wants to merge 2 commits into
Draft
test(delivery): fix bus-lag test deadlock via non-blocking observation send (RIG-2514)#501rigel-mintaka wants to merge 2 commits into
rigel-mintaka wants to merge 2 commits into
Conversation
…n send (RIG-2514) The bus-lag tests (TestBusLagResubscribesAndKeepsDelivering, TestBusLagResubscribeDeliversWindowMessageLive, TestBusLagTriggersSweepNotLoss) flake under cross-test load, hanging the internal/delivery package to the -timeout and reddening CI + the jj-hp pre-push gate ~50% of runs. Reproduced 7/12 on pristine main; each passes in isolation. The test fakes signal each observed dispatch/wake on a buffered recorded channel (cap 1024) with a BLOCKING send. The three tests deliberately flood 1100 > 1024 to force a live-buffer overrun. The recorded fact lands in a mutex-guarded slice before the token send and every waiter re-checks that slice each loop, so once a waiter finds its target it stops draining — the consumer's Run goroutine then wedges on the next blocking send, hanging the package. In isolation the waiter drains fast enough to never saturate; under load the buffer fills and deadlocks. Make the observation send non-blocking at all four identical sites via one documented helper, signalObserved(ch). The token is only a wakeup hint (the fact already lives in the calls slice), so a drop is safe and starvation-free: a drop happens only when the buffer is full, so a blocked waiter always has a token to drain and loop back; when empty the send always lands. Per the no-retries rule this is a determinism fix, not a retry: the observation channel stops exerting backpressure on the code under test. Verified: 20/20 green post-fix (was 7/12 failing); -race suite green; gofmt/vet/golangci-lint clean. Closes RIG-2514. Co-authored-by: Matt Wilkinson <matt@rigel.build>
|
Compass engineering docs preview: https://mintaka-rig-2514-flaky-bus-l.compass-eng-docs.pages.dev Deployed from |
…ling (RIG-2514 review low) The three bus-lag tests publish a hardcoded 1100 to overrun the events bus's per-subscriber live-tail buffer (liveBufferCapacity == ringCapacity == 1024), with the two magic numbers sitting in different files and no explicit tie. If the events caps ever rose past 1100 without the flood count following, the overrun would stop firing and the RIG-2514 regression guard would silently degrade to a no-op while the tests kept passing. Extract the flood count to a documented busLagFloodCount constant that states the must-exceed-liveBufferCapacity invariant, and derive all three flood loops from it. No behavior change: still 1100 > 1024, guard still triggers (15/15 green). Addresses the review low on PR #501. Co-authored-by: Matt Wilkinson <matt@rigel.build>
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.
Problem
TestBusLagResubscribesAndKeepsDelivering,TestBusLagResubscribeDeliversWindowMessageLive, andTestBusLagTriggersSweepNotLossingo/internal/delivery/consumer_test.goflake under cross-test load — intermittently hanging the wholeinternal/deliverypackage to the-timeout, which reddens the requiredCIcheck and the local jj-hp pre-pushmoon cigate. Reproduced 7 of 12 consecutivego test -count=1 -timeout 60s ./internal/delivery/runs on pristinemain; each test passes in isolation.Root cause
The test fakes signal each observed dispatch/wake on a buffered
recordedchannel (cap 1024), and the send is blocking (d.recorded <- struct{}{}). The three bus-lag tests deliberately flood 1100 > 1024 messages to force a live-buffer overrun. The recorded fact is appended to a mutex-guarded slice before the token send, and every waiter (waitForMessage/waitForDispatches/waitForWakes/waitFor) re-checks that slice each loop — so once a waiter finds its target it stops drainingrecorded. With the buffer full, the consumer'sRungoroutine then wedges on the next blocking send insideDispatchControl, and:...Resubscribes...,afterResubscribenever fires →t.Fatalat the 10s resubscribe assertion;...DeliversWindowMessageLive/...TriggersSweepNotLoss, cleanup'scancel()+<-donenever returns becauseRunis blocked on the send, not selecting on ctx.The stalled goroutine hangs the package to the outer
-timeout. In isolation the waiter drains fast enough that the buffer never saturates; under parallel-package load it does — a classic observation-channel-as-backpressure deadlock.Goroutine dumps confirm the wedge at
helpers_test.god.recorded <- struct{}{}, reached fromsweepSession/ livefanOut, with the test goroutine already past its assertion.Fix
Make the observation send non-blocking at all four identical sites via one documented helper,
signalObserved(ch)(select { case ch <- struct{}{}: default: }). The token is only a wakeup hint — the recorded fact already lives in the fake'scallsslice before the send — so dropping a token is safe and starvation-free: a drop happens only when the buffer is full (non-empty), so a blocked waiter always has a token to drain and loop back to re-check the snapshot; when the buffer is empty the send always lands.Per the house no-retries rule this is a determinism fix, not a retry: the observation channel simply stops exerting backpressure on the code under test.
Verification
mainf97e0438(each apanic: test timed outwithtesting.(*common).Fatalon the stack).go test -tags unix -count=1 -timeout 60s ./internal/delivery/runs green; total wall time for 20 runs dropped from 441s (hangs) to 26s.-racefull delivery suite green;gofmt/go vet/golangci-lintclean.Closes RIG-2514.