Dan Hecht has posted comments on this change. Change subject: IMPALA-3208: max_row_size option ......................................................................
Patch Set 11: (6 comments) http://gerrit.cloudera.org:8080/#/c/7629/11//COMMIT_MSG Commit Message: PS11, Line 18: The default value is 512KB. I picked that number because it doesn't : increase minimum reservations *too* much but should be large enough : for almost all reasonable workloads. what's the advantage of picking that below the default buffer size? Oh, I guess the reason is it puts a limit on the savings the scaled down buffer size optimization? http://gerrit.cloudera.org:8080/#/c/7629/11/be/src/exec/partitioned-aggregation-node.h File be/src/exec/partitioned-aggregation-node.h: Line 741: resource_profile_.max_row_buffer_size * 2; if we're going to use the resource profile to compute this, any reason not to just get the reservation value directly from there rather than recomputing? http://gerrit.cloudera.org:8080/#/c/7629/11/be/src/service/query-options.cc File be/src/service/query-options.cc: PS11, Line 522: Spillable should that say "Min spillable"? http://gerrit.cloudera.org:8080/#/c/7629/11/common/thrift/ImpalaInternalService.thrift File common/thrift/ImpalaInternalService.thrift: Line 270: 60: optional i64 max_row_size = 524288; doesn't have to be in this commit, but it'd be nice to document the interaction of all the query options that affect memory reservations. that should probably go in the docs, but may also help as a comment in the code somewhere. http://gerrit.cloudera.org:8080/#/c/7629/11/fe/src/main/java/org/apache/impala/planner/AggregationNode.java File fe/src/main/java/org/apache/impala/planner/AggregationNode.java: PS11, Line 323: max-size buffers let's spell that out a bit more: ... large enough to hold the maximum-sized row ... or similar. http://gerrit.cloudera.org:8080/#/c/7629/11/fe/src/main/java/org/apache/impala/planner/HashJoinNode.java File fe/src/main/java/org/apache/impala/planner/HashJoinNode.java: PS11, Line 235: max-size buffers same -- 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: 11 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
