-
Notifications
You must be signed in to change notification settings - Fork 165
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
testdata: add a lagging commit test #137
Conversation
b030d6d
to
912b496
Compare
912b496
to
9ef2cb9
Compare
@nvanbenschoten @ahrtr PTAL. This is a regression test for potential future fixes for #138. |
9ef2cb9
to
53d373c
Compare
This commit adds a test demonstrating the effect of delayed commit on a follower node after a network hiccup between the leader and this follower. In the described scenario, after the moment of committing an entry on the leader, it takes HeartbeatInterval + 3/2 * RTT until the follower learns this entry is committed. This is suboptimal, and could take HeartbeatInverval + 1/2 * RTT if the leader didn't cut the commit index at Progress.Match before sending it to the follower. Signed-off-by: Pavel Kalinnikov <[email protected]>
53d373c
to
8a26275
Compare
Why do we care? Follower reads? |
Yes, and also the general "convergence" of the system (resulting in fewer messages etc). For other kinds of reads, like quorum reads, this also seems relevant. |
@ahrtr Good to merge? |
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.
lgtm
This commit adds a test demonstrating the effect of delayed commit on a follower node after a network hiccup between the leader and this follower.
In the described scenario, after the moment of committing an entry on the leader, it takes
HeartbeatInterval + 3/2 * RTT
until the follower learns this entry is committed.This is suboptimal, and could take
HeartbeatInverval + 1/2 * RTT
if the leader didn't cut the commit index atProgress.Match
before sending it to the follower.Informs #138