Adar Dembo has posted comments on this change. Change subject: Add metrics for tablet state and tablet copy ......................................................................
Patch Set 1: (6 comments) http://gerrit.cloudera.org:8080/#/c/7540/1//COMMIT_MSG Commit Message: PS1, Line 9: This patch adds a metric to each tablet that records its state, : e.g. RUNNING or TOMBSTONED. KUDU-2044 is related and worth fixing (i.e. we shouldn't publish metrics for TOMBSTONED tablets). http://gerrit.cloudera.org:8080/#/c/7540/1/src/kudu/integration-tests/tablet_copy-itest.cc File src/kudu/integration-tests/tablet_copy-itest.cc: Line 1264: // We should see one source session on the leader and one client session on the follower. If we only inject 1s of latency, are we still guaranteed to see the open sessions? Would it be possible for the copy to finish so quickly that we miss it? Line 1277: ASSERT_EQ(bytes_sent, bytes_fetched); Could also check that they're non-zero. http://gerrit.cloudera.org:8080/#/c/7540/1/src/kudu/tserver/tablet_copy_client.h File src/kudu/tserver/tablet_copy_client.h: Line 85: std::shared_ptr<TabletCopyClientMetrics> tablet_copy_metrics); Why does this need shared ownership? Seems like optionality (via boost::optional) would suffice. http://gerrit.cloudera.org:8080/#/c/7540/1/src/kudu/tserver/tablet_copy_service.h File src/kudu/tserver/tablet_copy_service.h: Line 128: std::shared_ptr<TabletCopySourceMetrics> tablet_copy_metrics_; Again, why shared ownership? Aren't all sessions guaranteed to be destroyed before the ServiceImpl is destroyed? http://gerrit.cloudera.org:8080/#/c/7540/1/src/kudu/tserver/tablet_copy_source_session.h File src/kudu/tserver/tablet_copy_source_session.h: Line 100: FsManager* fs_manager, std::shared_ptr<TabletCopySourceMetrics> metrics); Why shared ownership? -- To view, visit http://gerrit.cloudera.org:8080/7540 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c945826f5fdb181d40e004fc8ef00688f0dedb3 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley <wdberke...@gmail.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
