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

SPARK-5425: Use synchronised methods in system properties to create SparkConf #4222

Closed

Conversation

jacek-lewandowski
Copy link
Contributor

SPARK-5425: Fixed usages of system properties

This patch fixes few problems caused by the fact that the Scala wrapper over system properties is not thread-safe and is basically invalid because it doesn't take into account the default values which could have been set in the properties object. The problem is fixed by modifying Utils.getSystemProperties method so that it uses stringPropertyNames method of the Properties class, which is thread-safe (internally it creates a defensive copy in a synchronized method) and returns keys of the properties which were set explicitly and which are defined as defaults.
The other related problem, which is fixed here. was in ResetSystemProperties mix-in. It created a copy of the system properties in the wrong way.

This patch also introduces a test case for thread-safeness of SparkConf creation.

Refer to the discussion in #4220 for more details.

@SparkQA
Copy link

SparkQA commented Jan 27, 2015

Test build #26160 has started for PR 4222 at commit 51987d2.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Jan 27, 2015

Test build #26160 has finished for PR 4222 at commit 51987d2.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class DenseMatrix(

@AmplabJenkins
Copy link

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

@andrewor14
Copy link
Contributor

retest this please

@SparkQA
Copy link

SparkQA commented Jan 28, 2015

Test build #26248 has started for PR 4222 at commit 51987d2.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Jan 28, 2015

Test build #26248 has finished for PR 4222 at commit 51987d2.

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

@AmplabJenkins
Copy link

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

@jacek-lewandowski
Copy link
Contributor Author

Explanation of the second commit is in #4220

@SparkQA
Copy link

SparkQA commented Jan 29, 2015

Test build #26315 has started for PR 4222 at commit 00f11a1.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Jan 29, 2015

Test build #26315 has finished for PR 4222 at commit 00f11a1.

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

@AmplabJenkins
Copy link

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

@SparkQA
Copy link

SparkQA commented Jan 29, 2015

Test build #26319 has started for PR 4222 at commit 4b6f366.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Jan 29, 2015

Test build #26319 has finished for PR 4222 at commit 4b6f366.

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

@AmplabJenkins
Copy link

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

@SparkQA
Copy link

SparkQA commented Jan 29, 2015

Test build #26322 has started for PR 4222 at commit 8faf2ea.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Jan 29, 2015

Test build #26322 has finished for PR 4222 at commit 8faf2ea.

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

@AmplabJenkins
Copy link

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

@SparkQA
Copy link

SparkQA commented Feb 2, 2015

Test build #26500 has started for PR 4222 at commit 03da61b.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Feb 2, 2015

Test build #26500 has finished for PR 4222 at commit 03da61b.

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

@AmplabJenkins
Copy link

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

@JoshRosen
Copy link
Contributor

I've merged this into master (1.3.0). Thanks!

@asfgit asfgit closed this in 5a55261 Feb 2, 2015
@pwendell
Copy link
Contributor

pwendell commented Jun 4, 2015

@jacek-lewandowski can you close this issue? It didnt' close properly because of the way github auto-closes patches into release branches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants