[KYUUBI #469] Using Optional Config for query timeout
 [](https://github.com/yaooqinn/kyuubi/pull/469)      [<img width="16" alt="Powered by Pull Request Badge" src="https://user-images.githubusercontent.com/1393946/111216524-d2bb8e00-85d4-11eb-821b-ed4c00989c02.png">](https://pullrequestbadge.com/?utm_medium=github&utm_source=yaooqinn&utm_campaign=badge_info)<!-- PR-BADGE: PLEASE DO NOT REMOVE THIS COMMENT --> <!-- 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/yaooqinn/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 ...'. --> ### _Why are the changes needed?_ <!-- 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. --> 1. bugfix for comparing a client-side value with a second unit to a server-side value with a millisecond unit 2. using OptionalConfigEntry for query timeout, it's more simple, it will take effect only when set with a valid value 3. add tests for system query timeout 4. move timeout conf getter to `initialize` to detect during Kyuubi server startup, the current behavior is a bit dangerous for deploying. ### _How was this patch tested?_ - [x] 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.readthedocs.io/en/latest/tools/testing.html#running-tests) locally before make a pull request Closes #469 from yaooqinn/querytimeout. Closes #469 a524771 [Kent Yao] Using Optional Config for query timeout Authored-by: Kent Yao <yao@apache.org> Signed-off-by: Kent Yao <yao@apache.org>
This commit is contained in:
Kent Yao
parent
5684407ce0
commit
77ecb0d527
@ -188,7 +188,7 @@ Key | Default | Meaning | Type | Since
|
||||
--- | --- | --- | --- | ---
|
||||
kyuubi\.operation\.idle<br>\.timeout|<div style='width: 65pt;word-wrap: break-word;white-space: normal'>PT3H</div>|<div style='width: 170pt;word-wrap: break-word;white-space: normal'>Operation will be closed when it's not accessed for this duration of time</div>|<div style='width: 30pt'>duration</div>|<div style='width: 20pt'>1.0.0</div>
|
||||
kyuubi\.operation<br>\.interrupt\.on\.cancel|<div style='width: 65pt;word-wrap: break-word;white-space: normal'>true</div>|<div style='width: 170pt;word-wrap: break-word;white-space: normal'>When true, all running tasks will be interrupted if one cancels a query. When false, all running tasks will remain until finished.</div>|<div style='width: 30pt'>boolean</div>|<div style='width: 20pt'>1.2.0</div>
|
||||
kyuubi\.operation<br>\.query\.timeout|<div style='width: 65pt;word-wrap: break-word;white-space: normal'>PT0S</div>|<div style='width: 170pt;word-wrap: break-word;white-space: normal'>Set a query duration timeout in seconds in Kyuubi. If the timeout is set to a positive value, a running query will be cancelled automatically if timeout. Otherwise the query continues to run till completion. If timeout values are set for each statement via `java.sql.Statement.setQueryTimeout` and they are smaller than this configuration value, they take precedence. If you set this timeout and prefer to cancel the queries right away without waiting task to finish, consider enabling kyuubi.operation.interrupt.on.cancel together.</div>|<div style='width: 30pt'>duration</div>|<div style='width: 20pt'>1.2.0</div>
|
||||
kyuubi\.operation<br>\.query\.timeout|<div style='width: 65pt;word-wrap: break-word;white-space: normal'><undefined></div>|<div style='width: 170pt;word-wrap: break-word;white-space: normal'>Timeout for query executions at server-side, take affect with client-side timeout(`java.sql.Statement.setQueryTimeout`) together, a running query will be cancelled automatically if timeout. It's off by default, which means only client-side take fully control whether the query should timeout or not. If set, client-side timeout capped at this point. To cancel the queries right away without waiting task to finish, consider enabling kyuubi.operation.interrupt.on.cancel together.</div>|<div style='width: 30pt'>duration</div>|<div style='width: 20pt'>1.2.0</div>
|
||||
kyuubi\.operation<br>\.status\.polling<br>\.timeout|<div style='width: 65pt;word-wrap: break-word;white-space: normal'>PT5S</div>|<div style='width: 170pt;word-wrap: break-word;white-space: normal'>Timeout(ms) for long polling asynchronous running sql query's status</div>|<div style='width: 30pt'>duration</div>|<div style='width: 20pt'>1.0.0</div>
|
||||
|
||||
|
||||
|
||||
@ -510,18 +510,18 @@ object KyuubiConf {
|
||||
.booleanConf
|
||||
.createWithDefault(true)
|
||||
|
||||
val OPERATION_QUERY_TIMEOUT: ConfigEntry[Long] =
|
||||
val OPERATION_QUERY_TIMEOUT: OptionalConfigEntry[Long] =
|
||||
buildConf("operation.query.timeout")
|
||||
.doc("Set a query duration timeout in seconds in Kyuubi. If the timeout is set to " +
|
||||
"a positive value, a running query will be cancelled automatically if timeout. " +
|
||||
"Otherwise the query continues to run till completion. If timeout values are " +
|
||||
"set for each statement via `java.sql.Statement.setQueryTimeout` and they are smaller " +
|
||||
"than this configuration value, they take precedence. If you set this timeout and prefer " +
|
||||
"to cancel the queries right away without waiting task to finish, consider enabling " +
|
||||
s"${OPERATION_FORCE_CANCEL.key} together.")
|
||||
.doc("Timeout for query executions at server-side, take affect with client-side timeout(" +
|
||||
"`java.sql.Statement.setQueryTimeout`) together, a running query will be cancelled" +
|
||||
" automatically if timeout. It's off by default, which means only client-side take fully" +
|
||||
" control whether the query should timeout or not. If set, client-side timeout capped at" +
|
||||
" this point. To cancel the queries right away without waiting task to finish, consider" +
|
||||
s" enabling ${OPERATION_FORCE_CANCEL.key} together.")
|
||||
.version("1.2.0")
|
||||
.timeConf
|
||||
.createWithDefault(Duration.ZERO.toMillis)
|
||||
.checkValue(_ >= 1000, "must >= 1s if set")
|
||||
.createOptional
|
||||
|
||||
val ENGINE_SHARED_LEVEL: ConfigEntry[String] = buildConf("session.engine.share.level")
|
||||
.doc("The SQL engine App will be shared in different levels, available configs are: <ul>" +
|
||||
|
||||
@ -133,4 +133,29 @@ class KyuubiConfSuite extends KyuubiFunSuite {
|
||||
.toMillis)
|
||||
}
|
||||
|
||||
test(KyuubiConf.OPERATION_QUERY_TIMEOUT.key) {
|
||||
val kyuubiConf = KyuubiConf()
|
||||
assert(kyuubiConf.get(OPERATION_QUERY_TIMEOUT).isEmpty)
|
||||
kyuubiConf.set(OPERATION_QUERY_TIMEOUT, 1000L)
|
||||
assert(kyuubiConf.get(OPERATION_QUERY_TIMEOUT) === Some(1000L))
|
||||
kyuubiConf.set(OPERATION_QUERY_TIMEOUT.key, "1000")
|
||||
assert(kyuubiConf.get(OPERATION_QUERY_TIMEOUT) === Some(1000L))
|
||||
kyuubiConf.set(OPERATION_QUERY_TIMEOUT.key, " 1000 ")
|
||||
assert(kyuubiConf.get(OPERATION_QUERY_TIMEOUT) === Some(1000L))
|
||||
kyuubiConf.set(OPERATION_QUERY_TIMEOUT.key, "1000A")
|
||||
val e = intercept[IllegalArgumentException](kyuubiConf.get(OPERATION_QUERY_TIMEOUT))
|
||||
assert(e.getMessage.contains("ISO-8601"))
|
||||
kyuubiConf.set(OPERATION_QUERY_TIMEOUT.key, " P1DT2H3.2S ")
|
||||
|
||||
assert(kyuubiConf.get(OPERATION_QUERY_TIMEOUT) ===
|
||||
Some(Duration.ofDays(1)
|
||||
.plusHours(2)
|
||||
.plusSeconds(3)
|
||||
.plusMillis(200)
|
||||
.toMillis))
|
||||
|
||||
kyuubiConf.set(OPERATION_QUERY_TIMEOUT.key, "0")
|
||||
val e1 = intercept[IllegalArgumentException](kyuubiConf.get(OPERATION_QUERY_TIMEOUT))
|
||||
assert(e1.getMessage.contains("must >= 1s if set"))
|
||||
}
|
||||
}
|
||||
|
||||
@ -328,7 +328,7 @@ trait JDBCTests extends BasicJDBCTests {
|
||||
}
|
||||
}
|
||||
|
||||
test("Support query auto timeout cancel on thriftserver - setQueryTimeout") {
|
||||
test("query time out shall respect client-side if no server-side control") {
|
||||
withJdbcStatement() { statement =>
|
||||
statement.setQueryTimeout(1)
|
||||
val e = intercept[SQLTimeoutException] {
|
||||
|
||||
@ -17,12 +17,13 @@
|
||||
|
||||
package org.apache.kyuubi.operation
|
||||
|
||||
import java.util.concurrent.ConcurrentHashMap
|
||||
import java.util.concurrent.{ConcurrentHashMap, TimeUnit}
|
||||
|
||||
import org.apache.hive.service.rpc.thrift.{TCLIService, TFetchResultsReq, TRowSet, TSessionHandle}
|
||||
|
||||
import org.apache.kyuubi.KyuubiSQLException
|
||||
import org.apache.kyuubi.config.KyuubiConf
|
||||
import org.apache.kyuubi.config.KyuubiConf.OPERATION_QUERY_TIMEOUT
|
||||
import org.apache.kyuubi.operation.FetchOrientation.FetchOrientation
|
||||
import org.apache.kyuubi.session.{Session, SessionHandle}
|
||||
import org.apache.kyuubi.util.ThriftUtils
|
||||
@ -34,6 +35,13 @@ class KyuubiOperationManager private (name: String) extends OperationManager(nam
|
||||
private val handleToClient = new ConcurrentHashMap[SessionHandle, TCLIService.Iface]()
|
||||
private val handleToTSessionHandle = new ConcurrentHashMap[SessionHandle, TSessionHandle]()
|
||||
|
||||
private var queryTimeout: Option[Long] = None
|
||||
|
||||
override def initialize(conf: KyuubiConf): Unit = {
|
||||
queryTimeout = conf.get(OPERATION_QUERY_TIMEOUT).map(TimeUnit.MILLISECONDS.toSeconds)
|
||||
super.initialize(conf)
|
||||
}
|
||||
|
||||
private def getThriftClient(sessionHandle: SessionHandle): TCLIService.Iface = {
|
||||
val client = handleToClient.get(sessionHandle)
|
||||
if (client == null) {
|
||||
@ -53,12 +61,11 @@ class KyuubiOperationManager private (name: String) extends OperationManager(nam
|
||||
private def getQueryTimeout(clientQueryTimeout: Long): Long = {
|
||||
// If clientQueryTimeout is smaller than systemQueryTimeout value,
|
||||
// we use the clientQueryTimeout value.
|
||||
val systemQueryTimeout = getConf.get(KyuubiConf.OPERATION_QUERY_TIMEOUT)
|
||||
if (clientQueryTimeout > 0 &&
|
||||
(systemQueryTimeout <= 0 || clientQueryTimeout < systemQueryTimeout)) {
|
||||
clientQueryTimeout
|
||||
} else {
|
||||
systemQueryTimeout
|
||||
queryTimeout match {
|
||||
case Some(systemQueryTimeout) if clientQueryTimeout > 0 =>
|
||||
math.min(systemQueryTimeout, clientQueryTimeout)
|
||||
case Some(systemQueryTimeout) => systemQueryTimeout
|
||||
case None => clientQueryTimeout
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -0,0 +1,64 @@
|
||||
/*
|
||||
* 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.
|
||||
*/
|
||||
|
||||
package org.apache.kyuubi.operation
|
||||
|
||||
import java.sql.SQLTimeoutException
|
||||
|
||||
import org.apache.kyuubi.config.KyuubiConf
|
||||
import org.apache.kyuubi.config.KyuubiConf.OPERATION_QUERY_TIMEOUT
|
||||
|
||||
class KyuubiOperationManagerSuite extends WithKyuubiServer with JDBCTestUtils {
|
||||
override protected val conf: KyuubiConf = {
|
||||
KyuubiConf().set(OPERATION_QUERY_TIMEOUT.key, "PT1S")
|
||||
}
|
||||
|
||||
test(OPERATION_QUERY_TIMEOUT.key + " initialize") {
|
||||
val kyuubiConf: KyuubiConf = KyuubiConf()
|
||||
val mgr1 = new KyuubiOperationManager()
|
||||
mgr1.initialize(kyuubiConf)
|
||||
assert(mgr1.getConf.get(OPERATION_QUERY_TIMEOUT).isEmpty)
|
||||
|
||||
val mgr2 = new KyuubiOperationManager()
|
||||
mgr2.initialize(kyuubiConf.set(OPERATION_QUERY_TIMEOUT, 1000000L))
|
||||
assert(mgr2.getConf.get(OPERATION_QUERY_TIMEOUT) === Some(1000000L))
|
||||
|
||||
val mgr3 = new KyuubiOperationManager()
|
||||
intercept[IllegalArgumentException] {
|
||||
mgr3.initialize(kyuubiConf.set(OPERATION_QUERY_TIMEOUT.key, "10000A"))
|
||||
}
|
||||
|
||||
val mgr4 = new KyuubiOperationManager()
|
||||
val conf4 = kyuubiConf.set(OPERATION_QUERY_TIMEOUT, -1000000L)
|
||||
intercept[IllegalArgumentException] { mgr4.initialize(conf4) }
|
||||
}
|
||||
|
||||
|
||||
test("query time out shall respect server-side first") {
|
||||
withJdbcStatement() { statement =>
|
||||
Range(-1, 20, 5).foreach { clientTimeout =>
|
||||
statement.setQueryTimeout(clientTimeout)
|
||||
val e = intercept[SQLTimeoutException] {
|
||||
statement.execute("select java_method('java.lang.Thread', 'sleep', 10000L)")
|
||||
}.getMessage
|
||||
assert(e.contains("Query timed out after"))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
override protected def jdbcUrl: String = getJdbcUrl
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user