You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kyuubi.apache.org by ch...@apache.org on 2022/02/21 10:23:46 UTC
[incubator-kyuubi] branch branch-1.4 updated: [KYUUBI #1946] Close the kyuubi connection on launch engine operation failure
This is an automated email from the ASF dual-hosted git repository.
chengpan pushed a commit to branch branch-1.4
in repository https://gitbox.apache.org/repos/asf/incubator-kyuubi.git
The following commit(s) were added to refs/heads/branch-1.4 by this push:
new 2a8cc59 [KYUUBI #1946] Close the kyuubi connection on launch engine operation failure
2a8cc59 is described below
commit 2a8cc593167ecf6d3ab74b322bd5090583e832fe
Author: Fei Wang <fw...@ebay.com>
AuthorDate: Mon Feb 21 18:13:02 2022 +0800
[KYUUBI #1946] Close the kyuubi connection on launch engine operation failure
<!--
Thanks for sending a pull request!
Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/contributions.html
2. If the PR is related to an issue in https://github.com/apache/incubator-kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'.
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'.
-->
<!--
Please clarify why the changes are needed. For instance,
1. If you add a feature, you can talk about the use case of it.
2. If you fix a bug, you can clarify why it is a bug.
-->
For kyuubi connection, if the launch engine operation fails, we need close the kyuubi connection.
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible
- [ ] Add screenshots for manual tests if appropriate
- [ ] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request
Closes #1946 from turboFei/close_conn_on_engine_failure.
Closes #1946
3a4a465a [Fei Wang] remove final:
81117268 [Fei Wang] add ut
74d3b28b [Fei Wang] address commments
0c41ac58 [Fei Wang] refactor
8dcc7ec1 [Fei Wang] Close the kyuubi connection on launch engine operation failure
Authored-by: Fei Wang <fw...@ebay.com>
Signed-off-by: Cheng Pan <ch...@apache.org>
(cherry picked from commit 0e6367ad5c3726ebff8c9ee4d611e8897084d583)
Signed-off-by: Cheng Pan <ch...@apache.org>
---
.../hive/beeline/KyuubiDatabaseConnection.java | 1 +
.../apache/kyuubi/jdbc/hive/KyuubiConnection.java | 32 ++++++++++++++++++----
.../KyuubiOperationPerConnectionSuite.scala | 13 +++++++++
3 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/kyuubi-hive-beeline/src/main/java/org/apache/hive/beeline/KyuubiDatabaseConnection.java b/kyuubi-hive-beeline/src/main/java/org/apache/hive/beeline/KyuubiDatabaseConnection.java
index 7aa4cf9..83daaa8 100644
--- a/kyuubi-hive-beeline/src/main/java/org/apache/hive/beeline/KyuubiDatabaseConnection.java
+++ b/kyuubi-hive-beeline/src/main/java/org/apache/hive/beeline/KyuubiDatabaseConnection.java
@@ -140,6 +140,7 @@ public class KyuubiDatabaseConnection extends DatabaseConnection {
new Thread(beeLine.commands.createLogRunnable(kyuubiConnection, eventNotifier));
logThread.setDaemon(true);
logThread.start();
+ kyuubiConnection.setEngineLogThread(logThread);
kyuubiConnection.waitLaunchEngineToComplete();
logThread.interrupt();
diff --git a/kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/hive/KyuubiConnection.java b/kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/hive/KyuubiConnection.java
index 711e2e9..ebf10f8 100644
--- a/kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/hive/KyuubiConnection.java
+++ b/kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/hive/KyuubiConnection.java
@@ -78,6 +78,7 @@ import org.slf4j.LoggerFactory;
public class KyuubiConnection implements java.sql.Connection, KyuubiLoggable {
public static final Logger LOG = LoggerFactory.getLogger(KyuubiConnection.class.getName());
public static final String BEELINE_MODE_PROPERTY = "BEELINE_MODE";
+ public static int DEFAULT_ENGINE_LOG_THREAD_TIMEOUT = 10 * 1000;
private String jdbcUriString;
private String host;
@@ -100,6 +101,7 @@ public class KyuubiConnection implements java.sql.Connection, KyuubiLoggable {
private boolean initFileCompleted = false;
private TOperationHandle launchEngineOpHandle = null;
+ private Thread engineLogThread;
private boolean engineLogInflight = true;
private volatile boolean launchEngineOpCompleted = false;
@@ -263,7 +265,7 @@ public class KyuubiConnection implements java.sql.Connection, KyuubiLoggable {
private void showLaunchEngineLog() {
if (launchEngineOpHandle != null) {
LOG.info("Starting to get launch engine log.");
- Thread logThread =
+ engineLogThread =
new Thread("engine-launch-log") {
@Override
@@ -282,10 +284,14 @@ public class KyuubiConnection implements java.sql.Connection, KyuubiLoggable {
LOG.info("Finished to get launch engine log.");
}
};
- logThread.start();
+ engineLogThread.start();
}
}
+ public void setEngineLogThread(Thread logThread) {
+ this.engineLogThread = logThread;
+ }
+
public void executeInitSql() throws SQLException {
if (initFileCompleted) return;
if (initFile != null) {
@@ -937,6 +943,18 @@ public class KyuubiConnection implements java.sql.Connection, KyuubiLoggable {
}
}
+ private void closeOnLaunchEngineFailure() throws SQLException {
+ if (engineLogThread != null && engineLogThread.isAlive()) {
+ engineLogThread.interrupt();
+ try {
+ engineLogThread.join(DEFAULT_ENGINE_LOG_THREAD_TIMEOUT);
+ } catch (Exception e) {
+ }
+ }
+ engineLogThread = null;
+ close();
+ }
+
/*
* (non-Javadoc)
*
@@ -1669,12 +1687,14 @@ public class KyuubiConnection implements java.sql.Connection, KyuubiLoggable {
break;
}
}
- } catch (SQLException e) {
- engineLogInflight = false;
- throw e;
} catch (Exception e) {
engineLogInflight = false;
- throw new SQLException(e.toString(), "08S01", e);
+ closeOnLaunchEngineFailure();
+ if (e instanceof SQLException) {
+ throw e;
+ } else {
+ throw new SQLException(e.getMessage(), "08S01", e);
+ }
}
}
}
diff --git a/kyuubi-server/src/test/scala/org/apache/kyuubi/operation/KyuubiOperationPerConnectionSuite.scala b/kyuubi-server/src/test/scala/org/apache/kyuubi/operation/KyuubiOperationPerConnectionSuite.scala
index 6891c1c..f6f15e8 100644
--- a/kyuubi-server/src/test/scala/org/apache/kyuubi/operation/KyuubiOperationPerConnectionSuite.scala
+++ b/kyuubi-server/src/test/scala/org/apache/kyuubi/operation/KyuubiOperationPerConnectionSuite.scala
@@ -26,6 +26,7 @@ import org.scalatest.time.SpanSugar.convertIntToGrainOfTime
import org.apache.kyuubi.{Utils, WithKyuubiServer}
import org.apache.kyuubi.config.KyuubiConf
import org.apache.kyuubi.jdbc.KyuubiHiveDriver
+import org.apache.kyuubi.jdbc.hive.KyuubiConnection
/**
* UT with Connection level engine shared cost much time, only run basic jdbc tests.
@@ -156,4 +157,16 @@ class KyuubiOperationPerConnectionSuite extends WithKyuubiServer with HiveJDBCTe
assert(resultSet.getString(1).nonEmpty)
}
}
+
+ test("close kyuubi connection on launch engine operation failure") {
+ withSessionConf(Map.empty)(Map.empty)(Map(
+ KyuubiConf.SESSION_ENGINE_LAUNCH_ASYNC.key -> "true",
+ "spark.master" -> "invalid")) {
+ val prop = new Properties()
+ prop.setProperty(KyuubiConnection.BEELINE_MODE_PROPERTY, "true")
+ val kyuubiConnection = new KyuubiConnection(jdbcUrlWithConf, prop)
+ intercept[SQLException](kyuubiConnection.waitLaunchEngineToComplete())
+ assert(kyuubiConnection.isClosed)
+ }
+ }
}