[BugFix] Validate query_timeout at analyze stage (#13653)

StarRocks · Nov 21, 2022 · 7247906 · 7247906
1 parent 1f5dc07
commit 7247906
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 21 deletions.
diff --git a/fe/fe-core/src/main/java/com/starrocks/qe/MasterOpExecutor.java b/fe/fe-core/src/main/java/com/starrocks/qe/MasterOpExecutor.java
@@ -66,6 +66,9 @@ public MasterOpExecutor(StatementBase parsedStmt, OriginStatement originStmt,
         // set thriftTimeoutMs to query_timeout + thrift_rpc_timeout_ms
         // so that we can return an execution timeout instead of a network timeout
         this.thriftTimeoutMs = ctx.getSessionVariable().getQueryTimeoutS() * 1000 + Config.thrift_rpc_timeout_ms;
+        if (this.thriftTimeoutMs < 0) {
+            this.thriftTimeoutMs = ctx.getSessionVariable().getQueryTimeoutS() * 1000;
+        }
         this.parsedStmt = parsedStmt;
     }
 

diff --git a/fe/fe-core/src/main/java/com/starrocks/qe/SessionVariable.java b/fe/fe-core/src/main/java/com/starrocks/qe/SessionVariable.java
@@ -77,8 +77,6 @@ public class SessionVariable implements Serializable, Writable, Cloneable {
     public static final String SQL_SAFE_UPDATES = "sql_safe_updates";
     public static final String NET_BUFFER_LENGTH = "net_buffer_length";
     public static final String CODEGEN_LEVEL = "codegen_level";
-    // mem limit can't smaller than bufferpool's default page size
-    public static final int MIN_EXEC_MEM_LIMIT = 2097152;
     public static final String BATCH_SIZE = "batch_size";
     public static final String CHUNK_SIZE = "chunk_size";
     public static final String DISABLE_STREAMING_PREAGGREGATIONS = "disable_streaming_preaggregations";
@@ -202,6 +200,12 @@ public class SessionVariable implements Serializable, Writable, Cloneable {
     public static final String RUNTIME_FILTER_SCAN_WAIT_TIME = "runtime_filter_scan_wait_time";
     public static final String ENABLE_OPTIMIZER_TRACE_LOG = "enable_optimizer_trace_log";
 
+    // Limitations
+    // mem limit can't smaller than bufferpool's default page size
+    public static final int MIN_EXEC_MEM_LIMIT = 2097152;
+    // query timeout cannot greater than one month
+    public static final int MAX_QUERY_TIMEOUT = 259200;
+
     @VariableMgr.VarAttr(name = ENABLE_PIPELINE_ENGINE)
     private boolean enablePipelineEngine = true;
 
@@ -595,16 +599,10 @@ public String getCollationServer() {
     }
 
     public long getSqlSelectLimit() {
-        if (sqlSelectLimit < 0) {
-            return DEFAULT_SELECT_LIMIT;
-        }
         return sqlSelectLimit;
     }
 
     public void setSqlSelectLimit(long limit) {
-        if (limit < 0) {
-            return;
-        }
         this.sqlSelectLimit = limit;
     }
 
@@ -617,11 +615,7 @@ public void setTimeZone(String timeZone) {
     }
 
     public void setMaxExecMemByte(long maxExecMemByte) {
-        if (maxExecMemByte < MIN_EXEC_MEM_LIMIT) {
-            this.maxExecMemByte = MIN_EXEC_MEM_LIMIT;
-        } else {
-            this.maxExecMemByte = maxExecMemByte;
-        }
+        this.maxExecMemByte = maxExecMemByte;
     }
 
     public void setLoadMemLimit(long loadMemLimit) {

diff --git a/fe/fe-core/src/test/java/com/starrocks/qe/VariableMgrTest.java b/fe/fe-core/src/test/java/com/starrocks/qe/VariableMgrTest.java
@@ -105,12 +105,12 @@ public void testNormal() throws IllegalAccessException, NoSuchFieldException, Us
         }
 
         // Set global variable
-        SetVar setVar = new SetVar(SetType.GLOBAL, "exec_mem_limit", new IntLiteral(1234L));
+        SetVar setVar = new SetVar(SetType.GLOBAL, "exec_mem_limit", new IntLiteral(12999934L));
         setVar.analyze(null);
         VariableMgr.setVar(var, setVar, false);
-        Assert.assertEquals(1234L, var.getMaxExecMemByte());
+        Assert.assertEquals(12999934L, var.getMaxExecMemByte());
         var = VariableMgr.newSessionVariable();
-        Assert.assertEquals(1234L, var.getMaxExecMemByte());
+        Assert.assertEquals(12999934L, var.getMaxExecMemByte());
 
         SetVar setVar2 = new SetVar(SetType.GLOBAL, "parallel_fragment_exec_instance_num", new IntLiteral(5L));
         setVar2.analyze(null);
@@ -134,18 +134,18 @@ public void testNormal() throws IllegalAccessException, NoSuchFieldException, Us
         Assert.assertEquals("CST", var.getTimeZone());
 
         // Set session variable
-        setVar = new SetVar(SetType.GLOBAL, "exec_mem_limit", new IntLiteral(1234L));
+        setVar = new SetVar(SetType.GLOBAL, "exec_mem_limit", new IntLiteral(12999934L));
         setVar.analyze(null);
         VariableMgr.setVar(var, setVar, false);
-        Assert.assertEquals(1234L, var.getMaxExecMemByte());
+        Assert.assertEquals(12999934L, var.getMaxExecMemByte());
 
         // onlySessionVar
-        setVar = new SetVar(SetType.GLOBAL, "exec_mem_limit", new IntLiteral(4321L));
+        setVar = new SetVar(SetType.GLOBAL, "exec_mem_limit", new IntLiteral(12999935L));
         setVar.analyze(null);
         VariableMgr.setVar(var, setVar, true);
-        Assert.assertEquals(4321L, var.getMaxExecMemByte());
+        Assert.assertEquals(12999935L, var.getMaxExecMemByte());
         var = VariableMgr.newSessionVariable();
-        Assert.assertEquals(1234L, var.getMaxExecMemByte());
+        Assert.assertEquals(12999934L, var.getMaxExecMemByte());
 
         setVar3 = new SetVar(SetType.SESSION, "time_zone", new StringLiteral("Asia/Jakarta"));
         setVar3.analyze(null);