Skip to content
This repository was archived by the owner on Jun 14, 2024. It is now read-only.

Add warning log - newly created Hyperspace context for different Spark Session #374

Open
sezruby wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add warning log - newly created Hyperspace context for different Spark Session #374

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
72afc4c
Add warning for newly created Hyperspace Object
sezruby Apr 12, 2021
0f3d839
Merge branch 'master' into multisession
sezruby Apr 13, 2021
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
9 changes: 7 additions & 2 deletions src/main/scala/com/microsoft/hyperspace/Hyperspace.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.microsoft.hyperspace

import org.apache.spark.internal.Logging
import org.apache.spark.sql.{DataFrame, SparkSession}

import com.microsoft.hyperspace.index._
Expand All @@ -24,7 +25,7 @@ import com.microsoft.hyperspace.index.plananalysis.PlanAnalyzer
import com.microsoft.hyperspace.index.sources.FileBasedSourceProviderManager

class Hyperspace(spark: SparkSession) {
private val indexManager: IndexManager = Hyperspace.getContext(spark).indexCollectionManager
private def indexManager: IndexManager = Hyperspace.getContext(spark).indexCollectionManager

/**
* Collect all the index metadata.
Expand Down Expand Up @@ -165,7 +166,7 @@ class Hyperspace(spark: SparkSession) {
}
}

object Hyperspace extends ActiveSparkSession {
object Hyperspace extends ActiveSparkSession with Logging {
private lazy val context = new ThreadLocal[HyperspaceContext]

private[hyperspace] def getContext(spark: SparkSession): HyperspaceContext = {
Expand All @@ -175,6 +176,10 @@ object Hyperspace extends ActiveSparkSession {
// the one HyperspaceContext is using because Hyperspace depends on the
// session's properties such as configs, etc.
context.set(new HyperspaceContext(spark))
if (!ctx.spark.equals(spark)) {
Copy link
Collaborator Author

@sezruby sezruby Apr 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@imback82 I'm thinking of this change & what can we do about this.
I think the problem is - we access using the active spark session when optimizing query plan, but it can be different from the session which is used to create Hyperspace object.
So.. how about

  1. not resetting hyperspace & just using the previous one even with different active spark session.
  2. if a user wants to use another configs/spark session, they need to redefine Hyperspace object.

I think this is clearer behavior because we do val hs = new Hyperspace(spark)

Copy link
Contributor

@imback82 imback82 Apr 19, 2021

Choose a reason for hiding this comment

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

  1. not resetting hyperspace & just using the previous one even with different active spark session.

Will it be ok to silently use a different active session?

logWarning("Accessed Hyperspace using different Spark Session. " +
"Hyperspace context is newly created.")
}
}

context.get()
Expand Down