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

jacek-lewandowski · 2015-01-27T12:17:05Z

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.

…parkConf

SparkQA · 2015-01-27T12:22:51Z

Test build #26161 has started for PR 4221 at commit 94aeacf.

This patch merges cleanly.

SparkQA · 2015-01-27T13:49:08Z

Test build #26161 has finished for PR 4221 at commit 94aeacf.

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

AmplabJenkins · 2015-01-27T13:49:11Z

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

SparkQA · 2015-01-29T11:17:47Z

Test build #26312 has started for PR 4221 at commit 4a0c50d.

This patch merges cleanly.

jacek-lewandowski · 2015-01-29T11:21:00Z

Explanation of the second commit is in #4220

SparkQA · 2015-01-29T13:12:33Z

Test build #26312 has finished for PR 4221 at commit 4a0c50d.

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

AmplabJenkins · 2015-01-29T13:12:37Z

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

…roperties trait

SparkQA · 2015-01-29T17:42:54Z

Test build #26321 has started for PR 4221 at commit 01dd5cb.

This patch merges cleanly.

SparkQA · 2015-01-29T19:35:34Z

Test build #26321 has finished for PR 4221 at commit 01dd5cb.

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

AmplabJenkins · 2015-01-29T19:35:37Z

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

… system properties - explicit + defaults

SparkQA · 2015-02-02T08:22:56Z

Test build #26498 has started for PR 4221 at commit 87951a2.

This patch merges cleanly.

SparkQA · 2015-02-02T10:09:08Z

Test build #26498 has finished for PR 4221 at commit 87951a2.

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

AmplabJenkins · 2015-02-02T10:09:11Z

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

JoshRosen · 2015-02-02T22:11:32Z

Let's leave this PR open for a few days until we complete the 1.2.1 release voting, then I'll merge it into branch-1.2 (1.2.2).

JoshRosen · 2015-02-08T01:21:22Z

Jenkins, retest this please.

SparkQA · 2015-02-08T01:22:59Z

Test build #27019 has started for PR 4221 at commit 87951a2.

This patch merges cleanly.

JoshRosen · 2015-02-08T01:23:12Z

I'm going to merge this into branch-1.2 (1.2.2) now since I'd like to see if resolves https://issues.apache.org/jira/browse/SPARK-5227.

SparkQA · 2015-02-08T03:14:49Z

Test build #27019 has finished for PR 4221 at commit 87951a2.

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

AmplabJenkins · 2015-02-08T03:14:54Z

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

…parkConf 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. Author: Jacek Lewandowski <lewandowski.jacek@gmail.com> Closes #4221 from jacek-lewandowski/SPARK-5425-1.2 and squashes the following commits: 87951a2 [Jacek Lewandowski] SPARK-5425: Modified Utils.getSystemProperties to return a map of all system properties - explicit + defaults 01dd5cb [Jacek Lewandowski] SPARK-5425: Use SerializationUtils to save properties in ResetSystemProperties trait 94aeacf [Jacek Lewandowski] SPARK-5425: Use synchronised methods in system properties to create SparkConf

JoshRosen · 2015-02-08T03:17:00Z

I've merged this into branch-1.2 (1.2.2). Thanks!

@jacek-lewandowski, you can close this PR now that it's been merged, since GitHub can't auto close.

srowen · 2015-02-18T12:46:40Z

Mind closing this PR @jacek-lewandowski ?

pwendell · 2015-02-18T21:35:54Z

Yeah our auto-close doesn't work on PR's into release branches like this.

SPARK-5425: Use synchronised methods in system properties to create S…

94aeacf

…parkConf

jacek-lewandowski force-pushed the SPARK-5425-1.2 branch from 4a0c50d to 01dd5cb Compare January 29, 2015 17:38

SPARK-5425: Use SerializationUtils to save properties in ResetSystemP…

01dd5cb

…roperties trait

SPARK-5425: Modified Utils.getSystemProperties to return a map of all…

87951a2

… system properties - explicit + defaults

asfgit closed this in 46462ff Feb 22, 2015

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

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

Uh oh!

Conversation

jacek-lewandowski commented Jan 27, 2015

Uh oh!

SparkQA commented Jan 27, 2015

Uh oh!

SparkQA commented Jan 27, 2015

Uh oh!

AmplabJenkins commented Jan 27, 2015

Uh oh!

SparkQA commented Jan 29, 2015

Uh oh!

jacek-lewandowski commented Jan 29, 2015

Uh oh!

SparkQA commented Jan 29, 2015

Uh oh!

AmplabJenkins commented Jan 29, 2015

Uh oh!

SparkQA commented Jan 29, 2015

Uh oh!

SparkQA commented Jan 29, 2015

Uh oh!

AmplabJenkins commented Jan 29, 2015

Uh oh!

SparkQA commented Feb 2, 2015

Uh oh!

SparkQA commented Feb 2, 2015

Uh oh!

AmplabJenkins commented Feb 2, 2015

Uh oh!

JoshRosen commented Feb 2, 2015

Uh oh!

JoshRosen commented Feb 8, 2015

Uh oh!

SparkQA commented Feb 8, 2015

Uh oh!

JoshRosen commented Feb 8, 2015

Uh oh!

SparkQA commented Feb 8, 2015

Uh oh!

AmplabJenkins commented Feb 8, 2015

Uh oh!

JoshRosen commented Feb 8, 2015

Uh oh!

srowen commented Feb 18, 2015

Uh oh!

pwendell commented Feb 18, 2015

Uh oh!

Uh oh!