impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-5713: always reserve memory for preaggs
Date Wed, 23 Aug 2017 23:20:57 GMT
Alex Behm has posted comments on this change.

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

Patch Set 4: Code-Review+1


FE changes lgtm. I only skimmed the BE code which also looked ok.

Let's wait for Mostafa's input.
File fe/src/main/java/org/apache/impala/planner/

PS3, Line 327: 16b
> I guess the number of buckets doesn't matter too much from the frontend's p
I vote for omitting that last calculation.

Line 330:         perInstanceMinReservation = (bufferSize + 64 * 1024) * PARTITION_FANOUT;
> Yeah it was. That's one of the reasons I don't think this change is a win i
Tim once suggested that the pre-agg should really be a separate exec node. I like that idea
a lot since the pre-agg is such a critical step with special characteristics. Why even have
16 partitions etc.

I prefer this patch over the old behavior because in most circumstances I believe users would
be willing to give up a little memory to have their query performance be more robust. The
really bad scenario where all rows are passed through could happen "sometimes" even for the
same query depending on the state of the system, so I think getting a reservation is the right
way to go.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I2b544f9ec1a906719e2bbb074743926b95a3a573
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Dan Hecht <>
Gerrit-Reviewer: Mostafa Mokhtar <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-HasComments: Yes

View raw message