Will Berkeley has posted comments on this change.

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


Patch Set 2:

(4 comments)

I added a test and fixed a bug (plus the changes from your comments)

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

PS1, Line 1259:         // pass
              :        
> Got some tabs in here.
Done


PS1, Line 1267: int TSTabletManager::NumNotInitializedTablets() {
              :   return 
UpdateTabletStateSummaryAndReturnCount(tablet::NOT_INITIALIZED);
              : }
> Since this pattern is repeated a lot, perhaps you could change UpdateTablet
Done


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

Line 94:         return num_not_initialized;
> warning: missing username/bug in TODO [google-readability-todo]
git blame'd on todd


Line 308:   // Handle the case on startup where we find a tablet that is not in
> Nit: convention is to add the "Unlocked" suffix to functions that must be c
Superseded by later comment + fix.


-- 
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: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <[email protected]>
Gerrit-HasComments: Yes

Reply via email to