[impala] 01/02: IMPALA-9754 buffer_pool_limit error message is confusing

[jo...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

joemcdonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git

commit 41813b27bc40c7fdf4e5eed3461d857dd1a2724b
Author: Qifan Chen <qc...@cloudera.com>
AuthorDate: Sun Oct 11 11:15:50 2020 -0400

    IMPALA-9754 buffer_pool_limit error message is confusing
    
    This fix reworded following two error messages for clarity.
    
    1. "Invalid --buffer_pool_limit value, must be a percentage or positive
        bytes value or percentage:"
    2. "Invalid --buffer_pool_clean_pages_limit value, must be a percentage
        or positive bytes value or percentage:"
    
    The fix also enhanced the code to verify that the JVM max heap size is
    less than the process memory limit when mem_limit_includes_jvm flag is
    set to true, and raise a new error message otherwise:
    
    "Invalid combination of --mem_limit_includes_jvm and JVM max
    heap size $0, which must be smaller than process memory limit $1".
    
    Testing:
    1. Unit testing;
    2. Ran Core tests successfully.
    
    Change-Id: I15ce1cdcc168163b3f5b21e778f9bf6e6b7730d5
    Reviewed-on: http://gerrit.cloudera.org:8080/16566
    Reviewed-by: Tim Armstrong <ta...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 be/src/runtime/exec-env.cc | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/be/src/runtime/exec-env.cc b/be/src/runtime/exec-env.cc
index 689ec8f..fe2bfd8 100644
--- a/be/src/runtime/exec-env.cc
+++ b/be/src/runtime/exec-env.cc
@@ -338,15 +338,23 @@ Status ExecEnv::Init() {
     // The JVM max heap size is static and therefore known at this point. Other categories
     // of JVM memory consumption are much smaller and dynamic so it is simpler not to
     // include them here.
-    admit_mem_limit_ -= JvmMemoryMetric::HEAP_MAX_USAGE->GetValue();
+    int64_t jvm_max_heap_size = JvmMemoryMetric::HEAP_MAX_USAGE->GetValue();
+    admit_mem_limit_ -= jvm_max_heap_size;
+    if (admit_mem_limit_ <= 0) {
+      return Status(
+          Substitute("Invalid combination of --mem_limit_includes_jvm and JVM max heap "
+                     "size $0, which must be smaller than process memory limit $1",
+              jvm_max_heap_size, bytes_limit));
+    }
   }
 
   bool is_percent;
   int64_t buffer_pool_limit = ParseUtil::ParseMemSpec(FLAGS_buffer_pool_limit,
       &is_percent, admit_mem_limit_);
   if (buffer_pool_limit <= 0) {
-    return Status(Substitute("Invalid --buffer_pool_limit value, must be a percentage or "
-          "positive bytes value or percentage: $0", FLAGS_buffer_pool_limit));
+    return Status(Substitute("Invalid --buffer_pool_limit value, must be a "
+                             "positive bytes value or percentage: $0",
+        FLAGS_buffer_pool_limit));
   }
   buffer_pool_limit = BitUtil::RoundDown(buffer_pool_limit, FLAGS_min_buffer_size);
   LOG(INFO) << "Buffer pool limit: "
@@ -356,7 +364,7 @@ Status ExecEnv::Init() {
       &is_percent, buffer_pool_limit);
   if (clean_pages_limit <= 0) {
     return Status(Substitute("Invalid --buffer_pool_clean_pages_limit value, must be a "
-        "percentage or positive bytes value or percentage: $0",
+                             "positive bytes value or percentage: $0",
         FLAGS_buffer_pool_clean_pages_limit));
   }
   InitBufferPool(FLAGS_min_buffer_size, buffer_pool_limit, clean_pages_limit);