Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13979 )

Change subject: IMPALA-8806: Add metrics to improve observability of executor 
groups
......................................................................


Patch Set 2:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/13979/1/be/src/scheduling/cluster-membership-mgr-test.cc
File be/src/scheduling/cluster-membership-mgr-test.cc:

http://gerrit.cloudera.org:8080/#/c/13979/1/be/src/scheduling/cluster-membership-mgr-test.cc@271
PS1, Line 271:   MetricGroup tmp_metrics1("test-metrics1");
> We should create a dummy metrics group here instead of adding extra logic i
Done


http://gerrit.cloudera.org:8080/#/c/13979/1/be/src/scheduling/cluster-membership-mgr.cc
File be/src/scheduling/cluster-membership-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/13979/1/be/src/scheduling/cluster-membership-mgr.cc@48
PS1, Line 48: static const string 
LIVE_EXEC_GROUP_KEY("cluster-membership.executor-groups.total");
> Related to another comment, we could maybe omit the "live" adjective, since
Done


http://gerrit.cloudera.org:8080/#/c/13979/1/common/thrift/metrics.json
File common/thrift/metrics.json:

http://gerrit.cloudera.org:8080/#/c/13979/1/common/thrift/metrics.json@2446
PS1, Line 2446:  exe
> It might be better to avoid introducing the new "live" terminology and simp
Done


http://gerrit.cloudera.org:8080/#/c/13979/1/common/thrift/metrics.json@2446
PS1, Line 2446: at leas
> "at least" - here and below
Done


http://gerrit.cloudera.org:8080/#/c/13979/1/tests/custom_cluster/test_executor_groups.py
File tests/custom_cluster/test_executor_groups.py:

http://gerrit.cloudera.org:8080/#/c/13979/1/tests/custom_cluster/test_executor_groups.py@43
PS1, Line 43:     self.num_executors = 1
> I have a change that should be setting it to 50ms across all custom cluster
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7745ea1c7c6778d3fb5e59adbc873697beb0f3b9
Gerrit-Change-Number: 13979
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig <[email protected]>
Gerrit-Reviewer: Andrew Sherman <[email protected]>
Gerrit-Reviewer: Bikramjeet Vig <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Lars Volker <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Comment-Date: Thu, 01 Aug 2019 18:55:54 +0000
Gerrit-HasComments: Yes

Reply via email to