[SPARK-15075][SPARK-15345][SQL] Clean up SparkSession builder and propagate config options to existing sessions if specified #13200

rxin · 2016-05-19T20:28:11Z

What changes were proposed in this pull request?

Currently SparkSession.Builder use SQLContext.getOrCreate. It should probably the the other way around, i.e. all the core logic goes in SparkSession, and SQLContext just calls that. This patch does that.

This patch also makes sure config options specified in the builder are propagated to the existing (and of course the new) SparkSession.

How was this patch tested?

Updated tests to reflect the change, and also introduced a new SparkSessionBuilderSuite that should cover all the branches.

SparkQA · 2016-05-19T20:44:48Z

Test build #58899 has finished for PR 13200 at commit b351024.

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

…ession

SparkQA · 2016-05-19T22:17:35Z

Test build #58907 has finished for PR 13200 at commit 55ef850.

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

rxin · 2016-05-19T23:32:02Z

sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala

+        sparkContext.addSparkListener(new SparkListener {
+          override def onApplicationEnd(applicationEnd: SparkListenerApplicationEnd): Unit = {
+            defaultSession.set(null)
+            // TODO(rxin): Do we need to also clear SQL listener?


cc @zsxwing any idea?

actually @zsxwing might be good for you to review the entire pr.

We need to clear it. Otherwise, after stopping the SparkContext, we leak it in object SQLContext.

rxin · 2016-05-19T23:33:07Z

cc @andrewor14 too who wrote some of this

and cc @zjffdu since you were working on #13160

SparkQA · 2016-05-19T23:48:06Z

Test build #58911 has finished for PR 13200 at commit c665771.

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

rxin · 2016-05-20T00:22:30Z

OK I've pushed a commit to handle sql listener properly.

SparkQA · 2016-05-20T00:43:24Z

Test build #58913 has finished for PR 13200 at commit 526896f.

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

andrewor14 · 2016-05-20T01:09:07Z

We should fix session.py to use the new scala thing as well. Also tests are failing because python is still trying to call wrapped.

rxin · 2016-05-20T01:14:15Z

That's been updated?

andrewor14 · 2016-05-20T01:16:19Z

Ah, yes.

andrewor14 · 2016-05-20T01:17:19Z

but not the builder, which still uses SQLContext.getOrCreate: https://github.com/rxin/spark/blob/c475453c59e6f795ea9283271fe83b35ef71bee6/python/pyspark/sql/session.py#L147

SparkQA · 2016-05-20T01:39:29Z

Test build #58918 has finished for PR 13200 at commit af481d6.

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

SparkQA · 2016-05-20T01:41:40Z

Test build #58919 has finished for PR 13200 at commit 918b47b.

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

rxin · 2016-05-20T01:59:24Z

I updated Python docs. The Python change seems slightly larger and since it is not user facing, I'm going to defer it to another pr.

SparkQA · 2016-05-20T02:03:15Z

Test build #58921 has finished for PR 13200 at commit c475453.

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

SparkQA · 2016-05-20T03:26:26Z

Test build #58931 has finished for PR 13200 at commit 4173a72.

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

rxin · 2016-05-20T04:48:42Z

@marmbrus i know you were looking at this. Did you end up going through it?

marmbrus · 2016-05-20T04:51:27Z

LGTM

rxin · 2016-05-20T04:53:12Z

Thanks - merging in master/2.0.

…pagate config options to existing sessions if specified ## What changes were proposed in this pull request? Currently SparkSession.Builder use SQLContext.getOrCreate. It should probably the the other way around, i.e. all the core logic goes in SparkSession, and SQLContext just calls that. This patch does that. This patch also makes sure config options specified in the builder are propagated to the existing (and of course the new) SparkSession. ## How was this patch tested? Updated tests to reflect the change, and also introduced a new SparkSessionBuilderSuite that should cover all the branches. Author: Reynold Xin <rxin@databricks.com> Closes #13200 from rxin/SPARK-15075. (cherry picked from commit f2ee0ed) Signed-off-by: Reynold Xin <rxin@databricks.com>

dilipbiswal · 2016-05-20T05:28:03Z

sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala

+        }
+        session = new SparkSession(sparkContext)
+        options.foreach { case (k, v) => session.conf.set(k, v) }
+        defaultSession.set(session)


@rxin Hi Reynold, i had a minor question just for my understanding. When users do a
new SQLContext() , we create a implicit SparkSession. Should this session be made
the defaultSession ? If we call, 1) new SQLContext 2) builder.getOrCreate() then whats the expected behaviour ?

We would create a new one in that case ...

I'm not too worried about the legacy corner cases here though.

@rxin Ok. Got it. Thank you.

SparkQA · 2016-05-20T06:19:34Z

Test build #58939 has finished for PR 13200 at commit e4a4bc1.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class ConsoleSink(options: Map[String, String]) extends Sink with Logging
- class ConsoleSinkProvider extends StreamSinkProvider with DataSourceRegister

…verriding confs of existing sessions ## What changes were proposed in this pull request? This fixes the python SparkSession builder to allow setting confs correctly. This was a leftover TODO from #13200. ## How was this patch tested? Python doc tests. cc andrewor14 Author: Eric Liang <ekl@databricks.com> Closes #13289 from ericl/spark-15520. (cherry picked from commit 8239fdc) Signed-off-by: Andrew Or <andrew@databricks.com>

