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 <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
