impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5158,IMPALA-5236: account for unused buffer pool reservations
Date Wed, 16 Aug 2017 22:56:43 GMT
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5158,IMPALA-5236: account for unused buffer pool reservations
......................................................................


Patch Set 6:

(2 comments)

We could rework the MemTracker so that more of the logic was directly in the MemTracker instead
of in how the MemTrackers are connected up and given metrics.

We'd probably need a mode flag to control whether the top-level MemTracker was based on a
consumption metric (a lot of tests assume it isn't) but it may be reasonable to do since we
don't really make use of the configurability.

http://gerrit.cloudera.org:8080/#/c/7380/6//COMMIT_MSG
Commit Message:

> After staring at it for a while, I see why this makes sense and fixes the a
I don't think it's indicative of a problem if it's large. I don't think the top-level number
is directly actionable but does provide visibility into the state of the system.

WRT the second question. You can look at the operator profile and see if there was a gap between
the peak reservation and the peak used reservation. I don't think it's all that interesting
right now but if we moved to reserving more memory upfront a large difference would indicate
that more memory was reserved for an operator than needed. 

I think generally I'd expect users to start off with a higher-level question like "where is
my memory going?" or "why is my query using so much memory?". In that case you could follow
the breadcrumbs top-down like:

* Why did this query need X amount of memory? -> Look at Exec Summary
* Why did this operator need X amount of memory? -> Look at the operator profile
* How much memory did the operator reserve? Did it use all of the memory that it reserved?
-> Look at the counters in the operator profile.


PS6, Line 29: -8.25 MB
> Yeah I'm in favor of removing the UIntGauges for the memory metrics.
Yeah it wouldn't show up on /metrics. Or I could just add a derived metric that negates the
value.

I feel like the negative version of a metric is likely to be less intuitive than the positive
version even if there's a reasonable name for it.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idb1fa3110dc893321f9f4e8ced6b7ede12194dad
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <joemcdonnell@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message