Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10058 )
Change subject: IMPALA-6847: work around high memory estimates for AC ...................................................................... Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/10058/6/be/src/scheduling/query-schedule.cc File be/src/scheduling/query-schedule.cc: http://gerrit.cloudera.org:8080/#/c/10058/6/be/src/scheduling/query-schedule.cc@195 PS6, Line 195: per_host_mem = min(query_option_memory_limit, request_.per_host_mem_estimate); > The frontend set mem_limit so we need a special case here regardless to avo Oops, yeah, we still have to special case. It'd be nice if we could organize this if-stmt branches in a way that makes it obvious that this new branch is really a subset of the "estimate" case and not the "has_query_option" best practice case. But maybe the code gets ugly that way. http://gerrit.cloudera.org:8080/#/c/10058/6/common/thrift/ImpalaInternalService.thrift File common/thrift/ImpalaInternalService.thrift: http://gerrit.cloudera.org:8080/#/c/10058/6/common/thrift/ImpalaInternalService.thrift@284 PS6, Line 284: // this value, use this as the memory limit for the purposes of admission. 0 or -1 > Copied over the text from the other place (this text was stale). It'd be good to also be clear that this option is only used in the non best practices case. -- To view, visit http://gerrit.cloudera.org:8080/10058 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia5fc32a507ad0f00f564dfe4f954a829ac55d14e Gerrit-Change-Number: 10058 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Mon, 16 Apr 2018 18:55:42 +0000 Gerrit-HasComments: Yes
