Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
[SPARK-23809][SQL] Active SparkSession should be set by getOrCreate
## What changes were proposed in this pull request?

Currently, the active spark session is set inconsistently (e.g., in createDataFrame, prior to query execution). Many places in spark also incorrectly query active session when they should be calling activeSession.getOrElse(defaultSession) and so might get None even if a Spark session exists.

The semantics here can be cleaned up if we also set the active session when the default session is set.

Related: https://github.com/apache/spark/pull/20926/files

## How was this patch tested?

Unit test, existing test. Note that if #20926 merges first we should also update the tests there.

Author: Eric Liang <[email protected]>

Closes #20927 from ericl/active-session-cleanup.
  • Loading branch information
ericl committed Apr 4, 2018
commit f2303dcef61660dabfd08be5568b7da10cf1b117
14 changes: 13 additions & 1 deletion sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala
Original file line number Diff line number Diff line change
Expand Up @@ -951,7 +951,8 @@ object SparkSession {

session = new SparkSession(sparkContext, None, None, extensions)
options.foreach { case (k, v) => session.initialSessionOptions.put(k, v) }
defaultSession.set(session)
setDefaultSession(session)
setActiveSession(session)

// Register a successfully instantiated context to the singleton. This should be at the
// end of the class definition so that the singleton is updated only if there is no
Expand Down Expand Up @@ -1027,6 +1028,17 @@ object SparkSession {
*/
def getDefaultSession: Option[SparkSession] = Option(defaultSession.get)

/**
* Returns the currently active SparkSession, otherwise the default one. If there is no default
* SparkSession, throws an exception.
*
* @since 2.4.0
*/
def active: SparkSession = {
getActiveSession.getOrElse(getDefaultSession.getOrElse(
throw new IllegalStateException("No active or default Spark session found")))
}

////////////////////////////////////////////////////////////////////////////////////////
// Private methods from now on
////////////////////////////////////////////////////////////////////////////////////////
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,24 @@ class SparkSessionBuilderSuite extends SparkFunSuite with BeforeAndAfterEach {
assert(SparkSession.builder().getOrCreate() == session)
}

test("sets default and active session") {
assert(SparkSession.getDefaultSession == None)
assert(SparkSession.getActiveSession == None)
val session = SparkSession.builder().master("local").getOrCreate()
assert(SparkSession.getDefaultSession == Some(session))
assert(SparkSession.getActiveSession == Some(session))
}

test("get active or default session") {
val session = SparkSession.builder().master("local").getOrCreate()
assert(SparkSession.active == session)
SparkSession.clearActiveSession()
assert(SparkSession.active == session)
SparkSession.clearDefaultSession()
intercept[IllegalStateException](SparkSession.active)
session.stop()
}

test("config options are propagated to existing SparkSession") {
val session1 = SparkSession.builder().master("local").config("spark-config1", "a").getOrCreate()
assert(session1.conf.get("spark-config1") == "a")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ private[spark] class TestSparkSession(sc: SparkContext) extends SparkSession(sc)
}

SparkSession.setDefaultSession(this)
SparkSession.setActiveSession(this)

@transient
override lazy val sessionState: SessionState = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,9 @@ private[hive] class TestHiveSparkSession(
loadTestTables)
}

SparkSession.setDefaultSession(this)
SparkSession.setActiveSession(this)

{ // set the metastore temporary configuration
val metastoreTempConf = HiveUtils.newTemporaryConfiguration(useInMemoryDerby = false) ++ Map(
ConfVars.METASTORE_INTEGER_JDO_PUSHDOWN.varname -> "true",
Expand Down