Dinesh Bhat has posted comments on this change. Change subject: [consensus] KUDU-1613: Fix replica eviction failure for WRONG_SERVER_UUID ......................................................................
Patch Set 7: (6 comments) TFTR Mike, I will be adding one more test which changes just the UUID keeping the tablets intact (which means all tablets are reported behind a new server UUID but behind same RPC endpoints) after I figure out a anomaly with non_participant I observed in the new test. 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) instea Done, please note that as per our offline discussions last week, the tablet copy is driven due to this error code deemed as tolerable until heartbeat timeout kicks out the replica and re-replication is triggered at taht point. http://gerrit.cloudera.org:8080/#/c/5111/7/src/kudu/consensus/consensus_queue-test.cc File src/kudu/consensus/consensus_queue-test.cc: Line 816: // Verify that tablet copy is triggered when peer responds with > Is this test still supposed to be here? Not after we removed the WRONG_SERVER_UUID from queue->ResponseFromPeer(). Updated. http://gerrit.cloudera.org:8080/#/c/5111/7/src/kudu/consensus/consensus_queue.cc File src/kudu/consensus/consensus_queue.cc: Line 597: peer->needs_tablet_copy = true; > Why this for WRONG_SERVER_UUID? I suppose this isn't needed if we are driving the tablet-copy via replica eviction and then re-replication, right ? 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 Done Line 2795: int leader_idx = -1; > nit: This is written in a C-like way. It would be a bit more readable to de Done Line 2816: > Please add a comment in here to explain what's going on: Done -- 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
