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-3208: max row size option
Date Thu, 17 Aug 2017 22:21:02 GMT
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-3208: max_row_size option
......................................................................


Patch Set 10:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/7629/10/common/thrift/PlanNodes.thrift
File common/thrift/PlanNodes.thrift:

PS10, Line 503:   4: optional i64 max_row_buffer_size
              : }
> maybe this will become clear to me as I review more code, but why does this
There's an invariant that I forgot to document that this is >= spillable_buffer_size. So
if max_row_size is low and different nodes have different buffer sizes, this can have different
values for different nodes.

I've generally tended to put all of the logic for deciding buffer sizes in the frontend rather
than the backend, so the idea here is that the frontend should figure this out per-node then
the backend doesn't need to do anything with the sizes except from pass the values through
to BufferedTupleStream (or whatever)


http://gerrit.cloudera.org:8080/#/c/7629/10/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java
File fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java:

PS10, Line 251:     // Analytic uses a single buffer size.
> Why? It looks like every other node has the same behavior - can this just d
The hash join and the agg use multiple buffer sizes - buffers use the default buffer size
and larger buffers are allocated only for larger rows.


http://gerrit.cloudera.org:8080/#/c/7629/10/fe/src/main/java/org/apache/impala/planner/ResourceProfileBuilder.java
File fe/src/main/java/org/apache/impala/planner/ResourceProfileBuilder.java:

PS10, Line 37: et
set


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

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

Mime
View raw message