You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by "zhaohehuhu (via GitHub)" <gi...@apache.org> on 2024/01/23 10:09:23 UTC
[PR] Support killing engine forcibly [kyuubi]
zhaohehuhu opened a new pull request, #6008:
URL: https://github.com/apache/kyuubi/pull/6008
# :mag: Description
## Issue References 🔗
<!-- Append the issue number after #. If there is no issue for you to link create one or -->
<!-- If there are no issues to link, please provide details here. -->
This pull request fixes #
## Describe Your Solution 🔧
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
## Types of changes :bookmark:
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
## Test Plan 🧪
#### Behavior Without This Pull Request :coffin:
#### Behavior With This Pull Request :tada:
#### Related Unit Tests
---
# Checklist 📝
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
- [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)
**Be nice. Be informative.**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "wForget (via GitHub)" <gi...@apache.org>.
wForget commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1465731148
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -270,6 +275,18 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
s"${e.getMessage}")
}
}
+
+ if (Option(refId).isDefined) {
+
+ val applicationManagerInfo = ApplicationManagerInfo(
+ Option(sparkMaster),
+ Option(sparK8Context),
+ Option(sparK8Namespace))
+
+ fe.be.sessionManager.asInstanceOf[KyuubiSessionManager]
Review Comment:
We should further handle the result of `killApplication`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1534079200
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -255,24 +259,47 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
}
val engine = normalizeEngineInfo(userName, engineType, shareLevel, subdomain, "default")
val engineSpace = calculateEngineSpace(engine)
+ var msg = s"Engine $engineSpace is deleted successfully."
withDiscoveryClient(fe.getConf) { discoveryClient =>
val engineNodes = discoveryClient.getChildren(engineSpace)
engineNodes.foreach { node =>
val nodePath = s"$engineSpace/$node"
- info(s"Deleting engine node:$nodePath")
- try {
- discoveryClient.delete(nodePath)
- } catch {
- case e: Exception =>
- error(s"Failed to delete engine node:$nodePath", e)
- throw new NotFoundException(s"Failed to delete engine node:$nodePath," +
- s"${e.getMessage}")
+ if (forceKill) {
+ val refIds = node.split(";")
+ .flatMap(_.split("=", 2) match {
+ case Array("refId", value) if StringUtils.isNotBlank(value) => Some(value)
+ case _ => None
+ })
+
+ refIds.foreach(refId => {
+ val applicationManagerInfo = ApplicationManagerInfo(
+ Option(resourceManager),
+ Option(kubernetesContext),
+ Option(kubernetesNamespace))
+ val killMessage = fe.be.sessionManager.asInstanceOf[KyuubiSessionManager]
+ .applicationManager.killApplication(applicationManagerInfo, refId)
+ if (!killMessage._1) {
+ msg = s"Engine $engineSpace failed to get deleted forcibly," +
+ s"cause ${killMessage._2}"
+ error(msg)
+ throw new NotFoundException(msg)
+ }
+ })
+ } else {
+ info(s"Deleting engine node:$nodePath")
+ try {
+ discoveryClient.delete(nodePath)
+ } catch {
+ case e: Exception =>
+ error(s"Failed to delete engine node:$nodePath", e)
+ throw new NotFoundException(s"Failed to delete engine node:$nodePath," +
+ s"${e.getMessage}")
Review Comment:
yes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "zhaohehuhu (via GitHub)" <gi...@apache.org>.
zhaohehuhu commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1525802197
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -246,7 +246,12 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
@QueryParam("sharelevel") shareLevel: String,
@QueryParam("subdomain") subdomain: String,
@QueryParam("proxyUser") kyuubiProxyUser: String,
- @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String,
+ @QueryParam("forceKill") @DefaultValue("false") forceKill: Boolean,
+ @QueryParam("refId") refId: String,
Review Comment:
OK, that makes sense. I can get refId by below listEngines method, right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "zhaohehuhu (via GitHub)" <gi...@apache.org>.
zhaohehuhu commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1527699776
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -255,24 +260,47 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
}
val engine = normalizeEngineInfo(userName, engineType, shareLevel, subdomain, "default")
val engineSpace = calculateEngineSpace(engine)
+ var msg = s"Engine $engineSpace is deleted successfully."
+ if (forceKill) {
+ val refIds = listEngines(
+ engineType,
+ shareLevel,
+ subdomain,
+ kyuubiProxyUser,
+ hs2ProxyUser).map(_.getAttributes.get("refId"))
- withDiscoveryClient(fe.getConf) { discoveryClient =>
- val engineNodes = discoveryClient.getChildren(engineSpace)
- engineNodes.foreach { node =>
- val nodePath = s"$engineSpace/$node"
- info(s"Deleting engine node:$nodePath")
- try {
- discoveryClient.delete(nodePath)
- } catch {
- case e: Exception =>
- error(s"Failed to delete engine node:$nodePath", e)
- throw new NotFoundException(s"Failed to delete engine node:$nodePath," +
- s"${e.getMessage}")
+ refIds.filter(StringUtils.isNotBlank(_)).foreach(refId => {
+ val applicationManagerInfo = ApplicationManagerInfo(
+ Option(resourceManager),
+ Option(kubernetesContext),
+ Option(kubernetesNamespace))
+ val killMessage = fe.be.sessionManager.asInstanceOf[KyuubiSessionManager]
+ .applicationManager.killApplication(applicationManagerInfo, refId)
+ if (!killMessage._1) {
+ msg = s"Engine $engineSpace failed to get deleted forcibly," +
+ s"cause ${killMessage._2}"
+ error(msg)
+ throw new NotFoundException(msg)
+ }
+ })
+ } else {
+ withDiscoveryClient(fe.getConf) { discoveryClient =>
+ val engineNodes = discoveryClient.getChildren(engineSpace)
+ engineNodes.foreach { node =>
+ val nodePath = s"$engineSpace/$node"
Review Comment:
OK. Let me take a look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "wForget (via GitHub)" <gi...@apache.org>.
wForget commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1465730011
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -270,6 +275,18 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
s"${e.getMessage}")
}
}
+
+ if (Option(refId).isDefined) {
Review Comment:
Can we add a new parameter to decide whether to kill forcefully or gracefully？
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "zhaohehuhu (via GitHub)" <gi...@apache.org>.
zhaohehuhu commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1494163863
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -255,24 +260,44 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
}
val engine = normalizeEngineInfo(userName, engineType, shareLevel, subdomain, "default")
val engineSpace = calculateEngineSpace(engine)
+ var msg = s"Engine $engineSpace is deleted successfully."
+ if (forceKill) {
+ if (StringUtils.isBlank(refId)) {
+ error(s"Invalid refId: $refId")
+ throw new NotFoundException(s"Invalid refId: $refId")
+ }
- withDiscoveryClient(fe.getConf) { discoveryClient =>
- val engineNodes = discoveryClient.getChildren(engineSpace)
- engineNodes.foreach { node =>
- val nodePath = s"$engineSpace/$node"
- info(s"Deleting engine node:$nodePath")
- try {
- discoveryClient.delete(nodePath)
- } catch {
- case e: Exception =>
- error(s"Failed to delete engine node:$nodePath", e)
- throw new NotFoundException(s"Failed to delete engine node:$nodePath," +
- s"${e.getMessage}")
+ val applicationManagerInfo = ApplicationManagerInfo(
+ Option(sparkMaster),
+ Option(sparK8Context),
+ Option(sparK8Namespace))
+
+ var killMessage: KillResponse = (false, "UNKNOWN")
+ killMessage = fe.be.sessionManager.asInstanceOf[KyuubiSessionManager]
+ .applicationManager.killApplication(applicationManagerInfo, refId)
+ if (!killMessage._1) {
+ msg = s"Engine $engineSpace failed to get deleted forcibly," +
+ s"cause ${killMessage._2}"
+ error(msg)
Review Comment:
done. Thanks！
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] RESTful API supports killing engine forcibly [kyuubi]
Posted by "HaoYang670 (via GitHub)" <gi...@apache.org>.
HaoYang670 commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1710994235
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -286,24 +288,38 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
}
val engine = normalizeEngineInfo(userName, engineType, shareLevel, subdomain, "default")
val engineSpace = calculateEngineSpace(engine)
+ val responseMsgBuilder = new StringBuilder()
withDiscoveryClient(fe.getConf) { discoveryClient =>
- val engineNodes = discoveryClient.getChildren(engineSpace)
- engineNodes.foreach { node =>
- val nodePath = s"$engineSpace/$node"
+ val engineNodes = discoveryClient.getServiceNodesInfo(engineSpace, silent = true)
+ engineNodes.foreach { engineNode =>
+ val nodePath = s"$engineSpace/${engineNode.nodeName}"
+ val engineRefId = engineNode.engineRefId.orNull
info(s"Deleting engine node:$nodePath")
Review Comment:
I don't think so. Even in GROUP level, we can only create one engine per subdomain, right. And subdomain has been included in `engineSpace` right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1525685744
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -246,7 +246,12 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
@QueryParam("sharelevel") shareLevel: String,
@QueryParam("subdomain") subdomain: String,
@QueryParam("proxyUser") kyuubiProxyUser: String,
- @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String,
+ @QueryParam("forceKill") @DefaultValue("false") forceKill: Boolean,
+ @QueryParam("refId") refId: String,
+ @QueryParam("sparK8Context") sparK8Context: String,
Review Comment:
I am not sure why here is specified for spark.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] RESTful API supports killing engine forcibly [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1650968520
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -277,7 +278,8 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
@QueryParam("sharelevel") shareLevel: String,
@QueryParam("subdomain") subdomain: String,
@QueryParam("proxyUser") kyuubiProxyUser: String,
- @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String,
+ @QueryParam("forceKill") @DefaultValue("false") forceKill: Boolean): Response = {
Review Comment:
```suggestion
@QueryParam("kill") @DefaultValue("false") kill: Boolean): Response = {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] RESTful API supports killing engine forcibly [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1650966184
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -3544,7 +3544,12 @@ object KyuubiConf {
.version("1.8.0")
.stringConf
.toSequence()
- .createWithDefault(Seq("spark.driver.memory", "spark.executor.memory"))
+ .createWithDefault(Seq(
+ "spark.driver.memory",
+ "spark.executor.memory",
+ "spark.kubernetes.context",
+ "spark.kubernetes.namespace",
+ "spark.master"))
Review Comment:
we don't need to expose those extra attrs as we decided to use `kyuubi.engine.appMgrInfo` to stored a base64 encoded string
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "zhaohehuhu (via GitHub)" <gi...@apache.org>.
zhaohehuhu commented on PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#issuecomment-1916298779
Done. Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1527008300
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -255,24 +260,47 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
}
val engine = normalizeEngineInfo(userName, engineType, shareLevel, subdomain, "default")
val engineSpace = calculateEngineSpace(engine)
+ var msg = s"Engine $engineSpace is deleted successfully."
+ if (forceKill) {
+ val refIds = listEngines(
+ engineType,
+ shareLevel,
+ subdomain,
+ kyuubiProxyUser,
+ hs2ProxyUser).map(_.getAttributes.get("refId"))
- withDiscoveryClient(fe.getConf) { discoveryClient =>
- val engineNodes = discoveryClient.getChildren(engineSpace)
- engineNodes.foreach { node =>
- val nodePath = s"$engineSpace/$node"
- info(s"Deleting engine node:$nodePath")
- try {
- discoveryClient.delete(nodePath)
- } catch {
- case e: Exception =>
- error(s"Failed to delete engine node:$nodePath", e)
- throw new NotFoundException(s"Failed to delete engine node:$nodePath," +
- s"${e.getMessage}")
+ refIds.filter(StringUtils.isNotBlank(_)).foreach(refId => {
+ val applicationManagerInfo = ApplicationManagerInfo(
+ Option(resourceManager),
+ Option(kubernetesContext),
+ Option(kubernetesNamespace))
+ val killMessage = fe.be.sessionManager.asInstanceOf[KyuubiSessionManager]
+ .applicationManager.killApplication(applicationManagerInfo, refId)
+ if (!killMessage._1) {
+ msg = s"Engine $engineSpace failed to get deleted forcibly," +
+ s"cause ${killMessage._2}"
+ error(msg)
+ throw new NotFoundException(msg)
+ }
+ })
+ } else {
+ withDiscoveryClient(fe.getConf) { discoveryClient =>
+ val engineNodes = discoveryClient.getChildren(engineSpace)
+ engineNodes.foreach { node =>
+ val nodePath = s"$engineSpace/$node"
Review Comment:
how about checking `forceKill` inside here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "zhaohehuhu (via GitHub)" <gi...@apache.org>.
zhaohehuhu commented on PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#issuecomment-1926155520
plz review it again @wForget
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1525748667
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -246,7 +246,12 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
@QueryParam("sharelevel") shareLevel: String,
@QueryParam("subdomain") subdomain: String,
@QueryParam("proxyUser") kyuubiProxyUser: String,
- @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String,
+ @QueryParam("forceKill") @DefaultValue("false") forceKill: Boolean,
+ @QueryParam("refId") refId: String,
Review Comment:
See DiscoveryClient::getServiceNodesInfo
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "zhaohehuhu (via GitHub)" <gi...@apache.org>.
zhaohehuhu commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1534980692
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -255,24 +259,47 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
}
val engine = normalizeEngineInfo(userName, engineType, shareLevel, subdomain, "default")
val engineSpace = calculateEngineSpace(engine)
+ var msg = s"Engine $engineSpace is deleted successfully."
withDiscoveryClient(fe.getConf) { discoveryClient =>
val engineNodes = discoveryClient.getChildren(engineSpace)
engineNodes.foreach { node =>
val nodePath = s"$engineSpace/$node"
- info(s"Deleting engine node:$nodePath")
- try {
- discoveryClient.delete(nodePath)
- } catch {
- case e: Exception =>
- error(s"Failed to delete engine node:$nodePath", e)
- throw new NotFoundException(s"Failed to delete engine node:$nodePath," +
- s"${e.getMessage}")
+ if (forceKill) {
+ val refIds = node.split(";")
+ .flatMap(_.split("=", 2) match {
+ case Array("refId", value) if StringUtils.isNotBlank(value) => Some(value)
+ case _ => None
+ })
+
+ refIds.foreach(refId => {
+ val applicationManagerInfo = ApplicationManagerInfo(
+ Option(resourceManager),
+ Option(kubernetesContext),
+ Option(kubernetesNamespace))
+ val killMessage = fe.be.sessionManager.asInstanceOf[KyuubiSessionManager]
+ .applicationManager.killApplication(applicationManagerInfo, refId)
+ if (!killMessage._1) {
+ msg = s"Engine $engineSpace failed to get deleted forcibly," +
+ s"cause ${killMessage._2}"
+ error(msg)
+ throw new NotFoundException(msg)
+ }
+ })
+ } else {
+ info(s"Deleting engine node:$nodePath")
+ try {
+ discoveryClient.delete(nodePath)
+ } catch {
+ case e: Exception =>
+ error(s"Failed to delete engine node:$nodePath", e)
+ throw new NotFoundException(s"Failed to delete engine node:$nodePath," +
+ s"${e.getMessage}")
Review Comment:
Thanks for your guidance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "zhaohehuhu (via GitHub)" <gi...@apache.org>.
zhaohehuhu commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1533369224
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -255,24 +259,47 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
}
val engine = normalizeEngineInfo(userName, engineType, shareLevel, subdomain, "default")
val engineSpace = calculateEngineSpace(engine)
+ var msg = s"Engine $engineSpace is deleted successfully."
withDiscoveryClient(fe.getConf) { discoveryClient =>
val engineNodes = discoveryClient.getChildren(engineSpace)
engineNodes.foreach { node =>
val nodePath = s"$engineSpace/$node"
- info(s"Deleting engine node:$nodePath")
- try {
- discoveryClient.delete(nodePath)
- } catch {
- case e: Exception =>
- error(s"Failed to delete engine node:$nodePath", e)
- throw new NotFoundException(s"Failed to delete engine node:$nodePath," +
- s"${e.getMessage}")
+ if (forceKill) {
+ val refIds = node.split(";")
+ .flatMap(_.split("=", 2) match {
+ case Array("refId", value) if StringUtils.isNotBlank(value) => Some(value)
+ case _ => None
+ })
+
+ refIds.foreach(refId => {
+ val applicationManagerInfo = ApplicationManagerInfo(
+ Option(resourceManager),
+ Option(kubernetesContext),
+ Option(kubernetesNamespace))
+ val killMessage = fe.be.sessionManager.asInstanceOf[KyuubiSessionManager]
+ .applicationManager.killApplication(applicationManagerInfo, refId)
+ if (!killMessage._1) {
+ msg = s"Engine $engineSpace failed to get deleted forcibly," +
+ s"cause ${killMessage._2}"
+ error(msg)
+ throw new NotFoundException(msg)
+ }
+ })
+ } else {
+ info(s"Deleting engine node:$nodePath")
+ try {
+ discoveryClient.delete(nodePath)
+ } catch {
+ case e: Exception =>
+ error(s"Failed to delete engine node:$nodePath", e)
+ throw new NotFoundException(s"Failed to delete engine node:$nodePath," +
+ s"${e.getMessage}")
Review Comment:
Great ! So we have to delete the node and then kill application
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "zhaohehuhu (via GitHub)" <gi...@apache.org>.
zhaohehuhu commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1500238454
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -255,24 +260,44 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
}
val engine = normalizeEngineInfo(userName, engineType, shareLevel, subdomain, "default")
val engineSpace = calculateEngineSpace(engine)
+ var msg = s"Engine $engineSpace is deleted successfully."
+ if (forceKill) {
+ if (StringUtils.isBlank(refId)) {
+ throw new IllegalArgumentException(s"Invalid refId: $refId")
+ }
- withDiscoveryClient(fe.getConf) { discoveryClient =>
- val engineNodes = discoveryClient.getChildren(engineSpace)
- engineNodes.foreach { node =>
- val nodePath = s"$engineSpace/$node"
- info(s"Deleting engine node:$nodePath")
- try {
- discoveryClient.delete(nodePath)
- } catch {
- case e: Exception =>
- error(s"Failed to delete engine node:$nodePath", e)
- throw new NotFoundException(s"Failed to delete engine node:$nodePath," +
- s"${e.getMessage}")
+ val applicationManagerInfo = ApplicationManagerInfo(
+ Option(sparkMaster),
+ Option(sparK8Context),
+ Option(sparK8Namespace))
+
+ var killMessage: KillResponse = (false, "UNKNOWN")
+ killMessage = fe.be.sessionManager.asInstanceOf[KyuubiSessionManager]
Review Comment:
killMessage = fe.be.sessionManager.asInstanceOf[KyuubiSessionManager]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "zhaohehuhu (via GitHub)" <gi...@apache.org>.
zhaohehuhu commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1500256696
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -255,24 +260,44 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
}
val engine = normalizeEngineInfo(userName, engineType, shareLevel, subdomain, "default")
val engineSpace = calculateEngineSpace(engine)
+ var msg = s"Engine $engineSpace is deleted successfully."
+ if (forceKill) {
+ if (StringUtils.isBlank(refId)) {
+ throw new IllegalArgumentException(s"Invalid refId: $refId")
+ }
- withDiscoveryClient(fe.getConf) { discoveryClient =>
- val engineNodes = discoveryClient.getChildren(engineSpace)
- engineNodes.foreach { node =>
- val nodePath = s"$engineSpace/$node"
- info(s"Deleting engine node:$nodePath")
- try {
- discoveryClient.delete(nodePath)
- } catch {
- case e: Exception =>
- error(s"Failed to delete engine node:$nodePath", e)
- throw new NotFoundException(s"Failed to delete engine node:$nodePath," +
- s"${e.getMessage}")
+ val applicationManagerInfo = ApplicationManagerInfo(
+ Option(sparkMaster),
+ Option(sparK8Context),
+ Option(sparK8Namespace))
+
+ var killMessage: KillResponse = (false, "UNKNOWN")
+ killMessage = fe.be.sessionManager.asInstanceOf[KyuubiSessionManager]
Review Comment:
Done. Thanks!!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "zhaohehuhu (via GitHub)" <gi...@apache.org>.
zhaohehuhu commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1496827249
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -246,7 +246,12 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
@QueryParam("sharelevel") shareLevel: String,
@QueryParam("subdomain") subdomain: String,
@QueryParam("proxyUser") kyuubiProxyUser: String,
- @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String,
+ @QueryParam("forceKill") forceKill: Boolean,
Review Comment:
Got it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] RESTful API supports killing engine forcibly [kyuubi]
Posted by "HaoYang670 (via GitHub)" <gi...@apache.org>.
HaoYang670 commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1710992272
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -286,24 +288,38 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
}
val engine = normalizeEngineInfo(userName, engineType, shareLevel, subdomain, "default")
val engineSpace = calculateEngineSpace(engine)
+ val responseMsgBuilder = new StringBuilder()
withDiscoveryClient(fe.getConf) { discoveryClient =>
- val engineNodes = discoveryClient.getChildren(engineSpace)
- engineNodes.foreach { node =>
- val nodePath = s"$engineSpace/$node"
+ val engineNodes = discoveryClient.getServiceNodesInfo(engineSpace, silent = true)
+ engineNodes.foreach { engineNode =>
+ val nodePath = s"$engineSpace/${engineNode.nodeName}"
+ val engineRefId = engineNode.engineRefId.orNull
info(s"Deleting engine node:$nodePath")
Review Comment:
Do you mean there can be multiple engines under same subdomain?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#issuecomment-1909782279
## [Codecov](https://app.codecov.io/gh/apache/kyuubi/pull/6008?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: `7 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`3af7551`)](https://app.codecov.io/gh/apache/kyuubi/commit/3af755115ac44737017a6e5d9448f87592d7d26c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 61.23% compared to head [(`da653ad`)](https://app.codecov.io/gh/apache/kyuubi/pull/6008?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 61.07%.
> Report is 27 commits behind head on master.
> :exclamation: Current head da653ad differs from pull request most recent head e3476a6. Consider uploading reports for the commit e3476a6 to get more accurate results
| [Files](https://app.codecov.io/gh/apache/kyuubi/pull/6008?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...rg/apache/kyuubi/server/api/v1/AdminResource.scala](https://app.codecov.io/gh/apache/kyuubi/pull/6008?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvYXBpL3YxL0FkbWluUmVzb3VyY2Uuc2NhbGE=) | 0.00% | [6 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/kyuubi/pull/6008?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #6008 +/- ##
============================================
- Coverage 61.23% 61.07% -0.16%
Complexity 23 23
============================================
Files 622 623 +1
Lines 36887 37072 +185
Branches 5014 5025 +11
============================================
+ Hits 22587 22642 +55
- Misses 11869 11988 +119
- Partials 2431 2442 +11
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/kyuubi/pull/6008?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "wForget (via GitHub)" <gi...@apache.org>.
wForget commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1466235888
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -33,18 +33,19 @@ import org.apache.kyuubi.{KYUUBI_VERSION, Logging}
import org.apache.kyuubi.client.api.v1.dto._
import org.apache.kyuubi.config.KyuubiConf
import org.apache.kyuubi.config.KyuubiConf._
+import org.apache.kyuubi.engine.{ApplicationManagerInfo, KillResponse}
import org.apache.kyuubi.ha.HighAvailabilityConf.HA_NAMESPACE
import org.apache.kyuubi.ha.client.{DiscoveryPaths, ServiceNodeInfo}
import org.apache.kyuubi.ha.client.DiscoveryClientProvider.withDiscoveryClient
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.session.{KyuubiSession, KyuubiSessionManager, SessionHandle}
@Tag(name = "Admin")
@Produces(Array(MediaType.APPLICATION_JSON))
private[v1] class AdminResource extends ApiRequestContext with Logging {
-
+ private var killMessage: KillResponse = (false, "UNKNOWN")
Review Comment:
Why is it defined outside of kill method?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] RESTful API supports killing engine forcibly [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#issuecomment-2182045326
@turboFei @zhaohehuhu Sorry for forgetting this one, the new approach lgtm, let fix the UT the get it in.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "zhaohehuhu (via GitHub)" <gi...@apache.org>.
zhaohehuhu commented on PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#issuecomment-2062873888
I fixed UT. Plz take a look @turboFei
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "wForget (via GitHub)" <gi...@apache.org>.
wForget commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1496776873
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -246,7 +246,12 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
@QueryParam("sharelevel") shareLevel: String,
@QueryParam("subdomain") subdomain: String,
@QueryParam("proxyUser") kyuubiProxyUser: String,
- @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String,
+ @QueryParam("forceKill") forceKill: Boolean,
Review Comment:
add default value
```
@QueryParam("forceKill") @DefaultValue("false") forceKill: Boolean,
```
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -255,24 +260,44 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
}
val engine = normalizeEngineInfo(userName, engineType, shareLevel, subdomain, "default")
val engineSpace = calculateEngineSpace(engine)
+ var msg = s"Engine $engineSpace is deleted successfully."
+ if (forceKill) {
+ if (StringUtils.isBlank(refId)) {
+ throw new IllegalArgumentException(s"Invalid refId: $refId")
+ }
- withDiscoveryClient(fe.getConf) { discoveryClient =>
- val engineNodes = discoveryClient.getChildren(engineSpace)
- engineNodes.foreach { node =>
- val nodePath = s"$engineSpace/$node"
- info(s"Deleting engine node:$nodePath")
- try {
- discoveryClient.delete(nodePath)
- } catch {
- case e: Exception =>
- error(s"Failed to delete engine node:$nodePath", e)
- throw new NotFoundException(s"Failed to delete engine node:$nodePath," +
- s"${e.getMessage}")
+ val applicationManagerInfo = ApplicationManagerInfo(
+ Option(sparkMaster),
+ Option(sparK8Context),
+ Option(sparK8Namespace))
+
+ var killMessage: KillResponse = (false, "UNKNOWN")
+ killMessage = fe.be.sessionManager.asInstanceOf[KyuubiSessionManager]
Review Comment:
use `val`
```
val killMessage = fe.be.sessionManager.asInstanceOf[KyuubiSessionManager]
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "zhaohehuhu (via GitHub)" <gi...@apache.org>.
zhaohehuhu commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1466015539
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -270,6 +275,18 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
s"${e.getMessage}")
}
}
+
+ if (Option(refId).isDefined) {
Review Comment:
Of course.
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -270,6 +275,18 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
s"${e.getMessage}")
}
}
+
+ if (Option(refId).isDefined) {
+
+ val applicationManagerInfo = ApplicationManagerInfo(
+ Option(sparkMaster),
+ Option(sparK8Context),
+ Option(sparK8Namespace))
+
+ fe.be.sessionManager.asInstanceOf[KyuubiSessionManager]
Review Comment:
Got it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] RESTful API supports killing engine forcibly [kyuubi]
Posted by "HaoYang670 (via GitHub)" <gi...@apache.org>.
HaoYang670 commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1711098104
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -286,24 +288,38 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
}
val engine = normalizeEngineInfo(userName, engineType, shareLevel, subdomain, "default")
val engineSpace = calculateEngineSpace(engine)
+ val responseMsgBuilder = new StringBuilder()
withDiscoveryClient(fe.getConf) { discoveryClient =>
- val engineNodes = discoveryClient.getChildren(engineSpace)
- engineNodes.foreach { node =>
- val nodePath = s"$engineSpace/$node"
+ val engineNodes = discoveryClient.getServiceNodesInfo(engineSpace, silent = true)
+ engineNodes.foreach { engineNode =>
+ val nodePath = s"$engineSpace/${engineNode.nodeName}"
+ val engineRefId = engineNode.engineRefId.orNull
info(s"Deleting engine node:$nodePath")
Review Comment:
Thanks for the explanation!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] RESTful API supports killing engine forcibly [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1710980527
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -286,24 +288,38 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
}
val engine = normalizeEngineInfo(userName, engineType, shareLevel, subdomain, "default")
val engineSpace = calculateEngineSpace(engine)
+ val responseMsgBuilder = new StringBuilder()
withDiscoveryClient(fe.getConf) { discoveryClient =>
- val engineNodes = discoveryClient.getChildren(engineSpace)
- engineNodes.foreach { node =>
- val nodePath = s"$engineSpace/$node"
+ val engineNodes = discoveryClient.getServiceNodesInfo(engineSpace, silent = true)
+ engineNodes.foreach { engineNode =>
+ val nodePath = s"$engineSpace/${engineNode.nodeName}"
+ val engineRefId = engineNode.engineRefId.orNull
info(s"Deleting engine node:$nodePath")
Review Comment:
except for CONNECTION level engine, we cannot guarantee that there are no other engines access the same `engineSpace`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1533250250
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -255,24 +259,47 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
}
val engine = normalizeEngineInfo(userName, engineType, shareLevel, subdomain, "default")
val engineSpace = calculateEngineSpace(engine)
+ var msg = s"Engine $engineSpace is deleted successfully."
withDiscoveryClient(fe.getConf) { discoveryClient =>
val engineNodes = discoveryClient.getChildren(engineSpace)
engineNodes.foreach { node =>
val nodePath = s"$engineSpace/$node"
- info(s"Deleting engine node:$nodePath")
- try {
- discoveryClient.delete(nodePath)
- } catch {
- case e: Exception =>
- error(s"Failed to delete engine node:$nodePath", e)
- throw new NotFoundException(s"Failed to delete engine node:$nodePath," +
- s"${e.getMessage}")
+ if (forceKill) {
+ val refIds = node.split(";")
+ .flatMap(_.split("=", 2) match {
+ case Array("refId", value) if StringUtils.isNotBlank(value) => Some(value)
+ case _ => None
+ })
+
+ refIds.foreach(refId => {
+ val applicationManagerInfo = ApplicationManagerInfo(
+ Option(resourceManager),
+ Option(kubernetesContext),
+ Option(kubernetesNamespace))
+ val killMessage = fe.be.sessionManager.asInstanceOf[KyuubiSessionManager]
+ .applicationManager.killApplication(applicationManagerInfo, refId)
+ if (!killMessage._1) {
+ msg = s"Engine $engineSpace failed to get deleted forcibly," +
+ s"cause ${killMessage._2}"
+ error(msg)
+ throw new NotFoundException(msg)
+ }
+ })
+ } else {
+ info(s"Deleting engine node:$nodePath")
+ try {
+ discoveryClient.delete(nodePath)
+ } catch {
+ case e: Exception =>
+ error(s"Failed to delete engine node:$nodePath", e)
+ throw new NotFoundException(s"Failed to delete engine node:$nodePath," +
+ s"${e.getMessage}")
Review Comment:
FYI:
```
....
withDiscoveryClient(fe.getConf) { discoveryClient =>
val engineNodes = discoveryClient.getServiceNodesInfo(engineSpace, silent = true)
engineNodes.foreach { engineNode =>
val nodePath = s"$engineSpace/${engineNode.nodeName}"
info(s"Deleting engine node:$nodePath")
try {
discoveryClient.delete(nodePath)
} catch {
case e: Exception =>
error(s"Failed to delete engine node:$nodePath", e)
throw new NotFoundException(s"Failed to delete engine node:$nodePath," +
s"${e.getMessage}")
}
if (forceKill) {
val applicationManagerInfo = ApplicationManagerInfo(
Option(resourceManager),
Option(kubernetesContext),
Option(kubernetesNamespace))
engineNode.engineRefId.foreach { engineRefId =>
fe.be.sessionManager.asInstanceOf[KyuubiSessionManager]
.applicationManager.killApplication(applicationManagerInfo, engineRefId)
.....
}
}
}
}
....
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "zhaohehuhu (via GitHub)" <gi...@apache.org>.
zhaohehuhu commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1494163863
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -255,24 +260,44 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
}
val engine = normalizeEngineInfo(userName, engineType, shareLevel, subdomain, "default")
val engineSpace = calculateEngineSpace(engine)
+ var msg = s"Engine $engineSpace is deleted successfully."
+ if (forceKill) {
+ if (StringUtils.isBlank(refId)) {
+ error(s"Invalid refId: $refId")
+ throw new NotFoundException(s"Invalid refId: $refId")
+ }
- withDiscoveryClient(fe.getConf) { discoveryClient =>
- val engineNodes = discoveryClient.getChildren(engineSpace)
- engineNodes.foreach { node =>
- val nodePath = s"$engineSpace/$node"
- info(s"Deleting engine node:$nodePath")
- try {
- discoveryClient.delete(nodePath)
- } catch {
- case e: Exception =>
- error(s"Failed to delete engine node:$nodePath", e)
- throw new NotFoundException(s"Failed to delete engine node:$nodePath," +
- s"${e.getMessage}")
+ val applicationManagerInfo = ApplicationManagerInfo(
+ Option(sparkMaster),
+ Option(sparK8Context),
+ Option(sparK8Namespace))
+
+ var killMessage: KillResponse = (false, "UNKNOWN")
+ killMessage = fe.be.sessionManager.asInstanceOf[KyuubiSessionManager]
+ .applicationManager.killApplication(applicationManagerInfo, refId)
+ if (!killMessage._1) {
+ msg = s"Engine $engineSpace failed to get deleted forcibly," +
+ s"cause ${killMessage._2}"
+ error(msg)
Review Comment:
done. Thanks！ @wForget
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "zhaohehuhu (via GitHub)" <gi...@apache.org>.
zhaohehuhu commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1496832608
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -255,24 +260,44 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
}
val engine = normalizeEngineInfo(userName, engineType, shareLevel, subdomain, "default")
val engineSpace = calculateEngineSpace(engine)
+ var msg = s"Engine $engineSpace is deleted successfully."
+ if (forceKill) {
+ if (StringUtils.isBlank(refId)) {
+ throw new IllegalArgumentException(s"Invalid refId: $refId")
+ }
- withDiscoveryClient(fe.getConf) { discoveryClient =>
- val engineNodes = discoveryClient.getChildren(engineSpace)
- engineNodes.foreach { node =>
- val nodePath = s"$engineSpace/$node"
- info(s"Deleting engine node:$nodePath")
- try {
- discoveryClient.delete(nodePath)
- } catch {
- case e: Exception =>
- error(s"Failed to delete engine node:$nodePath", e)
- throw new NotFoundException(s"Failed to delete engine node:$nodePath," +
- s"${e.getMessage}")
+ val applicationManagerInfo = ApplicationManagerInfo(
+ Option(sparkMaster),
+ Option(sparK8Context),
+ Option(sparK8Namespace))
+
+ var killMessage: KillResponse = (false, "UNKNOWN")
+ killMessage = fe.be.sessionManager.asInstanceOf[KyuubiSessionManager]
Review Comment:
killMessage need to be assigned a new value next line. Could we keep var here ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] RESTful API supports killing engine forcibly [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#issuecomment-2186496427
Please don't forget to update the RESTful API docs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] RESTful API supports killing engine forcibly [kyuubi]
Posted by "HaoYang670 (via GitHub)" <gi...@apache.org>.
HaoYang670 commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1710962039
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -286,24 +288,38 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
}
val engine = normalizeEngineInfo(userName, engineType, shareLevel, subdomain, "default")
val engineSpace = calculateEngineSpace(engine)
+ val responseMsgBuilder = new StringBuilder()
withDiscoveryClient(fe.getConf) { discoveryClient =>
- val engineNodes = discoveryClient.getChildren(engineSpace)
- engineNodes.foreach { node =>
- val nodePath = s"$engineSpace/$node"
+ val engineNodes = discoveryClient.getServiceNodesInfo(engineSpace, silent = true)
+ engineNodes.foreach { engineNode =>
+ val nodePath = s"$engineSpace/${engineNode.nodeName}"
+ val engineRefId = engineNode.engineRefId.orNull
info(s"Deleting engine node:$nodePath")
Review Comment:
Sorry for the late comment.
the `engineSpace` is not cleaned right? Why don't we delete it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "wForget (via GitHub)" <gi...@apache.org>.
wForget commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1477917098
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -255,24 +260,44 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
}
val engine = normalizeEngineInfo(userName, engineType, shareLevel, subdomain, "default")
val engineSpace = calculateEngineSpace(engine)
+ var msg = s"Engine $engineSpace is deleted successfully."
+ if (forceKill) {
+ if (StringUtils.isBlank(refId)) {
+ error(s"Invalid refId: $refId")
+ throw new NotFoundException(s"Invalid refId: $refId")
Review Comment:
change to `IllegalArgumentException`
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -255,24 +260,44 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
}
val engine = normalizeEngineInfo(userName, engineType, shareLevel, subdomain, "default")
val engineSpace = calculateEngineSpace(engine)
+ var msg = s"Engine $engineSpace is deleted successfully."
+ if (forceKill) {
+ if (StringUtils.isBlank(refId)) {
+ error(s"Invalid refId: $refId")
+ throw new NotFoundException(s"Invalid refId: $refId")
+ }
- withDiscoveryClient(fe.getConf) { discoveryClient =>
- val engineNodes = discoveryClient.getChildren(engineSpace)
- engineNodes.foreach { node =>
- val nodePath = s"$engineSpace/$node"
- info(s"Deleting engine node:$nodePath")
- try {
- discoveryClient.delete(nodePath)
- } catch {
- case e: Exception =>
- error(s"Failed to delete engine node:$nodePath", e)
- throw new NotFoundException(s"Failed to delete engine node:$nodePath," +
- s"${e.getMessage}")
+ val applicationManagerInfo = ApplicationManagerInfo(
+ Option(sparkMaster),
+ Option(sparK8Context),
+ Option(sparK8Namespace))
+
+ var killMessage: KillResponse = (false, "UNKNOWN")
+ killMessage = fe.be.sessionManager.asInstanceOf[KyuubiSessionManager]
+ .applicationManager.killApplication(applicationManagerInfo, refId)
+ if (!killMessage._1) {
+ msg = s"Engine $engineSpace failed to get deleted forcibly," +
+ s"cause ${killMessage._2}"
+ error(msg)
Review Comment:
Do we need to throw an exception if kill fails?
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -255,24 +260,44 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
}
val engine = normalizeEngineInfo(userName, engineType, shareLevel, subdomain, "default")
val engineSpace = calculateEngineSpace(engine)
+ var msg = s"Engine $engineSpace is deleted successfully."
+ if (forceKill) {
+ if (StringUtils.isBlank(refId)) {
+ error(s"Invalid refId: $refId")
Review Comment:
remove this log
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] RESTful API supports killing engine forcibly [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#issuecomment-2186494924
the failed UTs are related, because this PR appends a new configuration to the engine launch command, they should be easy to debug.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "zhaohehuhu (via GitHub)" <gi...@apache.org>.
zhaohehuhu commented on PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#issuecomment-2014731125
Thanks all for reviewing this pr@wForget @turboFei
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] RESTful API supports killing engine forcibly [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1711064538
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -286,24 +288,38 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
}
val engine = normalizeEngineInfo(userName, engineType, shareLevel, subdomain, "default")
val engineSpace = calculateEngineSpace(engine)
+ val responseMsgBuilder = new StringBuilder()
withDiscoveryClient(fe.getConf) { discoveryClient =>
- val engineNodes = discoveryClient.getChildren(engineSpace)
- engineNodes.foreach { node =>
- val nodePath = s"$engineSpace/$node"
+ val engineNodes = discoveryClient.getServiceNodesInfo(engineSpace, silent = true)
+ engineNodes.foreach { engineNode =>
+ val nodePath = s"$engineSpace/${engineNode.nodeName}"
+ val engineRefId = engineNode.engineRefId.orNull
info(s"Deleting engine node:$nodePath")
Review Comment:
there are some corner cases. for example, if the engine has no response due to full GC or overload, kyuubi will create a new one on the same engine space. I also see sometimes the dist lock does not work properly(which should not, may be bugs), that also causes multi engines live in the same subdomain. finally, in theory, there are race conditions for kyuubi server deleting and engine registering under the same subdomain.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1525746783
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -246,7 +246,12 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
@QueryParam("sharelevel") shareLevel: String,
@QueryParam("subdomain") subdomain: String,
@QueryParam("proxyUser") kyuubiProxyUser: String,
- @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String,
+ @QueryParam("forceKill") @DefaultValue("false") forceKill: Boolean,
+ @QueryParam("refId") refId: String,
Review Comment:
otherwise, Kyuubiserver can not check whether you have permission to kill the engine.
You can specify any refId to kill, it does not make senses.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "wForget (via GitHub)" <gi...@apache.org>.
wForget commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1466239762
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -255,7 +261,7 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
}
val engine = normalizeEngineInfo(userName, engineType, shareLevel, subdomain, "default")
val engineSpace = calculateEngineSpace(engine)
-
+ var msg = s"Engine $engineSpace is deleted successfully."
withDiscoveryClient(fe.getConf) { discoveryClient =>
Review Comment:
kill gracefully only when forceKill is false.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "wForget (via GitHub)" <gi...@apache.org>.
wForget commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1496867993
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -255,24 +260,44 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
}
val engine = normalizeEngineInfo(userName, engineType, shareLevel, subdomain, "default")
val engineSpace = calculateEngineSpace(engine)
+ var msg = s"Engine $engineSpace is deleted successfully."
+ if (forceKill) {
+ if (StringUtils.isBlank(refId)) {
+ throw new IllegalArgumentException(s"Invalid refId: $refId")
+ }
- withDiscoveryClient(fe.getConf) { discoveryClient =>
- val engineNodes = discoveryClient.getChildren(engineSpace)
- engineNodes.foreach { node =>
- val nodePath = s"$engineSpace/$node"
- info(s"Deleting engine node:$nodePath")
- try {
- discoveryClient.delete(nodePath)
- } catch {
- case e: Exception =>
- error(s"Failed to delete engine node:$nodePath", e)
- throw new NotFoundException(s"Failed to delete engine node:$nodePath," +
- s"${e.getMessage}")
+ val applicationManagerInfo = ApplicationManagerInfo(
+ Option(sparkMaster),
+ Option(sparK8Context),
+ Option(sparK8Namespace))
+
+ var killMessage: KillResponse = (false, "UNKNOWN")
+ killMessage = fe.be.sessionManager.asInstanceOf[KyuubiSessionManager]
Review Comment:
> killMessage need to be assigned a new value next line. Could we keep var here ?
which line?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "zhaohehuhu (via GitHub)" <gi...@apache.org>.
zhaohehuhu commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1533369224
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -255,24 +259,47 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
}
val engine = normalizeEngineInfo(userName, engineType, shareLevel, subdomain, "default")
val engineSpace = calculateEngineSpace(engine)
+ var msg = s"Engine $engineSpace is deleted successfully."
withDiscoveryClient(fe.getConf) { discoveryClient =>
val engineNodes = discoveryClient.getChildren(engineSpace)
engineNodes.foreach { node =>
val nodePath = s"$engineSpace/$node"
- info(s"Deleting engine node:$nodePath")
- try {
- discoveryClient.delete(nodePath)
- } catch {
- case e: Exception =>
- error(s"Failed to delete engine node:$nodePath", e)
- throw new NotFoundException(s"Failed to delete engine node:$nodePath," +
- s"${e.getMessage}")
+ if (forceKill) {
+ val refIds = node.split(";")
+ .flatMap(_.split("=", 2) match {
+ case Array("refId", value) if StringUtils.isNotBlank(value) => Some(value)
+ case _ => None
+ })
+
+ refIds.foreach(refId => {
+ val applicationManagerInfo = ApplicationManagerInfo(
+ Option(resourceManager),
+ Option(kubernetesContext),
+ Option(kubernetesNamespace))
+ val killMessage = fe.be.sessionManager.asInstanceOf[KyuubiSessionManager]
+ .applicationManager.killApplication(applicationManagerInfo, refId)
+ if (!killMessage._1) {
+ msg = s"Engine $engineSpace failed to get deleted forcibly," +
+ s"cause ${killMessage._2}"
+ error(msg)
+ throw new NotFoundException(msg)
+ }
+ })
+ } else {
+ info(s"Deleting engine node:$nodePath")
+ try {
+ discoveryClient.delete(nodePath)
+ } catch {
+ case e: Exception =>
+ error(s"Failed to delete engine node:$nodePath", e)
+ throw new NotFoundException(s"Failed to delete engine node:$nodePath," +
+ s"${e.getMessage}")
Review Comment:
Great ! So we have to delete the path and then kill the application.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "zhaohehuhu (via GitHub)" <gi...@apache.org>.
zhaohehuhu commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1533369224
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -255,24 +259,47 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
}
val engine = normalizeEngineInfo(userName, engineType, shareLevel, subdomain, "default")
val engineSpace = calculateEngineSpace(engine)
+ var msg = s"Engine $engineSpace is deleted successfully."
withDiscoveryClient(fe.getConf) { discoveryClient =>
val engineNodes = discoveryClient.getChildren(engineSpace)
engineNodes.foreach { node =>
val nodePath = s"$engineSpace/$node"
- info(s"Deleting engine node:$nodePath")
- try {
- discoveryClient.delete(nodePath)
- } catch {
- case e: Exception =>
- error(s"Failed to delete engine node:$nodePath", e)
- throw new NotFoundException(s"Failed to delete engine node:$nodePath," +
- s"${e.getMessage}")
+ if (forceKill) {
+ val refIds = node.split(";")
+ .flatMap(_.split("=", 2) match {
+ case Array("refId", value) if StringUtils.isNotBlank(value) => Some(value)
+ case _ => None
+ })
+
+ refIds.foreach(refId => {
+ val applicationManagerInfo = ApplicationManagerInfo(
+ Option(resourceManager),
+ Option(kubernetesContext),
+ Option(kubernetesNamespace))
+ val killMessage = fe.be.sessionManager.asInstanceOf[KyuubiSessionManager]
+ .applicationManager.killApplication(applicationManagerInfo, refId)
+ if (!killMessage._1) {
+ msg = s"Engine $engineSpace failed to get deleted forcibly," +
+ s"cause ${killMessage._2}"
+ error(msg)
+ throw new NotFoundException(msg)
+ }
+ })
+ } else {
+ info(s"Deleting engine node:$nodePath")
+ try {
+ discoveryClient.delete(nodePath)
+ } catch {
+ case e: Exception =>
+ error(s"Failed to delete engine node:$nodePath", e)
+ throw new NotFoundException(s"Failed to delete engine node:$nodePath," +
+ s"${e.getMessage}")
Review Comment:
Great !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "zhaohehuhu (via GitHub)" <gi...@apache.org>.
zhaohehuhu commented on PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#issuecomment-2008582437
Could you plz review it again ? @turboFei
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] RESTful API supports killing engine forcibly [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#issuecomment-2065659264
a little bit concern about the design, which exposed too many details to end users, let me think this weekend
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] RESTful API supports killing engine forcibly [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#issuecomment-2094912825
How about register the app mgr into to discovery service?
https://github.com/apache/kyuubi/pull/6360/commits/4cf2852ebb2343a47c7e5a182a58832076c0255b
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "zhaohehuhu (via GitHub)" <gi...@apache.org>.
zhaohehuhu commented on PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#issuecomment-1998903502
We're using Spark engine in most of time, so we would like a way to kill Spark engine forcibly thus fail to take other engines into account. Maybe I can make some improvements to support multiple engines by passing the ApplicationManagerInfo instead of Spark Info.
@turboFei
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Support killing engine forcibly [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #6008:
URL: https://github.com/apache/kyuubi/pull/6008#discussion_r1525744218
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -246,7 +246,12 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
@QueryParam("sharelevel") shareLevel: String,
@QueryParam("subdomain") subdomain: String,
@QueryParam("proxyUser") kyuubiProxyUser: String,
- @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String,
+ @QueryParam("forceKill") @DefaultValue("false") forceKill: Boolean,
+ @QueryParam("refId") refId: String,
Review Comment:
refId is not needed, we can get it with user and specified `sharelevel`, `subdomain`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org