Bikramjeet Vig has posted comments on this change. ( )

Change subject: IMPALA-6847: work around high memory estimates for AC

Patch Set 12:


+1 for the simplified version (patch 12). It is way cleaner and also looks more 
like a workaround, which is what is intended. However, I am a bit concerned 
about this option becoming mainstream, since this is a workaround and will 
probably be deprecated in the future. We should discourage use of this unless 
absolutely necessary and ask users to use it at a more granular level (pool 
level) so that it will be easily manageable and removable once deprecated.

I also really like the ideas in patch 8 and looks like a good direction to 
steer our solution towards in IMPALA-6460. It was essentially decoupling the 2 
functions performed by mem_limit ( 1. to enforce a runtime limit and 2. to be 
used in admission control for admitting and accounting of memory) and would 
have made impala more stable, but the fact that a query option was used to 
override mem_limit's functionality and had overlapping functionality with 
mem_estimate (acting as an upper limit for it) was making it confusing.
File fe/src/main/java/org/apache/impala/service/
PS12, Line 1063: set
typo: sent
PS12, Line 1129: checkForMemEstimateOverride
nit: how about checkAndOverrideMemEstimate
PS12, Line 1: ====
add case where mem_limit and max_mem_estimate_for_admission both are set, to 
show that mem_limit is used over max_mem_estimate_for_admission

To view, visit
To unsubscribe, visit

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 <>
Gerrit-Reviewer: Bikramjeet Vig <>
Gerrit-Reviewer: Dan Hecht <>
Gerrit-Reviewer: Philip Zeyliger <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-Comment-Date: Tue, 17 Apr 2018 05:07:45 +0000
Gerrit-HasComments: Yes

Reply via email to