docs: scaling dive 2026-05 (closes Phase 2 of #7756)

[JohnMcLear]

Summary

Numbers-backed answer to #7756: "how many editors can be on one pad, and where is the bottleneck?" Every claim links to a CI run whose report.json is downloadable for re-analysis.

This PR description is intentionally brief; the doc itself (docs/scaling-dive-2026-05.md) is the source of truth and contains the full methodology, every lever scored, the negative results, and the honest re-evaluations.

Headline findings

1. Real ceiling on a 4-vCPU GitHub runner: ~350-400 authors per pad. Beyond that, p95 cliffs from ~80 ms to ~2000 ms within one step. CPU consumed 7+ cores of work per wall-second — OS preemption, not slow code paths.

2. The "250-author cliff" earlier reports kept hitting was a measurement artefact. Per-IP commitRateLimiting + harness colocation (all simulated authors share 127.0.0.1). Fixed in load-test#105. Production deployments with many client IPs are not affected.

3. Runner noise is significant. Each GHA matrix entry runs on a potentially different physical runner. The same develop baseline produced p95 between 38 ms and 122 ms at step 350 across three identical runs. All single-run lever conclusions sit inside the noise envelope below the cliff. The doc adopts N=3-per-lever methodology and reports min/median/max.

4. Two changes hold up under N=3 scoring:

   • #7774 — engine.io socket flush deferral. Tightens the tail at mid-load (step 350 max: 122 → 110 ms; step 300 max: 71 → 58 ms). Wire-compatible, server-side only. The only N=3-confirmed perf win in this program.
   • #7768 — per-socket fan-out serialization. No measurable perf benefit (the earlier "70% drop" was single-run noise), but a real correctness fix: the previous code was racy under concurrent commits and could lose revisions on emit failure. Recommend merging for correctness, not perf.

5. Five directions did not pan out, documented in full:

   • WebSocket-only transport — reliably worse at cliff (N=3 confirms).
   • --max-old-space-size heap bump — no effect.
   • Closed PRs: #7766, #7770, #7769, #7772. Each has a one-paragraph diagnosis in the doc.

6. The cliff is hardware-bound, not code-bound, on the runner we measure. Production with more cores per host sees proportionally higher ceilings without code changes. Remaining application-level surface explored end-to-end.

What changed since the first draft of this doc

• Original draft claimed lever 3 (feat(scaling): serialize per-socket fan-out + NEW_CHANGES_BATCH (#7756 lever 3) #7768) gave a 70% p95 drop — this was a single-run outlier comparison and is corrected throughout.
• New methodology caveat section explains why matrix-entry-on-different-runners means within-run cross-lever comparisons are cross-hardware.
• N=3 envelope tables added for develop baseline, feat(scaling): serialize per-socket fan-out + NEW_CHANGES_BATCH (#7756 lever 3) #7768, and feat(scaling): engine.io socket flush deferral — modest tail-latency reduction at mid-load #7774.
• Lever 8 (feat(scaling): engine.io WS transport-level packing — lever 8 prototype #7772) closed with diagnosis; lever 8b (feat(scaling): engine.io socket flush deferral — modest tail-latency reduction at mid-load #7774) added.
• Recommendation section reordered: flush deferral first, serialization second-for-correctness.

Test plan

This is documentation only — no code changes. Reviewers should verify:

• The numbers in the tables match the linked CI runs' report.json files.
• The methodology caveat section accurately describes how GHA matrix entries map to physical runners.
• Recommendations match the actual disposition of each linked PR.

🤖 Generated with Claude Code

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Scaling dive 2026-05: Phase 2 analysis with fan-out bottleneck findings

📝 Documentation

Walkthroughs

Description

• Documents Phase 2 scaling analysis with empirical measurements
• Identifies fan-out as dominant bottleneck, not server-side apply
• Refutes websocket-only hypothesis; polling fallback beneficial
• Recommends fan-out batching as next concrete optimization lever

Diagram

flowchart LR
  baseline["Baseline measurements<br/>20-200 authors"]
  analysis["Decision rule analysis<br/>CPU/network bound"]
  levers["Per-lever evaluation<br/>4 configurations tested"]
  findings["Key findings:<br/>Fan-out O(N²) cost<br/>Websocket-only refuted"]
  recommendation["Recommendation:<br/>Prototype batching<br/>Verify perMessageDeflate"]
  
  baseline --> analysis
  analysis --> levers
  levers --> findings
  findings --> recommendation

Loading

File Changes

1. docs/scaling-dive-2026-05.md 📝 Documentation +133/-0

Phase 2 scaling analysis with empirical bottleneck identification

• New comprehensive documentation of Phase 2 scaling dive analysis
• Presents methodology, baseline curve (20-200 authors), and decision rules
• Evaluates four optimization levers with empirical data and verdicts
• Identifies fan-out as O(N²) bottleneck; CPU saturates at 200 authors
• Refutes websocket-only hypothesis; confirms polling fallback beneficial
• Recommends fan-out batching prototype as highest-leverage next step

docs/scaling-dive-2026-05.md

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (1)

Context used

✅ Tickets: 🎫 Look deeper into scaling N+ contributors per pad.

1. Fan-out batching lacks results 📎 Requirement gap ➹ Performance

Description

The doc recommends fan-out batching but explicitly defers running/recording any batching experiment,
so there are no load-test findings on message batching/buffering impact. This fails the requirement
to validate and report findings for batching/message-size reduction under load.

Code

docs/scaling-dive-2026-05.md[R77-82]

+## Lever 3 — fan-out batching
+
+**Deferred.** Requires a code change in `PadMessageHandler.ts` (specifically the per-socket loop in `updatePadClients` and/or the broadcast emit at line 627). Recommended as the next concrete code change. The harness is ready to score it as soon as a candidate branch exists — point the workflow's `core_ref` input at the branch.
+
+The `emits_new_changes` column on the curve table above is the direct measurement target. At 200 authors we're producing 66 032 emits per 10 s dwell. Halving the emit rate (by coalescing two changesets per emit on a sub-50 ms window) would directly reduce CPU.
+

Evidence

PR Compliance ID 4 requires a batching/buffering experiment (or implementation) with reported
load-test findings. The added doc section marks fan-out batching as Deferred and only describes an
expected benefit, without any measured results.

Explore message size reduction or batching for high edit rates
docs/scaling-dive-2026-05.md[77-82]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR proposes/recommends fan-out batching but provides no implemented or documented batching/buffering experiment and no load-test findings for it.

## Issue Context
PR Compliance ID 4 requires an implementation or documented experiment for batching/buffering (or other message-size reduction) plus reported load-test findings demonstrating the effect.

## Fix Focus Areas
- docs/scaling-dive-2026-05.md[77-82]
- docs/scaling-dive-2026-05.md[108-109]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Metrics misnamed as counters 🐞 Bug ≡ Correctness

Description

The methodology section says the scaling-dive instruments are "three Prometheus counters", but the
implementation is a Histogram + Counter + Gauge, which can mislead readers when interpreting scraped
metric series (_bucket/_sum/_count vs monotonic counters).

Code

docs/scaling-dive-2026-05.md[19]

+- **Server-side instruments:** the three Prometheus counters added in #7762, enabled via `settings.scalingDiveMetrics=true`.

Evidence

The doc claims all three instruments are counters, but the implementation clearly defines one
Histogram, one Counter, and one Gauge.

docs/scaling-dive-2026-05.md[18-20]
src/node/prom-instruments.ts[17-35]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`docs/scaling-dive-2026-05.md` describes the scaling-dive Prometheus instruments as "three Prometheus counters", but the code implements a histogram (`etherpad_changeset_apply_duration_seconds`), a counter (`etherpad_socket_emits_total`), and a gauge (`etherpad_pad_users`). This is factually incorrect and may confuse anyone consuming the metrics.

### Issue Context
The implementation lives in `src/node/prom-instruments.ts` and is gated behind `settings.scalingDiveMetrics`.

### Fix Focus Areas
- docs/scaling-dive-2026-05.md[18-20]

### Expected change
Update the wording to something like "three Prometheus metrics/instruments" or explicitly list the types (Histogram/Counter/Gauge).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Wrong line number reference 🐞 Bug ⚙ Maintainability

Description

The Lever 3 section tells readers to change a "broadcast emit at line 627" in PadMessageHandler.ts,
but line 627 is in handleSaveRevisionMessage; the NEW_CHANGES emit is inside
exports.updatePadClients at lines 985–999. This incorrect pointer will waste time during the
recommended batching prototype work.

Code

docs/scaling-dive-2026-05.md[79]

+**Deferred.** Requires a code change in `PadMessageHandler.ts` (specifically the per-socket loop in `updatePadClients` and/or the broadcast emit at line 627). Recommended as the next concrete code change. The harness is ready to score it as soon as a candidate branch exists — point the workflow's `core_ref` input at the branch.

Evidence

The doc references line 627 for broadcast fan-out work, but in the current PadMessageHandler.ts line
627 is unrelated; the NEW_CHANGES emit is located in updatePadClients at lines 984–1000.

docs/scaling-dive-2026-05.md[77-81]
src/node/handler/PadMessageHandler.ts[622-627]
src/node/handler/PadMessageHandler.ts[950-1000]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`docs/scaling-dive-2026-05.md` points developers to "the broadcast emit at line 627" in `PadMessageHandler.ts` for fan-out batching work, but that line number does not correspond to the NEW_CHANGES fan-out emit. The relevant per-socket emit happens in `exports.updatePadClients`.

### Issue Context
In the current code, line 627 is part of `handleSaveRevisionMessage`, while `exports.updatePadClients` is defined later and emits NEW_CHANGES via `socket.emit('message', msg)`.

### Fix Focus Areas
- docs/scaling-dive-2026-05.md[77-81]

### Expected change
Replace the incorrect line-number reference with the correct location (e.g., `exports.updatePadClients` near the NEW_CHANGES `socket.emit`) or remove hard-coded line numbers to avoid future drift.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-free-for-open-source-projects]

1. Fan-out batching lacks results 📎 Requirement gap ➹ Performance

The doc recommends fan-out batching but explicitly defers running/recording any batching experiment,
so there are no load-test findings on message batching/buffering impact. This fails the requirement
to validate and report findings for batching/message-size reduction under load.

Agent Prompt

## Issue description
The PR proposes/recommends fan-out batching but provides no implemented or documented batching/buffering experiment and no load-test findings for it.

## Issue Context
PR Compliance ID 4 requires an implementation or documented experiment for batching/buffering (or other message-size reduction) plus reported load-test findings demonstrating the effect.

## Fix Focus Areas
- docs/scaling-dive-2026-05.md[77-82]
- docs/scaling-dive-2026-05.md[108-109]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[JohnMcLear]

Addressing Qodo's three items (commit 80b3b74 — wholesale doc rewrite):

1. Fan-out batching lacks results → Fixed. The "Lever 3" section now has the measured numbers from run 25941483750: p95 89→24 ms (-73%), apply_mean 10.7→4.66 ms (-56%), 0 vs 8 errors at step 200. The Recommendation section explicitly lists #7768 at priority 1.
2. Metrics misnamed as counters → Fixed. The Methodology section now correctly labels each instrument: etherpad_changeset_apply_duration_seconds as a Histogram (with _bucket/_sum/_count series), etherpad_socket_emits_total as a Counter, etherpad_pad_users as a Gauge.
3. Wrong line number reference → Fixed. Lever 3 section now points at exports.updatePadClients (lines 985–999), which is where the NEW_CHANGES emit lives. The previous "line 627" pointed at handleSaveRevisionMessage.

Bonus rewrite: the doc now reflects everything we learned after the original draft — the rate-limit artefact, the real ~350-400 author ceiling on a 4-vCPU runner, the OS-preemption diagnosis of apply_mean ballooning at the cliff, the negative results on websocket-only and the closed fanoutDebounceMs (#7766) and rebase-prefetch (#7770) PRs, and the engine.io transport-level packing direction as the next concrete lever. Every claim links to a CI run for re-analysis.