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/kyuubi.git
The following commit(s) were added to refs/heads/master by this push:
new 7926cc66f [KYUUBI #5606][REST] Handle engine listing request properly
for users who have not created engine
7926cc66f is described below
commit 7926cc66f66a082aac24d69c8b7e7f6f2394d8aa
Author: Cheng Pan <[email protected]>
AuthorDate: Fri Nov 3 21:44:12 2023 +0800
[KYUUBI #5606][REST] Handle engine listing request properly for users who
have not created engine
### _Why are the changes needed?_
Close #5606
```
2023-11-03 19:30:13.215 ERROR KyuubiRestFrontendService-57
org.apache.kyuubi.server.api.v1.AdminResource: No such engine for user:
anonymous, engine type: SPARK_SQL, share level: USER, subdomain: null
org.apache.kyuubi.shaded.zookeeper.KeeperException$NoNodeException:
KeeperErrorCode = NoNode for /kyuubi_1.8.1-SNAPSHOT_USER_SPARK_SQL/anonymous
at
org.apache.kyuubi.shaded.zookeeper.KeeperException.create(KeeperException.java:114)
at
org.apache.kyuubi.shaded.zookeeper.KeeperException.create(KeeperException.java:54)
at
org.apache.kyuubi.shaded.zookeeper.ZooKeeper.getChildren(ZooKeeper.java:1659)
at
org.apache.kyuubi.shaded.curator.framework.imps.GetChildrenBuilderImpl$3.call(GetChildrenBuilderImpl.java:230)
at
org.apache.kyuubi.shaded.curator.framework.imps.GetChildrenBuilderImpl$3.call(GetChildrenBuilderImpl.java:219)
at
org.apache.kyuubi.shaded.curator.RetryLoop.callWithRetry(RetryLoop.java:109)
at
org.apache.kyuubi.shaded.curator.framework.imps.GetChildrenBuilderImpl.pathInForeground(GetChildrenBuilderImpl.java:216)
at
org.apache.kyuubi.shaded.curator.framework.imps.GetChildrenBuilderImpl.forPath(GetChildrenBuilderImpl.java:207)
at
org.apache.kyuubi.shaded.curator.framework.imps.GetChildrenBuilderImpl.forPath(GetChildrenBuilderImpl.java:40)
at
org.apache.kyuubi.ha.client.zookeeper.ZookeeperDiscoveryClient.getChildren(ZookeeperDiscoveryClient.scala:82)
at
org.apache.kyuubi.server.api.v1.AdminResource.$anonfun$listEngines$5(AdminResource.scala:306)
at
org.apache.kyuubi.ha.client.DiscoveryClientProvider$.withDiscoveryClient(DiscoveryClientProvider.scala:36)
at
org.apache.kyuubi.server.api.v1.AdminResource.listEngines(AdminResource.scala:304)
```
### _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.readthedocs.io/en/master/contributing/code/testing.html#running-tests)
locally before make a pull request
### _Was this patch authored or co-authored using generative AI tooling?_
No
Closes #5619 from pan3793/list-engine.
Closes #5606
783894b3e [Cheng Pan] nit
52eb538bb [Cheng Pan] nit
8d8097fda [Cheng Pan] [KYUUBI #5606][REST] Handle engine listing request
properly for users who have not created engine
Authored-by: Cheng Pan <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
---
.../kyuubi/server/api/v1/AdminResource.scala | 59 +++++++++-------------
1 file changed, 24 insertions(+), 35 deletions(-)
diff --git
a/kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala
b/kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala
index 75baba74f..0b017543e 100644
---
a/kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala
+++
b/kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala
@@ -40,7 +40,6 @@ import org.apache.kyuubi.operation.{KyuubiOperation,
OperationHandle}
import org.apache.kyuubi.server.KyuubiServer
import org.apache.kyuubi.server.api.{ApiRequestContext, ApiUtils}
import org.apache.kyuubi.session.{KyuubiSession, SessionHandle}
-import org.apache.kyuubi.shaded.zookeeper.KeeperException.NoNodeException
@Tag(name = "Admin")
@Produces(Array(MediaType.APPLICATION_JSON))
@@ -252,8 +251,8 @@ private[v1] class AdminResource extends ApiRequestContext
with Logging {
} else {
fe.getSessionUser(hs2ProxyUser)
}
- val engine = getEngine(userName, engineType, shareLevel, subdomain,
"default")
- val engineSpace = getEngineSpace(engine)
+ val engine = normalizeEngineInfo(userName, engineType, shareLevel,
subdomain, "default")
+ val engineSpace = calculateEngineSpace(engine)
withDiscoveryClient(fe.getConf) { discoveryClient =>
val engineNodes = discoveryClient.getChildren(engineSpace)
@@ -290,33 +289,24 @@ private[v1] class AdminResource extends ApiRequestContext
with Logging {
} else {
fe.getSessionUser(hs2ProxyUser)
}
- val engine = getEngine(userName, engineType, shareLevel, subdomain, "")
- val engineSpace = getEngineSpace(engine)
+ val engine = normalizeEngineInfo(userName, engineType, shareLevel,
subdomain, "")
+ val engineSpace = calculateEngineSpace(engine)
val engineNodes = ListBuffer[ServiceNodeInfo]()
- Option(subdomain).filter(_.nonEmpty) match {
- case Some(_) =>
- withDiscoveryClient(fe.getConf) { discoveryClient =>
- info(s"Listing engine nodes for $engineSpace")
+ withDiscoveryClient(fe.getConf) { discoveryClient =>
+ Option(subdomain).filter(_.nonEmpty) match {
+ case Some(_) =>
+ info(s"Listing engine nodes under $engineSpace")
engineNodes ++= discoveryClient.getServiceNodesInfo(engineSpace)
- }
- case None =>
- withDiscoveryClient(fe.getConf) { discoveryClient =>
- try {
- discoveryClient.getChildren(engineSpace).map { child =>
- info(s"Listing engine nodes for $engineSpace/$child")
- engineNodes ++=
discoveryClient.getServiceNodesInfo(s"$engineSpace/$child")
- }
- } catch {
- case nne: NoNodeException =>
- error(
- s"No such engine for user: $userName, " +
- s"engine type: $engineType, share level: $shareLevel,
subdomain: $subdomain",
- nne)
- throw new NotFoundException(s"No such engine for user:
$userName, " +
- s"engine type: $engineType, share level: $shareLevel,
subdomain: $subdomain")
+ case None if discoveryClient.pathNonExists(engineSpace) =>
+ warn(s"Path $engineSpace does not exist. user: $userName, engine
type: $engineType, " +
+ s"share level: $shareLevel, subdomain: $subdomain")
+ case None =>
+ discoveryClient.getChildren(engineSpace).map { child =>
+ info(s"Listing engine nodes under $engineSpace/$child")
+ engineNodes ++=
discoveryClient.getServiceNodesInfo(s"$engineSpace/$child")
}
- }
+ }
}
engineNodes.map(node =>
new Engine(
@@ -360,7 +350,7 @@ private[v1] class AdminResource extends ApiRequestContext
with Logging {
servers.toSeq
}
- private def getEngine(
+ private def normalizeEngineInfo(
userName: String,
engineType: String,
shareLevel: String,
@@ -373,6 +363,7 @@ private[v1] class AdminResource extends ApiRequestContext
with Logging {
.foreach(_ => clonedConf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN,
Option(subdomain)))
Option(shareLevel).filter(_.nonEmpty).foreach(clonedConf.set(ENGINE_SHARE_LEVEL,
_))
+ val serverSpace = clonedConf.get(HA_NAMESPACE)
val normalizedEngineType = clonedConf.get(ENGINE_TYPE)
val engineSubdomain =
clonedConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN).getOrElse(subdomainDefault)
val engineShareLevel = clonedConf.get(ENGINE_SHARE_LEVEL)
@@ -384,22 +375,20 @@ private[v1] class AdminResource extends ApiRequestContext
with Logging {
engineShareLevel,
engineSubdomain,
null,
- null,
+ serverSpace,
Collections.emptyMap())
}
- private def getEngineSpace(engine: Engine): String = {
- val serverSpace = fe.getConf.get(HA_NAMESPACE)
- val appUser = engine.getSharelevel match {
+ private def calculateEngineSpace(engine: Engine): String = {
+ val userOrGroup = engine.getSharelevel match {
case "GROUP" =>
fe.sessionManager.groupProvider.primaryGroup(engine.getUser,
fe.getConf.getAll.asJava)
case _ => engine.getUser
}
- DiscoveryPaths.makePath(
-
s"${serverSpace}_${engine.getVersion}_${engine.getSharelevel}_${engine.getEngineType}",
- appUser,
- engine.getSubdomain)
+ val engineSpace =
+
s"${engine.getNamespace}_${engine.getVersion}_${engine.getSharelevel}_${engine.getEngineType}"
+ DiscoveryPaths.makePath(engineSpace, userOrGroup, engine.getSubdomain)
}
@ApiResponse(
