Measure the slow-reader forward instead of timing it - #5
Merged
Conversation
This test failed once on a Windows runner, one frame short of four megabytes after a full minute, having done the same transfer in a second on that runner the run before and in 0.06s on a laptop. It was skipped there pending an explanation. The explanation is that the test could not give one. It set a single deadline for the whole transfer, so "the stream broke" and "this machine had a bad minute" produced the same failure and the same message. The evidence read like a stall — 32KB missing, which is exactly one frame — but a transfer that is uniformly slow ends one frame short too, and nothing in the output distinguished them. Measurements of the read pattern say the reads are buffer-bound at 4096 bytes and always full, so a minute for four megabytes means every read waited about 58ms: the pipeline was crawling for the whole minute, not stuck at the end. What this test asserts is that a slow reader is never reset — that bytes keep arriving. So the deadline now sits between reads, where that claim lives, and a failure says how long nothing arrived and how far in it happened. A slow machine finishes late and passes. A stream that actually stops fails in twenty seconds and says so. That is enough to run it on Windows again, which is the point: skipped, it could never tell us anything. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
This test failed once on a Windows runner, one frame short of four megabytes
after a full minute — having done the same transfer in a second on that same
runner the run before, and in 0.06s on a laptop. It was skipped there pending an
explanation. The explanation is that the test could not give one.
Why the evidence was unreadable
It set one deadline for the entire transfer, so "the stream broke" and "this
machine had a bad minute" produced the same failure with the same message.
The 32KB shortfall looked like a smoking gun — exactly one frame — but a
transfer that is uniformly slow also ends one frame short when an absolute
deadline expires. Nothing in the output told the two apart.
Measuring the read pattern settles which it was: the reads are buffer-bound at
4096 bytes and always come back full (1025 reads for 4MB, the floor). A minute
for that transfer means every read waited ~58ms — the pipeline was crawling for
the whole minute, not stuck at the end.
The change
What the test asserts is that a slow reader is never reset — that bytes keep
arriving. The deadline now sits between reads, which is where that claim lives:
nothing arrived and how far into the transfer it happened.
That is enough to run it on Windows again, and this drops the last entry from
that job's skip list. Skipped, it could never tell us anything.
Not in this PR
The investigation also turned up a genuine way for a forward to hang — the relay
retiring one without telling either end — which is #4. That is a real bug and
was not the cause of this.
🤖 Generated with Claude Code