Tim Armstrong 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 12: (7 comments) http://gerrit.cloudera.org:8080/#/c/10058/12//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10058/12//COMMIT_MSG@9 PS12, Line 9: MAX_MEM_LIMIT_FOR_ADMISSION > update Done http://gerrit.cloudera.org:8080/#/c/10058/12//COMMIT_MSG@14 PS12, Line 14: MAX_MEM_LIMIT_FOR_ADMISSION > same Done http://gerrit.cloudera.org:8080/#/c/10058/12/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/10058/12/common/thrift/ImpalaService.thrift@294 PS12, Line 294: // See comment in ImpalaInternalService.thrift > I like the idea of de-duping the comments, but perhaps it should go the oth Done http://gerrit.cloudera.org:8080/#/c/10058/12/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/10058/12/fe/src/main/java/org/apache/impala/service/Frontend.java@1063 PS12, Line 1063: set > typo: sent Done http://gerrit.cloudera.org:8080/#/c/10058/12/fe/src/main/java/org/apache/impala/service/Frontend.java@1065 PS12, Line 1065: // shown in the plan. > given that, will it be hard to diagnose admission decisions? though I guess Yeah, it also change the memory estimate shown in the profile, so you can see that the memory estimate in the plan embedded in the profile is different from the memory estimate in the profile counter. It seemed useful to have the original memory estimate in the profile in some form too so we could see how much the override changed the behaviour. http://gerrit.cloudera.org:8080/#/c/10058/12/fe/src/main/java/org/apache/impala/service/Frontend.java@1129 PS12, Line 1129: checkForMemEstimateOverride > nit: how about checkAndOverrideMemEstimate Done http://gerrit.cloudera.org:8080/#/c/10058/12/testdata/workloads/functional-query/queries/QueryTest/admission-reject-mem-estimate.test File testdata/workloads/functional-query/queries/QueryTest/admission-reject-mem-estimate.test: http://gerrit.cloudera.org:8080/#/c/10058/12/testdata/workloads/functional-query/queries/QueryTest/admission-reject-mem-estimate.test@1 PS12, Line 1: ==== > add case where mem_limit and max_mem_estimate_for_admission both are set, t Added cases where mem_limit is higher and lower. -- 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: 12 Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Tue, 17 Apr 2018 20:46:53 +0000 Gerrit-HasComments: Yes
