-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Randomize leader election #3693
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Others LGTM! Copy-pasted from Zullip for future sequencer-benchmarking reference:
the first argument to new is just the nodes that you want to allow to be leader.
#[derive(Clone, Debug, Eq, PartialEq, Hash)] | ||
|
||
/// The static committee election | ||
pub struct GeneralStaticCommittee<T: NodeType> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just name this GeneralRandomCommittee? or similar so we don't have 2 structs named the exact same thing, I could see this causing some weird headache in the future. Unless this is some common code style thing I wasn't aware of
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was just following the style from before, but yeah I agree -- renamed!
Closes #3684
This PR:
Membership
trait. This logic previously existed but was gated behind a feature flag.randomized-leader-election
feature is completely removed, whilefixed-leader-election
is now exclusively a feature in theexamples
crate.test_success
andtest_all_restart
now also run with randomized leader rotations.This PR does not:
Key places to review: