-
Notifications
You must be signed in to change notification settings - Fork 218
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
matchIndex lower bound for nextIndex #5614
Conversation
mku-pr5341@75418 aka 20230905.42 vs main ewma over 20 builds from 75075 to 75413 Click to see tablemain
mku-pr5341
|
@@ -950,8 +952,7 @@ HandleAppendEntriesResponse(i, j, m) == | |||
/\ nextIndex' = [nextIndex EXCEPT ![i][j] = max(@, m.lastLogIndex + 1)] | |||
\/ /\ \lnot m.success \* not successful | |||
/\ LET tm == FindHighestPossibleMatch(log[i], m.lastLogIndex, m.term) | |||
IN nextIndex' = [nextIndex EXCEPT ![i][j] = | |||
(IF matchIndex[i][j] = 0 THEN tm ELSE min(tm, matchIndex[i][j])) + 1 ] | |||
IN nextIndex' = [nextIndex EXCEPT ![i][j] = max(tm, matchIndex[i][j]) + 1 ] |
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.
Using the matchIndex
as a lower bound for the nextIndex
makes sense to me. Does this match the implementation?
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.
No:
Lines 1437 to 1440 in 2fbf878
const auto this_match = | |
find_highest_possible_match({r.term, r.last_log_idx}); | |
node->second.sent_idx = std::min(this_match, node->second.sent_idx); | |
return; |
which is Fix A
described in #5341 (comment):
/\ LET tm == FindHighestPossibleMatch(log[i], m.lastLogIndex, m.term)
IN nextIndex' = [nextIndex EXCEPT ![i][j] = min(@, tm) ]
Merged because the build failures were unrelated. |
#5341