Pooja Nilangekar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10928 )

Change subject: IMPALA-7811: optionally count JVM heap towards process mem limit
......................................................................


Patch Set 8:

(5 comments)

The approach used to compute the memory utilized and accounting for it in the 
process memory makes sense. Thanks for adding the test to validate it.

http://gerrit.cloudera.org:8080/#/c/10928/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10928/8//COMMIT_MSG@22
PS8, Line 22: row
nit: typo


http://gerrit.cloudera.org:8080/#/c/10928/8//COMMIT_MSG@25
PS8, Line 25: We should not admit query memory in addition to the JVM memory.
Initially, I had some trouble understanding this. Could you please clarify this?


http://gerrit.cloudera.org:8080/#/c/10928/8/be/src/runtime/exec-env.cc
File be/src/runtime/exec-env.cc:

http://gerrit.cloudera.org:8080/#/c/10928/8/be/src/runtime/exec-env.cc@277
PS8, Line 277:     post_jvm_bytes_limit -= 
JvmMemoryMetric::HEAP_MAX_USAGE->GetValue();
Is there a reason for not subtracting the "JvmMemoryMetric::NON_HEAP_COMMITTED" 
memory from this value?


http://gerrit.cloudera.org:8080/#/c/10928/8/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/10928/8/be/src/service/impala-server.cc@1817
PS8, Line 1817: ;
nit: extra semicolon


http://gerrit.cloudera.org:8080/#/c/10928/8/be/src/service/impala-server.cc@1817
PS8, Line 1817:     admit_mem_limit -= 
JvmMemoryMetric::HEAP_MAX_USAGE->GetValue();
Same concern as above.
Is there a reason for not subtracting the "JvmMemoryMetric::NON_HEAP_COMMITTED" 
memory from this value?



--
To view, visit http://gerrit.cloudera.org:8080/10928
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I39dd715882a32fc986755d573bd46f0fd9eefbfc
Gerrit-Change-Number: 10928
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <pooja.nilange...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Comment-Date: Wed, 21 Nov 2018 01:15:42 +0000
Gerrit-HasComments: Yes

Reply via email to