John Russell has posted comments on this change. ( http://gerrit.cloudera.org:8080/8003 )
Change subject: IMPALA-3200: [DOCS] Document user-facing aspects of new buffer pool ...................................................................... Patch Set 7: (9 comments) http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_buffer_pool_limit.xml File docs/topics/impala_buffer_pool_limit.xml: http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_buffer_pool_limit.xml@53 PS5, Line 53: : </p> : > This area of behaviour in the code is a bit of a mess and subject to change Done http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_default_spillable_buffer_size.xml File docs/topics/impala_default_spillable_buffer_size.xml: http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_default_spillable_buffer_size.xml@72 PS5, Line 72: spill-to-disk operations. Reducing this value may reduce memory consumption. > The last sentence I think creates a lot of potential for confusion. The con Done http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_default_spillable_buffer_size.xml@73 PS5, Line 73: > Impala daemons? Done http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_max_row_size.xml File docs/topics/impala_max_row_size.xml: http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_max_row_size.xml@40 PS5, Line 40: , but that is not guaranteed.) Applies when : constructing intermediate or final rows in the result set. This setting prevents : out-of-control memory use when accessing columns containing huge strings. : </p> > This summary is wrong (the behaviour is subtle). The query option has zero Done http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_max_row_size.xml@69 PS5, Line 69: his setting. > This is correct but I'm concerned some users might take it the wrong way. T OK, will do. But is it not reasonable to worry a little if some rows are > 512 KB? I would imagine that some subtle change (adding or removing a host, HDFS rebalancing, changing the WHERE condition, updating some rows in a Kudu table, changing an impalad config setting) could cause big-row queries to fail where previously they succeeded. http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_min_spillable_buffer_size.xml File docs/topics/impala_min_spillable_buffer_size.xml: http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_min_spillable_buffer_size.xml@49 PS5, Line 49: <p> > Same comments as default_spillable_buffer_size. OK. I added a summary in the first paragraph, which like the DEFAULT_ topic was originally blank. I copied the wording about the tradeoff with a too-high value... but could a user actually lower this setting below 64KB? I got the impression maybe 64KB was already at the low end of the allowable range. http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_scalability.xml File docs/topics/impala_scalability.xml: http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_scalability.xml@570 PS5, Line 570: On each host that participates in the query, each such operator in a query requires memory > I think this background is helpful for users to understand the implications OK, used your suggested wording. http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_scalability.xml@572 PS5, Line 572: up front for each operator that supports spill-to-disk that is sufficient to execute the > Could we keep this around but make clear that it describes the behaviour in Let's revisit that later. Trying to tease out what is needed in a "Previously," section could be problematic so close to the deadline. http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_scalability.xml@627 PS5, Line 627: > Should we mention the old names parenthetically (it was ScratchBytesWritten Done -- To view, visit http://gerrit.cloudera.org:8080/8003 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I49323f8ffbff3e195058e88762eedbb1fcb1bc0e Gerrit-Change-Number: 8003 Gerrit-PatchSet: 7 Gerrit-Owner: John Russell <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: John Russell <[email protected]> Gerrit-Reviewer: Mostafa Mokhtar <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Fri, 06 Oct 2017 23:23:39 +0000 Gerrit-HasComments: Yes
