Adar Dembo has posted comments on this change. Change subject: [consensus] KUDU-1407 replica is not evcited when TABLET_NOT_RUNNING ......................................................................
Patch Set 3: (4 comments) Again, will defer to Mike and David. http://gerrit.cloudera.org:8080/#/c/5352/3/src/kudu/consensus/consensus_queue-test.cc File src/kudu/consensus/consensus_queue-test.cc: Line 818: TEST_F(ConsensusQueueTest, TestTriggerTabletCopyIfTabletNotRunning) { This looks like it's exactly the same test as the one below, but with a different code. Could you consolidate all of the shared code? If this means running both "tests" in a single TEST_F(), that's fine too. http://gerrit.cloudera.org:8080/#/c/5352/3/src/kudu/integration-tests/raft_consensus-itest.cc File src/kudu/integration-tests/raft_consensus-itest.cc: Line 2772: // A new good copy should get created because follower returns Ah, this addresses one of my comments from your earlier patch. It looks like this hunk belonged in that patch. Can you move it? http://gerrit.cloudera.org:8080/#/c/5352/3/src/kudu/tserver/ts_tablet_manager.cc File src/kudu/tserver/ts_tablet_manager.cc: Line 574: Status TSTabletManager::StartTabletStateTransitionUnlocked( These changes affect the semantics of DeleteTablet() and StartTabletCopy() somewhat, right? AFAICT it's now possible for DeleteTablet() to return ALREADY_INPROGRESS, and for StartTabletCopy() to return TABLET_NOT_RUNNING. I don't think that's necessarily wrong, I just want to make sure you've thought through the side effects of that. How will the RPC callers of DeleteTablet() and StartTabletCopy() react to those errors? http://gerrit.cloudera.org:8080/#/c/5352/3/src/kudu/tserver/ts_tablet_manager.h File src/kudu/tserver/ts_tablet_manager.h: PS3, Line 207: replica Nit: the rest of this comment refers to "tablet" when describing a "tablet replica", so please do the same for the new section you added. -- To view, visit http://gerrit.cloudera.org:8080/5352 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I554ad61f25a7de78eda60fd50228e0f015c1b625 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat <din...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
