[GitHub] [kafka] ableegoldman commented on a change in pull request #8929: KAFKA-4996: Fix findbugs multithreaded correctness warnings for streams

[GitBox <gi...@apache.org>]

ableegoldman commented on a change in pull request #8929:
URL: https://github.com/apache/kafka/pull/8929#discussion_r445872345



##########
File path: streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsMetadataState.java
##########
@@ -50,7 +51,7 @@
     private final HostInfo thisHost;
     private List<StreamsMetadata> allMetadata = Collections.emptyList();
     private Cluster clusterMetadata;
-    private StreamsMetadata localMetadata;
+    private AtomicReference<StreamsMetadata> localMetadata;

Review comment:
       We need to actually initialize this now, although we can just initialize the value to null. ie
   ```
   private AtomicReference<StreamsMetadata> localMetadata = new AtomicReference<>(null);
   ```

##########
File path: streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsMetadataState.java
##########
@@ -226,7 +227,7 @@ public StreamsMetadata getLocalMetadata() {
             if (thisHost.equals(UNKNOWN_HOST)) {
                 return new KeyQueryMetadata(allMetadata.get(0).hostInfo(), Collections.emptySet(), -1);
             }
-            return new KeyQueryMetadata(localMetadata.hostInfo(), Collections.emptySet(), -1);
+            return new KeyQueryMetadata(localMetadata.get().hostInfo(), Collections.emptySet(), -1);

Review comment:
       Yeah good catch, see above

##########
File path: gradle/spotbugs-exclude.xml
##########
@@ -348,14 +348,11 @@ For a detailed description of spotbugs bug categories, see https://spotbugs.read
     </Match>
 
     <Match>
-        <!-- TODO: fix this (see KAFKA-4996) -->
-        <Or>
-            <Package name="org.apache.kafka.streams.state.internals"/>
+        <!-- Suppress warning about a value that gets initialized
+             before any other threads are created. -->
             <Package name="org.apache.kafka.streams.processor.internals"/>
-            <Package name="org.apache.kafka.streams.processor"/>
-            <Package name="org.apache.kafka.streams"/>
-        </Or>
-        <Bug pattern="IS2_INCONSISTENT_SYNC"/>
+            <Source name="InternalTopologyBuilder.java"/>
+            <Bug pattern="IS2_INCONSISTENT_SYNC"/>

Review comment:
       It's the `applicationId` -- I guess we should specify in the comment since both you and Boyang asked




----------------------------------------------------------------
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.

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