snazy commented on code in PR #3755:
URL: https://github.com/apache/polaris/pull/3755#discussion_r2805754392
##########
polaris-core/src/main/java/org/apache/polaris/core/config/PolarisConfiguration.java:
##########
@@ -71,12 +72,12 @@ private static void
registerConfiguration(PolarisConfiguration<?> configuration)
}
}
}
- allConfigurations.add(configuration);
+ ALL_CONFIGURATIONS.add(configuration);
}
/** Returns a list of all PolarisConfigurations that have been registered */
public static List<PolarisConfiguration<?>> getAllConfigurations() {
- return List.copyOf(allConfigurations);
+ return List.copyOf(ALL_CONFIGURATIONS);
Review Comment:
The writes to `ALL_CONFIGURATIONS` via `registerConfiguration` are fine, but
this one can still yield wrong results (no fence/barrier here).
I suspect that the there are maybe just a handful calls to
`registerConfiguration`.
WDYT of using a copy-on-write but save the `List.copyOf()` on every read?
```java
private static volatile List<PolarisConfiguration<?>> allConfigurations =
List.of();
// The `synchronized` is only needed to to have no concurrent executions of
registerConfiguration(), not needed as a memory fence/barrier..
synchronized void registerConfiguration(...) {
var updatedConfigs = new ArrayList(allConfigurations);
// register dance
allConfigurations = List.copyOf(updatedConfigs);
}
static List<PolarisConfiguration<?>> getAllConfigurations() {
// reads the current immutable list object
return allConfigurations;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
