docs(perf): rewrite the bare-metal report for its audience; correct a stale roadmap status - #40
Merged
Merged
Conversation
The report read like a worklog: a post-mortem of a deleted revision came before any current result, internal identifiers (RP-9, RP-10, OQ1, PR #29) appeared throughout, sections were titled "Still owed" and "Open Question 1, answered", and it named the specific machines it ran on. A reader arriving at the repository had to work through the history of the document to reach a number. 567 lines to 289. Nothing measured was dropped: every figure, sample set and NIC reading is still there, verified by spot-check. What changed: - Leads with what the numbers ARE — the round-trip/batched distinction and the headline figures — instead of with what a previous revision got wrong. - The withdrawn figures are one short note at the end. They deserve acknowledgement, since someone may arrive remembering 837,284 msg/s, but three sentences serve that reader better than four sections, and the narrative said nothing about current performance. - Internal roadmap identifiers removed. "Open Question 1, answered" became "What limits each number", which is what a reader wants from it. "Still owed" became "Known limits of this report". - Machine names removed; hardware specification kept, since that is the useful part. A public report should not map someone's LAN. - The bug-fix appendix is gone — that belongs in the changelog. - The measurement traps are kept and given their own section, because anyone benchmarking this system will hit them: sub-ten-second runs measuring the client's send buffer, `--linger-ms` destroying a synchronous benchmark, and throughput alone being unable to tell a fast broker from a failing one. Same three internal identifiers stripped from BASELINE_PERFORMANCE.md, whose structure was already reader-first.
The status read "CODE COMPLETE — unit-tested, not yet exercised on a cluster" while all three of its boxes were ticked, which understated what had actually been proven. Leader epochs were exercised on a cluster, and RP-3.3 is the evidence: staging real divergence surfaced a bug where replicated assignments were rebuilt without their epoch, so every follower's copy read epoch 0 forever and truncation could never fire. No unit test reaches that — it needs two nodes disagreeing about an epoch. tests/cluster/local_divergence.sh covers the path on every run. Leaving it as written implied the epoch work was less proven than it is, in the one phase where "proven" carries the most weight.
|
Important Review available on request
Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 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. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Documentation only. No code, no behaviour change.
The bare-metal performance report was written as a worklog
It opened with a post-mortem of a previous revision, carried internal identifiers
(
RP-9,RP-10,OQ1,PR #29) throughout, had sections titled "Still owed"and "Open Question 1, answered", and named the specific machines it ran on. A
reader arriving at the repository had to work through the document's own history
before reaching a number.
567 lines → 289. Nothing measured was dropped — every figure, sample set and
NIC reading is preserved, verified by spot-check.
What changed:
makes them comparable, instead of with what a previous revision got wrong.
acknowledgement — someone may arrive remembering 837,284 msg/s — but three
sentences serve that reader better than four sections, and the narrative said
nothing about current performance.
limits each number", which is what a reader actually wants from that section.
"Still owed" became "Known limits of this report".
the useful part. A public report should not map someone's LAN.
benchmarking this system will hit them: sub-ten-second runs measuring the
client's send buffer rather than the broker,
--linger-msdestroying asynchronous benchmark, and throughput alone being unable to distinguish a fast
broker from a failing one.
The same three internal identifiers are stripped from
BASELINE_PERFORMANCE.md,whose structure was already reader-first.
RP-3.1's status was stale
It read
CODE COMPLETE — unit-tested, not yet exercised on a clusterwhile allthree of its boxes were ticked. Leader epochs were exercised on a cluster, and
RP-3.3 is the evidence: staging real divergence surfaced a bug where replicated
assignments were rebuilt without their epoch, so every follower's copy read epoch
0 forever and truncation could never fire. No unit test reaches that — it needs
two nodes disagreeing about an epoch.
Leaving it as written implied the epoch work was less proven than it is, in the
phase where "proven" carries the most weight.
🤖 Generated with Claude Code