From 6a9d5ff244d6f2add29ae0ae84594cdf4e7c0cb0 Mon Sep 17 00:00:00 2001 From: Fei Wang Date: Mon, 27 Jun 2022 14:45:52 +0800 Subject: [PATCH] [KYUUBI #2948] Remove thrift request timeout for KyuubiSyncThriftClient ### _Why are the changes needed?_ Remove thrift request timeout for KyuubiSyncThriftClient ### _How was this patch tested?_ - [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [ ] Add screenshots for manual tests if appropriate - [x] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request Closes #2948 from turboFei/remove_request_timeout. Closes #2948 a9c5c084 [Fei Wang] remove request timeout Authored-by: Fei Wang Signed-off-by: Fei Wang --- docs/deployment/settings.md | 1 - .../org/apache/kyuubi/config/KyuubiConf.scala | 6 ------ .../client/KyuubiSyncThriftClient.scala | 5 ++--- .../KyuubiOperationPerConnectionSuite.scala | 21 ++----------------- 4 files changed, 4 insertions(+), 29 deletions(-) diff --git a/docs/deployment/settings.md b/docs/deployment/settings.md index abf1e737b..f4aaa3bdc 100644 --- a/docs/deployment/settings.md +++ b/docs/deployment/settings.md @@ -422,7 +422,6 @@ kyuubi.session.engine.initialize.timeout|PT3M|Timeout for starting the backgroun kyuubi.session.engine.launch.async|true|When opening kyuubi session, whether to launch backend engine asynchronously. When true, the Kyuubi server will set up the connection with the client without delay as the backend engine will be created asynchronously.|boolean|1.4.0 kyuubi.session.engine.log.timeout|PT24H|If we use Spark as the engine then the session submit log is the console output of spark-submit. We will retain the session submit log until over the config value.|duration|1.1.0 kyuubi.session.engine.login.timeout|PT15S|The timeout of creating the connection to remote sql query engine|duration|1.0.0 -kyuubi.session.engine.request.timeout|PT0S|The timeout of awaiting response after sending request to remote sql query engine|duration|1.4.0 kyuubi.session.engine.share.level|USER|(deprecated) - Using kyuubi.engine.share.level instead|string|1.0.0 kyuubi.session.engine.spark.main.resource|<undefined>|The package used to create Spark SQL engine remote application. If it is undefined, Kyuubi will use the default|string|1.0.0 kyuubi.session.engine.spark.max.lifetime|PT0S|Max lifetime for spark engine, the engine will self-terminate when it reaches the end of life. 0 or negative means not to self-terminate.|duration|1.6.0 diff --git a/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala index 57b839fc9..c90c33aa2 100644 --- a/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala +++ b/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala @@ -823,12 +823,6 @@ object KyuubiConf { .timeConf .createWithDefault(Duration.ofSeconds(15).toMillis) - val ENGINE_REQUEST_TIMEOUT: ConfigEntry[Long] = buildConf("kyuubi.session.engine.request.timeout") - .doc("The timeout of awaiting response after sending request to remote sql query engine") - .version("1.4.0") - .timeConf - .createWithDefault(0) - val ENGINE_ALIVE_PROBE_ENABLED: ConfigEntry[Boolean] = buildConf("kyuubi.session.engine.alive.probe.enabled") .doc("Whether to enable the engine alive probe, it true, we will create a companion thrift" + diff --git a/kyuubi-server/src/main/scala/org/apache/kyuubi/client/KyuubiSyncThriftClient.scala b/kyuubi-server/src/main/scala/org/apache/kyuubi/client/KyuubiSyncThriftClient.scala index 9e63c7f6c..ff5de5025 100644 --- a/kyuubi-server/src/main/scala/org/apache/kyuubi/client/KyuubiSyncThriftClient.scala +++ b/kyuubi-server/src/main/scala/org/apache/kyuubi/client/KyuubiSyncThriftClient.scala @@ -30,7 +30,7 @@ import org.apache.thrift.transport.TSocket import org.apache.kyuubi.{KyuubiSQLException, Logging, Utils} import org.apache.kyuubi.config.KyuubiConf -import org.apache.kyuubi.config.KyuubiConf.{ENGINE_LOGIN_TIMEOUT, ENGINE_REQUEST_TIMEOUT} +import org.apache.kyuubi.config.KyuubiConf.ENGINE_LOGIN_TIMEOUT import org.apache.kyuubi.operation.FetchOrientation import org.apache.kyuubi.operation.FetchOrientation.FetchOrientation import org.apache.kyuubi.service.authentication.PlainSASLHelper @@ -432,14 +432,13 @@ private[kyuubi] object KyuubiSyncThriftClient extends Logging { conf: KyuubiConf): KyuubiSyncThriftClient = { val passwd = Option(password).filter(_.nonEmpty).getOrElse("anonymous") val loginTimeout = conf.get(ENGINE_LOGIN_TIMEOUT).toInt - val requestTimeout = conf.get(ENGINE_REQUEST_TIMEOUT).toInt val requestMaxAttempts = conf.get(KyuubiConf.OPERATION_THRIFT_CLIENT_REQUEST_MAX_ATTEMPTS) val aliveProbeEnabled = conf.get(KyuubiConf.ENGINE_ALIVE_PROBE_ENABLED) val aliveProbeInterval = conf.get(KyuubiConf.ENGINE_ALIVE_PROBE_INTERVAL).toInt val aliveTimeout = conf.get(KyuubiConf.ENGINE_ALIVE_TIMEOUT) val (tProtocol, _) = withRetryingRequestNoLock( - createTProtocol(user, passwd, host, port, requestTimeout, loginTimeout), + createTProtocol(user, passwd, host, port, 0, loginTimeout), "CreatingTProtocol", requestMaxAttempts, false, 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 70a3146d8..4059eb084 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 @@ -65,23 +65,6 @@ class KyuubiOperationPerConnectionSuite extends WithKyuubiServer with HiveJDBCTe } } - test("client sync query cost time longer than engine.request.timeout") { - withSessionConf(Map( - KyuubiConf.ENGINE_REQUEST_TIMEOUT.key -> "PT5S"))(Map.empty)(Map.empty) { - withSessionHandle { (client, handle) => - val executeStmtReq = new TExecuteStatementReq() - executeStmtReq.setStatement("select java_method('java.lang.Thread', 'sleep', 6000L)") - executeStmtReq.setSessionHandle(handle) - executeStmtReq.setRunAsync(false) - val executeStmtResp = client.ExecuteStatement(executeStmtReq) - val getOpStatusReq = new TGetOperationStatusReq(executeStmtResp.getOperationHandle) - val getOpStatusResp = client.GetOperationStatus(getOpStatusReq) - assert(getOpStatusResp.getStatus.getStatusCode === TStatusCode.SUCCESS_STATUS) - assert(getOpStatusResp.getOperationState === TOperationState.FINISHED_STATE) - } - } - } - test("sync query causes engine crash") { withSessionHandle { (client, handle) => val executeStmtReq = new TExecuteStatementReq() @@ -233,8 +216,8 @@ class KyuubiOperationPerConnectionSuite extends WithKyuubiServer with HiveJDBCTe KyuubiConf.ENGINE_ALIVE_PROBE_ENABLED.key -> "true", KyuubiConf.ENGINE_ALIVE_PROBE_INTERVAL.key -> "1000", KyuubiConf.ENGINE_ALIVE_TIMEOUT.key -> "3000", - KyuubiConf.OPERATION_THRIFT_CLIENT_REQUEST_MAX_ATTEMPTS.key -> "10000", - KyuubiConf.ENGINE_REQUEST_TIMEOUT.key -> "1000"))(Map.empty)(Map.empty) { + KyuubiConf.OPERATION_THRIFT_CLIENT_REQUEST_MAX_ATTEMPTS.key -> "10000"))(Map.empty)( + Map.empty) { withSessionHandle { (client, handle) => val preReq = new TExecuteStatementReq() preReq.setStatement("select engine_name()")