[GitHub] [kafka] ableegoldman commented on a change in pull request #10000: KAFKA-9274: handle TimeoutException on task reset

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

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



##########
File path: streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamTask.java
##########
@@ -227,6 +230,27 @@ public void initializeIfNeeded() {
         }
     }
 
+    private void initOffsetsIfNeeded(final java.util.function.Consumer<Set<TopicPartition>> offsetResetter) {

Review comment:
       Hm...I'm not necessarily that concerned about calling `mainConsumer.committed` twice in rare cases (although maybe that would not be so good, since those rare cases happen to be those in which this is probably more likely to time out, right?)
   But personally, just coming into this code from the outside, it's super confusing to have two different methods for initializing the offsets. It seems more convoluted that way, to me. Also maybe I am missing some context here but why do we call `initOffsetsIfNeeded`  from `initializeIfNeeded` rather than from `completeRestoration` in the first place? We don't need to initialize main consumer offsets until it transitions to running




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