David Ribeiro Alves has posted comments on this change.
Change subject: KUDU-871. Support tombstoned voting
Patch Set 6:
leaving for luch, more comments forthcoming
PS6, Line 1519: LOG_WITH_PREFIX_UNLOCKED(INFO) << "voting while tombstoned
based on last-logged opid "
: << local_last_logged_opid;
move this to the else block above and thus avoid the extra if?
PS6, Line 2538: const optional<OpId>& tombstone_last_logged_opid
whats the point of passing an opid here if you're only checking for its
also you mention on LOC 1508 "in which case we are guaranteed that
// 'tombstone_last_logged_opid' is set by CheckSafeToVoteUnlocked()"
but that is not happening or am I missing something?
PS6, Line 124: static Status Create(ConsensusOptions options,
: RaftPeerPB local_peer_pb,
: ThreadPool* raft_pool,
didn't you delete this a while back?
PS6, Line 271: term
nit: current_term() would be more explicit
PS6, Line 292: Tombstoned voting
not super clear what "tombstoned voting" is maybe explain it somewhere?
PS6, Line 354: RaftConsensus has been stopped.
maybe explain what this means in the context of the other states? something
like: "RaftConsensus has been stopped. No more transactions or commits are
accepted but will still reply to election requests if tombstoned."
PS6, Line 195: // Ask TS1 for a vote that should be denied (old last-logged
: s = itest::RequestVote(ts1_ets, tablet_id, "B", current_term,
: /*ignore_live_leader=*/ true,
/*is_pre_election=*/ false, kTimeout);
can you add a test for same candidate same term but with an opid that is
greater that the tombstoned replica's op_id?
To view, visit http://gerrit.cloudera.org:8080/6960
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
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: Mike Percy <mpe...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>