[impala] 02/02: IMPALA-12023: Skip resource checking on last executor group set

[mi...@apache.org]

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

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

commit 1eb6ed6f2901aad6af66c206d446f8aebf4529fe
Author: Riza Suminto <ri...@cloudera.com>
AuthorDate: Thu Mar 23 11:46:37 2023 -0700

    IMPALA-12023: Skip resource checking on last executor group set
    
    This patch adds flag skip_resource_checking_on_last_executor_group_set.
    If this backend flag is set to true, memory and cpu resource checking
    will be skipped when a query is being planned against the last (largest)
    executor group set. Setting true will ensure that query will always get
    admitted into the last executor group set if it does not fit in any
    other group set.
    
    Testing
    - Tune test_query_cpu_count_divisor_fraction to run two test case:
      cpu within limit, and cpu outside limit.
    - Add test_no_skip_resource_checking
    
    Change-Id: I5848e4f67939d3dd2fb105c1ae4ca8e15f2e556f
    Reviewed-on: http://gerrit.cloudera.org:8080/19649
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Reviewed-by: Wenzhe Zhou <wz...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 be/src/util/backend-gflag-util.cc                  |  8 +++++++
 common/thrift/BackendGflags.thrift                 |  2 ++
 .../org/apache/impala/service/BackendConfig.java   |  4 ++++
 .../java/org/apache/impala/service/Frontend.java   |  7 ++++++
 tests/custom_cluster/test_executor_groups.py       | 27 ++++++++++++++++++++--
 5 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/be/src/util/backend-gflag-util.cc b/be/src/util/backend-gflag-util.cc
index 882b0c77a..cd8f1dc01 100644
--- a/be/src/util/backend-gflag-util.cc
+++ b/be/src/util/backend-gflag-util.cc
@@ -222,6 +222,12 @@ DEFINE_int64_hidden(min_processing_per_thread, 10000000,
     "number of cores in selected executor group, MT_DOP, or PROCESSING_COST_MIN_THREAD "
     "query option. Must be a positive integer. Default to 10M.");
 
+DEFINE_bool_hidden(skip_resource_checking_on_last_executor_group_set, true,
+    "(Advance) If true, memory and cpu resource checking will be skipped when a query "
+    "is being planned against the last (largest) executor group set. Setting true will "
+    "ensure that query will always get admitted into last executor group set if it does "
+    "not fit in any other group set.");
+
 using strings::Substitute;
 
 namespace impala {
@@ -399,6 +405,8 @@ Status PopulateThriftBackendGflags(TBackendGflags& cfg) {
   cfg.__set_processing_cost_use_equal_expr_weight(
       FLAGS_processing_cost_use_equal_expr_weight);
   cfg.__set_min_processing_per_thread(FLAGS_min_processing_per_thread);
+  cfg.__set_skip_resource_checking_on_last_executor_group_set(
+      FLAGS_skip_resource_checking_on_last_executor_group_set);
   return Status::OK();
 }
 
diff --git a/common/thrift/BackendGflags.thrift b/common/thrift/BackendGflags.thrift
index 5ea3cd107..3f1f03749 100644
--- a/common/thrift/BackendGflags.thrift
+++ b/common/thrift/BackendGflags.thrift
@@ -248,4 +248,6 @@ struct TBackendGflags {
   108: required bool processing_cost_use_equal_expr_weight
 
   109: required i64 min_processing_per_thread
+
+  110: required bool skip_resource_checking_on_last_executor_group_set
 }
diff --git a/fe/src/main/java/org/apache/impala/service/BackendConfig.java b/fe/src/main/java/org/apache/impala/service/BackendConfig.java
index f5daefa7e..820d60a17 100644
--- a/fe/src/main/java/org/apache/impala/service/BackendConfig.java
+++ b/fe/src/main/java/org/apache/impala/service/BackendConfig.java
@@ -390,4 +390,8 @@ public class BackendConfig {
   public long getMinProcessingPerThread() {
     return backendCfg_.min_processing_per_thread;
   }
+
+  public boolean isSkipResourceCheckingOnLastExecutorGroupSet() {
+    return backendCfg_.skip_resource_checking_on_last_executor_group_set;
+  }
 }
diff --git a/fe/src/main/java/org/apache/impala/service/Frontend.java b/fe/src/main/java/org/apache/impala/service/Frontend.java
index e59bf90bc..1952b6e33 100644
--- a/fe/src/main/java/org/apache/impala/service/Frontend.java
+++ b/fe/src/main/java/org/apache/impala/service/Frontend.java
@@ -2122,6 +2122,13 @@ public class Frontend {
         matchFound = true;
       }
 
+      if (!matchFound && (i >= num_executor_group_sets - 1)
+          && BackendConfig.INSTANCE.isSkipResourceCheckingOnLastExecutorGroupSet()) {
+        reason = "no executor group set fit. Admit to last executor group set.";
+        addInfoString(groupSetProfile, VERDICT, reason);
+        matchFound = true;
+      }
+
       if (matchFound) {
         // Set the group name prefix in both the returned query options and
         // the query context for non default group setup.
diff --git a/tests/custom_cluster/test_executor_groups.py b/tests/custom_cluster/test_executor_groups.py
index 4ae476e81..6c60b703c 100644
--- a/tests/custom_cluster/test_executor_groups.py
+++ b/tests/custom_cluster/test_executor_groups.py
@@ -909,11 +909,34 @@ class TestExecutorGroups(CustomClusterTestSuite):
   @pytest.mark.execute_serially
   def test_query_cpu_count_divisor_fraction(self):
     # Expect to run the query on the large group
-    coordinator_test_args = "-query_cpu_count_divisor=0.2 "
+    coordinator_test_args = "-query_cpu_count_divisor=0.03 "
     self._setup_three_exec_group_cluster(coordinator_test_args)
+    options = copy.deepcopy(CPU_DOP_OPTIONS)
+    options['MT_DOP'] = '1'
+    self._run_query_and_verify_profile(CPU_TEST_QUERY, options,
+        ["Executor Group: root.large-group", "EffectiveParallelism: 4",
+         "ExecutorGroupsConsidered: 3", "CpuAsk: 134",
+         "Verdict: Match"])
+
+    # Expect that a query still admitted to last group even if
+    # its resource requirement exceed the limit on that last executor group.
     self._run_query_and_verify_profile(CPU_TEST_QUERY, CPU_DOP_OPTIONS,
         ["Executor Group: root.large-group", "EffectiveParallelism: 7",
-         "ExecutorGroupsConsidered: 3"])
+         "ExecutorGroupsConsidered: 3", "CpuAsk: 234",
+         "Verdict: no executor group set fit. Admit to last executor group set."])
+    self.client.close()
+
+  @pytest.mark.execute_serially
+  def test_no_skip_resource_checking(self):
+    """This test check that executor group limit is enforced if
+    skip_resource_checking_on_last_executor_group_set=false."""
+    coordinator_test_args = ("-query_cpu_count_divisor=0.03 "
+        "-skip_resource_checking_on_last_executor_group_set=false ")
+    self._setup_three_exec_group_cluster(coordinator_test_args)
+    self.client.set_configuration(CPU_DOP_OPTIONS)
+    result = self.execute_query_expect_failure(self.client, CPU_TEST_QUERY)
+    assert ("AnalysisException: The query does not fit largest executor group sets. "
+        "Reason: not enough cpu cores (require=234, max=192).") in str(result)
     self.client.close()
 
   @pytest.mark.execute_serially