Skip to content

Commit 8345db4

Browse files
committed
[SPARK-52559] Synchronize SparkOperatorConfManager.getValue
### What changes were proposed in this pull request? This PR aims to fix a concurrency issue by synchronizing `SparkOperatorConfManager.getValue`. ### Why are the changes needed? `configOverrides` is supposed by being protected by `this` at `refresh` method. https://github.com/apache/spark-kubernetes-operator/blob/35214b00c777c724d9cd40208601839824811346/spark-operator/src/main/java/org/apache/spark/k8s/operator/config/SparkOperatorConfManager.java#L71-L76 However, currently `getValue` can access an empty value while `refresh` invokes `this.configOverrides = new Properties();` statement because `getValue` method is not synchronized properly. https://github.com/apache/spark-kubernetes-operator/blob/35214b00c777c724d9cd40208601839824811346/spark-operator/src/main/java/org/apache/spark/k8s/operator/config/SparkOperatorConfManager.java#L62-L65 ### Does this PR introduce _any_ user-facing change? Yes, this is a bug fix. ### How was this patch tested? Manual review. (It's a little difficult to add a concurrency failure test case). ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#256 from dongjoon-hyun/SPARK-52559. Authored-by: Dongjoon Hyun <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
1 parent 35214b0 commit 8345db4

File tree

1 file changed

+4
-2
lines changed

1 file changed

+4
-2
lines changed

spark-operator/src/main/java/org/apache/spark/k8s/operator/config/SparkOperatorConfManager.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,10 @@ protected SparkOperatorConfManager() {
6060
}
6161

6262
public String getValue(String key) {
63-
String currentValue = configOverrides.getProperty(key);
64-
return StringUtils.isEmpty(currentValue) ? getInitialValue(key) : currentValue;
63+
synchronized (this) {
64+
String currentValue = configOverrides.getProperty(key);
65+
return StringUtils.isEmpty(currentValue) ? getInitialValue(key) : currentValue;
66+
}
6567
}
6668

6769
public String getInitialValue(String key) {

0 commit comments

Comments
 (0)