Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11251 )
Change subject: KUDU-2245 Graceful leadership transfer ...................................................................... Patch Set 7: (30 comments) http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus-test-util.h File src/kudu/consensus/consensus-test-util.h: http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus-test-util.h@230 PS7, Line 230: virtual void StartElection(const RunLeaderElectionRequestPB* request, > warning: parameter 'request' is unused [misc-unused-parameters] Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus-test-util.h@231 PS7, Line 231: RunLeaderElectionResponsePB* response, > warning: parameter 'response' is unused [misc-unused-parameters] Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus-test-util.h@232 PS7, Line 232: rpc::RpcController* controller, > warning: parameter 'controller' is unused [misc-unused-parameters] Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus-test-util.h@233 PS7, Line 233: const rpc::ResponseCallback& callback) override {} > warning: parameter 'callback' is unused [misc-unused-parameters] Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus-test-util.h@299 PS7, Line 299: virtual void StartElection(const RunLeaderElectionRequestPB* request, > warning: parameter 'request' is unused [misc-unused-parameters] Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus-test-util.h@299 PS7, Line 299: virtual > nit: drop 'virtual' since 'override' is used. Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus-test-util.h@300 PS7, Line 300: RunLeaderElectionResponsePB* response, > warning: parameter 'response' is unused [misc-unused-parameters] Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus-test-util.h@301 PS7, Line 301: rpc::RpcController* controller, > warning: parameter 'controller' is unused [misc-unused-parameters] Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus-test-util.h@302 PS7, Line 302: const rpc::ResponseCallback& callback) override {} > warning: parameter 'callback' is unused [misc-unused-parameters] Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus-test-util.h@355 PS7, Line 355: virtual > nit: drop Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus-test-util.h@355 PS7, Line 355: virtual void StartElection(const RunLeaderElectionRequestPB* requestoverride , > warning: parameter 'requestoverride' is unused [misc-unused-parameters] Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus-test-util.h@356 PS7, Line 356: RunLeaderElectionResponsePB* response, > warning: parameter 'response' is unused [misc-unused-parameters] Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus-test-util.h@357 PS7, Line 357: rpc::RpcController* controller, > warning: parameter 'controller' is unused [misc-unused-parameters] Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus-test-util.h@358 PS7, Line 358: const rpc::ResponseCallback& callback) override {} > warning: parameter 'callback' is unused [misc-unused-parameters] Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus-test-util.h@362 PS7, Line 362: rpc::RpcController* controller, > warning: parameter 'controller' is unused [misc-unused-parameters] Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus-test-util.h@482 PS7, Line 482: virtual void StartElection(const RunLeaderElectionRequestPB* request, > warning: parameter 'request' is unused [misc-unused-parameters] Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus-test-util.h@482 PS7, Line 482: virtual > nit: drop Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus-test-util.h@483 PS7, Line 483: RunLeaderElectionResponsePB* response, > warning: parameter 'response' is unused [misc-unused-parameters] Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus-test-util.h@484 PS7, Line 484: rpc::RpcController* controller, > warning: parameter 'controller' is unused [misc-unused-parameters] Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus-test-util.h@485 PS7, Line 485: const rpc::ResponseCallback& callback) override {} > warning: parameter 'callback' is unused [misc-unused-parameters] Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus-test-util.h@489 PS7, Line 489: rpc::RpcController* controller, > warning: parameter 'controller' is unused [misc-unused-parameters] Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus.proto File src/kudu/consensus/consensus.proto: http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus.proto@463 PS7, Line 463: optional bytes new_leader_uuid = 4; > Why do we need this? Wouldn't it be better for the leader to always simply It's useful for leadership rebalancing and decommissioning to be able to pick a server. http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus_peers.h File src/kudu/consensus/consensus_peers.h: http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus_peers.h@226 PS7, Line 226: rpc::RpcController* controller, > warning: parameter 'controller' is unused [misc-unused-parameters] Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus_peers.h@227 PS7, Line 227: const rpc::ResponseCallback& callback) { > warning: parameter 'callback' is unused [misc-unused-parameters] Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus_queue.h File src/kudu/consensus/consensus_queue.h: http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus_queue.h@362 PS7, Line 362: boost:none > boost::none Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus_queue.h@365 PS7, Line 365: WatchForSuccessor > nit: maybe, rename to BeginWatchForSuccessor() to pair with the EndWatchFor Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus_queue.h@559 PS7, Line 559: designated_successor_ > designated_successor_uuid_ ? Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/consensus/consensus_queue.h@593 PS7, Line 593: virtual void NotifyPeerToStartElection(const std::string& peer_uuid) = 0; > Add documentation. Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/integration-tests/raft_consensus-itest.cc File src/kudu/integration-tests/raft_consensus-itest.cc: http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/integration-tests/raft_consensus-itest.cc@3054 PS7, Line 3054: : > nit: two extra lines Done http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/tools/kudu-admin-test.cc File src/kudu/tools/kudu-admin-test.cc: http://gerrit.cloudera.org:8080/#/c/11251/7/src/kudu/tools/kudu-admin-test.cc@1514 PS7, Line 1514: , { } > nit: drop Done -- To view, visit http://gerrit.cloudera.org:8080/11251 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic97343af9eb349556424c999799ed5e2941f0083 Gerrit-Change-Number: 11251 Gerrit-PatchSet: 7 Gerrit-Owner: Will Berkeley <wdberke...@gmail.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Fengling Wang <fw...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-Comment-Date: Fri, 21 Sep 2018 16:17:43 +0000 Gerrit-HasComments: Yes