…verriding confs of existing sessions ## What changes were proposed in this pull request? This fixes the python SparkSession builder to allow setting confs correctly. This was a leftover TODO from #13200. ## How was this patch tested? Python doc tests. cc andrewor14 Author: Eric Liang <ekl@databricks.com> Closes #13289 from ericl/spark-15520.

rxin changed the title ~~[SPARK-15075][SQL] Cleanup dependencies between SQLContext and SparkS…~~ [SPARK-15075][SQL] Cleanup dependencies between SQLContext and SparkSession May 19, 2016

rxin added 2 commits May 19, 2016 14:47

[SPARK-15075][SQL] Cleanup dependencies between SQLContext and SparkS…

46710a8

…ession

Fix compilation

55ef850

rxin mentioned this pull request May 19, 2016

[SPARK-15345][SQL][PYSPARK]. SparkSession's conf doesn't take effect when this already an existing SparkContext #13160

Closed

rxin force-pushed the SPARK-15075 branch from b351024 to 55ef850 Compare May 19, 2016 21:55

rxin changed the title ~~[SPARK-15075][SQL] Cleanup dependencies between SQLContext and SparkSession~~ [SPARK-15075][SQL] Clean up dependencies between SQLContext and SparkSession May 19, 2016

rxin added 2 commits May 19, 2016 15:28

fix compilation

c665771

Beef up test cases.

526896f

rxin changed the title ~~[SPARK-15075][SQL] Clean up dependencies between SQLContext and SparkSession~~ [SPARK-15075][SQL] Clean up SparkSession builder and propagate config options to existing sessions if specified May 19, 2016

rxin reviewed May 19, 2016
View reviewed changes

rxin changed the title ~~[SPARK-15075][SQL] Clean up SparkSession builder and propagate config options to existing sessions if specified~~ [SPARK-15075][SPARK-15345][SQL] Clean up SparkSession builder and propagate config options to existing sessions if specified May 19, 2016

rxin added 4 commits May 19, 2016 17:05

Fix python

af481d6

Also deprecate SQLContext constructors

c6a8224

Deprecate more

918b47b

Handle SQLListener properly

9d69f6a

Blah

c475453

Updated documentation for Python

4173a72

Merge branch 'master' into SPARK-15075

e4a4bc1

asfgit closed this in f2ee0ed May 20, 2016

dilipbiswal reviewed May 20, 2016
View reviewed changes

ericl mentioned this pull request May 25, 2016

[SPARK-15520] [SQL] SparkSession builder in python should also allow overriding confs of existing sessions #13289

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-15075][SPARK-15345][SQL] Clean up SparkSession builder and propagate config options to existing sessions if specified #13200

[SPARK-15075][SPARK-15345][SQL] Clean up SparkSession builder and propagate config options to existing sessions if specified #13200

rxin commented May 19, 2016 •

edited

Loading

SparkQA commented May 19, 2016

SparkQA commented May 19, 2016

rxin May 19, 2016

rxin May 19, 2016

zsxwing May 20, 2016

rxin commented May 19, 2016 •

edited

Loading

SparkQA commented May 19, 2016

rxin commented May 20, 2016

SparkQA commented May 20, 2016

andrewor14 commented May 20, 2016

rxin commented May 20, 2016

andrewor14 commented May 20, 2016

andrewor14 commented May 20, 2016

SparkQA commented May 20, 2016

SparkQA commented May 20, 2016

rxin commented May 20, 2016

SparkQA commented May 20, 2016

SparkQA commented May 20, 2016

rxin commented May 20, 2016

marmbrus commented May 20, 2016

rxin commented May 20, 2016

dilipbiswal May 20, 2016

rxin May 20, 2016

dilipbiswal May 20, 2016

SparkQA commented May 20, 2016

[SPARK-15075][SPARK-15345][SQL] Clean up SparkSession builder and propagate config options to existing sessions if specified #13200

[SPARK-15075][SPARK-15345][SQL] Clean up SparkSession builder and propagate config options to existing sessions if specified #13200

Conversation

rxin commented May 19, 2016 • edited Loading

What changes were proposed in this pull request?

How was this patch tested?

SparkQA commented May 19, 2016

SparkQA commented May 19, 2016

rxin May 19, 2016

Choose a reason for hiding this comment

rxin May 19, 2016

Choose a reason for hiding this comment

zsxwing May 20, 2016

Choose a reason for hiding this comment

rxin commented May 19, 2016 • edited Loading

SparkQA commented May 19, 2016

rxin commented May 20, 2016

SparkQA commented May 20, 2016

andrewor14 commented May 20, 2016

rxin commented May 20, 2016

andrewor14 commented May 20, 2016

andrewor14 commented May 20, 2016

SparkQA commented May 20, 2016

SparkQA commented May 20, 2016

rxin commented May 20, 2016

SparkQA commented May 20, 2016

SparkQA commented May 20, 2016

rxin commented May 20, 2016

marmbrus commented May 20, 2016

rxin commented May 20, 2016

dilipbiswal May 20, 2016

Choose a reason for hiding this comment

rxin May 20, 2016

Choose a reason for hiding this comment

dilipbiswal May 20, 2016

Choose a reason for hiding this comment

SparkQA commented May 20, 2016

rxin commented May 19, 2016 •

edited

Loading

rxin commented May 19, 2016 •

edited

Loading