-
Notifications
You must be signed in to change notification settings - Fork 393
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
Check pending conf change before campaign #225
Conversation
I think we should contribute to etcd later. |
PTAL @xiang90 |
|
||
/// Tests if unapplied conf change is checked before campaign. | ||
#[test] | ||
fn test_conf_change_check_before_campaign() { |
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.
if we don't call self.hub(true)
, the test can fail?
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.
Yes. The case was written before the fix, and it failed.
Seems criterion's API is broken. @Hoverbear Would you like to take a look? |
seem some failed tests were not caused by criterion API changes. |
@siddontang can you name some? I can't find them. |
|
Those tests are expected to panic. They are wrapped with UnwindSafe. |
src/raft.rs
Outdated
); | ||
}); | ||
let n = self.num_pending_conf(&ents); | ||
if n != 0 && self.raft_log.committed > self.raft_log.applied { |
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.
If there is a pending snapshot, what will raft_log.applied
be?
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.
In this case I think raft_log.applied
must be less than snapshot.index
, so I think only n != 0
is ok here.
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.
raft_log.applied
can be any value less than the snapshot index.
LGTM. |
* Check pending conf change before campaign (#225) Fix #221. * Add more convenient lite-weight interfaces (#227) This PR introduces two simple and lite weight interfaces: - ping to trigger heartbeats without ticking, - status_ref to borrow the progress set instead of cloning. * *: bump to 0.4.2 (#228) * Bump to v0.4.3 (#231) * raft: leader respond to learner read index message (#220) Signed-off-by: nolouch <nolouch@gmail.com> * Bump to v0.4.3 Signed-off-by: Neil Shen <overvenus@gmail.com> * Request snapshot (#243) Signed-off-by: Neil Shen <overvenus@gmail.com> * fix tests * cargo fmt * address comments.
Fix #221.
This is a fix for 0.4.x, I will do the fix for master later.