Skip to content

feat(stats): requested prefix tracks with rollup, self, and children - #5811

Merged
kixelated merged 5 commits into
mainfrom
quest/m2/stats-prefix-tracks
Oct 17, 2026
Merged

kixelated merged 5 commits into
mainfrom
quest/m2/stats-prefix-tracks

Conversation

@kixelated

@kixelated kixelated commented Oct 17, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Completes quest/m2/stats-prefix-tracks.md (deleted here, references updated).

A reader of a node's stats broadcast can request <path>/traffic.json[.z] for any broadcast prefix within the group. Each frame is keyed by tier, like totals.json, and each tier entry carries:

  • publisher / subscriber at the top level: the rollup of every path at or under the prefix, so a Totals reader parses it unchanged.
  • self: {publisher, subscriber}: the broadcast at exactly the prefix.
  • children: {"<segment>": {publisher, subscriber}}: each direct child's rollup.

Behavior, as decided in the quest:

  • Produced only while held; a prefix with no members is held open at {}; reclaimed once the last consumer leaves. No cap on holds or children.
  • Drain-time sums, like presence: each drain walks every row's ancestors and looks each up in the held set, so the cost is rows times path depth and nothing when no prefix is held. No moq-net change; the steady drain still allocates nothing with a prefix held (extended test).
  • Counters follow presence: live members plus the closing readouts of members that ended while held, so active is exact for a prefix held mid-stream.
  • A child leaves the map after the frame carrying its closing readout; its tail stays in the rollup and self stays flat.
  • A prefix outside the group, or deeper than Path::MAX_PARTS, is refused with NotFound.
  • An oversized frame ends only that flavor with FrameTooLarge (warn log); the sibling keeps publishing. This now comes from fix(stats): end a stats track whose frame outgrows the cache #5826, which applies it to every stats track.
  • The aggregator merges prefix tracks with the per-(node, epoch) reset baseline, so the merged rollup and self never regress. Children sum only the live nodes' current children: a departed node's children leave at once, while its rollup stays.
  • Mergeable now requires Clone instead of Copy, since Prefix holds a map.

Public API and wire impact

  • moq-stats adds Prefix, Usage, PrefixFrame, prefix_track, consume::Consumer::prefix / consume::Prefix, and aggregate::Consumer::prefix / aggregate::PrefixConsumer. Nothing removed or changed.
  • Wire: new requested track <path>/traffic.json[.z] with the payload above. The existing map tracks are unchanged. Documented in doc/concept/stats.md.

Decisions

Recommended choices applied while the maintainer was away:

  1. Entry type names. Keep Prefix for the per-tier entry, Usage for the {publisher, subscriber} pair, and own as the Rust field for JSON self (a keyword). Rejected: Rollup for the entry, Flow or Sides for the pair.
  2. Merged children. When a node leaves, its children leave the merged map immediately while its totals stay in the rollup, matching the producer's "child leaves, tail stays in the rollup". Rejected: keeping a departed node's children sticky with closed pairs until the grace ends, which would hold pruned children for up to ten minutes.

Checks

  • just check (lint, then tests for moq-stats, moq-relay, moq-cli): lint clean. One run hit the bursts_cross_a_transport_peer::impaired drill flake planned in quest(m1): plan fuzz crypto, richer sim workloads, and a relay drill flake #5785; the rerun with --no-fail-fast passed all 581.
  • New tests: prefix rollup, self, and children; a prefix held while 300 subscriptions are live, then watching them leave; a nested broadcast ending under a held parent; an idle prefix held then reclaimed; aggregator join, leave, and restart; aggregator reset baseline.
  • producer_tick bench gains stats/prefix, sweeping paths, path depth, and held prefixes. Measured on a shared, loaded machine, so the timings are noisy: holding 100 prefixes adds about 900 to 1,300 allocations per tick (frame building and encoding), and the time is dominated by the publisher.json encode.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

@kixelated

Copy link
Copy Markdown
Collaborator Author

Outcome: the quest's plan is implemented, and it stays a draft only for the two naming and merge-semantics decisions in the body. Local checks are green, apart from the known relay drill flake (#5785), which passed on rerun. Suggested follow-ups: flip abort_oversized on for every stats track under #5739's quest, and have the inspect CLI print a prefix track.

(Written by Claude Opus 5.5)

@kixelated
kixelated marked this pull request as ready for review October 17, 2026 00:29
@coderabbitai

coderabbitai Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 6 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 984a30b7-cd6c-4d52-bf87-f722e8bb3b07
📥 Commits

Reviewing files that changed from the base of the PR and between e3e8c44 and 2214d74.

📒 Files selected for processing: 15
  • doc/bin/inspect.md
  • doc/concept/stats.md
  • quest/m0/broadcast-epoch/README.md
  • quest/m2/README.md
  • quest/m2/qos-lag-dashboard.md
  • quest/m2/qos-publisher-timeliness.md
  • quest/m2/qos/README.md
  • quest/m2/qos/lag-histogram.md
  • quest/m2/stats-prefix-tracks.md
  • rs/moq-stats/benches/producer_tick.rs
  • rs/moq-stats/src/aggregate.rs
  • rs/moq-stats/src/consume.rs
  • rs/moq-stats/src/lib.rs
  • rs/moq-stats/src/produce.rs
  • rs/moq-stats/src/produce_bench.rs

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f0a436ac-2429-4756-8020-c8fa50f17b2f
📥 Commits

