C0urante commented on code in PR #13181: URL: https://github.com/apache/kafka/pull/13181#discussion_r1093398759
########## connect/mirror/src/main/java/org/apache/kafka/connect/mirror/MirrorSourceTask.java: ########## @@ -87,6 +90,7 @@ public MirrorSourceTask() {} @Override public void start(Map<String, String> props) { MirrorSourceTaskConfig config = new MirrorSourceTaskConfig(props); + pendingOffsetSyncs.clear(); Review Comment: SpotBugs wasn't giving me any grief. But it is inconsistent. I wanted there to be a single place where `pendingOffsetSyncs` was declared in order to guarantee that it was a `LinkedHashMap` with the ordering logic that we needed. After thinking it over a bit, I think it should be fine to change the left-hand type of the field to `LinkedHashMap` and use the no-args constructor to instantiate it in two places instead of one. With that, it's no different than the `consumerAccess` semaphore. EDIT: Actually, with that approach, SpotBugs does start failing the build. So I think we can leave this as-is for now instead of adding synchronized blocks in `start` and/or a constructor. ########## connect/mirror/src/main/java/org/apache/kafka/connect/mirror/MirrorSourceTask.java: ########## @@ -87,6 +90,7 @@ public MirrorSourceTask() {} @Override public void start(Map<String, String> props) { MirrorSourceTaskConfig config = new MirrorSourceTaskConfig(props); + pendingOffsetSyncs.clear(); Review Comment: SpotBugs wasn't giving me any grief. But it is inconsistent. I wanted there to be a single place where `pendingOffsetSyncs` was declared in order to guarantee that it was a `LinkedHashMap` with the ordering logic that we needed. After thinking it over a bit, I think it should be fine to change the left-hand type of the field to `LinkedHashMap` and use the no-args constructor to instantiate it in two places instead of one. With that, it's no different than the `consumerAccess` semaphore. EDIT: Actually, with that approach, SpotBugs does start failing the build. So I think we can leave this as-is for now instead of adding synchronized blocks in `start` and/or constructors. -- 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: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org