mjsax commented on code in PR #13925:
URL: https://github.com/apache/kafka/pull/13925#discussion_r1247273467
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/TaskManager.java:
##########
@@ -1141,25 +1141,30 @@ public Map<TaskId, Long> getTaskOffsetSums() {
// Not all tasks will create directories, and there may be directories
for tasks we don't currently own,
// so we consider all tasks that are either owned or on disk. This
includes stateless tasks, which should
// just have an empty changelogOffsets map.
- for (final TaskId id : union(HashSet::new, lockedTaskDirectories,
tasks.allTaskIds())) {
Review Comment:
It seems with the state updated enabled, `tasks` is actually only containing
"running tasks". It seems appropriate the rename this variable to
`runningTasks` (can also happen in a follow up PR).
I am actually also wondering if we still need this `Tasks` container any
longer to begin with? The purpose of the `Tasks` container was to simplify
`TaskManager` that manages both active and standby tasks. With the state
updated (from my understanding) the `TaskManager` only manages active tasks,
while standby tasks will be owned by the state-updated-thread (would it still
be useful for the state-updated-thread to use `Tasks` container, given that is
also own active tasks as long as they are restoring?
--
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
