Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13426 )

Change subject: KUDU-2797: the master aggregates tablet statistics
......................................................................


Patch Set 9:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/13426/9/src/kudu/master/master_path_handlers.cc
File src/kudu/master/master_path_handlers.cc:

http://gerrit.cloudera.org:8080/#/c/13426/9/src/kudu/master/master_path_handlers.cc@394
PS9, Line 394:         if (role == RaftPeerPB::LEADER) {
> 1.Hmm, should I sum up all of the replicas and displayed in the web UI?
1. Yes, I still think that's valuable, it's just that it'll also be valuable as 
a metric or for client APIs, so the summing ought to happen in a layer below 
the web UI.

2. Ideally it'd be more deterministic than that, so that there aren't weird 
fluctuations in the aggregate's value. Maybe pick the first after placing the 
replica IDs in alphabetically sorted order?


http://gerrit.cloudera.org:8080/#/c/13426/9/src/kudu/tserver/heartbeater.cc
File src/kudu/tserver/heartbeater.cc:

http://gerrit.cloudera.org:8080/#/c/13426/9/src/kudu/tserver/heartbeater.cc@378
PS9, Line 378:   // Update the tablet statistics if necessary.
> Yes, Andrew's suggestion is good and it really should be here.
The scheduling I'm talking about needn't use a dedicated thread; you could use 
a PeriodicTimer, though you'd need to plumb a Messenger into TsTabletManager. 
If that's too much work, we can remain with this approach for now.


http://gerrit.cloudera.org:8080/#/c/13426/9/src/kudu/tserver/tserver_path_handlers.cc
File src/kudu/tserver/tserver_path_handlers.cc:

http://gerrit.cloudera.org:8080/#/c/13426/9/src/kudu/tserver/tserver_path_handlers.cc@423
PS9, Line 423:     replica->tablet()->CountLiveRows(&row_count);
> I'm sorry I can't catch up your meaning. Could you please explain more? :(
See 
https://github.com/apache/kudu/blob/master/src/kudu/tablet/tablet_replica.cc#L201
 for an example of what I mean: a metric that uses a FunctionGauge to compute 
its value. That's probably the right way to expose live row count as a new 
tablet-level metric.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I74406ab7cca7c22fda455c328b8ee9989a6b2d99
Gerrit-Change-Number: 13426
Gerrit-PatchSet: 9
Gerrit-Owner: helifu <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <[email protected]>
Gerrit-Reviewer: helifu <[email protected]>
Gerrit-Comment-Date: Thu, 13 Jun 2019 03:48:25 +0000
Gerrit-HasComments: Yes

Reply via email to