You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/06/07 11:11:04 UTC

[GitHub] [spark] AngersZhuuuu opened a new pull request, #36786: [SPARK-39400][SQL] spark-sql should remove hive resource dir in all case

AngersZhuuuu opened a new pull request, #36786:
URL: https://github.com/apache/spark/pull/36786

   ### What changes were proposed in this pull request?
   In current code, when we use `spark-sql` `-e` , `-f` or use `ctrl + c` to close  `spark-sql` session, will remain hive session resource dir under `/tmp` path, this pr help to clean this files
   
   
   ### Why are the changes needed?
   Clean remained files
   
   
   ### Does this PR introduce _any_ user-facing change?
   No
   
   
   ### How was this patch tested?
   Manuel tested
   


-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AngersZhuuuu commented on pull request #36786: [SPARK-39400][SQL] spark-sql should remove hive resource dir in all case

Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on PR #36786:
URL: https://github.com/apache/spark/pull/36786#issuecomment-1149434288

   > Could we add a test?
   
   Need to test after built, seems hard to write test in UT...


-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AngersZhuuuu commented on a diff in pull request #36786: [SPARK-39400][SQL] spark-sql should remove hive resource dir in all case

Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on code in PR #36786:
URL: https://github.com/apache/spark/pull/36786#discussion_r892496313


##########
sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIDriver.scala:
##########
@@ -140,7 +143,10 @@ private[hive] object SparkSQLCLIDriver extends Logging {
     SessionState.setCurrentSessionState(sessionState)
 
     // Clean up after we exit
-    ShutdownHookManager.addShutdownHook { () => SparkSQLEnv.stop() }
+    ShutdownHookManager.addShutdownHook { () =>
+      sessionState.close()
+      SparkSQLEnv.stop()

Review Comment:
   `sessionState.close` already have try catch
   ```
     public void close() {
       try {
         super.close();
       } catch (IOException var2) {
         var2.printStackTrace();
       }
   
     }
   ```



-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AngersZhuuuu commented on pull request #36786: [SPARK-39400][SQL] spark-sql should remove hive resource dir in all case

Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on PR #36786:
URL: https://github.com/apache/spark/pull/36786#issuecomment-1148527000

   ping @cloud-fan @wangyum 


-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] wangyum commented on a diff in pull request #36786: [SPARK-39400][SQL] spark-sql should remove hive resource dir in all case

Posted by GitBox <gi...@apache.org>.
wangyum commented on code in PR #36786:
URL: https://github.com/apache/spark/pull/36786#discussion_r892493780


##########
sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIDriver.scala:
##########
@@ -140,7 +143,10 @@ private[hive] object SparkSQLCLIDriver extends Logging {
     SessionState.setCurrentSessionState(sessionState)
 
     // Clean up after we exit
-    ShutdownHookManager.addShutdownHook { () => SparkSQLEnv.stop() }
+    ShutdownHookManager.addShutdownHook { () =>
+      sessionState.close()
+      SparkSQLEnv.stop()

Review Comment:
   ```scala
       try {
         sessionState.close()
       } finally {
         SparkSQLEnv.stop()
       }
   ```
   ?



-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] wangyum closed pull request #36786: [SPARK-39400][SQL] spark-sql should remove hive resource dir in all case

Posted by GitBox <gi...@apache.org>.
wangyum closed pull request #36786: [SPARK-39400][SQL] spark-sql should remove hive resource dir in all case
URL: https://github.com/apache/spark/pull/36786


-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] wangyum commented on pull request #36786: [SPARK-39400][SQL] spark-sql should remove hive resource dir in all case

Posted by GitBox <gi...@apache.org>.
wangyum commented on PR #36786:
URL: https://github.com/apache/spark/pull/36786#issuecomment-1150532390

   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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org