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/03/24 07:00:03 UTC

[GitHub] [incubator-kyuubi] winfys opened a new pull request #2208: Fixed session close operator log session dir not deleted

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


   ### _Why are the changes needed?_
   Operator log Session dir not deleted when the session is closed，
   ![image](https://user-images.githubusercontent.com/13195083/159858277-ee1b4391-3f87-41e9-906e-bc02f8ef4655.png)
   As time goes on, more and more directories are created, which is a big risk
   
   ### _How was this patch tested?_
   1、open session
   ![image](https://user-images.githubusercontent.com/13195083/159859323-221ffd93-4500-4236-8646-9253e3019983.png)
   session operator dir
   ![image](https://user-images.githubusercontent.com/13195083/159859633-4bb3db5a-8d46-4e79-af3f-ad88cb1eb3d9.png)
   2、Closing the session directory is deleted
   ![image](https://user-images.githubusercontent.com/13195083/159860262-adcd7f60-2838-456b-beef-6413c50433b7.png)
   
   ![image](https://user-images.githubusercontent.com/13195083/159860191-9defcd23-875b-4f03-8e98-9469dcba52f9.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] codecov-commenter commented on pull request #2208: Fixed session close operator log session dir not deleted

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


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2208?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 [#2208](https://codecov.io/gh/apache/incubator-kyuubi/pull/2208?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cbcee43) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/8f2b7358e47bb5f01989d61bb4eb16a4af9b64ee?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8f2b735) will **decrease** coverage by `0.00%`.
   > The diff coverage is `100.00%`.
   
   > :exclamation: Current head cbcee43 differs from pull request most recent head 46725f0. Consider uploading reports for the commit 46725f0 to get more accurate results
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2208      +/-   ##
   ============================================
   - Coverage     61.69%   61.69%   -0.01%     
     Complexity       69       69              
   ============================================
     Files           332      332              
     Lines         15947    15952       +5     
     Branches       2027     2027              
   ============================================
   + Hits           9838     9841       +3     
   - Misses         5289     5290       +1     
   - Partials        820      821       +1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/2208?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/2208/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) | `85.71% <100.00%> (+0.59%)` | :arrow_up: |
   | [.../org/apache/kyuubi/engine/hive/HiveSQLEngine.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2208/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-ZXh0ZXJuYWxzL2t5dXViaS1oaXZlLXNxbC1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL2hpdmUvSGl2ZVNRTEVuZ2luZS5zY2FsYQ==) | `75.00% <0.00%> (-2.78%)` | :arrow_down: |
   | [...ache/kyuubi/operation/KyuubiOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2208/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==) | `94.11% <0.00%> (-1.97%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2208?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/2208?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 [8f2b735...46725f0](https://codecov.io/gh/apache/incubator-kyuubi/pull/2208?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: 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 edited a comment on pull request #2208: Fixed session close operator log session dir not deleted

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


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2208?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 [#2208](https://codecov.io/gh/apache/incubator-kyuubi/pull/2208?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cbcee43) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/c09cd654bbc918178112a2b9d20c1a0e35154f95?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c09cd65) will **increase** coverage by `0.00%`.
   > The diff coverage is `100.00%`.
   
   > :exclamation: Current head cbcee43 differs from pull request most recent head 7f261a3. Consider uploading reports for the commit 7f261a3 to get more accurate results
   
   ```diff
   @@            Coverage Diff            @@
   ##             master    #2208   +/-   ##
   =========================================
     Coverage     61.68%   61.69%           
     Complexity       69       69           
   =========================================
     Files           332      332           
     Lines         15949    15952    +3     
     Branches       2027     2027           
   =========================================
   + Hits           9838     9841    +3     
   + Misses         5292     5290    -2     
   - Partials        819      821    +2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/2208?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/2208/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) | `85.71% <100.00%> (+0.59%)` | :arrow_up: |
   | [...ache/kyuubi/operation/KyuubiOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2208/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==) | `94.11% <0.00%> (-1.97%)` | :arrow_down: |
   | [...ache/kyuubi/engine/flink/FlinkProcessBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2208/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvZmxpbmsvRmxpbmtQcm9jZXNzQnVpbGRlci5zY2FsYQ==) | `71.95% <0.00%> (-1.22%)` | :arrow_down: |
   | [...rg/apache/kyuubi/engine/flink/FlinkSQLEngine.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2208/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-ZXh0ZXJuYWxzL2t5dXViaS1mbGluay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9mbGluay9GbGlua1NRTEVuZ2luZS5zY2FsYQ==) | `22.85% <0.00%> (+0.63%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2208?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/2208?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 [c09cd65...7f261a3](https://codecov.io/gh/apache/incubator-kyuubi/pull/2208?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: 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 change in pull request #2208: Fixed session close operator log session dir not deleted

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



##########
File path: kyuubi-common/src/main/scala/org/apache/kyuubi/session/SessionManager.scala
##########
@@ -89,10 +89,23 @@ abstract class SessionManager(name: String) extends CompositeService(name) {
     if (session == null) {
       throw KyuubiSQLException(s"Invalid $sessionHandle")
     }
+    deleteOperationLogSessionDir(sessionHandle)

Review comment:
       my bad, previous I only considered the empty session case which a session is created without operators




-- 
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 edited a comment on pull request #2208: Fixed session close operator log session dir not deleted

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


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2208?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 [#2208](https://codecov.io/gh/apache/incubator-kyuubi/pull/2208?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cbcee43) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/c09cd654bbc918178112a2b9d20c1a0e35154f95?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c09cd65) will **increase** coverage by `0.00%`.
   > The diff coverage is `100.00%`.
   
   > :exclamation: Current head cbcee43 differs from pull request most recent head 2dad761. Consider uploading reports for the commit 2dad761 to get more accurate results
   
   ```diff
   @@            Coverage Diff            @@
   ##             master    #2208   +/-   ##
   =========================================
     Coverage     61.68%   61.69%           
     Complexity       69       69           
   =========================================
     Files           332      332           
     Lines         15949    15952    +3     
     Branches       2027     2027           
   =========================================
   + Hits           9838     9841    +3     
   + Misses         5292     5290    -2     
   - Partials        819      821    +2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/2208?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/2208/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) | `85.71% <100.00%> (+0.59%)` | :arrow_up: |
   | [...ache/kyuubi/operation/KyuubiOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2208/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==) | `94.11% <0.00%> (-1.97%)` | :arrow_down: |
   | [...ache/kyuubi/engine/flink/FlinkProcessBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2208/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvZmxpbmsvRmxpbmtQcm9jZXNzQnVpbGRlci5zY2FsYQ==) | `71.95% <0.00%> (-1.22%)` | :arrow_down: |
   | [...rg/apache/kyuubi/engine/flink/FlinkSQLEngine.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2208/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-ZXh0ZXJuYWxzL2t5dXViaS1mbGluay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9mbGluay9GbGlua1NRTEVuZ2luZS5zY2FsYQ==) | `22.85% <0.00%> (+0.63%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2208?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/2208?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 [c09cd65...2dad761](https://codecov.io/gh/apache/incubator-kyuubi/pull/2208?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: 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] winfys commented on pull request #2208: Fixed session close operator log session dir not deleted

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


   @SteNicholas ok,I'm going to add
   


-- 
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 #2208: Fixed session close operator log session dir not deleted

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


   thanks, merging 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 closed pull request #2208: Fixed session close operator log session dir not deleted

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


   


-- 
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 change in pull request #2208: Fixed session close operator log session dir not deleted

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



##########
File path: kyuubi-common/src/main/scala/org/apache/kyuubi/session/SessionManager.scala
##########
@@ -89,10 +89,23 @@ abstract class SessionManager(name: String) extends CompositeService(name) {
     if (session == null) {
       throw KyuubiSQLException(s"Invalid $sessionHandle")
     }
+    deleteOperationLogSessionDir(sessionHandle)

Review comment:
       When the `closeSession` method is called, the operation is not necessarily close. At this time, there are log files, and the log may also increase.
   
   When `session.close()`, it will close the operation first.
   
   So it is suggested here that `deleteOperationLogSessionDir` should be placed after `session.close()`
   Also because `Files.deleteIfExists` can only delete empty folders, consider using `org.apache.kyuubi.Utils#deleteDirectoryRecursively`.




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