FrankChen021 commented on code in PR #20424:
URL: https://github.com/apache/druid/pull/20424#discussion_r4412546891
##########
extensions-contrib/rabbit-stream-indexing-service/src/main/java/org/apache/druid/indexing/rabbitstream/RabbitStreamRecordSupplier.java:
##########
@@ -347,6 +348,7 @@ public void handle(MessageHandler.Context context, Message
message)
}
}
catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
Review Comment:
[P1] Restore the flag after consumer cleanup
**Finding:** When BlockingQueue.offer throws InterruptedException, this line
sets the current thread's interrupt status before stopBackgroundFetch(). That
method's stateSemaphore.acquire() is interruptible, so it immediately throws
and returns without closing consumers or setting isRunning to false. The
supplier can therefore keep consuming after the buffer interruption and leave
resources active during shutdown or reassignment.
**Suggestion:** Perform stopBackgroundFetch while the interrupt is cleared
and restore the status after cleanup, or make the cleanup acquire/retry
uninterruptibly before returning.
##########
extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/CommonCacheNotifier.java:
##########
@@ -141,6 +141,7 @@ public void start()
LOG.debug(callerName + ":Received responses for cache update
notifications.");
}
catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
Review Comment:
[P1] Propagate interruptions from response waits
**Finding:** The added restoration only handles interruption from
updateQueue.take(). Futures.allAsList(futures).get(...) is inside the inner
catch (Exception) at lines 137-139, so an InterruptedException while waiting
for notification responses is caught there, its flag is cleared, and execution
returns to the while loop; the outer catch at this line never runs. shutdownNow
can therefore leave the notifier thread blocking on the next updateQueue.take().
**Suggestion:** Handle InterruptedException separately in the inner
response-wait catch and restore or rethrow it so the outer loop exits during
shutdown.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
