Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18057 )
Change subject: KUDU-3341: No further retry when DeleteTabet with WRONG_SERVER_UUID error ...................................................................... Patch Set 3: (7 comments) http://gerrit.cloudera.org:8080/#/c/18057/1/src/kudu/integration-tests/delete_tablet-itest.cc File src/kudu/integration-tests/delete_tablet-itest.cc: http://gerrit.cloudera.org:8080/#/c/18057/1/src/kudu/integration-tests/delete_tablet-itest.cc@240 PS1, Line 240: > To speed up the test, could we reduce the Raft heartbeat interval down to s Done http://gerrit.cloudera.org:8080/#/c/18057/1/src/kudu/integration-tests/delete_tablet-itest.cc@241 PS1, Line 241: // Regression test for KUDU-3341: Ensure that master would not retry to send : // DeleteTablet() RPC to a "wrong > Does this test actually need 5 servers? Would we get the same test coverage No, we don't really need 5 servers. I tested with 3 tservers and when re-added the tserver with a new uuid, a replacement for the dead replica would also be triggered. So 3 servers is enough. http://gerrit.cloudera.org:8080/#/c/18057/1/src/kudu/integration-tests/delete_tablet-itest.cc@251 PS1, Line 251: NO_FATALS(StartCluster(/*num_tablet_servers=*/kNumTabletServers)); : TestWorkload wo > nit: could reduce to a single line? ASSERT_OK(v.RunKsck()); ? Done http://gerrit.cloudera.org:8080/#/c/18057/1/src/kudu/integration-tests/delete_tablet-itest.cc@267 PS1, Line 267: } > nit: we never called workload.Start(), so there's no need to call StopAndJo Done http://gerrit.cloudera.org:8080/#/c/18057/1/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/18057/1/src/kudu/master/catalog_manager.cc@4147 PS1, Line 4147: case TabletServerErrorPB::ALREADY_INPROGRESS: > Should we call MarkComplete() for this and TABLET_NOT_FOUND? It's a CAS wit Done http://gerrit.cloudera.org:8080/#/c/18057/1/src/kudu/mini-cluster/internal_mini_cluster.cc File src/kudu/mini-cluster/internal_mini_cluster.cc: http://gerrit.cloudera.org:8080/#/c/18057/1/src/kudu/mini-cluster/internal_mini_cluster.cc@221 PS1, Line 221: Status InternalMiniCluster::AddTabletServer(const HostPort& hp) { > warning: the parameter 'hp' is copied for each invocation but only used as Done http://gerrit.cloudera.org:8080/#/c/18057/1/src/kudu/mini-cluster/internal_mini_cluster.cc@230 PS1, Line 230: emplace_back(std::mov > nit: we could construct in place with Done -- To view, visit http://gerrit.cloudera.org:8080/18057 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieaa36086300bda7f958570c690b951dc090c342a Gerrit-Change-Number: 18057 Gerrit-PatchSet: 3 Gerrit-Owner: Yifan Zhang <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Comment-Date: Tue, 30 Nov 2021 08:25:43 +0000 Gerrit-HasComments: Yes
