Todd Lipcon has posted comments on this change.

Change subject: Cleanup/refactor tracking of consensus watermarks

Patch Set 6:

File src/kudu/consensus/

Line 137:   if (current_term != queue_state_.current_term) {
> are we verifying that the term doesn't go back? should we?
sure, I'll give that a try. I think we are, elsewhere, but maybe not here.

Line 263:       // TODO: this code seems wrong. When we are leader, we've 
already bumped
> yeah, this seems like it's making things a bit messier (not that they were 
oops, I actually meant to remove this TODO in the current revision.

I agree it would be nice to do the SetLeaderAndAppendNoOp thing, but we're 
currently abusing SetLeaderMode to handle refreshing the tracked peers on a 
configuration change, and I didn't want to open the can of worms of trying to 
add more refactoring on top. So, I just kept this as is. I'll update the TODO, 
though, to suggest this refactor.

Line 459:     // TODO: The fact that we only consider peers whose last exchange 
> this is technically true, but not sure how important/impactful it is. If A 
yea, agreed that it's not a super impactful optimization, but more about code 
cleanliness and the fact that our "watermarks" actually move backwards, not 
just forwards (and thus aren't technically watermarks!)

But, would like to address it as a follow-on.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I2aa294472f018013d88f36a9358e9ebd9d5ed8f8
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <>
Gerrit-Reviewer: David Ribeiro Alves <>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <>
Gerrit-Reviewer: Todd Lipcon <>
Gerrit-HasComments: Yes

Reply via email to