[SPARK-7553] [STREAMING] Added methods to maintain a singleton StreamingContext #6070

tdas · 2015-05-12T02:56:21Z

In a REPL/notebook environment, its very easy to lose a reference to a StreamingContext by overriding the variable name. So if you happen to execute the following commands

val ssc = new StreamingContext(...) // cmd 1
ssc.start() // cmd 2
...
val ssc = new StreamingContext(...) // accidentally run cmd 1 again

The value of ssc will be overwritten. Now you can neither start the new context (as only one context can be started), nor stop the previous context (as the reference is lost).
Hence its best to maintain a singleton reference to the active context, so that we never loose reference for the active context.
Since this problem occurs useful in REPL environments, its best to add this as an Experimental support in the Scala API only so that it can be used in Scala REPLs and notebooks.

tdas · 2015-05-12T02:56:26Z

@pwendell

AmplabJenkins · 2015-05-12T02:57:10Z

Merged build triggered.

AmplabJenkins · 2015-05-12T02:57:17Z

Merged build started.

SparkQA · 2015-05-12T02:58:59Z

Test build #32453 has started for PR 6070 at commit d37a846.

SparkQA · 2015-05-12T03:52:04Z

Test build #32453 has finished for PR 6070 at commit d37a846.

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

AmplabJenkins · 2015-05-12T03:52:09Z

Merged build finished. Test FAILed.

AmplabJenkins · 2015-05-12T03:52:09Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/32453/
Test FAILed.

pwendell · 2015-05-12T04:31:19Z

streaming/src/main/scala/org/apache/spark/streaming/StreamingContext.scala

I think you renamed this to getActiveOrCreate

pwendell · 2015-05-12T04:42:13Z

The API design all looks good to me. I'm a bit confused about the locking (seems strange to have both a guarding lock and the atomic reference) and I was wondering if maybe there is an opportunity for racing.

zsxwing · 2015-05-12T04:57:13Z

streaming/src/test/scala/org/apache/spark/streaming/StreamingContextSuite.scala

nit: nonEmpty -> isEmpty

tdas · 2015-05-12T06:20:12Z

@pwendell @zsxwing I should have mentioned in the title that this is WIP as #6060 will cause merge conflicts and is needed the full picture regarding locks.

AmplabJenkins · 2015-05-12T06:32:12Z

Merged build triggered.

AmplabJenkins · 2015-05-12T06:32:18Z

Merged build started.

SparkQA · 2015-05-12T06:34:12Z

Test build #32470 has started for PR 6070 at commit 3884a25.

SparkQA · 2015-05-12T07:46:14Z

Test build #32470 has finished for PR 6070 at commit 3884a25.

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

AmplabJenkins · 2015-05-12T07:46:19Z

Merged build finished. Test FAILed.

AmplabJenkins · 2015-05-12T07:46:19Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/32470/
Test FAILed.

pwendell · 2015-05-12T17:57:20Z

Okay, LGTM

AmplabJenkins · 2015-05-12T18:07:12Z

Merged build triggered.

AmplabJenkins · 2015-05-12T18:07:18Z

Merged build started.

SparkQA · 2015-05-12T18:09:11Z

Test build #32519 has started for PR 6070 at commit 64706c9.

zsxwing · 2015-05-12T18:10:34Z

LGTM

AmplabJenkins · 2015-05-12T19:52:21Z

Merged build finished. Test PASSed.

AmplabJenkins · 2015-05-12T19:52:21Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/32519/
Test PASSed.

AmplabJenkins · 2015-05-12T20:12:13Z

Merged build triggered.

AmplabJenkins · 2015-05-12T20:12:19Z

Merged build started.

SparkQA · 2015-05-12T20:13:00Z

Test build #32531 has started for PR 6070 at commit a797171.

SparkQA · 2015-05-12T20:14:40Z

Test build #32531 has finished for PR 6070 at commit a797171.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2015-05-12T20:14:41Z

Merged build finished. Test FAILed.

AmplabJenkins · 2015-05-12T20:14:42Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/32531/
Test FAILed.

AmplabJenkins · 2015-05-12T20:32:13Z

Merged build triggered.

AmplabJenkins · 2015-05-12T20:32:19Z

Merged build started.

SparkQA · 2015-05-12T20:33:00Z

