You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by jo...@apache.org on 2022/10/17 21:52:14 UTC
[impala] 01/03: Revert "IMPALA-11617: Pool service should be made aware of processing cost limit"
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 1b59d32eff725664a7a224d980f08ea4d245d63b
Author: Wenzhe Zhou <wz...@cloudera.com>
AuthorDate: Thu Oct 13 16:23:53 2022 +0000
Revert "IMPALA-11617: Pool service should be made aware of processing cost limit"
This reverts commit 1d62bddb84753a040a46687391d9c797d849bd0e.
Change-Id: I1ebf5ff9685072079e18497d869d06b2c55153fe
Reviewed-on: http://gerrit.cloudera.org:8080/19139
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Wenzhe Zhou <wz...@cloudera.com>
---
be/src/scheduling/admission-controller.cc | 8 --------
be/src/scheduling/admission-controller.h | 1 -
common/thrift/ImpalaInternalService.thrift | 5 -----
common/thrift/metrics.json | 10 ---------
docs/topics/impala_admission_config.xml | 4 ----
.../java/org/apache/impala/service/Frontend.java | 4 ----
.../org/apache/impala/util/RequestPoolService.java | 10 ++-------
.../apache/impala/util/TestRequestPoolService.java | 9 ++++----
fe/src/test/resources/llama-site-2-groups.xml | 10 ---------
fe/src/test/resources/llama-site-test.xml | 4 ----
fe/src/test/resources/llama-site-test2.xml | 4 ----
.../test/resources/mem-limit-test-llama-site.xml | 24 ----------------------
fe/src/test/resources/minicluster-llama-site.xml | 7 -------
tests/common/resource_pool_config.py | 4 +---
tests/custom_cluster/test_executor_groups.py | 7 +++----
www/admission_controller.tmpl | 5 -----
16 files changed, 10 insertions(+), 106 deletions(-)
diff --git a/be/src/scheduling/admission-controller.cc b/be/src/scheduling/admission-controller.cc
index c2957baa9..1e31813ba 100644
--- a/be/src/scheduling/admission-controller.cc
+++ b/be/src/scheduling/admission-controller.cc
@@ -139,8 +139,6 @@ const string POOL_MIN_QUERY_MEM_LIMIT_METRIC_KEY_FORMAT =
"admission-controller.pool-min-query-mem-limit.$0";
const string POOL_CLAMP_MEM_LIMIT_QUERY_OPTION_METRIC_KEY_FORMAT =
"admission-controller.pool-clamp-mem-limit-query-option.$0";
-const string POOL_MAX_QUERY_PROCESSING_COST_LIMIT_METRIC_KEY_FORMAT =
- "admission-controller.pool-max-query-processing-cost-limit.$0";
// Profile info strings
const string AdmissionController::PROFILE_INFO_KEY_ADMISSION_RESULT = "Admission result";
@@ -1163,8 +1161,6 @@ void AdmissionController::PoolStats::UpdateConfigMetrics(const TPoolConfig& pool
metrics_.max_query_mem_limit->SetValue(pool_cfg.max_query_mem_limit);
metrics_.min_query_mem_limit->SetValue(pool_cfg.min_query_mem_limit);
metrics_.clamp_mem_limit_query_option->SetValue(pool_cfg.clamp_mem_limit_query_option);
- metrics_.max_query_processing_cost_limit->SetValue(
- pool_cfg.max_query_processing_cost_limit);
}
Status AdmissionController::SubmitForAdmission(const AdmissionRequest& request,
@@ -2351,8 +2347,6 @@ void AdmissionController::PoolStats::ToJson(
document->GetAllocator());
pool->AddMember("clamp_mem_limit_query_option",
metrics_.clamp_mem_limit_query_option->GetValue(), document->GetAllocator());
- pool->AddMember("max_query_processing_cost_limit",
- metrics_.max_query_processing_cost_limit->GetValue(), document->GetAllocator());
pool->AddMember("wait_time_ms_ema", wait_time_ms_ema_, document->GetAllocator());
Value histogram(kArrayType);
for (int bucket = 0; bucket < peak_mem_histogram_.size(); bucket++) {
@@ -2437,8 +2431,6 @@ void AdmissionController::PoolStats::InitMetrics() {
POOL_MIN_QUERY_MEM_LIMIT_METRIC_KEY_FORMAT, 0, name_);
metrics_.clamp_mem_limit_query_option = parent_->metrics_group_->AddProperty<bool>(
POOL_CLAMP_MEM_LIMIT_QUERY_OPTION_METRIC_KEY_FORMAT, false, name_);
- metrics_.max_query_processing_cost_limit = parent_->metrics_group_->AddGauge(
- POOL_MAX_QUERY_PROCESSING_COST_LIMIT_METRIC_KEY_FORMAT, 0, name_);
}
void AdmissionController::PopulatePerHostMemReservedAndAdmitted(
diff --git a/be/src/scheduling/admission-controller.h b/be/src/scheduling/admission-controller.h
index f8914c079..fec28d5d4 100644
--- a/be/src/scheduling/admission-controller.h
+++ b/be/src/scheduling/admission-controller.h
@@ -543,7 +543,6 @@ class AdmissionController {
IntGauge* max_query_mem_limit;
IntGauge* min_query_mem_limit;
BooleanProperty* clamp_mem_limit_query_option;
- IntGauge* max_query_processing_cost_limit;
};
PoolStats(AdmissionController* parent, const std::string& name)
diff --git a/common/thrift/ImpalaInternalService.thrift b/common/thrift/ImpalaInternalService.thrift
index 59949db26..1f95d7284 100644
--- a/common/thrift/ImpalaInternalService.thrift
+++ b/common/thrift/ImpalaInternalService.thrift
@@ -195,11 +195,6 @@ struct TPoolConfig {
// maximum, the mt_dop setting is reduced to the maximum. If the max_mt_dop is
// negative, no limit is enforced.
9: required i64 max_mt_dop = -1;
-
- // Maximum processing cost which is the amount of data in bytes to process for a query.
- // A value of 0 effectively disables the pool. -1 indicates no limit. Default value is
- // set as -1.
- 10: required i64 max_query_processing_cost_limit = -1;
}
struct TParseDateStringResult {
diff --git a/common/thrift/metrics.json b/common/thrift/metrics.json
index 5321921d4..a5abd6651 100644
--- a/common/thrift/metrics.json
+++ b/common/thrift/metrics.json
@@ -89,16 +89,6 @@
"kind": "PROPERTY",
"key": "admission-controller.pool-clamp-mem-limit-query-option.$0"
},
- {
- "description": "Resource Pool $0 Max Query Processing Cost Limit",
- "contexts": [
- "RESOURCE_POOL"
- ],
- "label": "Resource Pool $0 Max Query Processing Cost Limit",
- "units": "BYTES",
- "kind": "GAUGE",
- "key": "admission-controller.pool-max-query-processing-cost-limit.$0"
- },
{
"description": "Resource Pool $0 Aggregate Queue Size",
"contexts": [
diff --git a/docs/topics/impala_admission_config.xml b/docs/topics/impala_admission_config.xml
index 846226afc..5b6ecf5de 100644
--- a/docs/topics/impala_admission_config.xml
+++ b/docs/topics/impala_admission_config.xml
@@ -224,10 +224,6 @@ impala.admission-control.pool-queue-timeout-ms.<varname>queue_name</varname></ph
<name>impala.admission-control.<b>clamp-mem-limit-query-option</b>.root.default.regularPool</name>
<value>true</value>
</property>
- <property>
- <name>impala.admission-control.<b>max-query-processing-cost-limit</b>.root.default.regularPool</name>
- <value>1610612736</value><!--1.5GB-->
- </property>
</configuration>
</codeblock>
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 a5d523b29..eeb5b59ea 100644
--- a/fe/src/main/java/org/apache/impala/service/Frontend.java
+++ b/fe/src/main/java/org/apache/impala/service/Frontend.java
@@ -1838,10 +1838,6 @@ public class Frontend {
Preconditions.checkNotNull(poolConfig);
new_entry.setMax_mem_limit(poolConfig.getMax_query_mem_limit() > 0 ?
poolConfig.getMax_query_mem_limit() : Long.MAX_VALUE);
- // TODO IMPALA-11604: set max_processing_cost_limit for executor group set
- // new_entry.setMax_processing_cost_limit(
- // poolConfig.getMax_query_processing_cost_limit() >= 0 ?
- // poolConfig.getMax_query_processing_cost_limit() : Long.MAX_VALUE);
} else {
// Set to max possible threshold value when there is no pool service
new_entry.setMax_mem_limit(Long.MAX_VALUE);
diff --git a/fe/src/main/java/org/apache/impala/util/RequestPoolService.java b/fe/src/main/java/org/apache/impala/util/RequestPoolService.java
index 18ea505a9..828fe3792 100644
--- a/fe/src/main/java/org/apache/impala/util/RequestPoolService.java
+++ b/fe/src/main/java/org/apache/impala/util/RequestPoolService.java
@@ -123,10 +123,6 @@ public class RequestPoolService {
// Key for specifying the "Max mt_dop" configuration of the pool
private final static String MAX_MT_DOP = "impala.admission-control.max-mt-dop";
- // Keys for the pool max query processing cost limits (in bytes).
- private final static String MAX_QUERY_PROCESSING_COST_LIMIT_BYTES =
- "impala.admission-control.max-query-processing-cost-limit";
-
// String format for a per-pool configuration key. First parameter is the key for the
// default, e.g. MAX_PLACED_RESERVATIONS_KEY, and the second parameter is the
// pool name.
@@ -392,18 +388,16 @@ public class RequestPoolService {
getPoolConfigValue(currentConf, pool, CLAMP_MEM_LIMIT_QUERY_OPTION, true));
result.setMax_mt_dop(
getPoolConfigValue(currentConf, pool, MAX_MT_DOP, -1));
- result.setMax_query_processing_cost_limit(getPoolConfigValue(
- currentConf, pool, MAX_QUERY_PROCESSING_COST_LIMIT_BYTES, -1));
}
if (LOG.isTraceEnabled()) {
LOG.debug("getPoolConfig(pool={}): max_mem_resources={}, max_requests={},"
+ " max_queued={}, queue_timeout_ms={}, default_query_options={},"
+ " max_query_mem_limit={}, min_query_mem_limit={},"
- + " clamp_mem_limit_query_option={}, max_query_processing_cost_limit={}",
+ + " clamp_mem_limit_query_option={}",
pool, result.max_mem_resources, result.max_requests, result.max_queued,
result.queue_timeout_ms, result.default_query_options,
result.max_query_mem_limit, result.min_query_mem_limit,
- result.clamp_mem_limit_query_option, result.max_query_processing_cost_limit);
+ result.clamp_mem_limit_query_option);
}
return result;
}
diff --git a/fe/src/test/java/org/apache/impala/util/TestRequestPoolService.java b/fe/src/test/java/org/apache/impala/util/TestRequestPoolService.java
index be4bdc889..efc9412ab 100644
--- a/fe/src/test/java/org/apache/impala/util/TestRequestPoolService.java
+++ b/fe/src/test/java/org/apache/impala/util/TestRequestPoolService.java
@@ -205,7 +205,7 @@ public class TestRequestPoolService {
10000L, "mem_limit=1024m,query_timeout_s=10");
checkPoolConfigResult("root.queueB", 5, 10, -1, 30000L, "mem_limit=1024m");
checkPoolConfigResult("root.queueC", 5, 10, 1024 * ByteUnits.MEGABYTE, 30000L,
- "mem_limit=1024m", 1000, 10, false, 1000);
+ "mem_limit=1024m", 1000, 10, false);
}
@Test
@@ -213,7 +213,7 @@ public class TestRequestPoolService {
createPoolService(ALLOCATION_FILE_EMPTY, LLAMA_CONFIG_FILE_EMPTY);
Assert.assertEquals("root.userA", poolService_.assignToPool("", "userA"));
Assert.assertTrue(poolService_.hasAccess("root.userA", "userA"));
- checkPoolConfigResult("root", -1, 200, -1, null, "", 0, 0, true, -1);
+ checkPoolConfigResult("root", -1, 200, -1, null, "", 0, 0, true);
}
@Ignore("IMPALA-4868") @Test
@@ -309,7 +309,7 @@ public class TestRequestPoolService {
private void checkPoolConfigResult(String pool, long expectedMaxRequests,
long expectedMaxQueued, long expectedMaxMem, Long expectedQueueTimeoutMs,
String expectedQueryOptions, long max_query_mem_limit, long min_query_mem_limit,
- boolean clamp_mem_limit_query_option, long max_query_processing_cost_limit) {
+ boolean clamp_mem_limit_query_option) {
TPoolConfig expectedResult = new TPoolConfig();
expectedResult.setMax_requests(expectedMaxRequests);
expectedResult.setMax_queued(expectedMaxQueued);
@@ -317,7 +317,6 @@ public class TestRequestPoolService {
expectedResult.setMax_query_mem_limit(max_query_mem_limit);
expectedResult.setMin_query_mem_limit(min_query_mem_limit);
expectedResult.setClamp_mem_limit_query_option(clamp_mem_limit_query_option);
- expectedResult.setMax_query_processing_cost_limit(max_query_processing_cost_limit);
if (expectedQueueTimeoutMs != null) {
expectedResult.setQueue_timeout_ms(expectedQueueTimeoutMs);
}
@@ -332,7 +331,7 @@ public class TestRequestPoolService {
long expectedMaxQueued, long expectedMaxMem, Long expectedQueueTimeoutMs,
String expectedQueryOptions) {
checkPoolConfigResult(pool, expectedMaxRequests, expectedMaxQueued, expectedMaxMem,
- expectedQueueTimeoutMs, expectedQueryOptions, 0, 0, true, -1);
+ expectedQueueTimeoutMs, expectedQueryOptions, 0, 0, true);
}
private void checkPoolConfigResult(String pool, long expectedMaxRequests,
diff --git a/fe/src/test/resources/llama-site-2-groups.xml b/fe/src/test/resources/llama-site-2-groups.xml
index 5328edcb2..1a48a847c 100644
--- a/fe/src/test/resources/llama-site-2-groups.xml
+++ b/fe/src/test/resources/llama-site-2-groups.xml
@@ -23,11 +23,6 @@
<!-- 0MB -->
<value>0</value>
</property>
- <property>
- <name>impala.admission-control.max-query-processing-cost-limit.root.small</name>
- <!-- 64 MB -->
- <value>67108864</value>
- </property>
<property>
<name>impala.admission-control.max-query-mem-limit.root.large</name>
<!-- 8 PB -->
@@ -38,9 +33,4 @@
<!-- 64MB+1 -->
<value>67108865</value>
</property>
- <property>
- <name>impala.admission-control.max-query-processing-cost-limit.root.large</name>
- <!-- 8 PB -->
- <value>9007199254740992</value>
- </property>
</configuration>
diff --git a/fe/src/test/resources/llama-site-test.xml b/fe/src/test/resources/llama-site-test.xml
index 47c61b281..373870516 100644
--- a/fe/src/test/resources/llama-site-test.xml
+++ b/fe/src/test/resources/llama-site-test.xml
@@ -55,8 +55,4 @@
<name>impala.admission-control.clamp-mem-limit-query-option.root.queueC</name>
<value>false</value>
</property>
- <property>
- <name>impala.admission-control.max-query-processing-cost-limit.root.queueC</name>
- <value>1000</value>
- </property>
</configuration>
diff --git a/fe/src/test/resources/llama-site-test2.xml b/fe/src/test/resources/llama-site-test2.xml
index b2fa18761..8890d994d 100644
--- a/fe/src/test/resources/llama-site-test2.xml
+++ b/fe/src/test/resources/llama-site-test2.xml
@@ -65,10 +65,6 @@
<name>impala.admission-control.min-query-mem-limit.root.queueD</name>
<value>10</value>
</property>
- <property>
- <name>impala.admission-control.max-query-processing-cost-limit.root.queueD</name>
- <value>62914560</value>
- </property>
<property>
<name>llama.am.throttling.maximum.placed.reservations.root.queueD</name>
<value>6</value>
diff --git a/fe/src/test/resources/mem-limit-test-llama-site.xml b/fe/src/test/resources/mem-limit-test-llama-site.xml
index a36135ed4..2c4838838 100644
--- a/fe/src/test/resources/mem-limit-test-llama-site.xml
+++ b/fe/src/test/resources/mem-limit-test-llama-site.xml
@@ -13,10 +13,6 @@
<name>impala.admission-control.clamp-mem-limit-query-option.root.regularPoolWithoutClamping</name>
<value>false</value>
</property>
- <property>
- <name>impala.admission-control.max-query-processing-cost-limit.root.regularPoolWithoutClamping</name>
- <value>1610612736</value><!--1.5GB-->
- </property>
<!--poolLowMinLimit pool config-->
<property>
<name>impala.admission-control.min-query-mem-limit.root.poolLowMinLimit</name>
@@ -40,10 +36,6 @@
<name>impala.admission-control.clamp-mem-limit-query-option.root.regularPool</name>
<value>true</value>
</property>
- <property>
- <name>impala.admission-control.max-query-processing-cost-limit.root.regularPool</name>
- <value>1610612736</value><!--1.5GB-->
- </property>
<!--regularPoolNoMinLimit pool config-->
<property>
<name>impala.admission-control.max-query-mem-limit.root.regularPoolNoMinLimit</name>
@@ -57,10 +49,6 @@
<name>impala.admission-control.clamp-mem-limit-query-option.root.regularPoolNoMinLimit</name>
<value>true</value>
</property>
- <property>
- <name>impala.admission-control.max-query-processing-cost-limit.root.regularPoolNoMinLimit</name>
- <value>1610612736</value><!--1.5GB-->
- </property>
<!--poolNoMemLimits pool config-->
<property>
<name>impala.admission-control.max-query-mem-limit.root.poolNoMemLimits</name>
@@ -70,10 +58,6 @@
<name>impala.admission-control.min-query-mem-limit.root.poolNoMemLimits</name>
<value>0</value>
</property>
- <property>
- <name>impala.admission-control.max-query-processing-cost-limit.root.poolNoMemLimits</name>
- <value>-1</value>
- </property>
<!--maxLessThanMinLimit pool config-->
<property>
<name>impala.admission-control.max-query-mem-limit.root.maxLessThanMinLimit</name>
@@ -83,10 +67,6 @@
<name>impala.admission-control.min-query-mem-limit.root.maxLessThanMinLimit</name>
<value>100001</value>
</property>
- <property>
- <name>impala.admission-control.max-query-processing-cost-limit.root.maxLessThanMinLimit</name>
- <value>100000</value>
- </property>
<!--maxMemLessThanMinLimit pool config-->
<property>
<name>impala.admission-control.min-query-mem-limit.root.maxMemLessThanMinLimit</name>
@@ -101,10 +81,6 @@
<name>impala.admission-control.min-query-mem-limit.root.invalidTestPool</name>
<value>26214400</value>
</property>
- <property>
- <name>impala.admission-control.max-query-processing-cost-limit.root.invalidTestPool</name>
- <value>-1</value>
- </property>
<property>
<name>llama.am.throttling.maximum.placed.reservations.root.invalidTestPool</name>
<value>1</value>
diff --git a/fe/src/test/resources/minicluster-llama-site.xml b/fe/src/test/resources/minicluster-llama-site.xml
index ec5194634..affc9a484 100644
--- a/fe/src/test/resources/minicluster-llama-site.xml
+++ b/fe/src/test/resources/minicluster-llama-site.xml
@@ -19,13 +19,6 @@
<name>impala.admission-control.clamp-mem-limit-query-option.root.default</name>
<value>false</value>
</property>
- <property>
- <!-- Max processing cost given to queries by default. This will allow
- running one large query and multiple small queries on a typical
- minicluster where each impalad has ~7gb of memory. -->
- <name>impala.admission-control.max-query-processing-cost-limit.root.default</name>
- <value>4294967296</value><!--4GB-->
- </property>
<!-- We need to increase the pool queue timeout to avoid flakiness from queries
getting stuck behind queries from other tests and timed out. Set to a
very high value to avoid failures unless queries are genuinely stuck. -->
diff --git a/tests/common/resource_pool_config.py b/tests/common/resource_pool_config.py
index 47e9b9827..47a37b5f3 100644
--- a/tests/common/resource_pool_config.py
+++ b/tests/common/resource_pool_config.py
@@ -29,9 +29,7 @@ class ResourcePoolConfig(object):
# Mapping of config strings used in the llama_site file with those used on the impala
# metrics debug page. Add to this dictionary if other configs are need for tests.
- CONFIG_TO_METRIC_STR_MAPPING = {
- 'max-query-mem-limit': 'pool-max-query-mem-limit',
- 'max-query-processing-cost-limit': 'pool-max-query-processing-cost-limit'}
+ CONFIG_TO_METRIC_STR_MAPPING = {'max-query-mem-limit': 'pool-max-query-mem-limit'}
"""'impala_service' should point to an impalad to be used for running queries.
'ac_service' should point to the service running the admission controller and is used
diff --git a/tests/custom_cluster/test_executor_groups.py b/tests/custom_cluster/test_executor_groups.py
index 2603948cf..e0f945e1d 100644
--- a/tests/custom_cluster/test_executor_groups.py
+++ b/tests/custom_cluster/test_executor_groups.py
@@ -695,10 +695,9 @@ class TestExecutorGroups(CustomClusterTestSuite):
"resources")
# Define two group sets: small and large
fs_allocation_path = os.path.join(RESOURCES_DIR, "fair-scheduler-2-groups.xml")
- # Define the min-query-mem-limit, max-query-mem-limit and
- # max-query-processing-cost-limit properties of the two sets:
- # small: [0, 64MB, 64MB]
- # large: [64MB+1Byte, 8PB, 8PB]
+ # Define the min-query-mem-limit and max-query-mem-limit properties of the two sets:
+ # small: [0, 64MB]
+ # large: [64MB+1Byte, 8PB]
llama_site_path = os.path.join(RESOURCES_DIR, "llama-site-2-groups.xml")
# Start with a regular admission config with multiple pools and no resource limits.
self._restart_coordinators(num_coordinators=1,
diff --git a/www/admission_controller.tmpl b/www/admission_controller.tmpl
index be97886bd..b51c3dab1 100644
--- a/www/admission_controller.tmpl
+++ b/www/admission_controller.tmpl
@@ -36,7 +36,6 @@ Example of json received from the impala server
"max_query_mem_limit": 0,
"min_query_mem_limit": 0,
"clamp_mem_limit_query_option": true,
- "max_query_processing_cost_limit": -1,
"wait_time_ms_EMA": 0.0,
"histogram": [
[
@@ -262,10 +261,6 @@ Time since last statestore update containing admission control topic state (ms):
<td>Clamp MEM_LIMIT query option</td>
<td>{{clamp_mem_limit_query_option}}</td>
</tr>
- <tr>
- <td>Query processing cost</td>
- <td>{{max_query_processing_cost_limit}}</td>
- </tr>
</table>
<h4>Queued queries in order of being queued (submitted to this coordinator)</h4>