GH-4100: bring the Pulsar reliability test's timeout back inside the CI job cap - #4101
Merged
Merged
Conversation
…CI job cap
run_setup_with_simulated_exception_in_handler waited on its tracked session for
TimeSpan.FromSeconds(1000) -- 16m40s, against this workflow's 20 minute
timeout-minutes cap. That timeout can essentially never fire in CI:
* the job starts, so the cap lands 20 minutes later
* CIPulsar spends ~6.5 minutes building and standing up the broker before the
first test runs
* so a test starting then would have to finish inside ~13 minutes, and 1000s
does not fit
On run 32754218016 the arithmetic was exact: cap at 17:35:32, the test started
at 17:22:11, and its own timeout would have fired at 17:38:51 -- nearly three
minutes after the job was already dead.
The consequence is not a slow test. It is that when the underlying Pulsar
flakiness hits (GH-4100: producers and consumers stalling in
DotPulsar.Internal.Producer.Setup, every thread parked), the test cannot fail.
The job runs to the cap and is CANCELLED instead, and a cancelled job's logs are
discarded outright -- BlobNotFound, see GH-4098 -- so the failure leaves no
evidence at all. That has now happened twice, on unrelated branches.
100 seconds is what every other test in this file uses (lines 129, 187, 216) for
the same shape of assertion, and the retry schedule under test is 4 + 2 + 3
seconds plus a dead-letter hop. This was the only
TrackActivity(TimeSpan.FromSeconds(1000)) in the repository.
Measured against a local broker before changing it: the test passes in ~12.8s,
so 100s leaves roughly 8x headroom.
This does NOT fix the wedge. It converts a 20 minute silent cancellation into a
100 second failure with a tracking dump, which is the difference between a bug
that can be diagnosed and one that cannot.
Refs #4100. Refs #4098.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Member
Author
|
All five runs finished after the PR was opened — adding the last two for completeness. Passes are unanimous and the spread is 66ms across the set: 12.81–12.87s against a 100s budget — ~8x headroom, and none of the five came close to the flakiness described in #4100, so the local runs say nothing about how often the stall hits. |
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.
One character. Closes the first half of #4100.
PulsarNativeReliabilityTests.run_setup_with_simulated_exception_in_handlerwaited on its trackedsession for
TimeSpan.FromSeconds(1000)— 16m40s, against this workflow's 20 minutetimeout-minutescap.That timeout can essentially never fire in CI
CIPulsarspends ~6.5 minutes building and standing up the broker before the first test runsOn run 32754218016 the arithmetic
was exact:
Why that matters
The consequence is not a slow test. It is that when the underlying Pulsar flakiness hits — #4100:
seven producers and three consumers stalled in
DotPulsar.Internal.Producer/Consumer.Setup, all 18threads parked — the test cannot fail. The job runs to the cap and is
cancelledinstead, and acancelled job's logs are discarded outright (
BlobNotFound, #4098), so the failure leaves noevidence at all.
That has now happened twice, on unrelated branches, three seconds apart in duration (1218s / 1221s).
This PR does not fix the wedge. It converts a 20 minute silent cancellation into a 100 second
failure with a tracking dump — the difference between a bug that can be diagnosed and one that
cannot. The stall itself is the other half of #4100.
Why 100
TimeSpan.FromSeconds(100)— lines 129, 187, 216 — for thesame shape of assertion
4 + 2 + 3seconds plus a dead-letter hopTrackActivity(TimeSpan.FromSeconds(1000))in the repositoryMeasured, not assumed
The risk in this change is that the test genuinely needs more than 100s. It does not — run against a
local broker on
:6650:~12.8s, very tightly clustered, so 100s leaves roughly 8× headroom — and 1000s was ~78×.
Worth considering separately
Should any tracked-session timeout be allowed to exceed the job cap? One that does can only ever
produce a cancellation instead of a failure. This was the only 1000s case, but the general rule may
be worth enforcing. Raised in #4100, not addressed here.
Refs #4100. Refs #4098.
🤖 Generated with Claude Code