gianm commented on code in PR #19006:
URL: https://github.com/apache/druid/pull/19006#discussion_r3143153921
##########
processing/src/main/java/org/apache/druid/frame/channel/BlockingQueueFrameChannel.java:
##########
@@ -261,7 +293,12 @@ public ListenableFuture<?> readabilityFuture()
public void close()
{
synchronized (lock) {
- queue.clear();
+ if (readerClosed) {
+ // close() should not be called twice.
+ throw DruidException.defensive("Closed, cannot call close() again");
+ }
+
+ readerClosed = true;
Review Comment:
It's true although the old code is the same way. The old code cleared the
queue here, but typically the queue is of length just 1 or 2, and it's easy for
it to fill back up again. I believe isn't a problem in practice, because in
production early-close scenarios (mainly `LIMIT`) processors are directly
canceled by `Worker#postCleanupStage`. Generally we aren't relying on the
channels themselves to propagate such information.
--
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]
