Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14260 )
Change subject: [consensus] KUDU-2947 fix voting in case of slow WAL ...................................................................... Patch Set 3: Code-Review+1 (4 comments) Overall looks good. Just a couple nits and a couple questions http://gerrit.cloudera.org:8080/#/c/14260/3/src/kudu/consensus/raft_consensus.cc File src/kudu/consensus/raft_consensus.cc: http://gerrit.cloudera.org:8080/#/c/14260/3/src/kudu/consensus/raft_consensus.cc@1420 PS3, Line 1420: SnoozeFailureDetector(); I know they're not coupled right now, but I wonder if it would make sense to consider putting WithholdVotesUnlocked() into SnoozeFailureDetector(). It seems their goals are similar -- to inform consensus (in some form) that things aren't in a bad state (so I shouldn't call an election, so I shouldn't vote yes to someone else). It affects more than just what's needed here, but have you considered whether something like that would be beneficial? http://gerrit.cloudera.org:8080/#/c/14260/3/src/kudu/integration-tests/raft_consensus_election-itest.cc File src/kudu/integration-tests/raft_consensus_election-itest.cc: http://gerrit.cloudera.org:8080/#/c/14260/3/src/kudu/integration-tests/raft_consensus_election-itest.cc@578 PS3, Line 578: if (!AllowSlowTests()) { : LOG(WARNING) << "test is skipped; set KUDU_ALLOW_SLOW_TESTS=1 to run"; : return; : } nit: you can use SKIP_IF_SLOW_NOT_ALLOWED() for this http://gerrit.cloudera.org:8080/#/c/14260/3/src/kudu/integration-tests/raft_consensus_election-itest.cc@613 PS3, Line 613: but it's assumed a loopback interface : // that should not be the case. The consensus RPC queues are not supposed : // to be overflown. I don't quite follow this bit here. What's the loopback interface? And why would one expect there to be an RPC queue overflow? http://gerrit.cloudera.org:8080/#/c/14260/3/src/kudu/integration-tests/raft_consensus_election-itest.cc@711 PS3, Line 711: not Remove -- To view, visit http://gerrit.cloudera.org:8080/14260 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c061b498e727a1a11e94e03c55530eeebfdf8dd Gerrit-Change-Number: 14260 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin <[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: Volodymyr Verovkin <[email protected]> Gerrit-Comment-Date: Fri, 20 Sep 2019 20:09:54 +0000 Gerrit-HasComments: Yes
