Zakelly commented on a change in pull request #16341:
URL: https://github.com/apache/flink/pull/16341#discussion_r664208282



##########
File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/state/changelog/StateChangelogStorageLoader.java
##########
@@ -28,16 +34,51 @@
 /** A thin wrapper around {@link PluginManager} to load {@link 
StateChangelogStorage}. */
 @Internal
 public class StateChangelogStorageLoader {
-    private final PluginManager pluginManager;
 
-    public StateChangelogStorageLoader(PluginManager pluginManager) {
-        this.pluginManager = pluginManager;
+    private static final Logger LOG = 
LoggerFactory.getLogger(StateChangelogStorageLoader.class);
+
+    /**
+     * Mapping of state changelog storage identifier to the corresponding 
storage factories,
+     * populated in {@link 
StateChangelogStorageLoader#initialize(PluginManager)}.
+     */
+    private static final HashMap<String, StateChangelogStorageFactory>
+            STATE_CHANGELOG_STORAGE_FACTORIES = new HashMap<>();
+
+    static {
+        // Guarantee to trigger once.
+        initialize(null);
     }
 
-    @SuppressWarnings({"rawtypes"})
-    public Iterator<StateChangelogStorage> load() {
-        return concat(
-                pluginManager.load(StateChangelogStorage.class),
-                ServiceLoader.load(StateChangelogStorage.class).iterator());
+    public static void initialize(PluginManager pluginManager) {
+        STATE_CHANGELOG_STORAGE_FACTORIES.clear();
+        Iterator<StateChangelogStorageFactory> iterator =
+                pluginManager == null
+                        ? 
ServiceLoader.load(StateChangelogStorageFactory.class).iterator()
+                        : concat(
+                                
pluginManager.load(StateChangelogStorageFactory.class),
+                                
ServiceLoader.load(StateChangelogStorageFactory.class).iterator());
+        iterator.forEachRemaining(
+                factory ->
+                        STATE_CHANGELOG_STORAGE_FACTORIES.putIfAbsent(
+                                factory.getIdentifier().toLowerCase(), 
factory));

Review comment:
       We do ```clear()``` before load and ```putIfAbsent```, so initializing 
twice won't cause duplicated factories. Duplicated factories occur when 
```PluginManager``` produces several factories with same name or 
```ServiceLoader``` loads one with same name. This may happen when users put 
several jars in ```lib```.
   Since JVM will load the first class from the first jar it finds, and the 
later ones with same identifiers are ignored, we should do the same. However, I 
do agree we remind user with a WARN message.




-- 
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: issues-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to