Will Berkeley has posted comments on this change.

Change subject: Add tablet state summary metrics
......................................................................


Patch Set 5:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/7980/5/src/kudu/integration-tests/tablet_copy-itest.cc
File src/kudu/integration-tests/tablet_copy-itest.cc:

Line 1530:   workload.StopAndJoin();
> Why not stop the workload on line 1523, as soon as we have the number of ro
Done


PS5, Line 1544: Sleep for > 10ms
> This comment seems out of date.
Done


http://gerrit.cloudera.org:8080/#/c/7980/5/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:

Line 115: DEFINE_int32(tablet_state_walk_min_period_ms, 1000,
> What do you think about putting the metrics prototypes, callbacks, and regi
Done. It's a wee bit awkward since the metrics struct and the tablet manager 
both need to reference each other because the (refactored) 
UpdateTabletStateSummaryAndReturnCount function most naturally lives in the 
tablet manager, but it does cut down the boilerplate in the tablet manager code.


Line 1226:   MonoDelta period = 
MonoDelta::FromMilliseconds(FLAGS_tablet_state_walk_min_period_ms);
> We can initialize this object before acquiring the lock
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/7980
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9fda2061d341586f0aa343787af59984a627080a
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
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-HasComments: Yes

Reply via email to