Mike Percy has posted comments on this change. Change subject: [consensus] KUDU-1613: Fix replica eviction failure for WRONG_SERVER_UUID ......................................................................
Patch Set 7: (4 comments) http://gerrit.cloudera.org:8080/#/c/5111/7/src/kudu/consensus/consensus_peers.cc File src/kudu/consensus/consensus_peers.cc: Line 305: if (response_.error().code() != TabletServerErrorPB::WRONG_SERVER_UUID && I think we should consider WRONG_SERVER_UUID as unresponsive (ERROR) instead of ERROR_BUT_ALIVE, since it seems pointless to try to run tablet copy on it when that would never succeed due to it having a different UUID. http://gerrit.cloudera.org:8080/#/c/5111/7/src/kudu/integration-tests/raft_consensus-itest.cc File src/kudu/integration-tests/raft_consensus-itest.cc: Line 2789: MonoDelta kTimeout = MonoDelta::FromSeconds(30); nit: const Line 2795: int leader_idx = -1; nit: This is written in a C-like way. It would be a bit more readable to declare the variables at the site where they are first used. Also the nullptr initializations are not needed and the -1s are never used. Line 2816: Please add a comment in here to explain what's going on: // At this point, the leader will evict the old tablet due to unresponsiveness and the master will assign the replica to the tablet server with the new UUID. We wait for the leader to copy the tablet to the newly-wiped replica. -- To view, visit http://gerrit.cloudera.org:8080/5111 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0d3f84fd297a8e4760208a213c1ee393e92499a3 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Dinesh Bhat <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
