Mike Percy has posted comments on this change. Change subject: [consensus] Add consensus op-level lag metrics ......................................................................
Patch Set 10: (2 comments) http://gerrit.cloudera.org:8080/#/c/6451/10/src/kudu/consensus/consensus_queue-test.cc File src/kudu/consensus/consensus_queue-test.cc: Line 247: // Note: This estimation must be precise in order to determine the exact number of messages that This was a little hard to me to grok. Can you clarify in this message that all optional fields in the protobuf must appear in order for the estimate in this test to be accurate? http://gerrit.cloudera.org:8080/#/c/6451/10/src/kudu/consensus/raft_consensus.cc File src/kudu/consensus/raft_consensus.cc: Line 1152: queue_->UpdateLagMetrics(request->last_idx_appended_to_leader()); I think this will cause the field to be 'required' without a default being specified in the .proto file. That will break rolling upgrades. -- 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: 10 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: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
