Will Berkeley has posted comments on this change.

Change subject: KUDU-763 consensus queue metrics on followers are messed up
......................................................................


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3501/5/src/kudu/consensus/consensus_queue-test.cc
File src/kudu/consensus/consensus_queue-test.cc:

PS5, Line 822: as a followe
> this seems to conflict with the 'SetLeaderMode' call below?
See next comment.


Line 827:   queue_->SetNonLeaderMode();
> oh, I see, you change it to non-leader. perhaps just update the comment abo
Done. Is this a reasonable way to do this? I just frankenstein'd the test from 
other tests.


Line 851:   // Committed index should be the same.
> I'm not sure why this patch is changing the committed index metric to not a
This patch doesn't change how followers handle the committed index. It changes 
how followers update metrics. The situation before this patch (and after) is 
that followers don't update the committed index-- I tested this a little bit to 
try to confirm it was what was leading to the bad metrics. I'm checking it's 
not updated in the test because if the committed index ever starts to be 
updated I'd like this test to fail, so somebody will take a look at how the 
change to the committed index code affects how metrics should work (e.g. maybe 
the follower could have the same metrics as leader at that point).


-- 
To view, visit http://gerrit.cloudera.org:8080/3501
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9fb0d45f85786b9e2631b5dc0bf044a9d3192a39
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wdberke...@gmail.com>
Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com>
Gerrit-HasComments: Yes

Reply via email to