Copilot commented on code in PR #22595:
URL: https://github.com/apache/kafka/pull/22595#discussion_r3450899051


##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/StreamsRebalanceData.java:
##########
@@ -355,12 +358,14 @@ public StreamsRebalanceData(final UUID processId,
                                 final Optional<HostInfo> endpoint,
                                 final Optional<String> rackId,
                                 final Map<String, Subtopology> subtopologies,
-                                final Map<String, String> clientTags) {
+                                final Map<String, String> clientTags,
+                                final Supplier<Map<TaskId, Long>> 
taskOffsetSum) {
         this.processId = Objects.requireNonNull(processId, "Process ID cannot 
be null");
         this.endpoint = Objects.requireNonNull(endpoint, "Endpoint cannot be 
null");
         this.rackId = Objects.requireNonNull(rackId, "Rack ID cannot be null");
         this.subtopologies = Map.copyOf(Objects.requireNonNull(subtopologies, 
"Subtopologies cannot be null"));
         this.clientTags = Map.copyOf(Objects.requireNonNull(clientTags, 
"Client tags cannot be null"));
+        this.taskOffsetSum = Objects.requireNonNull(taskOffsetSum, 
"TaskOffsetSum cannot be null");

Review Comment:
   The null-check message "TaskOffsetSum cannot be null" is inconsistent with 
the other argument messages in this constructor (sentence case with spaces). 
Consider renaming it to a clearer message.



##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/StateDirectory.java:
##########
@@ -310,6 +310,10 @@ public Map<TaskId, Long> taskOffsetSums(final Set<TaskId> 
tasks) {
                 .collect(Collectors.toMap(Map.Entry::getKey, 
Map.Entry::getValue));
     }
 
+    public Map<TaskId, Long> taskOffsetSums() {
+        return taskOffsetSums;
+    }

Review Comment:
   `taskOffsetSums()` returns the internal `ConcurrentHashMap`, which allows 
external callers to mutate/clear the state directory’s offset-sum tracking. 
This breaks encapsulation and can lead to hard-to-debug corruption. Return an 
unmodifiable view instead (callers that need a snapshot can still copy it 
themselves).



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

Reply via email to