[GitHub] [doris] dataroaring commented on a diff in pull request #16343: [Enhancement](Stmt) Set insert_into timeout session variable separately

["dataroaring (via GitHub)" <gi...@apache.org>]

dataroaring commented on code in PR #16343:
URL: https://github.com/apache/doris/pull/16343#discussion_r1101211587


##########
be/src/runtime/runtime_state.cpp:
##########
@@ -21,7 +21,9 @@
 #include "runtime/runtime_state.h"
 
 #include <fmt/format.h>
+#include <gen_cpp/PaloInternalService_types.h>
 
+#include <algorithm>

Review Comment:
   uselss include?



##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -1562,8 +1566,9 @@ private void handleInsertStmt() throws Exception {
                 QeProcessorImpl.INSTANCE.registerQuery(context.queryId(), coord);
 
                 coord.exec();
-
-                boolean notTimeout = coord.join(context.getSessionVariable().getQueryTimeoutS());
+                int execTimeout = context.getExecTimeout();
+                LOG.info("Insert execution timeout:{}", execTimeout);

Review Comment:
   info -> debug.



##########
fe/fe-core/src/main/java/org/apache/doris/qe/ConnectContext.java:
##########
@@ -577,25 +581,27 @@ public void checkTimeout(long now) {
                 killConnection = true;
             }
         } else {
+            long timeout;
+            String timeoutTag = "query";
             if (userQueryTimeout > 0) {
                 // user set query_timeout property
-                if (delta > userQueryTimeout * 1000) {
-                    LOG.warn("kill query timeout, remote: {}, query timeout: {}",
-                            getMysqlChannel().getRemoteHostPortString(), userQueryTimeout);
-
-                    killFlag = true;
-                }
+                timeout = userQueryTimeout * 1000L;
             } else {
-                // default use session query_timeout
-                if (delta > sessionVariable.getQueryTimeoutS() * 1000) {
-                    LOG.warn("kill query timeout, remote: {}, query timeout: {}",
-                            getMysqlChannel().getRemoteHostPortString(), sessionVariable.getQueryTimeoutS());
+                //to ms
+                timeout = executionTimeoutS * 1000L;
+            }
+            //deal with insert stmt particularly
+            if (executor != null && executor.isInsertStmt()) {
+                timeoutTag = "insert";
+            }
 
-                    // Only kill
-                    killFlag = true;
-                }
+            if (delta > timeout) {
+                LOG.warn("kill {} timeout, remote: {}, query timeout: {}",
+                        timeoutTag, getMysqlChannel().getRemoteHostPortString(), timeout);
+                killFlag = true;
             }
         }
+

Review Comment:
   executionTimeoutS is set in setExecTimeout, we can just use it ?```suggestion
   
   ```



##########
be/src/runtime/fragment_mgr.cpp:
##########
@@ -19,9 +19,11 @@
 
 #include <bvar/latency_recorder.h>
 #include <gen_cpp/HeartbeatService_types.h>
+#include <glog/logging.h>
 #include <gperftools/profiler.h>
 #include <thrift/protocol/TDebugProtocol.h>
 
+#include <algorithm>

Review Comment:
   useless includes.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org