Skip to content

devstack front door: a read's test fails only on bytes that came early - #802

Merged
futurepaul merged 1 commit into
masterfrom
claude/front-read-seen-in-flight
Oct 31, 2026
Merged

futurepaul merged 1 commit into
masterfrom
claude/front-read-seen-in-flight

Conversation

@futurepaul

Copy link
Copy Markdown
Collaborator

front::tests::a_read_is_seen_before_it_is_passed_on failed #783's check (job). The code under test was fine; the test was too strict.

Why it failed

The test drained the far end inside seen and required arrived == passed.

On a loaded runner, the sender held small writes while waiting for an ACK. arrived sat at 22,341 bytes for 60 callbacks while passed reached 58,671. The bytes were late, not early, so the code under test was fine.

Why arrived ≤ passed is enough

The property the test guards, from Alex's review of #748, is that a read is recorded before it is passed on. At seen time, the far end must hold none of that read. In terms of the counters, that is arrived ≤ passed. The test now fails only when arrived > passed.

Checks

🤖 Generated with Claude Code

a_read_is_seen_before_it_is_passed_on asked that the far end hold every
byte passed on before each read. A sender may hold small writes while an
ACK is out (CI held 36 KB, #783's check), so it failed with bytes late,
not early. It now fails only when the far end holds more than was seen:
what the property is, and what the bug of #748 does (checked by passing
on before seeing: it fails at once).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@futurepaul
futurepaul requested a review from alexlwn123 October 31, 2026 06:15
@futurepaul

Copy link
Copy Markdown
Collaborator Author

Checked against what I meant the test to guard (I wrote it in #748): this is right, and my != was too strict.

  • With seen before write_all, none of a read's bytes can be out when it's seen, so arrived ≤ passed always holds. The devstack: the front door logs each request and its answer #748 bug (write, then see) is exactly what makes arrived > passed possible. Late bytes (arrived < passed) were never the property: the test's raw pair has no TCP_NODELAY, unlike pass's sockets, so Nagle can hold small writes while an ACK is out, as CI showed.
  • The closing check that every byte passed on arrives ((passed, arrived) == (sent, sent)) still holds, so nothing is lost.
  • Sensitivity: on a runner where the early bytes haven't landed yet when seen runs, the bug could slip by in one run. each_request_is_logged_with_its_own_answer guards the same ordering at the connection level and failed 5/5 with the bug, so the pair still catches it.

Nothing to change.

@alexlwn123 alexlwn123 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pass at a1c363c8402b9b7040b07d1d4e925df8c023603b.

The test now permits earlier writes still in flight while rejecting bytes arriving before their read is recorded. The final byte-count assertion still checks complete delivery. This is a suitably small correction with no runtime changes or added machinery; I found no blocking regression.

Validation: cargo test -p fragment-devstack --lib front::tests passed all five tests. CI is still incomplete; e2e shard 4 failed before building or testing because downloading the Rust stable manifest timed out, unrelated to this change. Full e2e was not run locally.

@futurepaul
futurepaul added this pull request to the merge queue Oct 31, 2026
Merged via the queue into master with commit 90fa6b8 Oct 31, 2026
24 of 26 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.

2 participants