From f12a225844638cf9b8994822e7f9aafc34554726 Mon Sep 17 00:00:00 2001
From: zhiqiang <seuhezhiqiang@163.com>
Date: Sat, 16 Dec 2023 10:59:05 +0800
Subject: [PATCH] [fix](session variables) Make default value of
 max_execution_time same to query_timeout #28474

Current problem, UNSET global VARIABLE ALL will write an oplog, which makes query_timeout = 0 when we replay it in a future time-stamp. So we change default value of max_execution_time to 90000 which is consistent to query_timeout default value.
---
 .../apache/doris/mysql/AcceptListener.java    | 12 ++++--
 .../org/apache/doris/qe/SessionVariable.java  | 24 +++++++++--
 .../java/org/apache/doris/qe/VariableMgr.java |  7 ++-
 .../test_set_session_var_exception.groovy     | 43 +++++++++++++++++++
 4 files changed, 77 insertions(+), 9 deletions(-)
 create mode 100644 regression-test/suites/correctness/test_set_session_var_exception.groovy

diff --git a/fe/fe-core/src/main/java/org/apache/doris/mysql/AcceptListener.java b/fe/fe-core/src/main/java/org/apache/doris/mysql/AcceptListener.java
index b652a0069c..e88d91316c 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/mysql/AcceptListener.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/mysql/AcceptListener.java
@@ -56,7 +56,9 @@ public class AcceptListener implements ChannelListener<AcceptingChannel<StreamCo
             // connection has been established, so need to call context.cleanup()
             // if exception happens.
             ConnectContext context = new ConnectContext(connection);
-            LOG.info("Connection query timeout: {}", context.getSessionVariable().getQueryTimeoutS());
+            if (context.getSessionVariable().getQueryTimeoutS() <= 0) {
+                LOG.warn("Connection query timeout is invalid: {}", context.getSessionVariable().getQueryTimeoutS());
+            }
             context.setEnv(Env.getCurrentEnv());
             connectScheduler.submit(context);
 
@@ -79,10 +81,12 @@ public class AcceptListener implements ChannelListener<AcceptingChannel<StreamCo
                         throw new AfterConnectedException("Reach limit of connections");
                     }
                     context.setStartTime();
-                    context.setUserQueryTimeout(
-                            context.getEnv().getAuth().getQueryTimeout(context.getQualifiedUser()));
-                    LOG.info("Connection set query timeout {}",
+                    int userQueryTimeout = context.getEnv().getAuth().getQueryTimeout(context.getQualifiedUser());
+                    if (userQueryTimeout <= 0) {
+                        LOG.warn("Connection set query timeout to {}",
                                     context.getSessionVariable().getQueryTimeoutS());
+                    }
+                    context.setUserQueryTimeout(userQueryTimeout);
                     context.setUserInsertTimeout(
                             context.getEnv().getAuth().getInsertTimeout(context.getQualifiedUser()));
                     ConnectProcessor processor = new MysqlConnectProcessor(context);
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 af81911dba..52d7c934d9 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
@@ -542,8 +542,9 @@ public class SessionVariable implements Serializable, Writable {
     // no MAX_EXECUTION_TIME(N) optimizer hint or for which N is 0.
     // https://dev.mysql.com/doc/refman/5.7/en/server-system-variables.html
     // So that it is == query timeout in doris
-    @VariableMgr.VarAttr(name = MAX_EXECUTION_TIME, fuzzy = true, setter = "setMaxExecutionTimeMS")
-    public int maxExecutionTimeMS = -1;
+    @VariableMgr.VarAttr(name = MAX_EXECUTION_TIME, checker = "checkMaxExecutionTimeMSValid",
+                        setter = "setMaxExecutionTimeMS")
+    public int maxExecutionTimeMS = 900000;
 
     @VariableMgr.VarAttr(name = INSERT_TIMEOUT)
     public int insertTimeoutS = 14400;
@@ -2527,8 +2528,23 @@ public class SessionVariable implements Serializable, Writable {
     public void checkQueryTimeoutValid(String newQueryTimeout) {
         int value = Integer.valueOf(newQueryTimeout);
         if (value <= 0) {
-            LOG.warn("Setting invalid query timeout {}", value, new RuntimeException(""));
-            throw new UnsupportedOperationException("Query timeout must be greater than 0");
+            UnsupportedOperationException exception =
+                    new UnsupportedOperationException(
+                        "query_timeout can not be set to " + newQueryTimeout + ", it must be greater than 0");
+            LOG.warn("Check query_timeout failed", exception);
+            throw exception;
+        }
+    }
+
+    public void checkMaxExecutionTimeMSValid(String newValue) {
+        int value = Integer.valueOf(newValue);
+        if (value < 1000) {
+            UnsupportedOperationException exception =
+                    new UnsupportedOperationException(
+                        "max_execution_time can not be set to " + newValue
+                        + ", it must be greater or equal to 1000, the time unit is millisecond");
+            LOG.warn("Check max_execution_time failed", exception);
+            throw exception;
         }
     }
 
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 a5d1daf5ba..170c86e47e 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
@@ -470,7 +470,12 @@ public class VariableMgr {
                     LOG.error("failed to get global variable {} when replaying", (String) varName);
                     continue;
                 }
-                setValue(varContext.getObj(), varContext.getField(), root.get((String) varName).toString());
+                try {
+                    setValue(varContext.getObj(), varContext.getField(), root.get((String) varName).toString());
+                } catch (Exception exception) {
+                    LOG.warn("Exception during replay global variabl {} oplog, {}, THIS EXCEPTION WILL BE IGNORED.",
+                            (String) varName, exception.getMessage());
+                }
             }
         } finally {
             wlock.unlock();
diff --git a/regression-test/suites/correctness/test_set_session_var_exception.groovy b/regression-test/suites/correctness/test_set_session_var_exception.groovy
new file mode 100644
index 0000000000..9e5f005b9c
--- /dev/null
+++ b/regression-test/suites/correctness/test_set_session_var_exception.groovy
@@ -0,0 +1,43 @@
+// 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_set_session_var_exception") {
+    test {
+        sql """
+        set query_timeout=0;
+        """
+        exception "query_timeout can not be set to 0, it must be greater than 0"
+    }
+    test {
+        sql """
+        set max_execution_time=999;
+        """
+        exception "max_execution_time can not be set to 999"
+    }
+
+    sql """
+    set max_execution_time=1000;
+    """
+
+    sql """
+    UNSET VARIABLE max_execution_time;
+    """
+}
+
+
+
+