cadonna commented on code in PR #13925:
URL: https://github.com/apache/kafka/pull/13925#discussion_r1250497598
##########
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).
The old code path with disabled state updater does still exist and we can
disable the state updater if we encounter a major bug after releasing. So, I
would postpone such renamings to the removal of the old code path.
> I am actually also wondering if we still need this Tasks container any
longer to begin with?
I would keep it, because it allows to cleanly set a specific state of the
task manager in unit tests. Anyways, I would wait for the upcoming thread
refactoring to make such changes.
> 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?
I do not think so, since access by the state updater would imply that the
tasks registry (aka tasks container) needs to be concurrently accessed. For
this reason, we defined a invariant, that a task can only be owned either by
the stream thread or by the state updater, but not both. Sharing the tasks
registry between stream thread and state updater would break that invariant. If
you meant to use an separate instance of the tasks registry for the state
updater, that would be not useful IMO.
--
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
