mjsax commented on code in PR #21059:
URL: https://github.com/apache/kafka/pull/21059#discussion_r2590120331
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamThread.java:
##########
@@ -1066,13 +1023,6 @@ void maybeGetClientInstanceIds() {
);
}
}
-
- if (mainConsumerInstanceIdFuture.isDone()
Review Comment:
I think the condition is correct. -- But we should not remove this whole
`if`, but only remove line `&& (!stateUpdaterEnabled &&
restoreConsumerInstanceIdFuture.isDone())`.
The condition is correct, because we only care about
`restoreConsumerInstanceIdFuture.isDone()` here if state-updater is disabled.
What we do here is, we check if `StreamThread` did complete all it's futures,
and if it did, we set `fetchDeadlineClientInstanceId = -1L;` indicating we are
done.
If state-updater is enabled, it's not `StreamsThread`'s duty to complete the
`restoreConsumerInstanceIdFuture`, but it's `StateUpdater`'s duty, so we don't
care if the future is completed or not, to determine if `StreamsThread` is done
and we can reset the fetch deadline or not.
If we remove this whole block, we would never reset fetch deadline, implying
unnecessary busy work to re-check if the futures got completed -- after the
futures are completed, there is nothing to be checked any longer, and we want
to make `maybeGetClientInstanceIds` a no-op (even if it might not make a
difference in practice, as the overhead to check the futures should be small --
the intention was just to make it "clean")
--
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]
