Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix PR926 local properties issues in Spark Streaming like scenarios #937

Merged
merged 2 commits into from
Sep 22, 2013

Conversation

jerryshao
Copy link
Contributor

PR926 changes local properties from DynamicVariable to ThreadLocal, which cannot pass properties set in parent thread to child thread. Scenarios like Spark Streaming which separate runJob() function in child thread will always get null properties and pass to DAGScheduler. So thread local properties should be inheritable, also should be isolated in different threads.


threads.foreach(_.start())
assert(sc.getLocalProperty("test") === "parent")
assert(sc.getLocalProperty("Foo") === null)
Copy link
Member

Choose a reason for hiding this comment

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

Don't we need a barrier here to properly test this?

@kayousterhout
Copy link
Contributor

Thanks for fixing this -- sorry for not realizing this use case! The fix looks good.

@jerryshao
Copy link
Contributor Author

Hi @rxin ,thanks for your reviewing. I think it's more reasonable to add a barrier to unit test, I've already update the patch.

@rxin
Copy link
Member

rxin commented Sep 22, 2013

Jenkins, test this please.

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@AmplabJenkins
Copy link

Merged build finished.

@AmplabJenkins
Copy link

One or more automated tests failed
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/946/

@rxin
Copy link
Member

rxin commented Sep 22, 2013

Jenkins, retest this please.

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@AmplabJenkins
Copy link

Merged build finished.

@AmplabJenkins
Copy link

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/948/

rxin added a commit that referenced this pull request Sep 22, 2013
Fix PR926 local properties issues in Spark Streaming like scenarios
@rxin rxin merged commit a2ea069 into mesos:master Sep 22, 2013
@rxin
Copy link
Member

rxin commented Sep 22, 2013

We should also merge this back into 0.8 branch once 0.8 is released.

@jerryshao jerryshao deleted the localProperties-fix branch September 22, 2013 06:07
@rxin
Copy link
Member

rxin commented Sep 26, 2013

I've merged this into asf spark branch-0.8.

markhamstra pushed a commit to markhamstra/spark-old that referenced this pull request Sep 29, 2013
Fix PR926 local properties issues in Spark Streaming like scenarios
(cherry picked from commit a2ea069)

Signed-off-by: Reynold Xin <rxin@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants