Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/11157 )
Change subject: IMPALA-7349: Add Admission control support for automatically setting mem_limit ...................................................................... Patch Set 7: (3 comments) http://gerrit.cloudera.org:8080/#/c/11157/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11157/7//COMMIT_MSG@10 PS7, Line 10: With this patch the mem_limit of a query is automatically set based on : the mem_limit set in the query options and the mem_estimate calculated : by the planner. > For the new behavior (Assuming either min or max memlimit is set in the poo In the "else" clause, I understand why we have the min/max clamping values (because we're using heuristics to cook up a mem_limit). But in the "then" clause, why does it make sense to override the mem_limit if it was explicitly set by the user? Also, in terms of terminology in this part, my suggestion would be to come up with a different term rather than mem_limit for the thing before it gets clamped, and keep using mem_limit to mean only the final value used to limit memory consumption. But it's difficult to do that given the "then" clause and the pre-existing mem_limit query option name, which is another reason for my question above. http://gerrit.cloudera.org:8080/#/c/11157/7//COMMIT_MSG@15 PS7, Line 15: in-query-mem-limit-bytes, max-query-mem-limit-bytes these names seem okay. http://gerrit.cloudera.org:8080/#/c/11157/7//COMMIT_MSG@18 PS7, Line 18: strict-min-max-query-mem-limit this is a bit unclear, but not sure I have a better suggestion. In any case, let's finish the discussion above first anyway. -- To view, visit http://gerrit.cloudera.org:8080/11157 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifec00141651982f5975803c2165b7d7a10ebeaa6 Gerrit-Change-Number: 11157 Gerrit-PatchSet: 7 Gerrit-Owner: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Wed, 22 Aug 2018 17:09:13 +0000 Gerrit-HasComments: Yes
