David Ribeiro Alves has posted comments on this change.

Change subject: WIP: KUDU-1506 Add consensus lag metrics
......................................................................


Patch Set 6:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/6451/6/src/kudu/consensus/consensus.proto
File src/kudu/consensus/consensus.proto:

PS6, Line 358: The highest index that has been replicated by any peer in the 
config. Leaders will set this
             :   // to be the highest committed index it has seen as it 
receives responses from followers.
             :   // On the follower side, this can be used to determine how far 
behind a given peer is.
Wait, this doesn't seem to be quite true, according to the implementation. When 
you increase this in PeerMessageQueue::AppendOperations the message might not 
have been replicated anywhere, right?

My comments on the metric name were to make it consistent with this index, but 
it seems like it would make more sense to call this index something like: 
'max_appended_index'.

Btw 'max' makes sense here since even if we end up appending messages that are 
overwritten in the log, we still keep the highest index, according to the 
implementation (and which I think makes sense).


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

PS6, Line 251: page_size_estimator.set_max_replicated_index(0);
why is this needed in this particular test? btw could you add checking for the 
new index to some existing test (likely not worth it having its own test)


http://gerrit.cloudera.org:8080/#/c/6451/6/src/kudu/consensus/consensus_queue.cc
File src/kudu/consensus/consensus_queue.cc:

PS6, Line 86: ops_behind_max_replicated
make it consistent with the proto field, when you change it


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ida8e992cc2397ca8d5873e62961a65f618d52c36
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.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-HasComments: Yes

Reply via email to