Test build #32533 has started for PR 6070 at commit 731c9a1.

harishreedharan · 2015-05-12T22:14:33Z

If I create a new StreamingContext using the constructor it will not be considered as an active one which can be retrieved from getActiveOrCreate. This should be clearly documented, as this might cause leaks like the one this PR is trying to prevent. Also, how would this work when we are recovering from a checkpoint? How does one tell that the one from checkpoint should be the active one?

SparkQA · 2015-05-12T22:18:40Z

Test build #32533 has finished for PR 6070 at commit 731c9a1.

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

AmplabJenkins · 2015-05-12T22:18:45Z

Merged build finished. Test PASSed.

AmplabJenkins · 2015-05-12T22:18:45Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/32533/
Test PASSed.

tdas · 2015-05-12T22:21:27Z

Is the current documentation on getActiveOrCreate insufficient? It states that either get the active one which is started but not stopped, or get from checkpoint / create new. And getActiveOrCreate also takes a checkpoint path. And as the docs says, getActiveOrCreate(checkpointPath) will either

get the active context, or
recover from checkpoint, or
create a new context
In 1, the context will be already active. In the last 2, the contexts will be in INITIALIZED state and wil have to be started to start anything. BTW, start() as also been made idempotent, or in other words, no-op in case the cntext is already started. So in all the three above cases, you can call start().

So the following lines become idempotent

StreamingContext.getActiveOrCreate(checkpointPath, creatingFunc).start()

harishreedharan · 2015-05-12T22:26:30Z

The current docs don't clearly say that if a new StreamingContext is created directly using the constructor and not one of the getActiveOrCreate methods, it is not tracked. It should probably be made clear that an app that uses getActiveOrCreate should not directly call the constructor.

Sorry, I didn't notice the getActiveOrCreate(checkpointPath) - so you can ignore that comment.

tdas · 2015-05-12T22:58:19Z

Actually, it is tracked! A new StreamingContext created directly is also tracked. So the semantics is what is intuitive, and so I dont think it needs an extra documentation.

harishreedharan · 2015-05-12T23:10:12Z

OK, then we are good :)

tdas · 2015-05-12T23:44:10Z

Alright! Merging this then. Thank you all for reviewing.

On Tue, May 12, 2015 at 4:10 PM, Hari Shreedharan [email protected]
wrote:

OK, then we are good :)

—
Reply to this email directly or view it on GitHub
#6070 (comment).

…ingContext In a REPL/notebook environment, its very easy to lose a reference to a StreamingContext by overriding the variable name. So if you happen to execute the following commands ``` val ssc = new StreamingContext(...) // cmd 1 ssc.start() // cmd 2 ... val ssc = new StreamingContext(...) // accidentally run cmd 1 again ``` The value of ssc will be overwritten. Now you can neither start the new context (as only one context can be started), nor stop the previous context (as the reference is lost). Hence its best to maintain a singleton reference to the active context, so that we never loose reference for the active context. Since this problem occurs useful in REPL environments, its best to add this as an Experimental support in the Scala API only so that it can be used in Scala REPLs and notebooks. Author: Tathagata Das <[email protected]> Closes #6070 from tdas/SPARK-7553 and squashes the following commits: 731c9a1 [Tathagata Das] Fixed style a797171 [Tathagata Das] Added more unit tests 19fc70b [Tathagata Das] Added :: Experimental :: in docs 64706c9 [Tathagata Das] Fixed test 634db5d [Tathagata Das] Merge remote-tracking branch 'apache-github/master' into SPARK-7553 3884a25 [Tathagata Das] Fixing test bug d37a846 [Tathagata Das] Added getActive and getActiveOrCreate (cherry picked from commit 00e7b09) Signed-off-by: Tathagata Das <[email protected]>

