Skip to content

Commit

Permalink
[enhance](session)check invalid value when set parallel instance vari…
Browse files Browse the repository at this point in the history
…ables (apache#28141)

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();
  • Loading branch information
zhangstar333 committed Dec 15, 2023
1 parent bde9dc9 commit 98d0c72
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 14 deletions.
26 changes: 24 additions & 2 deletions fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -1673,6 +1675,26 @@ public void setMaxExecutionTimeMS(String maxExecutionTimeMS) {
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;
}
Expand Down
4 changes: 4 additions & 0 deletions fe/fe-core/src/main/java/org/apache/doris/qe/VariableMgr.java
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
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;
Expand Down Expand Up @@ -152,6 +153,7 @@ public static SessionVariable getDefaultSessionVariable() {
// 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();
}
Expand All @@ -168,6 +170,8 @@ private static boolean setValue(Object obj, Field field, String value) throws Dd
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());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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"))
}
}

0 comments on commit 98d0c72

Please sign in to comment.