Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/7981 )
Change subject: KUDU-2044 Tombstoned tablets show up in /metrics ...................................................................... Patch Set 6: (6 comments) http://gerrit.cloudera.org:8080/#/c/7981/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/7981/6//COMMIT_MSG@20 PS6, Line 20: tombstoned's tablets > tombstoned tablet's Done http://gerrit.cloudera.org:8080/#/c/7981/6/src/kudu/integration-tests/tablet_copy-itest.cc File src/kudu/integration-tests/tablet_copy-itest.cc: http://gerrit.cloudera.org:8080/#/c/7981/6/src/kudu/integration-tests/tablet_copy-itest.cc@1726 PS6, Line 1726: // Find the leader so we can tombstone a follower, which makes the test faster and more focused. : int leader_index; : int follower_index; : TServerDetails* leader_ts; : TServerDetails* follower_ts; : ASSERT_OK(FindTabletLeader(ts_map_, tablet_id, kTimeout, &leader_ts)); : leader_index = cluster_->tablet_server_index_by_uuid(leader_ts->uuid()); : follower_index = (leader_index + 1) % cluster_->num_tablet_servers(); : follower_ts = ts_map_[cluster_->tablet_server(follower_index)->uuid()]; : : // Make sure the metrics reflect that some rows have been inserted. : ASSERT_GT(CountRowsInserted(cluster_->tablet_server(follower_index)), 0); : : // Tombstone the tablet on the follower. : ASSERT_OK(itest::DeleteTablet(follower_ts, tablet_id, : TABLET_DATA_TOMBSTONED, : boost::none, kTimeout)); > This bit should be wrapped in ASSERT_EVENTUALLY() { ... } in order to ensu Done http://gerrit.cloudera.org:8080/#/c/7981/6/src/kudu/tserver/tablet_server-test.cc File src/kudu/tserver/tablet_server-test.cc: http://gerrit.cloudera.org:8080/#/c/7981/6/src/kudu/tserver/tablet_server-test.cc@2583 PS6, Line 2583: It takes three calls > Thanks for the explanation. Can you please add: "at the time of writing, ba Done http://gerrit.cloudera.org:8080/#/c/7981/6/src/kudu/tserver/tablet_server-test.cc@2591 PS6, Line 2591: mini > nit: indent this another 4 spaces to line up with the parameter on the abov Done http://gerrit.cloudera.org:8080/#/c/7981/6/src/kudu/tserver/tablet_server-test.cc@2592 PS6, Line 2592: &buf > nit: the indentation of this line seems random It's the second argument to FetchURL, so it's indented to line up with the first argument (strings::Substitute(...)) http://gerrit.cloudera.org:8080/#/c/7981/6/src/kudu/util/metrics.h File src/kudu/util/metrics.h: http://gerrit.cloudera.org:8080/#/c/7981/6/src/kudu/util/metrics.h@544 PS6, Line 544: bool unpublished_; > Sometimes thinking in negatives can be confusing. How about flipping this a Done -- To view, visit http://gerrit.cloudera.org:8080/7981 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I21b52262203410ded1e514502b59a79be12f8fb3 Gerrit-Change-Number: 7981 Gerrit-PatchSet: 6 Gerrit-Owner: Will Berkeley <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Thu, 05 Oct 2017 17:52:27 +0000 Gerrit-HasComments: Yes
