Skip to content

deps: bump moq-noq for the lost handshake ACK fix - #5457

Draft
kixelated wants to merge 2 commits into
mainfrom
quest/m1/test-flakes-2/handshake-ack
Draft

kixelated wants to merge 2 commits into
mainfrom
quest/m1/test-flakes-2/handshake-ack

Conversation

@kixelated

@kixelated kixelated commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

Quest: quest/m1/test-flakes-2/handshake-ack.md (deleted here).

Draft until moq-dev/noq releases 2.0.5. The fix is in moq-dev/noq#38. This PR still needs the pin bump, below.

Problem

MOQ_SHAPER_SEED=13876846873975217772 cargo test -p moq-relay --test drills bursts_cross_a_cluster::impaired fails every run on main with publisher connect timed out (2 of 2 today). The client's ACKs of the server's first Initial are all lost, and noq writes an ACK only while PendingAcks::can_send() holds, so the client's probes in between carry none. The server keeps resending its two-datagram flight, the shaper drops the tail each time, and the 10s connect budget runs out. The quest has the full trace.

Fix

moq-dev/noq#38 makes every Initial and Handshake packet repeat the pending ACK ranges, the handshake half of what quinn-rs/quinn#2747 does (and moq-quic already has). With it patched in locally:

  • The failing seed passes 3 of 3 runs, 6.3 to 6.8s.
  • 50 random seeds pass, 50 of 50.

This PR adds the same mock-time test to moq-quic, lost_handshake_ack_repeats_in_next_packet, so the switch off noq keeps the behavior. It passes on moq-quic as is and fails if can_send_with_other_frames returns false.

The PTO cascade the quest asked about (an ACK resets pto_count while the Handshake PTO deadline is already past, and noq refires it in one handle_timeout call, 6 times and 12 probes in the trace) is left alone. moq-quic's handle_timeout fires each timer at most once per call, so it does not cascade, and the switch removes it.

Remaining

Once moq-noq-proto 2.0.5 is on crates.io:

  1. Cargo.toml: moq-noq-proto and web-transport-moq to 2.0.5.
  2. cargo update -p moq-noq-proto -p moq-noq -p moq-noq-udp -p web-transport-moq.
  3. Rerun the seed above, then mark ready.

API: none. Wire: none (noq may now include an ACK frame in an Initial or Handshake packet that previously omitted it).

Tests

  • cargo test -p moq-quic: 447 + 3 passed. cargo clippy -p moq-quic --tests: clean.
  • The drill runs above, with moq-noq-proto patched to the noq#38 branch.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

kixelated and others added 2 commits October 13, 2026 13:54
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
moq-quic already bundles pending ACK ranges into packets carrying other
frames. Pin that for the handshake, so the switch off noq keeps the fix
moq-dev/noq#38 ports for the impaired cluster drill.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title fix(relay): a lost handshake ACK no longer stalls the impaired cluster drill deps: bump moq-noq for the lost handshake ACK fix Oct 13, 2026
@kixelated

Copy link
Copy Markdown
Collaborator Author

Outcome: the fix is moq-dev/noq#38 (draft), which repeats pending Initial and Handshake ACK ranges in every packet. Patched in, the failing seed passes 3 of 3 and 50 of 50 random seeds pass. This PR carries the matching moq-quic regression test and the quest deletion; it needs the 2.0.5 pin bump once noq is released.

Open decisions for the maintainer:

  1. Backport Relay support #38 to noq 1.3.x? Recommend no: Implement (forked) moq-transport-00 #34 was not backported either, and this repo pins 2.0.x.
  2. The PTO cascade was left unfixed in noq, reading the quest's "fix it only if moq-quic does the same" as "only if moq-quic also cascades". It does not, since quinn's handle_timeout fires each timer once per call. If the intent was instead "make noq match moq-quic", it is a small change in noq's handle_timeout loop. Recommend leaving it to the switch.

(Written by Claude Opus 5.5)

This branch has not been deployed

No deployments
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.

1 participant