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 2022/01/19 09:11:40 UTC

[GitHub] [incubator-kyuubi] cfmcgrady opened a new pull request #1807: [KYUUBI #1806] Fix LaunchEngine may be closed multiple times

cfmcgrady opened a new pull request #1807:
URL: https://github.com/apache/incubator-kyuubi/pull/1807


   <!--
   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.
   -->
   
   close #1806
   
   ### _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.readthedocs.io/en/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: commits-unsubscribe@kyuubi.apache.org

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

[GitHub] [incubator-kyuubi] turboFei commented on pull request #1807: [KYUUBI #1806] Fix LaunchEngine may be closed multiple times

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


   > indeed, we don't need to add it into OperationManger as it's a member of the session.
   because we need fetch the launch engine log in client side, so it seems that we have to add it into operationManager.
   
   > do we call CloseOperation separately at the client-side now?
   not.


-- 
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 #1807: [KYUUBI #1806] Fix LaunchEngine may be closed multiple times

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


   indeed, we don't need to add it into OperationManger as it's a member of the session. 


-- 
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 #1807: [KYUUBI #1806] Fix LaunchEngine may be closed multiple times

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


   > It seems we should not remove the `LaunchEngine` until session close.
   
   not true


-- 
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 #1807: [KYUUBI #1806] Fix LaunchEngine may be closed multiple times

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


   > > indeed, we don't need to add it into OperationManger as it's a member of the session.
   > 
   > because we need fetch the launch engine log in client side, so it seems that we have to add it into operationManager.
   > 
   > > do we call CloseOperation separately at the client-side now?
   > 
   > not now.
   
   I see


-- 
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] turboFei edited a comment on pull request #1807: [KYUUBI #1806] Fix LaunchEngine may be closed multiple times

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


   > indeed, we don't need to add it into OperationManger as it's a member of the session.
   because we need fetch the launch engine log in client side, so it seems that we have to add it into operationManager.
   
   > do we call CloseOperation separately at the client-side now?
   not now.


-- 
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 #1807: [KYUUBI #1806] Fix LaunchEngine may be closed multiple times

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


   indeed, we don't need to add it into OperationManger as it's a member of the session. cc @turboFei， do we call `CloseOperation` separately at the client-side now?


-- 
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 pull request #1807: [KYUUBI #1806] Fix LaunchEngine may be closed multiple times

Posted by GitBox <gi...@apache.org>.
ulysses-you commented on pull request #1807:
URL: https://github.com/apache/incubator-kyuubi/pull/1807#issuecomment-1016245395


   It seems we should not remove the `LaunchEngine` until session close.
   
   Can we override the `isTimedOut` of `LaunchEngine` and mark it never timeout ?


-- 
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 #1807: [KYUUBI #1806] Fix LaunchEngine may be closed multiple times

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


   indeed, we don't need to add it into OperationManger as it's a member of the session. cc @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: commits-unsubscribe@kyuubi.apache.org

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

[GitHub] [incubator-kyuubi] yaooqinn closed pull request #1807: [KYUUBI #1806] Fix LaunchEngine may be closed multiple times

Posted by GitBox <gi...@apache.org>.
yaooqinn closed pull request #1807:
URL: https://github.com/apache/incubator-kyuubi/pull/1807


   


-- 
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 #1807: [KYUUBI #1806] Fix LaunchEngine may be closed multiple times

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


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1807?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 [#1807](https://codecov.io/gh/apache/incubator-kyuubi/pull/1807?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c70a594) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/53bbbb71616c6857fe6cbb730956542bd26e57b7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (53bbbb7) will **decrease** coverage by `0.02%`.
   > The diff coverage is `50.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-kyuubi/pull/1807/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/1807?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    #1807      +/-   ##
   ============================================
   - Coverage     59.89%   59.86%   -0.03%     
   + Complexity      264      263       -1     
   ============================================
     Files           275      275              
     Lines         13660    13680      +20     
     Branches       1750     1746       -4     
   ============================================
   + Hits           8181     8189       +8     
   - Misses         4777     4793      +16     
   + Partials        702      698       -4     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/1807?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [.../org/apache/kyuubi/session/KyuubiSessionImpl.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1807/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL0t5dXViaVNlc3Npb25JbXBsLnNjYWxh) | `82.75% <50.00%> (-1.46%)` | :arrow_down: |
   | [...apache/kyuubi/server/api/v1/SessionsResource.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1807/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvYXBpL3YxL1Nlc3Npb25zUmVzb3VyY2Uuc2NhbGE=) | `65.34% <0.00%> (-12.31%)` | :arrow_down: |
   | [...apache/kyuubi/service/TBinaryFrontendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1807/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL1RCaW5hcnlGcm9udGVuZFNlcnZpY2Uuc2NhbGE=) | `85.41% <0.00%> (-3.72%)` | :arrow_down: |
   | [...ache/kyuubi/server/api/v1/OperationsResource.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1807/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvYXBpL3YxL09wZXJhdGlvbnNSZXNvdXJjZS5zY2FsYQ==) | `67.46% <0.00%> (-2.66%)` | :arrow_down: |
   | [...ache/kyuubi/engine/trino/TrinoProcessBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1807/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvdHJpbm8vVHJpbm9Qcm9jZXNzQnVpbGRlci5zY2FsYQ==) | `44.82% <0.00%> (-2.55%)` | :arrow_down: |
   | [...a/org/apache/kyuubi/metrics/MetricsConstants.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1807/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-a3l1dWJpLW1ldHJpY3Mvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvbWV0cmljcy9NZXRyaWNzQ29uc3RhbnRzLnNjYWxh) | `100.00% <0.00%> (ø)` | |
   | [...e/kyuubi/server/KyuubiTBinaryFrontendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1807/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvS3l1dWJpVEJpbmFyeUZyb250ZW5kU2VydmljZS5zY2FsYQ==) | `96.55% <0.00%> (+0.12%)` | :arrow_up: |
   | [...org/apache/kyuubi/engine/trino/schema/RowSet.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1807/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-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL3NjaGVtYS9Sb3dTZXQuc2NhbGE=) | `90.20% <0.00%> (+0.13%)` | :arrow_up: |
   | [...org/apache/kyuubi/engine/flink/schema/RowSet.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1807/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-ZXh0ZXJuYWxzL2t5dXViaS1mbGluay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9mbGluay9zY2hlbWEvUm93U2V0LnNjYWxh) | `63.76% <0.00%> (+1.81%)` | :arrow_up: |
   | [...cala/org/apache/kyuubi/metrics/MetricsSystem.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1807/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-a3l1dWJpLW1ldHJpY3Mvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvbWV0cmljcy9NZXRyaWNzU3lzdGVtLnNjYWxh) | `62.79% <0.00%> (+2.79%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1807?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/1807?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 [53bbbb7...c70a594](https://codecov.io/gh/apache/incubator-kyuubi/pull/1807?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] turboFei edited a comment on pull request #1807: [KYUUBI #1806] Fix LaunchEngine may be closed multiple times

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


   > indeed, we don't need to add it into OperationManger as it's a member of the session.
   
   because we need fetch the launch engine log in client side, so it seems that we have to add it into operationManager.
   
   > do we call CloseOperation separately at the client-side now?
   
   not now.


-- 
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