Andrew Wong has posted comments on this change.

Change subject: [consensus] Add consensus op-level lag metrics
......................................................................


Patch Set 6:

(3 comments)

As you guys in the above comments, since this doesn't address time, I've 
changed the tag to [consensus] rather than the jira number, and will link this 
as a related patch to KUDU-1506.

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. 
Right, the leader only knows that it appended this op to its own WAL.

Seeing as there's also a "last_received_current_leader" which sounds similar 
but is not the same, I'm renaming this to "last_idx_appended_by_leader" (one 
source of ambiguity may be leaving out "by"s, "on"s, "from"s, etc.).


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 page_size_estimator is used to set the max_batch_size_bytes flag. Without 
adding this index, we have an underestimate of the request size, which is 
results in an underestimate in the max batch size, calculated in RequestForPeer 
(consensus_queue.cc:384). This test expects a certain number of ops in the 
request and doesn't get it because of this.

Done


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
Done


-- 
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