Skip to content

Conversation

@ulysses-you
Copy link
Contributor

What changes were proposed in this pull request?

Reset listenerRegistered when application end.

Why are the changes needed?

Within a jvm, stop and create SparkContext multi times will cause the bug.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Add UT.

@ulysses-you
Copy link
Contributor Author

cc @cloud-fan @vinooganesh

assert(session.conf.get(WAREHOUSE_PATH) === "SPARK-31532-db-2")
}

test("SPARK-32062: reset listenerRegistered in SparkSession") {
Copy link
Contributor

@cloud-fan cloud-fan Jun 23, 2020

Choose a reason for hiding this comment

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

This test relies on other tests to create and stop spark session, which is fragile.

Can we explicitly create and stop a spark session in this test?

val conf = new SparkConf()
.setMaster("local")
.setAppName(s"test-SPARK-32062-$i")
val context = new SparkContext(conf)
Copy link
Contributor

Choose a reason for hiding this comment

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

does this work? The test doesn't stop the spark context, and AFAIK we don't support having multiple spark context instance at the same time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

missed it.

@SparkQA
Copy link

SparkQA commented Jun 23, 2020

Test build #124389 has finished for PR 28899 at commit a486f1f.

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

@SparkQA
Copy link

SparkQA commented Jun 23, 2020

Test build #124375 has finished for PR 28899 at commit fb88f3a.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jun 23, 2020

Test build #124390 has finished for PR 28899 at commit 708c9ff.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@cloud-fan
Copy link
Contributor

retest this please

@SparkQA
Copy link

SparkQA commented Jun 23, 2020

Test build #124416 has finished for PR 28899 at commit 708c9ff.

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

@vinooganesh
Copy link
Contributor

@ulysses-you - quick question for you (mostly for my own knowledge), do people usually destroy and re-create a spark context in the lifetime of a JVM? I was actually thinking about including this, but assumed that most people would create the singleton context and then use session instances for these types of short lives operations.

@ulysses-you
Copy link
Contributor Author

@vinooganesh
Not usual, but in local mode it may happen.
e.g. in spark test case, we create and stop context multi times. If the bug exists, other test case will be affected.

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in 9f540fa Jun 24, 2020
@ulysses-you
Copy link
Contributor Author

ulysses-you commented Jun 24, 2020

@cloud-fan thanks for merging !
BTW do we need a backport for 3.0.1 ?

@cloud-fan
Copy link
Contributor

It's not a bug that people can hit in practice, I think it's OK to leave 3.0.

@ulysses-you ulysses-you deleted the SPARK-32062 branch March 5, 2021 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants