[Impala-ASF-CR] WIP CDPD-8989 Improve admission control pool stats logging to be more explicit
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16220 ) Change subject: WIP CDPD-8989 Improve admission control pool stats logging to be more explicit .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/16220/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16220/1//COMMIT_MSG@7 PS1, Line 7: WIP CDPD-8989 > Can you file an upstream JIRA and add that here instead +1 http://gerrit.cloudera.org:8080/#/c/16220/1/be/src/runtime/mem-tracker.cc File be/src/runtime/mem-tracker.cc: http://gerrit.cloudera.org:8080/#/c/16220/1/be/src/runtime/mem-tracker.cc@411 PS1, Line 411: if (pool_stats.min_memory_consumed > mem_consumed) : pool_stats.min_memory_consumed = mem_consumed; : : if (pool_stats.max_memory_consumed < mem_consumed) : pool_stats.max_memory_consumed = mem_consumed; multi-line if statements need curly braces. -- To view, visit http://gerrit.cloudera.org:8080/16220 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id995a9d044082c3b8f044e1ec25bb4c64347f781 Gerrit-Change-Number: 16220 Gerrit-PatchSet: 1 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 21 Jul 2020 16:56:23 + Gerrit-HasComments: Yes
[Impala-ASF-CR] WIP CDPD-8989 Improve admission control pool stats logging to be more explicit
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16220 ) Change subject: WIP CDPD-8989 Improve admission control pool stats logging to be more explicit .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/6675/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/16220 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id995a9d044082c3b8f044e1ec25bb4c64347f781 Gerrit-Change-Number: 16220 Gerrit-PatchSet: 3 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 21 Jul 2020 03:41:45 + Gerrit-HasComments: No
[Impala-ASF-CR] WIP CDPD-8989 Improve admission control pool stats logging to be more explicit
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16220 ) Change subject: WIP CDPD-8989 Improve admission control pool stats logging to be more explicit .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/16220/3/be/src/scheduling/admission-controller.cc File be/src/scheduling/admission-controller.cc: http://gerrit.cloudera.org:8080/#/c/16220/3/be/src/scheduling/admission-controller.cc@1331 PS3, Line 1331: // the total memory consumption, and the number of all queries running on this line has trailing whitespace -- To view, visit http://gerrit.cloudera.org:8080/16220 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id995a9d044082c3b8f044e1ec25bb4c64347f781 Gerrit-Change-Number: 16220 Gerrit-PatchSet: 3 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 21 Jul 2020 03:20:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] WIP CDPD-8989 Improve admission control pool stats logging to be more explicit
Qifan Chen has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/16220 ) Change subject: WIP CDPD-8989 Improve admission control pool stats logging to be more explicit .. WIP CDPD-8989 Improve admission control pool stats logging to be more explicit This work addresses the current limitation in admission controller by appending the last known memory consumption statistics about a host to the existing host memory exhaustion message. The message is logged in impalad.INFO when a query is queued or timed out due to memory pressure at the host. This new memory consumption statistics covers the following content: num_running: the total number of queries running top_queries: a list of query Ids for up to 5 queries with top memory consumptions min: the minimal memory consumption of all running queries max: the maximal memory consumption of all running queries total: the total memory consumption of all running queries average: the average memory consumption of all running queries One example of the statistics is as follows. Memory consumed: num_running=2, top_queries=[ dc4fd356433812be:6902546f, c54d5ab3f4773ee8:8fb6a628], min=12.30 MB, max=12.37 MB, total=24.67 MB, average=12.34 MB Testing: TBD Change-Id: Id995a9d044082c3b8f044e1ec25bb4c64347f781 --- M be/src/runtime/mem-tracker.cc M be/src/runtime/mem-tracker.h M be/src/scheduling/admission-controller.cc M common/thrift/StatestoreService.thrift 4 files changed, 190 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/16220/3 -- To view, visit http://gerrit.cloudera.org:8080/16220 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id995a9d044082c3b8f044e1ec25bb4c64347f781 Gerrit-Change-Number: 16220 Gerrit-PatchSet: 3 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] WIP CDPD-8989 Improve admission control pool stats logging to be more explicit
Bikramjeet Vig has posted comments on this change. (
http://gerrit.cloudera.org:8080/16220 )
Change subject: WIP CDPD-8989 Improve admission control pool stats logging to
be more explicit
..
Patch Set 1:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/16220/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16220/1//COMMIT_MSG@7
PS1, Line 7: WIP CDPD-8989
Can you file an upstream JIRA and add that here instead
http://gerrit.cloudera.org:8080/#/c/16220/1/be/src/runtime/mem-tracker.h
File be/src/runtime/mem-tracker.h:
http://gerrit.cloudera.org:8080/#/c/16220/1/be/src/runtime/mem-tracker.h@362
PS1, Line 362: UpdatePoolStatsForQueries
I think only this needs to be public, rest can be private if they are only
helper functions
http://gerrit.cloudera.org:8080/#/c/16220/1/be/src/runtime/mem-tracker.cc
File be/src/runtime/mem-tracker.cc:
http://gerrit.cloudera.org:8080/#/c/16220/1/be/src/runtime/mem-tracker.cc@568
PS1, Line 568:
: // Recursively append info about this memory tracker and all its
children to ss.
: void MemTracker::GetAllMemTracker(std::stringstream& ss, int
indent) {
: ss << std::string(indent, ' ') << " MemTracker: label=" <<
label_;
:
: if (!pool_name_.empty()) {
: ss << ", pool_name =" << pool_name_;
: }
:
: if (is_query_mem_tracker_) {
: ss << ", qid=" << PrintId(query_id_);
: }
:
: ss << std::endl;
: indent += 3;
: for (MemTracker* child : child_trackers_) {
: child->GetAllMemTracker(ss, indent);
: }
: }
:
: // Return a debug string for all memory trackers reachable from
the root memory
: // tracker reachable from this.
: string MemTracker::GetAllMemTrackers() {
: lock_guard l(child_trackers_lock_);
: std::stringstream ss;
: GetRootMemTracker()->GetAllMemTracker(ss, 0);
: return ss.str();
: }
leftover code from debugging perhaps?
http://gerrit.cloudera.org:8080/#/c/16220/1/be/src/scheduling/admission-controller.cc
File be/src/scheduling/admission-controller.cc:
http://gerrit.cloudera.org:8080/#/c/16220/1/be/src/scheduling/admission-controller.cc@556
PS1, Line 556: DebugPoolStatsForConsumedMemory
Here the check is at the host level, but the log line is produced using pool
level stats which is misleading. You will either have to merge the results from
all pools like we do for updating HostStats (which might get unwieldy when
aggregating the top 5 queries among pools) or add another statestore update
that updates host level stats
http://gerrit.cloudera.org:8080/#/c/16220/1/be/src/scheduling/admission-controller.cc@1328
PS1, Line 1328: if (tracker) {
: // update local_stats_ with the query Ids of the top N
queries, plus the min, the max,
: // the total memory consumption, and the number of all
queries in this pool.
: tracker->UpdatePoolStatsForQueries(5 /*limit*/,
this->local_stats_);
:
: }
This is being called for the pool level memtracker, which will only give up the
top 5 queries in the pool.
http://gerrit.cloudera.org:8080/#/c/16220/1/common/thrift/StatestoreService.thrift
File common/thrift/StatestoreService.thrift:
http://gerrit.cloudera.org:8080/#/c/16220/1/common/thrift/StatestoreService.thrift@52
PS1, Line 52: 5: required i64 min_memory_consumed;
:
: // Max memory consumption among all queries.
: 6: required i64 max_memory_consumed;
Now that i think of this, I am not sure how helpful the min/max memory consumed
will be. I think it will be more helpful to add the current consumption of the
top K queries and print them instead.
http://gerrit.cloudera.org:8080/#/c/16220/1/common/thrift/StatestoreService.thrift@58
PS1, Line 58: 7: required i64 total_memory_consumed;
fyi: this can be fetched using the consumption of the parent tracker instead of
adding all the queries.
Also, seems like this is already tracked locally using
metrics_.local_backend_mem_usage at the pool level
http://gerrit.cloudera.org:8080/#/c/16220/1/common/thrift/StatestoreService.thrift@60
PS1, Line 60: The current number of requests admitted by any admission
controllers
: // that are currently running. This is an instantaneous value
(as opposed to a
: // cumulative sum).
I think a more appropriate description will be that this is the num of queries
that have live fragments taking up memory on the host.
--
To view, visit http://gerrit.cloudera.org:8080/16220
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
[Impala-ASF-CR] WIP CDPD-8989 Improve admission control pool stats logging to be more explicit
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16220 ) Change subject: WIP CDPD-8989 Improve admission control pool stats logging to be more explicit .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/6661/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/16220 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id995a9d044082c3b8f044e1ec25bb4c64347f781 Gerrit-Change-Number: 16220 Gerrit-PatchSet: 1 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 20 Jul 2020 21:25:11 + Gerrit-HasComments: No
[Impala-ASF-CR] WIP CDPD-8989 Improve admission control pool stats logging to be more explicit
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/16220 )
Change subject: WIP CDPD-8989 Improve admission control pool stats logging to
be more explicit
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16220/1/be/src/scheduling/admission-controller.cc
File be/src/scheduling/admission-controller.cc:
http://gerrit.cloudera.org:8080/#/c/16220/1/be/src/scheduling/admission-controller.cc@560
PS1, Line 560: PrintBytes(admit_mem_limit), mem_consumed_by_bost,
GetStalenessDetailLocked(" "));
line too long (96 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/16220
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id995a9d044082c3b8f044e1ec25bb4c64347f781
Gerrit-Change-Number: 16220
Gerrit-PatchSet: 1
Gerrit-Owner: Qifan Chen
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sahil Takiar
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Mon, 20 Jul 2020 20:57:48 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] WIP CDPD-8989 Improve admission control pool stats logging to be more explicit
Qifan Chen has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16220 Change subject: WIP CDPD-8989 Improve admission control pool stats logging to be more explicit .. WIP CDPD-8989 Improve admission control pool stats logging to be more explicit This work addresses the current limitation in admission controller by appending the last known memory consumption statistics about a host to the existing host memory exhaustion message. The message is logged in impalad.INFO when a query is queued or timed out due to memory exhaustion at the host. This new memory consumption statistics covers the following content: num_running: the total number of queries running top_queries: a list of query Ids for up to 5 queries with top memory consumptions min: the minimal memory consumption of all running queries max: the maximal memory consumption of all running queries total: the total memory consumption of all running queries average: the average memory consumption of all running queries One example of the appended string is as follows. Memory consumed: num_running=2, top_queries=[ dc4fd356433812be:6902546f, c54d5ab3f4773ee8:8fb6a628], min=12.30 MB, max=12.37 MB, total=24.67 MB, average=12.34 MB Testing: TBD Change-Id: Id995a9d044082c3b8f044e1ec25bb4c64347f781 --- M be/src/runtime/mem-tracker.cc M be/src/runtime/mem-tracker.h M be/src/scheduling/admission-controller.cc M common/thrift/StatestoreService.thrift 4 files changed, 191 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/16220/1 -- To view, visit http://gerrit.cloudera.org:8080/16220 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Id995a9d044082c3b8f044e1ec25bb4c64347f781 Gerrit-Change-Number: 16220 Gerrit-PatchSet: 1 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong
