This is an automated email from the ASF dual-hosted git repository.
kxiao pushed a commit to branch branch-2.0
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-2.0 by this push:
new b4ac28260b0 [enhance](session)check invalid value when set parallel
instance variables (#28141) (#28463)
b4ac28260b0 is described below
commit b4ac28260b06a2be0197275fea1edb19fbf34812
Author: zhangstar333 <[email protected]>
AuthorDate: Sat Dec 16 09:45:59 2023 +0800
[enhance](session)check invalid value when set parallel instance variables
(#28141) (#28463)
in some case, if set incorrectly, will be cause BE core dump
10:18:19 *** SIGFPE integer divide by zero (@0x564853c204c8) received by
PID 2132555
int max_scanners =
config::doris_scanner_thread_pool_thread_num /
state->query_parallel_instance_num();
---
.../java/org/apache/doris/qe/SessionVariable.java | 26 +++++++++++++++++--
.../main/java/org/apache/doris/qe/VariableMgr.java | 4 +++
.../doris/regression/action/TestAction.groovy | 12 ---------
.../session_variable/test_invalid_session.groovy | 30 ++++++++++++++++++++++
4 files changed, 58 insertions(+), 14 deletions(-)
diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java
b/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java
index 0bc8c2f2c20..3a61e861e94 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java
@@ -638,10 +638,12 @@ public class SessionVariable implements Serializable,
Writable {
* the parallel exec instance num for one Fragment in one BE
* 1 means disable this feature
*/
- @VariableMgr.VarAttr(name = PARALLEL_FRAGMENT_EXEC_INSTANCE_NUM,
needForward = true, fuzzy = true)
+ @VariableMgr.VarAttr(name = PARALLEL_FRAGMENT_EXEC_INSTANCE_NUM,
needForward = true, fuzzy = true,
+ setter = "setFragmentInstanceNum")
public int parallelExecInstanceNum = 1;
- @VariableMgr.VarAttr(name = PARALLEL_PIPELINE_TASK_NUM, fuzzy = true,
needForward = true)
+ @VariableMgr.VarAttr(name = PARALLEL_PIPELINE_TASK_NUM, fuzzy = true,
needForward = true,
+ setter = "setPipelineTaskNum")
public int parallelPipelineTaskNum = 0;
@VariableMgr.VarAttr(name = MAX_INSTANCE_NUM)
@@ -1673,6 +1675,26 @@ public class SessionVariable implements Serializable,
Writable {
this.queryTimeoutS = this.maxExecutionTimeMS / 1000;
}
+ public void setPipelineTaskNum(String value) throws Exception {
+ int val = checkFieldValue(PARALLEL_PIPELINE_TASK_NUM, 0, value);
+ this.parallelPipelineTaskNum = val;
+ }
+
+ public void setFragmentInstanceNum(String value) throws Exception {
+ int val = checkFieldValue(PARALLEL_FRAGMENT_EXEC_INSTANCE_NUM, 1,
value);
+ this.parallelExecInstanceNum = val;
+ }
+
+ private int checkFieldValue(String variableName, int minValue, String
value) throws Exception {
+ int val = Integer.valueOf(value);
+ if (val < minValue) {
+ throw new Exception(
+ variableName + " value should greater than or equal " +
String.valueOf(minValue)
+ + ", you set value is: " + value);
+ }
+ return val;
+ }
+
public String getWorkloadGroup() {
return workloadGroup;
}
diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/VariableMgr.java
b/fe/fe-core/src/main/java/org/apache/doris/qe/VariableMgr.java
index c729f0a444c..7115d777b3a 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/qe/VariableMgr.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/qe/VariableMgr.java
@@ -51,6 +51,7 @@ import java.io.IOException;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
@@ -152,6 +153,7 @@ public class VariableMgr {
// Set value to a variable
private static boolean setValue(Object obj, Field field, String value)
throws DdlException {
VarAttr attr = field.getAnnotation(VarAttr.class);
+
if (VariableVarConverters.hasConverter(attr.name())) {
value = VariableVarConverters.encode(attr.name(),
value).toString();
}
@@ -168,6 +170,8 @@ public class VariableMgr {
Preconditions.checkArgument(obj instanceof SessionVariable);
try {
SessionVariable.class.getDeclaredMethod(attr.setter(),
String.class).invoke(obj, value);
+ } catch (InvocationTargetException e) {
+ ErrorReport.reportDdlException(((InvocationTargetException)
e).getTargetException().getMessage());
} catch (Exception e) {
ErrorReport.reportDdlException(ErrorCode.ERR_INVALID_VALUE,
attr.name(), value, e.getMessage());
}
diff --git
a/regression-test/framework/src/main/groovy/org/apache/doris/regression/action/TestAction.groovy
b/regression-test/framework/src/main/groovy/org/apache/doris/regression/action/TestAction.groovy
index 32333c799e0..c6b19bccd23 100644
---
a/regression-test/framework/src/main/groovy/org/apache/doris/regression/action/TestAction.groovy
+++
b/regression-test/framework/src/main/groovy/org/apache/doris/regression/action/TestAction.groovy
@@ -54,11 +54,9 @@ class TestAction implements SuiteAction {
private String exception
private Closure check
SuiteContext context
- private Random rd
TestAction(SuiteContext context) {
this.context = context
- this.rd = new Random()
}
@Override
@@ -193,16 +191,6 @@ class TestAction implements SuiteAction {
}
void sql(String sql, boolean setRandomParallel = true) {
- if (setRandomParallel && (! sql.contains('SET_VAR')) &&
sql.containsIgnoreCase('select')) {
- def num = rd.nextInt(16)
- def replace_str = 'select
/*+SET_VAR(parallel_fragment_exec_instance_num=' + num.toString() + ')*/'
- if(sql.contains('SELECT')) {
- sql = sql.replaceFirst('SELECT', replace_str)
- }
- else if (sql.contains('select')) {
- sql = sql.replaceFirst('select', replace_str)
- }
- }
this.sql = sql
}
diff --git
a/regression-test/suites/query_p0/session_variable/test_invalid_session.groovy
b/regression-test/suites/query_p0/session_variable/test_invalid_session.groovy
new file mode 100644
index 00000000000..19800284793
--- /dev/null
+++
b/regression-test/suites/query_p0/session_variable/test_invalid_session.groovy
@@ -0,0 +1,30 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+suite("test_invalid_session") {
+ try {
+ sql "set parallel_pipeline_task_num = -1;"
+ } catch (Exception ex) {
+ assert("${ex}".contains("parallel_pipeline_task_num value should
greater than or equal 0, you set value is: -1"))
+ }
+
+ try {
+ sql "set parallel_fragment_exec_instance_num = 0;"
+ } catch (Exception ex) {
+ assert("${ex}".contains("parallel_fragment_exec_instance_num value
should greater than or equal 1, you set value is: 0"))
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
