fix(epoxy): make quorum sizes match paper#4234
fix(epoxy): make quorum sizes match paper#4234MasterPtato wants to merge 1 commit into02-13-fix_rivetkit_stall_stop_handler_until_start_completesfrom
Conversation
|
🚅 Deployed to the rivet-pr-4234 environment in rivet-frontend
|
Code Review: fix(epoxy): make quorum sizes match paperThe core idea is solid — separating the "sender-excluded" fanout quorum calculation out of the HTTP client and into a dedicated Issues
The PR correctly replaces // utils.rs line 1 — unchanged
use anyhow::*;This should be updated to explicit imports (e.g., Missing unit tests for the quorum calculation logic The PR fixes a correctness bug in consensus quorum sizing — this is exactly the kind of logic that should have explicit unit tests. Without them, it's very hard to verify the edge-case behavior or catch regressions. At a minimum, the following should be tested:
The relationship between
The 2 => 1,For
if target_responses == 0 {
tracing::warn\!("no fanout, target is 0");
return Ok(Vec::new());
}
Quorum formula for even N — worth a comment or test The comment explains that even N gives no additional fault tolerance (N=4 acts like N=3), but the resulting quorum sizes are noticeably smaller than a traditional majority. For example:
If EPaxos section 4.3 justifies this for even N clusters, a brief inline comment with that citation (beyond just "See EPaxos 4.3") would make the reasoning more accessible. A table-driven test would also make the expected values auditable. Minor
SummaryThe directional change is correct and the logic refactoring is an improvement. Main asks before merging:
|
788e9db to
e7553c7
Compare
c7242fc to
e418d91
Compare
e7553c7 to
710de07
Compare
e418d91 to
0e4f8dc
Compare
710de07 to
fbd667c
Compare
0e4f8dc to
233582b
Compare
233582b to
682f215
Compare
fbd667c to
6b4b34d
Compare
6b4b34d to
5973cf6
Compare
5973cf6 to
e86f64c
Compare
e86f64c to
a4fe6de
Compare
a4fe6de to
754e84e
Compare
754e84e to
4e3fd8f
Compare
4e3fd8f to
e144ca2
Compare
e144ca2 to
f01fcb4
Compare

Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context.
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist: