scwhittle commented on code in PR #33979:
URL: https://github.com/apache/beam/pull/33979#discussion_r1955749436
##########
sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/ConvertHelpers.java:
##########
@@ -85,57 +100,58 @@ public ConvertedSchemaInformation(
public static <T> ConvertedSchemaInformation<T>
getConvertedSchemaInformation(
Schema inputSchema, TypeDescriptor<T> outputType, SchemaRegistry
schemaRegistry) {
- ConvertedSchemaInformation<T> schemaInformation = null;
+ ConvertedSchemaInformation<T> schemaInformation;
Review Comment:
nit: move this definition to where assigned
##########
sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/ConvertHelpers.java:
##########
@@ -57,9 +61,20 @@
"rawtypes"
})
public class ConvertHelpers {
- private static class SchemaInformationProviders {
- private static final ServiceLoader<SchemaInformationProvider> INSTANCE =
- ServiceLoader.load(SchemaInformationProvider.class);
+
+ private static final AtomicReference<List<SchemaInformationProvider>>
+ SCHEMA_INFORMATION_PROVIDERS = new AtomicReference<>();
+
+ private static List<SchemaInformationProvider>
getSchemaInformationProviders() {
+ return SCHEMA_INFORMATION_PROVIDERS.updateAndGet(
Review Comment:
I'm also not sure when providers will be registered. From ServiceLoader
docs it caches internally but looks for new things to load each time.
It seems like a smaller change to semantics to synchronize access to the
loader.
Can you add a static Object to use as a lock, annotate INSTANCE as GuardedBy
it and synchronize iteration?
--
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]
