Skip to content

[SPARK-21993][SQL] Close sessionState when finish #19219

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

[SPARK-21993][SQL] Close sessionState when finish #19219

wants to merge 1 commit into from

Conversation

@jinxing64
Copy link

@jinxing64 jinxing64 commented Sep 13, 2017

What changes were proposed in this pull request?

In current code, SessionState in SparkSQLCLIDriver is not guaranteed to be closed. As a result, some tmp files/dirs are not removed when job finished. We can close SessionState in shutdown hook.

srowen
srowen reviewed Sep 13, 2017
View reviewed changes
...e-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIDriver.scala Outdated
Copy link
Member

@srowen srowen Sep 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if it matters, but keep in mind this will hold a reference to sessionState then until the JVM exits

@SparkQA
Copy link

SparkQA commented Sep 13, 2017

Test build #81713 has finished for PR 19219 at commit 2a06c0c.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@jinxing64
Copy link
Author

jinxing64 commented Sep 14, 2017 •
edited
Loading

@gatorsmile @HyukjinKwon
Could you please give some advice on this?

@jinxing64
Copy link
Author

jinxing64 commented Sep 14, 2017

It seems there are still some other places where session state is not guaranteed to be closed. I will update this pr soon.

@jinxing64 jinxing64 changed the title [SPARK-21993][SQL] Close sessionState in shutdown hook. [SPARK-21993][SQL][WIP] Close sessionState when finish Sep 16, 2017
@SparkQA
Copy link

SparkQA commented Sep 16, 2017

Test build #81838 has finished for PR 19219 at commit 1187dac.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

cenyuhai
cenyuhai reviewed Sep 16, 2017
View reviewed changes
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionStateBuilder.scala Outdated
Copy link
Contributor

@cenyuhai cenyuhai Sep 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

newSession is to isolate SparkSession in Spark ThriftServer, You must judge whether is thriftserver or not.

@SparkQA
Copy link

SparkQA commented Sep 18, 2017

Test build #81874 has finished for PR 19219 at commit d7a484d.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Sep 20, 2017

Test build #81997 has finished for PR 19219 at commit df49a19.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Sep 21, 2017

Test build #82017 has finished for PR 19219 at commit e421113.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@jinxing64 jinxing64 changed the title [SPARK-21993][SQL][WIP] Close sessionState when finish [SPARK-21993][SQL] Close sessionState when finish Sep 21, 2017
@gatorsmile
Copy link
Member

gatorsmile commented Feb 1, 2018

cc @liufengdb

@liufengdb
Copy link

liufengdb commented Feb 1, 2018

The major issue this PR tries to cover has been fixed by #20029, so I think we are good if there are no calls to HiveClientImpl.newSession. We can close this PR with no-fix.

@SparkQA
Copy link

SparkQA commented Feb 1, 2018

Test build #86911 has finished for PR 19219 at commit e421113.

  • This patch fails PySpark unit tests.
  • This patch does not merge cleanly.
  • This patch adds no public classes.

@srowen srowen mentioned this pull request Oct 24, 2018
Closed
@asfgit asfgit closed this in 65c653f Oct 25, 2018
zifeif2 pushed a commit to zifeif2/spark that referenced this pull request Nov 22, 2025
@srowen @zifeif2
[BUILD] Close stale PRs
25b8667 
Closes apache#22567
Closes apache#18457
Closes apache#21517
Closes apache#21858
Closes apache#22383
Closes apache#19219
Closes apache#22401
Closes apache#22811
Closes apache#20405
Closes apache#21933

Closes apache#22819 from srowen/ClosePRs.

Authored-by: Sean Owen <[email protected]>
Signed-off-by: Sean Owen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@srowen srowen srowen left review comments

+1 more reviewer

@cenyuhai cenyuhai cenyuhai left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@jinxing64 @SparkQA @gatorsmile @liufengdb @srowen @cenyuhai