This is an automated email from the ASF dual-hosted git repository.
chengpan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-kyuubi.git
The following commit(s) were added to refs/heads/master by this push:
new 93e9745 [KYUUBI #1293] Fix potential failure of engine discovery when
mixed use subdomain
93e9745 is described below
commit 93e9745a39a9d69db7cbd70ead63de0fff1e3c0b
Author: yanyu34946 <[email protected]>
AuthorDate: Thu Nov 18 18:00:30 2021 +0800
[KYUUBI #1293] Fix potential failure of engine discovery when mixed use
subdomain
### _Why are the changes needed?_
Fix #1293
This PR is a rework of #1303, also related to #962
We have two options to fix this issue. This PR implements option 2.
```
[option 1]
kyuubi.engine.share.level.subdomain: String -- default: "default"
kyuubi.engine.pool.size: Int -- default: -1
val subdomain =
if (kyuubi.engine.share.level.subdomain == "default" &&
kyuubi.engine.pool.size > 0) {
s"engine-pool-[num]"
} else {
kyuubi.engine.share.level.subdomain
}
[option 2]
kyuubi.engine.share.level.subdomain: Option[String] -- default: None
kyuubi.engine.pool.size: Int -- default: -1
val subdomain =
kyuubi.engine.share.level.subdomain match {
case Some(value) => value
case None if kyuubi.engine.pool.size > 0 => s"engine-pool-[num]"
case None => "default"
}
```
```
[diff]
[[case1]]
kyuubi.engine.share.level.subdomain=default
kyuubi.engine.pool.size=2
option 1 result: subdomain="engine-pool-[num]"
option 2 result: subdomain="default"
[[case2]]
kyuubi.engine.share.level.subdomain=hello
kyuubi.engine.pool.size=2
option 1 result: subdomain="hello"
option 2 result: subdomain="hello"
[[case3]]
kyuubi.engine.share.level.subdomain=
kyuubi.engine.pool.size=2
option 1 result: subdomain="engine-pool-[num]"
option 2 result: subdomain="engine-pool-[num]"
[[case4]]
kyuubi.engine.share.level.subdomain=
kyuubi.engine.pool.size=
option 1 result: subdomain="default"
option 2 result: subdomain="default"
[[case5]]
kyuubi.engine.share.level.subdomain="hello"
kyuubi.engine.pool.size=
option 1 result: subdomain="hello"
option 2 result: subdomain="hello"
```
### _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/develop_tools/testing.html#running-tests)
locally before make a pull request
Closes #1402 from pan3793/1293.
Closes #1293
f6c4a7db [Cheng Pan] nit
fd427bfb [yanyu34946] [KYUUBI #1293] Fix bootstrap potential failure when
mixed use subdomain
Lead-authored-by: yanyu34946 <[email protected]>
Co-authored-by: Cheng Pan <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
---
docs/deployment/settings.md | 2 +-
.../org/apache/kyuubi/config/KyuubiConf.scala | 6 ++--
.../scala/org/apache/kyuubi/engine/EngineRef.scala | 38 ++++++++------------
.../org/apache/kyuubi/engine/EngineRefSuite.scala | 42 +++++++++++++---------
.../operation/KyuubiOperationPerUserSuite.scala | 26 +++++++++++++-
5 files changed, 68 insertions(+), 46 deletions(-)
diff --git a/docs/deployment/settings.md b/docs/deployment/settings.md
index fa658cd..02c315d 100644
--- a/docs/deployment/settings.md
+++ b/docs/deployment/settings.md
@@ -186,7 +186,7 @@ kyuubi\.engine\.pool<br>\.size\.threshold|<div
style='width: 65pt;word-wrap: bre
kyuubi\.engine\.session<br>\.initialize\.sql|<div style='width:
65pt;word-wrap: break-word;white-space: normal'></div>|<div style='width:
170pt;word-wrap: break-word;white-space: normal'>SemiColon-separated list of
SQL statements to be initialized in the newly created engine session before
queries. This configuration can not be used in JDBC url due to the limitation
of Beeline/JDBC driver.</div>|<div style='width: 30pt'>seq</div>|<div
style='width: 20pt'>1.3.0</div>
kyuubi\.engine\.share<br>\.level|<div style='width: 65pt;word-wrap:
break-word;white-space: normal'>USER</div>|<div style='width: 170pt;word-wrap:
break-word;white-space: normal'>Engines will be shared in different levels,
available configs are: <ul> <li>CONNECTION: engine will not be shared but only
used by the current client connection</li> <li>USER: engine will be shared by
all sessions created by a unique username, see also
kyuubi.engine.share.level.subdomain</li> <li>GROUP: engine w [...]
kyuubi\.engine\.share<br>\.level\.sub\.domain|<div style='width:
65pt;word-wrap: break-word;white-space: normal'><undefined></div>|<div
style='width: 170pt;word-wrap: break-word;white-space: normal'>(deprecated) -
Using kyuubi.engine.share.level.subdomain instead</div>|<div style='width:
30pt'>string</div>|<div style='width: 20pt'>1.2.0</div>
-kyuubi\.engine\.share<br>\.level\.subdomain|<div style='width: 65pt;word-wrap:
break-word;white-space: normal'><undefined></div>|<div style='width:
170pt;word-wrap: break-word;white-space: normal'>Allow end-users to create a
subdomain for the share level of an engine. A subdomain is a case-insensitive
string values that must be a valid zookeeper sub path. For example, for `USER`
share level, an end-user can share a certain engine within a subdomain, not for
all of its clients. End- [...]
+kyuubi\.engine\.share<br>\.level\.subdomain|<div style='width: 65pt;word-wrap:
break-word;white-space: normal'><undefined></div>|<div style='width:
170pt;word-wrap: break-word;white-space: normal'>Allow end-users to create a
subdomain for the share level of an engine. A subdomain is a case-insensitive
string values that must be a valid zookeeper sub path. For example, for `USER`
share level, an end-user can share a certain engine within a subdomain, not for
all of its clients. End- [...]
kyuubi\.engine\.single<br>\.spark\.session|<div style='width: 65pt;word-wrap:
break-word;white-space: normal'>false</div>|<div style='width: 170pt;word-wrap:
break-word;white-space: normal'>When set to true, this engine is running in a
single session mode. All the JDBC/ODBC connections share the temporary views,
function registries, SQL configuration and the current database.</div>|<div
style='width: 30pt'>boolean</div>|<div style='width: 20pt'>1.3.0</div>
kyuubi\.engine\.type|<div style='width: 65pt;word-wrap:
break-word;white-space: normal'>SPARK_SQL</div>|<div style='width:
170pt;word-wrap: break-word;white-space: normal'>Specify the detailed engine
that supported by the Kyuubi. The engine type bindings to SESSION scope. This
configuration is experimental. Currently, available configs are: <ul>
<li>SPARK_SQL: specify this engine type will launch a Spark engine which can
provide all the capacity of the Apache Spark. Note, it's a default [...]
kyuubi\.engine\.ui<br>\.retainedSessions|<div style='width: 65pt;word-wrap:
break-word;white-space: normal'>200</div>|<div style='width: 170pt;word-wrap:
break-word;white-space: normal'>The number of SQL client sessions kept in the
Kyuubi Query Engine web UI.</div>|<div style='width: 30pt'>int</div>|<div
style='width: 20pt'>1.4.0</div>
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 a5c309c..2ad1368 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
@@ -784,15 +784,13 @@ object KyuubiConf {
.checkValues(ShareLevel.values.map(_.toString))
.createWithDefault(ShareLevel.USER.toString)
- private val validEngineSubDomain: Pattern = "^[a-zA-Z_-]{1,14}$".r.pattern
-
// [ZooKeeper Data Model]
//
(http://zookeeper.apache.org/doc/r3.7.0/zookeeperProgrammers.html#ch_zkDataModel)
private val validEngineSubdomain: Pattern = ("(?!^[\\u002e]{1,2}$)" +
"(^[\\u0020-\\u002e\\u0030-\\u007e\\u00a0-\\ud7ff\\uf900-\\uffef]{1,}$)").r.pattern
@deprecated(s"using kyuubi.engine.share.level.subdomain instead", "1.4.0")
- val ENGINE_SHARE_LEVEL_SUB_DOMAIN: OptionalConfigEntry[String] =
+ val ENGINE_SHARE_LEVEL_SUB_DOMAIN: ConfigEntry[Option[String]] =
buildConf("engine.share.level.sub.domain")
.doc("(deprecated) - Using kyuubi.engine.share.level.subdomain instead")
.version("1.2.0")
@@ -809,7 +807,7 @@ object KyuubiConf {
" subdomain is a case-insensitive string values that must be a valid
zookeeper sub path." +
" For example, for `USER` share level, an end-user can share a certain
engine within" +
" a subdomain, not for all of its clients. End-users are free to
create multiple" +
- " engines in the `USER` share level")
+ " engines in the `USER` share level. When disable engine pool, use
'default' if absent.")
.version("1.4.0")
.fallbackConf(ENGINE_SHARE_LEVEL_SUB_DOMAIN)
diff --git
a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/EngineRef.scala
b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/EngineRef.scala
index 617b778..c33ffbd 100644
--- a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/EngineRef.scala
+++ b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/EngineRef.scala
@@ -68,25 +68,21 @@ private[kyuubi] class EngineRef(
// Server-side engine pool size threshold
private val poolThreshold: Int = conf.get(ENGINE_POOL_SIZE_THRESHOLD)
+ private val clientPoolSize: Int = conf.get(ENGINE_POOL_SIZE)
+
@VisibleForTesting
- private[kyuubi] val subdomain: Option[String] =
conf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN).orElse {
- val clientPoolSize: Int = conf.get(ENGINE_POOL_SIZE)
-
- if (clientPoolSize > 0) {
- val poolSize = if (clientPoolSize <= poolThreshold) {
- clientPoolSize
- } else {
- warn(s"Request engine pool size($clientPoolSize) exceeds, fallback to
system threshold " +
- s"$poolThreshold")
- poolThreshold
+ private[kyuubi] val subdomain: String =
conf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN) match {
+ case Some(_subdomain) => _subdomain
+ case None if clientPoolSize > 0 =>
+ val poolSize = math.min(clientPoolSize, poolThreshold)
+ if (poolSize < clientPoolSize) {
+ warn(s"Request engine pool size($clientPoolSize) exceeds, fallback to
" +
+ s"system threshold $poolThreshold")
}
-
// TODO: Currently, we use random policy, and later we can add a
sequential policy,
// such as AtomicInteger % poolSize.
- Some("engine-pool-" + Random.nextInt(poolSize))
- } else {
- None
- }
+ "engine-pool-" + Random.nextInt(poolSize)
+ case _ => "default" // [KYUUBI #1293]
}
// Launcher of the engine
@@ -113,10 +109,7 @@ private[kyuubi] class EngineRef(
val commonNamePrefix = s"kyuubi_${shareLevel}_${engineType}_${appUser}"
shareLevel match {
case CONNECTION => s"${commonNamePrefix}_$engineRefId"
- case _ => subdomain match {
- case Some(domain) => s"${commonNamePrefix}_${domain}_$engineRefId"
- case _ => s"${commonNamePrefix}_$engineRefId"
- }
+ case _ => s"${commonNamePrefix}_${subdomain}_$engineRefId"
}
}
@@ -137,10 +130,7 @@ private[kyuubi] class EngineRef(
val commonParent = s"${serverSpace}_${shareLevel}_$engineType"
shareLevel match {
case CONNECTION => ZKPaths.makePath(commonParent, appUser, engineRefId)
- case _ => subdomain match {
- case Some(domain) => ZKPaths.makePath(commonParent, appUser, domain)
- case None => ZKPaths.makePath(commonParent, appUser)
- }
+ case _ => ZKPaths.makePath(commonParent, appUser, subdomain)
}
}
@@ -152,7 +142,7 @@ private[kyuubi] class EngineRef(
case CONNECTION => f
case _ =>
val lockPath =
- ZKPaths.makePath(s"${serverSpace}_$shareLevel", "lock", appUser,
subdomain.orNull)
+ ZKPaths.makePath(s"${serverSpace}_$shareLevel", "lock", appUser,
subdomain)
var lock: InterProcessSemaphoreMutex = null
try {
try {
diff --git
a/kyuubi-server/src/test/scala/org/apache/kyuubi/engine/EngineRefSuite.scala
b/kyuubi-server/src/test/scala/org/apache/kyuubi/engine/EngineRefSuite.scala
index dc3405e..808114d 100644
--- a/kyuubi-server/src/test/scala/org/apache/kyuubi/engine/EngineRefSuite.scala
+++ b/kyuubi-server/src/test/scala/org/apache/kyuubi/engine/EngineRefSuite.scala
@@ -77,8 +77,8 @@ class EngineRefSuite extends KyuubiFunSuite {
conf.set(KyuubiConf.ENGINE_SHARE_LEVEL, USER.toString)
conf.set(KyuubiConf.ENGINE_TYPE, FLINK_SQL.toString)
val appName = new EngineRef(conf, user, id)
- assert(appName.engineSpace ===
ZKPaths.makePath(s"kyuubi_${USER}_$FLINK_SQL", user))
- assert(appName.defaultEngineName ===
s"kyuubi_${USER}_${FLINK_SQL}_${user}_$id")
+ assert(appName.engineSpace ===
ZKPaths.makePath(s"kyuubi_${USER}_$FLINK_SQL", user, "default"))
+ assert(appName.defaultEngineName ===
s"kyuubi_${USER}_${FLINK_SQL}_${user}_default_$id")
Seq(KyuubiConf.ENGINE_SHARE_LEVEL_SUBDOMAIN,
KyuubiConf.ENGINE_SHARE_LEVEL_SUB_DOMAIN).foreach { k =>
@@ -97,8 +97,10 @@ class EngineRefSuite extends KyuubiFunSuite {
conf.set(KyuubiConf.ENGINE_TYPE, SPARK_SQL.toString)
val engineRef = new EngineRef(conf, user, id)
val primaryGroupName =
UserGroupInformation.createRemoteUser(user).getPrimaryGroupName
- assert(engineRef.engineSpace ===
ZKPaths.makePath(s"kyuubi_GROUP_SPARK_SQL", primaryGroupName))
- assert(engineRef.defaultEngineName ===
s"kyuubi_GROUP_SPARK_SQL_${primaryGroupName}_$id")
+ assert(engineRef.engineSpace ===
+ ZKPaths.makePath(s"kyuubi_GROUP_SPARK_SQL", primaryGroupName, "default"))
+ assert(engineRef.defaultEngineName ===
+ s"kyuubi_GROUP_SPARK_SQL_${primaryGroupName}_default_$id")
Seq(KyuubiConf.ENGINE_SHARE_LEVEL_SUBDOMAIN,
KyuubiConf.ENGINE_SHARE_LEVEL_SUB_DOMAIN).foreach { k =>
@@ -125,8 +127,8 @@ class EngineRefSuite extends KyuubiFunSuite {
conf.set(KyuubiConf.ENGINE_TYPE, FLINK_SQL.toString)
val appName = new EngineRef(conf, user, id)
assert(appName.engineSpace ===
- ZKPaths.makePath(s"kyuubi_${SERVER}_${FLINK_SQL}", user))
- assert(appName.defaultEngineName ===
s"kyuubi_${SERVER}_${FLINK_SQL}_${user}_$id")
+ ZKPaths.makePath(s"kyuubi_${SERVER}_${FLINK_SQL}", user, "default"))
+ assert(appName.defaultEngineName ===
s"kyuubi_${SERVER}_${FLINK_SQL}_${user}_default_$id")
conf.set(KyuubiConf.ENGINE_SHARE_LEVEL_SUBDOMAIN.key, "abc")
val appName2 = new EngineRef(conf, user, id)
@@ -138,27 +140,35 @@ class EngineRefSuite extends KyuubiFunSuite {
test("check the engine space of engine pool") {
val id = UUID.randomUUID().toString
- // test subdomain
+ // set subdomain and disable engine pool
conf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN.key, "abc")
+ conf.set(ENGINE_POOL_SIZE, -1)
val engine1 = new EngineRef(conf, user, id)
- assert(engine1.subdomain === Some("abc"))
+ assert(engine1.subdomain === "abc")
- // unset domain
+ // unset subdomain and disable engine pool
conf.unset(ENGINE_SHARE_LEVEL_SUBDOMAIN)
+ conf.set(ENGINE_POOL_SIZE, -1)
val engine2 = new EngineRef(conf, user, id)
- assert(engine2.subdomain === None)
+ assert(engine2.subdomain === "default")
- // 1 <= engine pool size < threshold
+ // set subdomain and 1 <= engine pool size < threshold
+ conf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN.key, "abc")
+ conf.set(ENGINE_POOL_SIZE, 1)
+ val engine3 = new EngineRef(conf, user, id)
+ assert(engine3.subdomain === "abc")
+
+ // unset subdomain and 1 <= engine pool size < threshold
conf.unset(ENGINE_SHARE_LEVEL_SUBDOMAIN)
conf.set(ENGINE_POOL_SIZE, 3)
- val engine3 = new EngineRef(conf, user, id)
- assert(engine3.subdomain.get.startsWith("engine-pool-"))
+ val engine4 = new EngineRef(conf, user, id)
+ assert(engine4.subdomain.startsWith("engine-pool-"))
- // engine pool size > threshold
+ // unset subdomain and engine pool size > threshold
conf.unset(ENGINE_SHARE_LEVEL_SUBDOMAIN)
conf.set(ENGINE_POOL_SIZE, 100)
- val engine4 = new EngineRef(conf, user, id)
- val engineNumber = Integer.parseInt(engine4.subdomain.get.substring(12))
+ val engine5 = new EngineRef(conf, user, id)
+ val engineNumber = Integer.parseInt(engine5.subdomain.substring(12))
val threshold = ENGINE_POOL_SIZE_THRESHOLD.defaultVal.get
assert(engineNumber <= threshold)
}
diff --git
a/kyuubi-server/src/test/scala/org/apache/kyuubi/operation/KyuubiOperationPerUserSuite.scala
b/kyuubi-server/src/test/scala/org/apache/kyuubi/operation/KyuubiOperationPerUserSuite.scala
index ee31b27..a4358b5 100644
---
a/kyuubi-server/src/test/scala/org/apache/kyuubi/operation/KyuubiOperationPerUserSuite.scala
+++
b/kyuubi-server/src/test/scala/org/apache/kyuubi/operation/KyuubiOperationPerUserSuite.scala
@@ -83,7 +83,7 @@ class KyuubiOperationPerUserSuite extends WithKyuubiServer
with SparkQueryTests
}
}
- test("ensure two connections share the same engine when specifying
subDomain.") {
+ test("ensure two connections share the same engine when specifying
subdomain.") {
withSessionConf()(
Map(
KyuubiConf.ENGINE_SHARE_LEVEL_SUBDOMAIN.key -> "abc"
@@ -114,4 +114,28 @@ class KyuubiOperationPerUserSuite extends WithKyuubiServer
with SparkQueryTests
assert(r1 === r2)
}
}
+
+ test("ensure engine discovery works when mixed use subdomain") {
+ var r1: String = null
+ var r2: String = null
+ withSessionConf()(Map.empty)(Map.empty) {
+ withJdbcStatement() { statement =>
+ val res = statement.executeQuery("set spark.app.name")
+ assert(res.next())
+ r1 = res.getString("value")
+ }
+ }
+ assert(r1 contains "default")
+
+ withSessionConf()(Map(KyuubiConf.ENGINE_SHARE_LEVEL_SUBDOMAIN.key ->
"abc"))(Map.empty) {
+ withJdbcStatement() { statement =>
+ val res = statement.executeQuery("set spark.app.name")
+ assert(res.next())
+ r2 = res.getString("value")
+ }
+ }
+ assert(r2 contains "abc")
+
+ assert(r1 !== r2)
+ }
}
