You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by GitBox <gi...@apache.org> on 2022/11/09 10:38:45 UTC
[GitHub] [incubator-kyuubi] cxzl25 opened a new pull request, #3792: [SPARK] Engine UI support grace stop
cxzl25 opened a new pull request, #3792:
URL: https://github.com/apache/incubator-kyuubi/pull/3792
### _Why are the changes needed?_
Now the kill operation of the Spark engine ui will directly shut down, and the SQL that is being executed by the engine will fail.
We can support grace stop, first log off the engine from the znode, and then wait for the SQL execution to complete.
### _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
- [ ] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request
--
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
[GitHub] [incubator-kyuubi] cxzl25 commented on a diff in pull request #3792: [SPARK] Engine UI support grace stop
Posted by GitBox <gi...@apache.org>.
cxzl25 commented on code in PR #3792:
URL: https://github.com/apache/incubator-kyuubi/pull/3792#discussion_r1024942274
##########
externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/SparkSQLEngine.scala:
##########
@@ -83,6 +84,33 @@ case class SparkSQLEngine(spark: SparkSession) extends Serverable("SparkSQLEngin
checker,
Duration(shutdownTimeout, TimeUnit.MILLISECONDS))
})
+ stopEngineExec.foreach(exec => {
+ ThreadUtils.shutdown(
+ exec,
+ Duration(60, TimeUnit.SECONDS))
+ })
+ }
+
+ def gracefulStop(): Unit = {
+ val deregistered = new AtomicBoolean(false)
+ val stopTask: Runnable = () => {
+ if (!shutdown.get) {
+ if (deregistered.compareAndSet(false, true)) {
+ info(s"Spark engine is de-registering from engine discovery space.")
+ frontendServices.flatMap(_.discoveryService).foreach(_.stop())
+ }
+ val openSessionCount = backendService.sessionManager.getOpenSessionCount
+ if (openSessionCount > 0) {
+ info(s"$openSessionCount connection(s) are active, delay shutdown")
+ } else {
+ info(s"Spark engine has no open session now, terminating.")
+ stop()
Review Comment:
Yes. also remove #3774
--
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
[GitHub] [incubator-kyuubi] cxzl25 commented on a diff in pull request #3792: [SPARK] Engine UI support grace stop
Posted by GitBox <gi...@apache.org>.
cxzl25 commented on code in PR #3792:
URL: https://github.com/apache/incubator-kyuubi/pull/3792#discussion_r1030378543
##########
externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/SparkSQLEngine.scala:
##########
@@ -83,6 +84,31 @@ case class SparkSQLEngine(spark: SparkSession) extends Serverable("SparkSQLEngin
checker,
Duration(shutdownTimeout, TimeUnit.MILLISECONDS))
})
+ stopEngineExec.foreach(exec => {
+ ThreadUtils.shutdown(
+ exec,
+ Duration(60, TimeUnit.SECONDS))
+ })
+ }
+
+ def gracefulStop(): Unit = {
+ val deregistered = new AtomicBoolean(false)
+ val stopTask: Runnable = () => {
+ if (!shutdown.get) {
+ if (deregistered.compareAndSet(false, true)) {
+ info(s"Spark engine is de-registering from engine discovery space.")
+ frontendServices.flatMap(_.discoveryService).foreach(_.stop())
+ }
+
+ if (backendService.sessionManager.getOpenSessionCount <= 0) {
+ info(s"Spark engine has no open session now, terminating.")
+ stop()
+ }
+ }
+ }
+ stopEngineExec =
Review Comment:
sure
--
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
[GitHub] [incubator-kyuubi] cxzl25 commented on a diff in pull request #3792: [SPARK] Engine UI support grace stop
Posted by GitBox <gi...@apache.org>.
cxzl25 commented on code in PR #3792:
URL: https://github.com/apache/incubator-kyuubi/pull/3792#discussion_r1024946458
##########
externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/spark/ui/EngineTab.scala:
##########
@@ -107,4 +122,10 @@ case class EngineTab(
engine.get.stop()
}
}
+
+ def handleGracefulKillRequest(request: HttpServletRequest): Unit = {
+ if (killEnabled && engine.isDefined && engine.get.getServiceState != ServiceState.STOPPED) {
+ engine.get.gracefulStop()
Review Comment:
Because the service can only be stopped once, when the service is in the stop state, multiple clicks will not affect.
## Engine log
### First click
```
INFO EngineServiceDiscovery: Service[EngineServiceDiscovery] is stopped.
```
## Second click
```
WARN EngineServiceDiscovery: Service[EngineServiceDiscovery] is not started(STOPPED) yet.
```
--
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
[GitHub] [incubator-kyuubi] wForget commented on a diff in pull request #3792: [SPARK] Engine UI support grace stop
Posted by GitBox <gi...@apache.org>.
wForget commented on code in PR #3792:
URL: https://github.com/apache/incubator-kyuubi/pull/3792#discussion_r1018564254
##########
externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/spark/ui/EngineTab.scala:
##########
@@ -63,20 +63,34 @@ case class EngineTab(
sparkUI.foreach { ui =>
try {
// Spark shade the jetty package so here we use reflection
- val sparkServletContextHandlerClz = loadSparkServletContextHandler
Review Comment:
seems to revert #3630
--
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
[GitHub] [incubator-kyuubi] cxzl25 closed pull request #3792: [SPARK] Engine UI support grace stop
Posted by GitBox <gi...@apache.org>.
cxzl25 closed pull request #3792: [SPARK] Engine UI support grace stop
URL: https://github.com/apache/incubator-kyuubi/pull/3792
--
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
[GitHub] [incubator-kyuubi] pan3793 commented on pull request #3792: [SPARK] Engine UI support grace stop
Posted by GitBox <gi...@apache.org>.
pan3793 commented on PR #3792:
URL: https://github.com/apache/incubator-kyuubi/pull/3792#issuecomment-1311576913
How about "Stop Immediately" / "Stop Gracefully"?
And we can explain more when mouse hover the button, or on the confirm dialog after clicking.
--
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
[GitHub] [incubator-kyuubi] ulysses-you commented on a diff in pull request #3792: [SPARK] Engine UI support grace stop
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on code in PR #3792:
URL: https://github.com/apache/incubator-kyuubi/pull/3792#discussion_r1021109724
##########
externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/spark/ui/EngineTab.scala:
##########
@@ -107,4 +122,10 @@ case class EngineTab(
engine.get.stop()
}
}
+
+ def handleGracefulKillRequest(request: HttpServletRequest): Unit = {
+ if (killEnabled && engine.isDefined && engine.get.getServiceState != ServiceState.STOPPED) {
+ engine.get.gracefulStop()
Review Comment:
what happens if user multiple clicks `gracefulStop` ?
--
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
[GitHub] [incubator-kyuubi] cxzl25 commented on pull request #3792: [SPARK] Engine UI support grace stop
Posted by GitBox <gi...@apache.org>.
cxzl25 commented on PR #3792:
URL: https://github.com/apache/incubator-kyuubi/pull/3792#issuecomment-1308554581
## Current
<img width="303" alt="image" src="https://user-images.githubusercontent.com/3898450/200809058-59558c07-c239-425e-abed-c56827d0ddd8.png">
## Fix
<img width="288" alt="image" src="https://user-images.githubusercontent.com/3898450/200808894-a0bef092-9564-4a46-bcd6-1fea5931a4d1.png">
--
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
[GitHub] [incubator-kyuubi] ulysses-you commented on pull request #3792: [SPARK] Engine UI support grace stop
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on PR #3792:
URL: https://github.com/apache/incubator-kyuubi/pull/3792#issuecomment-1309648764
seems confused to end user, just the word for some softwares: `setup` / `auto setup`/ `one click setup`..
--
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
[GitHub] [incubator-kyuubi] codecov-commenter commented on pull request #3792: [SPARK] Engine UI support grace stop
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #3792:
URL: https://github.com/apache/incubator-kyuubi/pull/3792#issuecomment-1311208277
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/3792?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#3792](https://codecov.io/gh/apache/incubator-kyuubi/pull/3792?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6e54ddb) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/76b0c6fa53eb8716f0e8fd192d58fc971a6830d5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (76b0c6f) will **decrease** coverage by `0.04%`.
> The diff coverage is `61.53%`.
> :exclamation: Current head 6e54ddb differs from pull request most recent head 25203ef. Consider uploading reports for the commit 25203ef to get more accurate results
```diff
@@ Coverage Diff @@
## master #3792 +/- ##
============================================
- Coverage 52.85% 52.81% -0.05%
Complexity 13 13
============================================
Files 496 496
Lines 27967 28003 +36
Branches 3856 3858 +2
============================================
+ Hits 14783 14789 +6
- Misses 11786 11818 +32
+ Partials 1398 1396 -2
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/3792?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...rg/apache/kyuubi/engine/spark/SparkSQLEngine.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3792/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9TcGFya1NRTEVuZ2luZS5zY2FsYQ==) | `68.98% <10.00%> (-6.62%)` | :arrow_down: |
| [...src/main/scala/org/apache/spark/ui/EngineTab.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3792/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvc3BhcmsvdWkvRW5naW5lVGFiLnNjYWxh) | `66.66% <90.90%> (-2.09%)` | :arrow_down: |
| [...rc/main/scala/org/apache/spark/ui/EnginePage.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3792/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvc3BhcmsvdWkvRW5naW5lUGFnZS5zY2FsYQ==) | `78.65% <100.00%> (+0.31%)` | :arrow_up: |
| [.../kyuubi/server/mysql/constant/MySQLErrorCode.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3792/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvbXlzcWwvY29uc3RhbnQvTXlTUUxFcnJvckNvZGUuc2NhbGE=) | `13.84% <0.00%> (-6.16%)` | :arrow_down: |
| [...ache/kyuubi/server/mysql/MySQLCommandHandler.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3792/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvbXlzcWwvTXlTUUxDb21tYW5kSGFuZGxlci5zY2FsYQ==) | `76.84% <0.00%> (-4.22%)` | :arrow_down: |
| [...ache/kyuubi/server/mysql/MySQLGenericPackets.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3792/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvbXlzcWwvTXlTUUxHZW5lcmljUGFja2V0cy5zY2FsYQ==) | `76.59% <0.00%> (-2.13%)` | :arrow_down: |
| [...a/org/apache/kyuubi/service/TFrontendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3792/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL1RGcm9udGVuZFNlcnZpY2Uuc2NhbGE=) | `90.98% <0.00%> (-0.30%)` | :arrow_down: |
| [...g/apache/kyuubi/operation/BatchJobSubmission.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3792/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vQmF0Y2hKb2JTdWJtaXNzaW9uLnNjYWxh) | `74.64% <0.00%> (ø)` | |
| [...yuubi/server/metadata/jdbc/JDBCMetadataStore.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3792/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvbWV0YWRhdGEvamRiYy9KREJDTWV0YWRhdGFTdG9yZS5zY2FsYQ==) | `89.96% <0.00%> (+0.69%)` | :arrow_up: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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
[GitHub] [incubator-kyuubi] cxzl25 commented on pull request #3792: [SPARK] Engine UI support grace stop
Posted by GitBox <gi...@apache.org>.
cxzl25 commented on PR #3792:
URL: https://github.com/apache/incubator-kyuubi/pull/3792#issuecomment-1326092539
Thanks, merged to master.
--
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
[GitHub] [incubator-kyuubi] ulysses-you commented on a diff in pull request #3792: [SPARK] Engine UI support grace stop
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on code in PR #3792:
URL: https://github.com/apache/incubator-kyuubi/pull/3792#discussion_r1021114248
##########
externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/SparkSQLEngine.scala:
##########
@@ -83,6 +84,33 @@ case class SparkSQLEngine(spark: SparkSession) extends Serverable("SparkSQLEngin
checker,
Duration(shutdownTimeout, TimeUnit.MILLISECONDS))
})
+ stopEngineExec.foreach(exec => {
+ ThreadUtils.shutdown(
+ exec,
+ Duration(60, TimeUnit.SECONDS))
+ })
+ }
+
+ def gracefulStop(): Unit = {
+ val deregistered = new AtomicBoolean(false)
+ val stopTask: Runnable = () => {
+ if (!shutdown.get) {
+ if (deregistered.compareAndSet(false, true)) {
+ info(s"Spark engine is de-registering from engine discovery space.")
+ frontendServices.flatMap(_.discoveryService).foreach(_.stop())
+ }
+ val openSessionCount = backendService.sessionManager.getOpenSessionCount
+ if (openSessionCount > 0) {
+ info(s"$openSessionCount connection(s) are active, delay shutdown")
+ } else {
+ info(s"Spark engine has no open session now, terminating.")
+ stop()
Review Comment:
seems `DeRegisterWatcher` will do this, `frontendServices.flatMap(_.discoveryService).foreach(_.stop())` is enough ?
--
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
[GitHub] [incubator-kyuubi] ulysses-you commented on a diff in pull request #3792: [SPARK] Engine UI support grace stop
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on code in PR #3792:
URL: https://github.com/apache/incubator-kyuubi/pull/3792#discussion_r1025077751
##########
externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/SparkSQLEngine.scala:
##########
@@ -83,6 +84,31 @@ case class SparkSQLEngine(spark: SparkSession) extends Serverable("SparkSQLEngin
checker,
Duration(shutdownTimeout, TimeUnit.MILLISECONDS))
})
+ stopEngineExec.foreach(exec => {
+ ThreadUtils.shutdown(
+ exec,
+ Duration(60, TimeUnit.SECONDS))
+ })
+ }
+
+ def gracefulStop(): Unit = {
+ val deregistered = new AtomicBoolean(false)
+ val stopTask: Runnable = () => {
+ if (!shutdown.get) {
+ if (deregistered.compareAndSet(false, true)) {
+ info(s"Spark engine is de-registering from engine discovery space.")
+ frontendServices.flatMap(_.discoveryService).foreach(_.stop())
+ }
+
+ if (backendService.sessionManager.getOpenSessionCount <= 0) {
+ info(s"Spark engine has no open session now, terminating.")
+ stop()
+ }
+ }
+ }
+ stopEngineExec =
Review Comment:
shall we make `deregistered ` as a global variable and check `deregistered` is true here to avoid user multi-clicks ?
--
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
[GitHub] [incubator-kyuubi] ulysses-you commented on a diff in pull request #3792: [SPARK] Engine UI support grace stop
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on code in PR #3792:
URL: https://github.com/apache/incubator-kyuubi/pull/3792#discussion_r1025077751
##########
externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/SparkSQLEngine.scala:
##########
@@ -83,6 +84,31 @@ case class SparkSQLEngine(spark: SparkSession) extends Serverable("SparkSQLEngin
checker,
Duration(shutdownTimeout, TimeUnit.MILLISECONDS))
})
+ stopEngineExec.foreach(exec => {
+ ThreadUtils.shutdown(
+ exec,
+ Duration(60, TimeUnit.SECONDS))
+ })
+ }
+
+ def gracefulStop(): Unit = {
+ val deregistered = new AtomicBoolean(false)
+ val stopTask: Runnable = () => {
+ if (!shutdown.get) {
+ if (deregistered.compareAndSet(false, true)) {
+ info(s"Spark engine is de-registering from engine discovery space.")
+ frontendServices.flatMap(_.discoveryService).foreach(_.stop())
+ }
+
+ if (backendService.sessionManager.getOpenSessionCount <= 0) {
+ info(s"Spark engine has no open session now, terminating.")
+ stop()
+ }
+ }
+ }
+ stopEngineExec =
Review Comment:
shall we check `deregistered` is true here to avoid user multi-clicks ?
--
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