You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kyuubi.apache.org by GitBox <gi...@apache.org> on 2021/08/05 07:14:34 UTC

[GitHub] [incubator-kyuubi] ulysses-you opened a new pull request #899: Avoid RejectedExecutionException if engine is shutdown

ulysses-you opened a new pull request #899:
URL: https://github.com/apache/incubator-kyuubi/pull/899


   <!--
   Thanks for sending a pull request!
   
   Here are some tips for you:
     1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/contributions.html
     2. If the PR is related to an issue in https://github.com/apache/incubator-kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'.
     3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'.
   -->
   
   ### _Why are the changes needed?_
   <!--
   Please clarify why the changes are needed. For instance,
     1. If you add a feature, you can talk about the use case of it.
     2. If you fix a bug, you can clarify why it is a bug.
   -->
   
   Engine can be shutdown but not stopped when an operation is submitted , then we might get such exception:
   
   ```
   2021-08-05 14:19:33.787 WARN service.FrontendService: Error executing statement:
   org.apache.kyuubi.KyuubiSQLException: Error submitting query in background, query rejected
   	at org.apache.kyuubi.KyuubiSQLException$.apply(KyuubiSQLException.scala:68)
   	at org.apache.kyuubi.engine.spark.operation.ExecuteStatement.runInternal(ExecuteStatement.scala:124)
   	at org.apache.kyuubi.operation.AbstractOperation.run(AbstractOperation.scala:130)
   	at org.apache.kyuubi.session.AbstractSession.runOperation(AbstractSession.scala:95)
   	at org.apache.kyuubi.session.AbstractSession.$anonfun$executeStatement$1(AbstractSession.scala:123)
   	at org.apache.kyuubi.session.AbstractSession.withAcquireRelease(AbstractSession.scala:78)
   	at org.apache.kyuubi.session.AbstractSession.executeStatement(AbstractSession.scala:120)
   	at org.apache.kyuubi.service.AbstractBackendService.executeStatement(AbstractBackendService.scala:61)
   	at org.apache.kyuubi.service.FrontendService.ExecuteStatement(FrontendService.scala:256)
   	at org.apache.hive.service.rpc.thrift.TCLIService$Processor$ExecuteStatement.getResult(TCLIService.java:1557)
   	at org.apache.hive.service.rpc.thrift.TCLIService$Processor$ExecuteStatement.getResult(TCLIService.java:1542)
   	at org.apache.thrift.ProcessFunction.process(ProcessFunction.java:38)
   	at org.apache.thrift.TBaseProcessor.process(TBaseProcessor.java:39)
   	at org.apache.kyuubi.service.authentication.TSetIpAddressProcessor.process(TSetIpAddressProcessor.scala:36)
   	at org.apache.thrift.server.TThreadPoolServer$WorkerProcess.run(TThreadPoolServer.java:310)
   	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
   	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
   	at java.lang.Thread.run(Thread.java:748)
   Caused by: java.util.concurrent.RejectedExecutionException: Task java.util.concurrent.FutureTask@1202ad6e rejected from java.util.concurrent.ThreadPoolExecutor@a50ca2b[Terminated, pool size = 0, active threads = 0, queued tasks = 0, completed tasks = 0]
   
   ```
   
   It's noisy to print this excption.
   
   ### _How was this patch tested?_
   Pass CI


-- 
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: commits-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] ulysses-you commented on a change in pull request #899: Avoid RejectedExecutionException if engine is shutdown

Posted by GitBox <gi...@apache.org>.
ulysses-you commented on a change in pull request #899:
URL: https://github.com/apache/incubator-kyuubi/pull/899#discussion_r808795264



##########
File path: kyuubi-common/src/main/scala/org/apache/kyuubi/session/SessionManager.scala
##########
@@ -198,6 +198,8 @@ abstract class SessionManager(name: String) extends CompositeService(name) {
     }
   }
 
+  def isShutdown: Boolean = shutdown

Review comment:
       `shutdown` is a private mutable field, it should be good to add a method 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: commits-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] yaooqinn commented on a change in pull request #899: Avoid RejectedExecutionException if engine is shutdown

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #899:
URL: https://github.com/apache/incubator-kyuubi/pull/899#discussion_r809730155



