Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/8297 )
Change subject: consensus: support changing between VOTER and NON_VOTER ...................................................................... Patch Set 4: (4 comments) http://gerrit.cloudera.org:8080/#/c/8297/4/src/kudu/consensus/raft_consensus.h File src/kudu/consensus/raft_consensus.h: http://gerrit.cloudera.org:8080/#/c/8297/4/src/kudu/consensus/raft_consensus.h@569 PS4, Line 569: boost::optional<MonoDelta> delta = boost::none); > can you explain what 'delta' is here? I know it's just passing through to Done http://gerrit.cloudera.org:8080/#/c/8297/4/src/kudu/consensus/raft_consensus.cc File src/kudu/consensus/raft_consensus.cc: http://gerrit.cloudera.org:8080/#/c/8297/4/src/kudu/consensus/raft_consensus.cc@2424 PS4, Line 2424: ToggleFailureDetector(cmeta_->CommittedConfig()); > can't you remove the config param from the ToggleFailureDetector function Indeed -- that's a good observation, thanks! http://gerrit.cloudera.org:8080/#/c/8297/4/src/kudu/consensus/raft_consensus.cc@2622 PS4, Line 2622: ToggleFailureDetector(new_config); > same here, we just set the pending config, so if the function just uses Act Done http://gerrit.cloudera.org:8080/#/c/8297/4/src/kudu/integration-tests/raft_consensus_nonvoter-itest.cc File src/kudu/integration-tests/raft_consensus_nonvoter-itest.cc: http://gerrit.cloudera.org:8080/#/c/8297/4/src/kudu/integration-tests/raft_consensus_nonvoter-itest.cc@103 PS4, Line 103: // Promote non-voter replica to voter one. : Status PromoteReplica(const string& tablet_id, : const TServerDetails* replica, : const MonoDelta& timeout) { : return ChangeReplicaMembership(tablet_id, replica, RaftPeerPB::VOTER, timeout); : } : : // Demote voter replica to non-voter one. : Status DemoteReplica(const string& tablet_id, : const TServerDetails* replica, : const MonoDelta& timeout) { : return ChangeReplicaMembership(tablet_id, replica, RaftPeerPB::NON_VOTER, timeout); : } > I'd go one further and just inline these ChangeReplicaMembership calls dow Done -- To view, visit http://gerrit.cloudera.org:8080/8297 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I978cbed683b0e95a903f2bd2b57496aee71cb33a Gerrit-Change-Number: 8297 Gerrit-PatchSet: 4 Gerrit-Owner: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: David Ribeiro Alves <davidral...@gmail.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Tue, 24 Oct 2017 06:43:25 +0000 Gerrit-HasComments: Yes