…ingContext In a REPL/notebook environment, its very easy to lose a reference to a StreamingContext by overriding the variable name. So if you happen to execute the following commands ``` val ssc = new StreamingContext(...) // cmd 1 ssc.start() // cmd 2 ... val ssc = new StreamingContext(...) // accidentally run cmd 1 again ``` The value of ssc will be overwritten. Now you can neither start the new context (as only one context can be started), nor stop the previous context (as the reference is lost). Hence its best to maintain a singleton reference to the active context, so that we never loose reference for the active context. Since this problem occurs useful in REPL environments, its best to add this as an Experimental support in the Scala API only so that it can be used in Scala REPLs and notebooks. Author: Tathagata Das <[email protected]> Closes apache#6070 from tdas/SPARK-7553 and squashes the following commits: 731c9a1 [Tathagata Das] Fixed style a797171 [Tathagata Das] Added more unit tests 19fc70b [Tathagata Das] Added :: Experimental :: in docs 64706c9 [Tathagata Das] Fixed test 634db5d [Tathagata Das] Merge remote-tracking branch 'apache-github/master' into SPARK-7553 3884a25 [Tathagata Das] Fixing test bug d37a846 [Tathagata Das] Added getActive and getActiveOrCreate

Added getActive and getActiveOrCreate

d37a846

pwendell reviewed May 12, 2015
View reviewed changes

zsxwing reviewed May 12, 2015
View reviewed changes

streaming/src/test/scala/org/apache/spark/streaming/StreamingContextSuite.scala Outdated

Copy link

Member

zsxwing May 12, 2015

Choose a reason for hiding this comment

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

nit: nonEmpty -> isEmpty

tdas changed the title ~~[SPARK-7553] [STREAMING] Added methods to maintain a singleton StreamingContext~~ [SPARK-7553] [STREAMING][WIP] Added methods to maintain a singleton StreamingContext May 12, 2015

Fixing test bug

3884a25

Merge remote-tracking branch 'apache-github/master' into SPARK-7553

634db5d

Fixed test

64706c9

Added more unit tests

a797171

Fixed style

731c9a1

tdas changed the title ~~[SPARK-7553] [STREAMING][WIP] Added methods to maintain a singleton StreamingContext~~ [SPARK-7553] [STREAMING] Added methods to maintain a singleton StreamingContext May 12, 2015

asfgit closed this in 00e7b09 May 12, 2015

[SPARK-7553] [STREAMING] Added methods to maintain a singleton StreamingContext #6070

[SPARK-7553] [STREAMING] Added methods to maintain a singleton StreamingContext #6070

Uh oh!

Conversation

tdas commented May 12, 2015

Uh oh!

tdas commented May 12, 2015

Uh oh!

AmplabJenkins commented May 12, 2015

Uh oh!

AmplabJenkins commented May 12, 2015

Uh oh!

SparkQA commented May 12, 2015

Uh oh!

SparkQA commented May 12, 2015

Uh oh!

AmplabJenkins commented May 12, 2015

Uh oh!

AmplabJenkins commented May 12, 2015

Uh oh!

pwendell May 12, 2015

Choose a reason for hiding this comment

Uh oh!

tdas May 12, 2015

Choose a reason for hiding this comment

Uh oh!

pwendell commented May 12, 2015

Uh oh!

zsxwing May 12, 2015

Choose a reason for hiding this comment

Uh oh!

tdas commented May 12, 2015

Uh oh!

AmplabJenkins commented May 12, 2015

Uh oh!

AmplabJenkins commented May 12, 2015

Uh oh!

SparkQA commented May 12, 2015

Uh oh!

SparkQA commented May 12, 2015

Uh oh!

AmplabJenkins commented May 12, 2015

Uh oh!

AmplabJenkins commented May 12, 2015

Uh oh!

pwendell commented May 12, 2015

Uh oh!

AmplabJenkins commented May 12, 2015

Uh oh!

AmplabJenkins commented May 12, 2015

Uh oh!

SparkQA commented May 12, 2015

Uh oh!

zsxwing commented May 12, 2015

Uh oh!

AmplabJenkins commented May 12, 2015

Uh oh!

AmplabJenkins commented May 12, 2015

Uh oh!

AmplabJenkins commented May 12, 2015

Uh oh!

AmplabJenkins commented May 12, 2015

Uh oh!

SparkQA commented May 12, 2015

Uh oh!

SparkQA commented May 12, 2015

Uh oh!

AmplabJenkins commented May 12, 2015

Uh oh!

AmplabJenkins commented May 12, 2015

Uh oh!

AmplabJenkins commented May 12, 2015

Uh oh!

AmplabJenkins commented May 12, 2015

Uh oh!

SparkQA commented May 12, 2015

Uh oh!

harishreedharan commented May 12, 2015

Uh oh!

SparkQA commented May 12, 2015