lucasbru commented on code in PR #22951:
URL: https://github.com/apache/kafka/pull/22951#discussion_r3977266791
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/StateManagerUtil.java:
##########
@@ -171,6 +171,20 @@ static void closeStateManager(final Logger log,
final TaskId id = stateMgr.taskId();
log.trace("Closing state manager for {} task {}", taskType, id);
+ // Nothing to close: the state manager has no registered stores (e.g.
they were already
+ // closed during a previous hand-off, or the task is stateless). Skip
acquiring the
+ // per-task state-directory lock so we don't emit a misleading
lock-contention warning
+ // for a benign back-to-back rebalance. We must still run the close
path when we intend
+ // to wipe the state store, since that deletes the on-disk task
directory.
+ if (!wipeStateStore && !stateMgr.hasRegisteredStores()) {
Review Comment:
That's the right instinct - it avoids the stateful hand-off gap and keeps
close()/maybeDowngradeOffsets() running unconditionally. But I don't think it
actually preserves the WARN for "genuinely unexpected failures" the way the
description says: lock() returns false whenever lockedTasksToOwner has a
different thread recorded as owner, and that entry only clears when the real
owner calls unlock() itself. So a lock that's stuck because the owning thread
died or hung (never unlocks) would also show lockOwner() != null && !=
currentThread forever, and would now always log at DEBUG - that's exactly the
case we want a WARN for. And since lock() and lockOwner() are two separate
calls rather than one atomic check, if the real owner unlocks in the gap
between them, lockOwner() returns null and we fall through to WARN for what was
actually a normal hand-off. Feels like the DEBUG/WARN split ends up close to
backwards from what we want.
Separate question: now that the early-return guard is gone, is
`hasRegisteredStores()` on ProcessorStateManager still used anywhere? Looks
like it might be dead code now.
--
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]
