Repository navigation
fix(relay): backport the mesh gossip fixes and relay counters to release - #7108
Conversation
Release already counts announces; render them on /metrics. Drop main's refusal and request metric tests, which cover code release lacks. (cherry picked from commit ec0b3d0) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Port the control-message byte counters: moq_net::stats::{Control, Bytes},
Snapshot::control(), and moq_relay_control_{tx,rx}_bytes_total{tier,class},
counted at the codec for lite 01-07 and IETF drafts 14-22. Release's codec has
no Encoder/Decoder context, so classes come from defaulted crate-private
Encode/Decode hooks, and raw IETF frame reads count through Reader::read_message.
The wire_{tx,rx} counters are not ported: they need #5773's wire registry.
(cherry picked from commit 8ee3c80)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Ports the due/parked pending queues, the per-entry update hold, the read-only readiness check in poll_next/try_next, the allocation-free no-op TableCursor::update, RouteEntry::reach, lazy RouteTable::covering, and TableCursor::best. Release keeps its lexicographic delivery order (due is a BTreeSet of prefixes) and its Option clock; it has no epochs or restarts. (cherry picked from commit 45007f6) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ackport #6423) Ports moq_tokio::Restart / Client::with_restart, the relay's Restart::Wait for cluster dials, the jittered 30s-capped cluster redial, and the cluster docs. The cluster_drain test runs on Tokio's paused clock (release has no moq-clock) in a flat tests/ file; the old empty-URI redial test in goaway_cluster is removed. (cherry picked from commit 4e5c68b) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ort #6427) Adds cluster.max_hops (--cluster-max-hops, MOQ_CLUSTER_MAX_HOPS; 1..=32, default 32) via origin::Config.max_hops: origin::HopLimit. Receive drops longer paths on lite and IETF subscribers, counting release's anonymous-publisher stamp; the egress cap joins release's cursor Horizon so selection falls back to an in-cap route. (cherry picked from commit 1582fdc) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b279140c37
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: b279140
Finding already reported: I agree with the existing P2 draft-14 SUBSCRIBE_ERROR classification finding in rs/moq-net/src/ietf/message.rs:20–23. The subscription-specific refusal is counted as Other; add the draft-14-only classification and a refusal-counter regression test while preserving later drafts' shared error classification. Referencing the existing thread rather than duplicating its inline comment. No additional actionable bugs found.
Direction: The release-specific adaptation is reasonable: selecting an in-cap route before advertising preserves fallback, and separating due/parked announcements avoids repeated full pending scans. Keeping these changes on the existing codec and reconnect abstractions is simpler than importing main's surrounding refactors; the added bookkeeping is justified by those constraints.
Verification limits: Static review of the GitHub diff and relevant surrounding code, including hop-limit, cursor/hold, reconnect, control-accounting paths and added tests. I did not execute tests, benchmarks, or semver checks; the PR's reported results were not independently verified.
Draft 14 sends 0x05 only to refuse a subscription; later drafts reuse it for the shared REQUEST_ERROR, which stays in the other class. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: ff6b115
Compared with my previous review at b279140 (base unchanged). The previously referenced P2 classification finding is fixed: rs/moq-net/src/ietf/message.rs:24–27 classifies SUBSCRIBE_ERROR as Subscribe only for draft 14, preserving later drafts' shared-error classification. The added test covers draft 14 and draft 15.
No new actionable bugs found in this update. The narrowly version-gated fix is the simplest appropriate approach and does not add unnecessary abstraction.
Verification: static inspection of the one-file, one-commit update and classifier context; I did not execute the tests.
Problem
On
release, a relay mesh under announce churn pays for it several times over:Operators also cannot see it:
/metricsomits the announce counters the relay already keeps (moq-dev/moq#5696) and has no control-message byte attribution (moq-dev/moq#6235).Approach
Backport those five
mainPRs, one commit each, plus one review fix.release'smoq-nethas diverged too far to cherry-pick, so each was ported by hand torelease's code, keepingmain's semantics, names, and public items. Each commit message notes its adaptations.releasec44953eb3releasealready counts announces; this adds only the/metricsrendering, its test, and the doc line.bb42e2711release's codec has noEncoder/Decodercontext, so classes come from crate-privateEncode/Decodehooks, and raw IETF frame reads count through a privateReader::read_message. Messagesreleasedoesn't implement stay uncounted (class="auth"stays 0).061110a33releasedelivers in prefix order rather than insertion order, sodueis keyed by prefix. Keepsrelease's optional clock. Ports the K log K step test, the allocation test, and both bench groups.f1e8cb6fcmoq_tokio::Restart::{Redial, Wait}andClient::with_restart; cluster dials useWait; jittered redial capped at 30s.releasehas nomoq-clock, so the drain test runs on Tokio's paused clock.b279140c3cluster.max_hopsmain. The egress cap joinsrelease's cursorHorizon, so selection falls back to an in-cap route; requests stay uncapped.main's random-mesh harness doesn't exist here, so a capped 8-relay withdrawal test replaces it.ff6b11554SUBSCRIBE_ERROR(0x05) counts assubscribe; from draft 15 on, 0x05 is the sharedREQUEST_ERRORand staysother.Impact
cargo semver-checksreports no semver update required formoq-net,moq-relay, andmoq-tokio. Every new item matchesmain:stats::Control,stats::Bytes,Snapshot::control(),origin::HopLimit(new,get),origin::Config.max_hopsRestart,Client::with_restartcluster::Config.max_hops/metricsseries:moq_relay_announces_{started,ended}_totalandmoq_relay_control_{tx,rx}_bytes_total{tier,class}.max_hopsdefaults to 32, the old limit.releasestill stamps a random hop ahead of an unnamed publisher's path, so such a route counts one more hop against the cap than onmain.Validation
just check origin/releasepasses onb279140c3, including 5162 Rust tests. Onff6b11554,cargo test -p moq-net -p moq-relay,cargo fmt --check, andcargo clippy -D warningspass.cargo semver-checks -p moq-net -p moq-relay -p moq-tokio --baseline-rev origin/release: no semver update required.Alternatives
wire_{tx,rx}half of feat(relay): expose wire and control byte counters #6235 comes too: it hooksmain's QUIC drivers, whichrelease's don't share, so it is left for its own backport.Follow-ups
moq_relay_wire_{tx,rx}_bytes_totalcounters torelease.(Written by Claude Opus 5.5)