loadgen: route room-read/read-receipt/members through the batched pacer

[hmchangw]

The room-read, read-receipt, and members workloads each rolled their own
one-event-per-tick time.Ticker dispatch loop instead of using the shared
pacedDispatch/pacer. At sub-millisecond intervals the Go runtime silently
coalesces ticks, so achieved RPS plateaued at a few thousand regardless of
--steps (room-read stuck ~1-2k, reported INCONCLUSIVE). Raising MAX_IN_FLIGHT
could not help: the in-flight pool never filled because the emitter could not
release enough events — the bottleneck was emit-side, not pool-side, and the
legacy path did not even account emit underrun, so the verdict fell through to
a generic "load box limited" with no actionable knob.

Route all three generators through the batched pacer (MaxInFlight<=0 keeps the
legacy serial path for bisection) and wire an emit-underrun counter into each:
RoomReadCollector/ReadReceiptCollector gain RecordUnderrun/UnderrunCount,
buildRoomReadInputs/buildReadReceiptInputs populate EmitUnderrun, and the
members generator tallies an "underrun" publish-error reason while preserving
its takeNext/giveBack/ErrPoolsExhausted semantics.

Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01P9bAdsThfEHmwzVNJapCyH

Summary by CodeRabbit

Release Notes

• New Features

  • Added underrun tracking to load generators, capturing when generated requests fall behind the scheduled rate
  • Implemented pacer-based request scheduling for improved timing accuracy

• Tests

  • Added tests to verify underrun behavior across all collectors

• Chores

  • Updated metrics documentation to include underrun as a tracked error reason

[coderabbitai]

📝 Walkthrough

Walkthrough

Underrun tracking is added to ReadReceiptCollector and RoomReadCollector via new RecordUnderrun/UnderrunCount methods. The buildReadReceiptInputs and buildRoomReadInputs functions wire UnderrunCount() into EmitUnderrun. ReadReceiptGenerator, roomReadGenerator, and SustainedMembersGenerator replace inline ticker/semaphore/drain loops with shared serialDispatch/pacedDispatch helpers.

Changes

Underrun Tracking and Paced Dispatch Refactor

Layer / File(s)	Summary
Collector underrun fields and methods tools/loadgen/readreceipt_collector.go, tools/loadgen/readreceipt_collector_test.go, tools/loadgen/roomread_collector.go, tools/loadgen/roomread_collector_test.go	Both collectors gain an underrun int field, a mutex-guarded RecordUnderrun(n int) that no-ops on n<=0, and a UnderrunCount() int accessor. Tests verify zero initial state, no-op on zero input, and correct accumulation.
Generator dispatch refactored to serialDispatch/pacedDispatch tools/loadgen/readreceipt_generator.go, tools/loadgen/roomread_generator.go, tools/loadgen/members_generator.go, tools/loadgen/metrics.go	ReadReceiptGenerator.Run and roomReadGenerator.Run remove inline ticker/semaphore/drain code and delegate to serialDispatch or pacedDispatch, passing collector underrun/saturation callbacks. SustainedMembersGenerator.Run replaces the ticker with newPacer, adds a drain closure, and records "underrun" metrics per pacer tick. Metrics help text documents the underrun reason label.
EmitUnderrun wired into rpsStepInputs tools/loadgen/maxrps_readreceipt.go, tools/loadgen/maxrps_readreceipt_test.go, tools/loadgen/maxrps_roomread.go, tools/loadgen/maxrps_roomread_test.go	buildReadReceiptInputs and buildRoomReadInputs each populate EmitUnderrun from c.UnderrunCount(). Tests confirm the field equals the sum of underruns recorded before the call.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• hmchangw/chat#264: Touches ReadReceiptCollector/ReadReceiptGenerator and buildReadReceiptInputs at the same code level as the underrun/max-rps step input wiring in this PR.
• hmchangw/chat#266: Originally introduced the room-read workload, collector, and buildRoomReadInputs, which this PR extends with underrun accounting.
• hmchangw/chat#270: Implements the same RecordUnderrun/UnderrunCount → EmitUnderrun paced dispatch plumbing that this PR completes.

Suggested labels

ready

Suggested reviewers

• mliu33

Poem

🐇 Hop hop, the ticker is gone,
A pacer now carries things on.
Underruns counted with care,
No missed beats left in the air,
The dispatch is tidy and drawn! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: routing three workloads (room-read, read-receipt, members) through a shared batched pacer, which is the core objective of the changeset.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/room-read-load-testing-uh5umc

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

tools/loadgen/members_generator.go (1)

180-229: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Optional: this inline pacer loop largely duplicates pacedDispatch. The ticker/pacer/semaphore/drain scaffolding mirrors pacedDispatch (pacer.go), differing only in the per-event takeNext/giveBack/ErrPoolsExhausted semantics. Reuse isn't trivial because this path returns an error and mutates a finite pool, so this is acceptable as-is — but if pacedDispatch later grows a per-event hook returning a "continue/stop" signal, consider collapsing this onto it to avoid drift between the two pacing loops.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tools/loadgen/members_generator.go` around lines 180 - 229, The pacer loop in
the members_generator.go file (lines 180-229) duplicates the ticker, pacer,
semaphore, and drain scaffolding from pacedDispatch in pacer.go. If
pacedDispatch is later enhanced to support per-event hooks with continue/stop
signals, refactor these two loops to share a common pacing implementation.
Extract the shared scaffolding (ticker initialization, pacer tick logic,
semaphore management, and drain function) into a reusable helper that accepts a
callback function for per-event processing, allowing the members_generator loop
to pass its takeNext/giveBack/error handling logic as a callback while
pacedDispatch passes its own event logic, thereby eliminating drift between the
two pacing patterns.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tools/loadgen/members_generator.go`:
- Around line 180-229: The pacer loop in the members_generator.go file (lines
180-229) duplicates the ticker, pacer, semaphore, and drain scaffolding from
pacedDispatch in pacer.go. If pacedDispatch is later enhanced to support
per-event hooks with continue/stop signals, refactor these two loops to share a
common pacing implementation. Extract the shared scaffolding (ticker
initialization, pacer tick logic, semaphore management, and drain function) into
a reusable helper that accepts a callback function for per-event processing,
allowing the members_generator loop to pass its takeNext/giveBack/error handling
logic as a callback while pacedDispatch passes its own event logic, thereby
eliminating drift between the two pacing patterns.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8e2ed64d-d030-4d3a-b297-fb7397e83fec

📥 Commits

Reviewing files that changed from the base of the PR and between 0d89e7f and 27750fc.

📒 Files selected for processing (12)

• tools/loadgen/maxrps_readreceipt.go
• tools/loadgen/maxrps_readreceipt_test.go
• tools/loadgen/maxrps_roomread.go
• tools/loadgen/maxrps_roomread_test.go
• tools/loadgen/members_generator.go
• tools/loadgen/metrics.go
• tools/loadgen/readreceipt_collector.go
• tools/loadgen/readreceipt_collector_test.go
• tools/loadgen/readreceipt_generator.go
• tools/loadgen/roomread_collector.go
• tools/loadgen/roomread_collector_test.go
• tools/loadgen/roomread_generator.go