Reviewing files that changed from the base of the PR and between b7d7e99 and e3e8c44.

📒 Files selected for processing: 16
  • doc/bin/inspect.md
  • doc/concept/stats.md
  • quest/m0/broadcast-epoch/README.md
  • quest/m1/stats-frame-too-large.md
  • quest/m2/README.md
  • quest/m2/qos-lag-dashboard.md
  • quest/m2/qos-publisher-timeliness.md
  • quest/m2/qos/README.md
  • quest/m2/qos/lag-histogram.md
  • quest/m2/stats-prefix-tracks.md
  • rs/moq-stats/benches/producer_tick.rs
  • rs/moq-stats/src/aggregate.rs
  • rs/moq-stats/src/consume.rs
  • rs/moq-stats/src/lib.rs
  • rs/moq-stats/src/produce.rs
  • rs/moq-stats/src/produce_bench.rs
💤 Files with no reviewable changes: 3
  • quest/m2/stats-prefix-tracks.md
  • quest/m2/README.md
  • quest/m0/broadcast-epoch/README.md

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.

Walkthrough

The change adds subscription-driven <path>/traffic.json prefix tracks with tiered rollups, exact-prefix counters, and direct-child counters. The producer validates requests, initializes and publishes track frames, and handles oversized frames for plain and compressed tracks independently. Consumers can read typed prefix frames, and aggregate consumers merge snapshots across nodes. The change also adds aggregation and producer benchmarks and updates statistics documentation and related plans.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to e3e8c

No identified issue prevents merging the prefix traffic tracks after normal checks.

Pre-merge checks: Passed 5
Check name Status Explanation
Docstring Coverage Passed Docstring coverage is 80.25% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 81 functions across 6 files. (7 skipped: 7 …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly summarizes the main change: requested prefix stats tracks with rollup, exact-prefix, and child data.
Description check Passed The description explains the prefix-track behavior, API and wire changes, implementation decisions, and reported checks.
Full details: Docstring Coverage

Explanation

Docstring coverage is 80.25% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 81 functions across 6 files. (7 skipped: 7 unsupported.)

✨ Finishing Touches
✨ Simplify code
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-17T14:58:56.080444Z 2214d74 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

kixelated and others added 3 commits October 16, 2026 17:47
A reader can request `<path>/traffic.json` for any broadcast prefix in a
stats group and gets, per tier, the prefix's rollup, the broadcast at
exactly the prefix (`self`), and a one-level map of each direct child,
produced only while held. Sums are drain-time like presence: each row
walks its ancestors in the held set, so nothing touches the per-frame
path. An oversized prefix frame ends that flavor with FrameTooLarge.

The aggregator merges prefix tracks with a per-node baseline, so the
rollup never regresses; children sum the live nodes only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated force-pushed the quest/m2/stats-prefix-tracks branch from 80c54da to e3e8c44 Compare October 17, 2026 00:51
@kixelated

Copy link
Copy Markdown
Collaborator Author

Ready to merge. The two open decisions are settled as recommended (recorded in the body): names stay Prefix / Usage / own, and a departed node's children leave the merged map at once while its totals stay.

Codex reviewed 80c54da with no findings. Since then, a rebase onto main plus one quest-doc fix: #5739 landed quest/m1/stats-frame-too-large.md with a link to the quest this PR deletes, which failed Check. It now points at this PR's TrackPair::abort_oversized. moq-stats tests pass locally (74/74).

Follow-ups: turn abort_oversized on for every stats track (quest/m1/stats-frame-too-large.md), and have the inspect CLI print a prefix track.

(Written by Claude Opus 5.5)

@kixelated
kixelated enabled auto-merge October 17, 2026 00:51
@kixelated
kixelated added this pull request to the merge queue Oct 17, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Oct 17, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated added this pull request to the merge queue Oct 17, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Oct 17, 2026
Main's #5826 ends any stats track whose frame outgrows the cache, so the prefix-only abort_oversized opt-in and its duplicate test are dropped.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated marked this pull request as draft October 17, 2026 14:56
@kixelated
kixelated marked this pull request as ready for review October 17, 2026 14:56
@kixelated
kixelated enabled auto-merge October 17, 2026 14:59
@kixelated
kixelated added this pull request to the merge queue Oct 17, 2026
Merged via the queue into main with commit b54589d Oct 17, 2026
11 checks passed
@kixelated
kixelated deleted the quest/m2/stats-prefix-tracks branch October 17, 2026 15:53
kixelated added a commit that referenced this pull request Oct 17, 2026
Resolves #5811's prefix tracks against label-keyed presence: presence
tracks and requests take the stats label, prefix tracks keep the
broadcast path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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