Conversation
|
🚅 Deployed to the rivet-pr-4234 environment in rivet-frontend
|
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
How to use the Graphite Merge QueueAdd the label merge-queue to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
PR Review: fix(epoxy): make quorum sizes match paperSummaryThis PR makes good improvements to edge case handling and code clarity, but contains a correctness issue in the fast quorum formula that could violate EPaxos safety invariants. Critical: Fast Quorum Formula Produces Unsafe ValuesThe new formula in With integer (floor) division, this gives values that are smaller than EPaxos requires. The EPaxos safety requirement for fast quorums is that any two fast quorums must intersect in at least Comparing the three formulas:
The old formula The corrected formula in Rust integer arithmetic (equivalent to Verification:
Since Good ChangesSlow quorum formula fix (
This is a genuine improvement. Edge case handling: The explicit
Minor IssuesDebug log lost Consider keeping Missing unit tests: The quorum math is subtle enough that unit tests would catch regressions like this. Consider adding a Suggested Fix for
|
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

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: