From 125b5e6093f61553d9e50fb44a862b4e0edea29d Mon Sep 17 00:00:00 2001
From: Mark Bastian <markbastian@gmail.com>
Date: Fri, 26 May 2023 17:39:26 -0600
Subject: [PATCH] Change MySQL DDL creation timeout back to 10 minutes (from 30
seconds) (#31104)
We use async to monitor ddl execution for MySQL since it does not have the concept of a (non-select) statement timeout.
@snoe had originally set this to 10 minutes, but (we believe) this was causing tests to run exceptionally long and 30 seconds felt more reasonable. After receiving customer feedback that we actually do need longer timeouts, we are restoring the timeout to 10 minutes.
Links containing context:
- https://github.com/metabase/metabase/pull/23443
- https://github.com/metabase/metabase/pull/27858
- https://metaboat.slack.com/archives/CKZEMT1MJ/p1685128597747419
Fixes #31102
---
src/metabase/driver/mysql/ddl.clj | 2 +-
test/metabase/driver/mysql_test.clj | 21 +++++++--------------
2 files changed, 8 insertions(+), 15 deletions(-)
diff --git a/src/metabase/driver/mysql/ddl.clj b/src/metabase/driver/mysql/ddl.clj
index 6086ca0c76d..be38b455cf3 100644
--- a/src/metabase/driver/mysql/ddl.clj
+++ b/src/metabase/driver/mysql/ddl.clj
@@ -68,7 +68,7 @@
;; That is ok, the persisted-info will be marked inactive and the next refresh will try again.
(execute-with-timeout! conn
db-spec
- (.toMillis (t/seconds 30))
+ (.toMillis (t/minutes 10))
(into [(sql.ddl/create-table-sql database definition query)] params))
{:state :success})))
diff --git a/test/metabase/driver/mysql_test.clj b/test/metabase/driver/mysql_test.clj
index 10c84b3e363..72251cb8df1 100644
--- a/test/metabase/driver/mysql_test.clj
+++ b/test/metabase/driver/mysql_test.clj
@@ -613,24 +613,17 @@
(mt/db)
(t2/select-one Table :db_id (mt/id) :name "bigint-and-bool-table")))))))))
-(deftest ddl-execute-with-timeout-test1
- (mt/test-driver :mysql
- (mt/dataset json
- (let [db-spec (sql-jdbc.conn/db->pooled-connection-spec (mt/db))]
- (is (thrown-with-msg?
- Exception
- #"Killed mysql process id [\d,]+ due to timeout."
- (#'mysql.ddl/execute-with-timeout! db-spec db-spec 10 ["select sleep(5)"])))))))
-
(deftest ddl-execute-with-timeout-test
(mt/test-driver :mysql
(mt/dataset json
(let [db-spec (sql-jdbc.conn/db->pooled-connection-spec (mt/db))]
- (is (thrown-with-msg?
- Exception
- #"Killed mysql process id [\d,]+ due to timeout."
- (#'mysql.ddl/execute-with-timeout! db-spec db-spec 10 ["select sleep(5)"])))
- (is (some? (#'mysql.ddl/execute-with-timeout! db-spec db-spec 5000 ["select sleep(0.1) as val"])))))))
+ (testing "When the query takes longer that the timeout, it is killed."
+ (is (thrown-with-msg?
+ Exception
+ #"Killed mysql process id [\d,]+ due to timeout."
+ (#'mysql.ddl/execute-with-timeout! db-spec db-spec 10 ["select sleep(5)"]))))
+ (testing "When the query takes less time than the timeout, it is successful."
+ (is (some? (#'mysql.ddl/execute-with-timeout! db-spec db-spec 5000 ["select sleep(0.1) as val"]))))))))
(deftest syncable-schemas-test
(mt/test-driver :mysql
--
GitLab