Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14128 )
Change subject: [metrics] Add a metric to count sub-entities when merge metrics ...................................................................... Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/14128/3//COMMIT_MSG Commit Message: PS3: In discussing this, are you sure we want to introduce the concept of a "sub-entity"? It doesn't seem necessary to me: when talking about merging entities, when we're done merging, we publish a metric that describes the number of merged entities, a merged entity count, or something along those lines. "Sub-entity" muddies the waters: does this refer to a different kind of entity that is somehow subordinate to a regular entity? If you agree, change 'sub_entities_count' to 'merged_entities_count' and adjust the rest of the commit message to reflect "merged" entities rather than "sub-entities". Plus whatever references exist in the code itself. http://gerrit.cloudera.org:8080/#/c/14128/3//COMMIT_MSG@13 PS3, Line 13: check whether a table is balanced or not by thirdparty monitor system. I saw this in my first pass but forgot to comment: how is this an effective proxy for table balance? Why not leverage the rebalancer? http://gerrit.cloudera.org:8080/#/c/14128/3/src/kudu/tablet/tablet.cc File src/kudu/tablet/tablet.cc: http://gerrit.cloudera.org:8080/#/c/14128/3/src/kudu/tablet/tablet.cc@163 PS3, Line 163: METRIC_DEFINE_gauge_size(tablet, sub_entities_count, Would it be possible to prevent the count from being displayed when regular tablet metrics are emitted, and instead only show it after they're merged? http://gerrit.cloudera.org:8080/#/c/14128/2/src/kudu/util/metrics.cc File src/kudu/util/metrics.cc: http://gerrit.cloudera.org:8080/#/c/14128/2/src/kudu/util/metrics.cc@479 PS2, Line 479: // metrics, we should keep them around until the next poll. > Thanks for your advice. Is tablet->table the only kind of merge allowed? Or is it possible to merge table entities together? Or server entities? Why not future-proof this by defining a "merged entity count" metric prototype whenever we define a new metric entity prototype? -- To view, visit http://gerrit.cloudera.org:8080/14128 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I652b457a2df73414f95f5d1d5efaa003cc262bd1 Gerrit-Change-Number: 14128 Gerrit-PatchSet: 3 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Tue, 27 Aug 2019 23:15:48 +0000 Gerrit-HasComments: Yes
