Tim Armstrong 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 10: (5 comments) 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: // The JVM max heap size is static and therefore known at this point > In that case would it make sense to always account for "JvmMemoryMetric::NO The maximum amount of non-heap memory isn't fixed though is the problem. On /metrics on my system: jvm.non-heap.max-usage-bytes -1.00 B Jvm non-heap Max Usage Bytes http://gerrit.cloudera.org:8080/#/c/10928/10/be/src/scheduling/query-schedule.h File be/src/scheduling/query-schedule.h: http://gerrit.cloudera.org:8080/#/c/10928/10/be/src/scheduling/query-schedule.h@75 PS10, Line 75: should > nit: can Done http://gerrit.cloudera.org:8080/#/c/10928/10/be/src/util/memory-metrics.cc File be/src/util/memory-metrics.cc: http://gerrit.cloudera.org:8080/#/c/10928/10/be/src/util/memory-metrics.cc@196 PS10, Line 196: HEAP_MAX_USAGE > is it worth documenting in the header file that these correspond to the "he Documented the metric names (I think using the public name is more intuitive than referencing another part of the code). http://gerrit.cloudera.org:8080/#/c/10928/10/common/thrift/StatestoreService.thrift File common/thrift/StatestoreService.thrift: http://gerrit.cloudera.org:8080/#/c/10928/10/common/thrift/StatestoreService.thrift@76 PS10, Line 76: admit_mem_limit > we might have to change the wording on AdmissionController::REASON_REQ_OVER Done http://gerrit.cloudera.org:8080/#/c/10928/10/tests/custom_cluster/test_jvm_mem_tracking.py File tests/custom_cluster/test_jvm_mem_tracking.py: http://gerrit.cloudera.org:8080/#/c/10928/10/tests/custom_cluster/test_jvm_mem_tracking.py@50 PS10, Line 50: GB > nit: MB? doh -- 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: 10 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Pooja Nilangekar <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Tue, 04 Dec 2018 00:43:19 +0000 Gerrit-HasComments: Yes
