Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14976 )
Change subject: KUDU-3011 p1: metric to count tablet leaders ...................................................................... Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/14976/3/src/kudu/consensus/raft_consensus.h File src/kudu/consensus/raft_consensus.h: http://gerrit.cloudera.org:8080/#/c/14976/3/src/kudu/consensus/raft_consensus.h@85 PS3, Line 85: // Gauge indicating how many Raft leaders are hosted on the server. Nit: "Raft tablet leaders" http://gerrit.cloudera.org:8080/#/c/14976/3/src/kudu/consensus/raft_consensus.cc File src/kudu/consensus/raft_consensus.cc: http://gerrit.cloudera.org:8080/#/c/14976/3/src/kudu/consensus/raft_consensus.cc@701 PS3, Line 701: return AppendNewRoundToQueueUnlocked(round); What if this fails? http://gerrit.cloudera.org:8080/#/c/14976/3/src/kudu/consensus/raft_consensus.cc@2907 PS3, Line 2907: if (server_ctx_.num_leaders) server_ctx_.num_leaders->IncrementBy(-1); Do we also need to do this in RaftConsensus::Stop? Or something that's common to that and to HandleTermAdvanceUnlocked? Might also be good to add a comment explaining why we don't do it in BecomeReplicaUnlocked (which would be symmetric with BecomeLeaderUnlocked): because we call that on startup when we're not already a leader. http://gerrit.cloudera.org:8080/#/c/14976/3/src/kudu/master/sys_catalog.cc File src/kudu/master/sys_catalog.cc: http://gerrit.cloudera.org:8080/#/c/14976/3/src/kudu/master/sys_catalog.cc@382 PS3, Line 382: RETURN_NOT_OK_SHUTDOWN(tablet_replica_->Init({ /*num_leaders*/nullptr, Could be useful for masters though, right? To indicate which master is currently the leader? http://gerrit.cloudera.org:8080/#/c/14976/3/src/kudu/tserver/ts_tablet_manager.h File src/kudu/tserver/ts_tablet_manager.h: http://gerrit.cloudera.org:8080/#/c/14976/3/src/kudu/tserver/ts_tablet_manager.h@410 PS3, Line 410: scoped_refptr<AtomicGauge<int32_t>> num_leaders_; Maybe "num_tablet_leaders" to be more clear? -- To view, visit http://gerrit.cloudera.org:8080/14976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaa6554458a860e34f97af168da7ed786c8ef47e4 Gerrit-Change-Number: 14976 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Sat, 04 Jan 2020 22:37:16 +0000 Gerrit-HasComments: Yes