##########
File path: externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/operation/ExecuteStatement.scala
##########
@@ -112,17 +112,19 @@ class ExecuteStatement(
         }
       }
 
+      val sparkSQLSessionManager = session.sessionManager
       try {
-        val sparkSQLSessionManager = session.sessionManager
         val backgroundHandle = sparkSQLSessionManager.submitBackgroundOperation(asyncOperation)
         setBackgroundHandle(backgroundHandle)
       } catch {
-        case rejected: RejectedExecutionException =>
+        case rejected: RejectedExecutionException if !sparkSQLSessionManager.isShutdown =>
           setState(OperationState.ERROR)
           val ke = KyuubiSQLException("Error submitting query in background, query rejected",

Review comment:
       how about we refine the error message to show that the engine is in shutting down phase.




-- 
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: commits-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] yaooqinn edited a comment on pull request #899: Avoid RejectedExecutionException if engine is shutdown

Posted by GitBox <gi...@apache.org>.
yaooqinn edited a comment on pull request #899:
URL: https://github.com/apache/incubator-kyuubi/pull/899#issuecomment-1044043763


   I am -1 with this.
   
   Instead, we still need to report the operation status as ERROR even the engine is shutting down. At least, it can stop sending requests.
   
   But, we can improve the error for this case.


-- 
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: commits-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] SteNicholas commented on a change in pull request #899: Avoid RejectedExecutionException if engine is shutdown

Posted by GitBox <gi...@apache.org>.
SteNicholas commented on a change in pull request #899:
URL: https://github.com/apache/incubator-kyuubi/pull/899#discussion_r808620379



##########
File path: kyuubi-common/src/main/scala/org/apache/kyuubi/session/SessionManager.scala
##########
@@ -198,6 +198,8 @@ abstract class SessionManager(name: String) extends CompositeService(name) {
     }
   }
 
+  def isShutdown: Boolean = shutdown

Review comment:
       IMO, here is unnecessary. You could directly use the `shutdown` to check.

##########
File path: kyuubi-common/src/main/scala/org/apache/kyuubi/session/SessionManager.scala
##########
@@ -198,6 +198,8 @@ abstract class SessionManager(name: String) extends CompositeService(name) {
     }
   }
 
+  def isShutdown: Boolean = shutdown

Review comment:
       IMO, here is unnecessary. You could directly use the `shutdown` to check.
   cc @pan3793.




