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-5713: always reserve memory for preaggs
Date Wed, 23 Aug 2017 22:13:34 GMT
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5713: always reserve memory for preaggs
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7739/3/fe/src/main/java/org/apache/impala/planner/AggregationNode.java
File fe/src/main/java/org/apache/impala/planner/AggregationNode.java:

PS3, Line 327: 16b
> should we add a static_assert to backend to update this if needed, or you t
I guess the number of buckets doesn't matter too much from the frontend's perspective - I
could just omit the calculation and say 64kb per hash table.


Line 330:         perInstanceMinReservation = (bufferSize + 64 * 1024) * PARTITION_FANOUT;
> this means we end up reserving around the same amount of memory for streami
Yeah it was. That's one of the reasons I don't think this change is a win in all situations.

I think that is less important though with the new buffer pool since the minimum requirements
with 2mb buffers is more sane (34mb versus 264mb to spill).

The memory reservation here is min(34mb, estimated size of all groups in memory), which seems
like a good amount of memory to reserve for a mid-sized aggregation if memory isn't too scarce.
This breaks down if the estimates are off and/or memory requirements are very tight (and we
don't care about the degraded performance).

And yeah, THP is a concern - preaggregations actually benefited the most from it.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2b544f9ec1a906719e2bbb074743926b95a3a573
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message