Will Berkeley has posted comments on this change. Change subject: KUDU-1755 Part 2: Improve tablet on-disk size metric ......................................................................
Patch Set 17: (4 comments) http://gerrit.cloudera.org:8080/#/c/6968/17/src/kudu/consensus/log.cc File src/kudu/consensus/log.cc: PS17, Line 909: std::lock_guard > Does this need to be lock_guard? Could it be lock_shared<rw_spinlock> l(sta I think percpu_rwlock doesn't support shared_lock. http://gerrit.cloudera.org:8080/#/c/6968/17/src/kudu/tablet/diskrowset.cc File src/kudu/tablet/diskrowset.cc: PS17, Line 736: OnDiskDataSize() > nit: may be worth commenting in the API declaration that this does not incl Added (no deltas) for clarity. The explanatory comment above the declarations, when matched with the comments on the declarations, does make it clear exactly what a given *OnDiskSize* metric includes. http://gerrit.cloudera.org:8080/#/c/6968/17/src/kudu/tablet/tablet_metadata-test.cc File src/kudu/tablet/tablet_metadata-test.cc: PS17, Line 138: The on-disk size > nit: maybe "The tablet metadata"? Done http://gerrit.cloudera.org:8080/#/c/6968/17/src/kudu/tablet/tablet_replica.cc File src/kudu/tablet/tablet_replica.cc: PS17, Line 193: ) != nullptr > nit: at L701, you have a similar check that doesn't make this explicit comp Done -- To view, visit http://gerrit.cloudera.org:8080/6968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia83f8aae0a544abbd19f5a34520b4828b89b6315 Gerrit-PatchSet: 17 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley <wdberke...@gmail.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-HasComments: Yes