-- 
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: commits-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #899: Avoid RejectedExecutionException if engine is shutdown

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #899:
URL: https://github.com/apache/incubator-kyuubi/pull/899#issuecomment-893236002


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?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 [#899](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (57a8841) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/5602c29b3694b3d2fdec6ab8334687180d9135d9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5602c29) will **decrease** coverage by `0.09%`.
   > The diff coverage is `25.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-kyuubi/pull/899/graphs/tree.svg?width=650&height=150&src=pr&token=925D4tb9AH&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master     #899      +/-   ##
   ============================================
   - Coverage     79.24%   79.15%   -0.10%     
     Complexity       90       90              
   ============================================
     Files           177      177              
     Lines          6620     6624       +4     
     Branches        783      784       +1     
   ============================================
   - Hits           5246     5243       -3     
   - Misses          919      927       +8     
   + Partials        455      454       -1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ala/org/apache/kyuubi/session/SessionManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/899/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL1Nlc3Npb25NYW5hZ2VyLnNjYWxh) | `52.42% <0.00%> (-0.52%)` | :arrow_down: |
   | [...uubi/engine/spark/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/899/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-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9vcGVyYXRpb24vRXhlY3V0ZVN0YXRlbWVudC5zY2FsYQ==) | `85.71% <33.33%> (-3.18%)` | :arrow_down: |
   | [.../org/apache/kyuubi/operation/KyuubiOperation.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/899/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vS3l1dWJpT3BlcmF0aW9uLnNjYWxh) | `50.00% <0.00%> (-5.36%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [5602c29...57a8841](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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: commits-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] timothy65535 commented on pull request #899: Avoid RejectedExecutionException if engine is shutdown

Posted by GitBox <gi...@apache.org>.
timothy65535 commented on pull request #899:
URL: https://github.com/apache/incubator-kyuubi/pull/899#issuecomment-899473602


   hi @ulysses-you
   
   > Engine can be shutdown but not stopped 
   
   How can I encounter this situation? The `shutdown` variable will be set to true only when the service is stopped in the codebase.


-- 
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: commits-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] yaooqinn commented on pull request #899: Avoid RejectedExecutionException if engine is shutdown

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #899:
URL: https://github.com/apache/incubator-kyuubi/pull/899#issuecomment-1044043763


   I am -1 with this.
   
   Instead, we still need to report the operation status as ERROR even the engine is shutting down.
   
   But, we can improve the error for this case.


-- 
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: commits-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] codecov-commenter commented on pull request #899: Avoid RejectedExecutionException if engine is shutdown

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #899:
URL: https://github.com/apache/incubator-kyuubi/pull/899#issuecomment-893236002


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?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 [#899](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1dcf0a8) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/34925fb3ebfb39173f7d12071a193e090ce4c557?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (34925fb) will **decrease** coverage by `0.02%`.
   > The diff coverage is `25.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-kyuubi/pull/899/graphs/tree.svg?width=650&height=150&src=pr&token=925D4tb9AH&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master     #899      +/-   ##
   ============================================
   - Coverage     79.16%   79.13%   -0.03%     
     Complexity       11       11              
   ============================================
     Files           143      143              
     Lines          5346     5348       +2     
     Branches        649      650       +1     
   ============================================
     Hits           4232     4232              
   - Misses          751      753       +2     
     Partials        363      363              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ala/org/apache/kyuubi/session/SessionManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/899/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL1Nlc3Npb25NYW5hZ2VyLnNjYWxh) | `52.42% <0.00%> (-0.52%)` | :arrow_down: |
   | [...uubi/engine/spark/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/899/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-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9vcGVyYXRpb24vRXhlY3V0ZVN0YXRlbWVudC5zY2FsYQ==) | `87.34% <28.57%> (-1.12%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [34925fb...1dcf0a8](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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: commits-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #899: Avoid RejectedExecutionException if engine is shutdown

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #899:
URL: https://github.com/apache/incubator-kyuubi/pull/899#issuecomment-893236002


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?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 [#899](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1dcf0a8) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/5602c29b3694b3d2fdec6ab8334687180d9135d9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5602c29) will **decrease** coverage by `0.11%`.
   > The diff coverage is `25.00%`.
   
   > :exclamation: Current head 1dcf0a8 differs from pull request most recent head 57a8841. Consider uploading reports for the commit 57a8841 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-kyuubi/pull/899/graphs/tree.svg?width=650&height=150&src=pr&token=925D4tb9AH&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master     #899      +/-   ##
   ============================================
   - Coverage     79.24%   79.13%   -0.12%     
   + Complexity       90       11      -79     
   ============================================
     Files           177      143      -34     
     Lines          6620     5348    -1272     
     Branches        783      650     -133     
   ============================================
   - Hits           5246     4232    -1014     
   + Misses          919      753     -166     
   + Partials        455      363      -92     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ala/org/apache/kyuubi/session/SessionManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/899/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL1Nlc3Npb25NYW5hZ2VyLnNjYWxh) | `52.42% <0.00%> (-0.52%)` | :arrow_down: |
   | [...uubi/engine/spark/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/899/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-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9vcGVyYXRpb24vRXhlY3V0ZVN0YXRlbWVudC5zY2FsYQ==) | `87.34% <28.57%> (-1.55%)` | :arrow_down: |
   | [...scala/org/apache/kyuubi/operation/GetColumns.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/899/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vR2V0Q29sdW1ucy5zY2FsYQ==) | `0.00% <0.00%> (-88.89%)` | :arrow_down: |
   | [...a/org/apache/kyuubi/operation/OperationState.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/899/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vT3BlcmF0aW9uU3RhdGUuc2NhbGE=) | `27.27% <0.00%> (-27.02%)` | :arrow_down: |
   | [...he/kyuubi/engine/spark/shim/CatalogShim\_v3\_0.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/899/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-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9zaGltL0NhdGFsb2dTaGltX3YzXzAuc2NhbGE=) | `61.84% <0.00%> (-26.32%)` | :arrow_down: |
   | [.../apache/kyuubi/client/KyuubiSyncThriftClient.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/899/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jbGllbnQvS3l1dWJpU3luY1RocmlmdENsaWVudC5zY2FsYQ==) | `72.72% <0.00%> (-10.26%)` | :arrow_down: |
   | [.../scala/org/apache/kyuubi/server/KyuubiServer.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/899/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvS3l1dWJpU2VydmVyLnNjYWxh) | `50.00% <0.00%> (-8.70%)` | :arrow_down: |
   | [...n/scala/org/apache/kyuubi/KyuubiSQLException.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/899/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9LeXV1YmlTUUxFeGNlcHRpb24uc2NhbGE=) | `86.36% <0.00%> (-5.21%)` | :arrow_down: |
   | [...ache/kyuubi/operation/KyuubiOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/899/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vS3l1dWJpT3BlcmF0aW9uTWFuYWdlci5zY2FsYQ==) | `60.00% <0.00%> (-4.16%)` | :arrow_down: |
   | [...org/apache/kyuubi/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/899/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vRXhlY3V0ZVN0YXRlbWVudC5zY2FsYQ==) | `78.94% <0.00%> (-3.85%)` | :arrow_down: |
   | ... and [85 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/899/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [5602c29...57a8841](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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: commits-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] timothy65535 edited a comment on pull request #899: Avoid RejectedExecutionException if engine is shutdown

Posted by GitBox <gi...@apache.org>.
timothy65535 edited a comment on pull request #899:
URL: https://github.com/apache/incubator-kyuubi/pull/899#issuecomment-899473602


   hi @ulysses-you
   
   > Engine can be shutdown but not stopped 
   
   How to encounter this situation? The `shutdown` variable will be set to true only when the service is stopped in the codebase.


-- 
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: commits-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] codecov-commenter commented on pull request #899: Avoid RejectedExecutionException if engine is shutdown

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #899:
URL: https://github.com/apache/incubator-kyuubi/pull/899#issuecomment-893236002


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?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 [#899](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1dcf0a8) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/34925fb3ebfb39173f7d12071a193e090ce4c557?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (34925fb) will **decrease** coverage by `0.02%`.
   > The diff coverage is `25.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-kyuubi/pull/899/graphs/tree.svg?width=650&height=150&src=pr&token=925D4tb9AH&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master     #899      +/-   ##
   ============================================
   - Coverage     79.16%   79.13%   -0.03%     
     Complexity       11       11              
   ============================================
     Files           143      143              
     Lines          5346     5348       +2     
     Branches        649      650       +1     
   ============================================
     Hits           4232     4232              
   - Misses          751      753       +2     
     Partials        363      363              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ala/org/apache/kyuubi/session/SessionManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/899/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL1Nlc3Npb25NYW5hZ2VyLnNjYWxh) | `52.42% <0.00%> (-0.52%)` | :arrow_down: |
   | [...uubi/engine/spark/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/899/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-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9vcGVyYXRpb24vRXhlY3V0ZVN0YXRlbWVudC5zY2FsYQ==) | `87.34% <28.57%> (-1.12%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [34925fb...1dcf0a8](https://codecov.io/gh/apache/incubator-kyuubi/pull/899?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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: commits-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-kyuubi] pan3793 commented on a change in pull request #899: Avoid RejectedExecutionException if engine is shutdown

Posted by GitBox <gi...@apache.org>.
pan3793 commented on a change in pull request #899:
URL: https://github.com/apache/incubator-kyuubi/pull/899#discussion_r710651036



##########
File path: externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/operation/ExecuteStatement.scala
##########
@@ -114,17 +114,21 @@ class ExecuteStatement(
         }
       }
 
+      val sparkSQLSessionManager = session.sessionManager
       try {
-        val sparkSQLSessionManager = session.sessionManager
         val backgroundHandle = sparkSQLSessionManager.submitBackgroundOperation(asyncOperation)
         setBackgroundHandle(backgroundHandle)
       } catch {
         case rejected: RejectedExecutionException =>
-          setState(OperationState.ERROR)
-          val ke = KyuubiSQLException("Error submitting query in background, query rejected",
-            rejected)
-          setOperationException(ke)
-          throw ke
+          if (sparkSQLSessionManager.isShutdown) {

Review comment:
       ```
   case rejected: RejectedExecutionException if !sparkSQLSessionManager.isShutdown =>
   ```




-- 
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: commits-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org