ankitsultana commented on code in PR #10099:
URL: https://github.com/apache/pinot/pull/10099#discussion_r1072507912
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/mailbox/channel/MailboxContentStreamObserver.java:
##########
@@ -75,7 +98,9 @@ public MailboxContentStreamObserver(GrpcMailboxService
mailboxService,
StreamObserver<Mailbox.MailboxStatus> responseObserver, boolean
isEnabledFeedback) {
_mailboxService = mailboxService;
_responseObserver = responseObserver;
- _receivingBuffer = new
ArrayBlockingQueue<>(DEFAULT_MAILBOX_QUEUE_CAPACITY);
+ // TODO: Replace unbounded queue with bounded queue when we have
backpressure in place.
+ // It is possible this will create high memory pressure since we have
memory leak issues.
+ _receivingBuffer = new LinkedBlockingQueue();
Review Comment:
Making the queue unbounded means that if the sender sends 20 blocks then all
of them would be leaked to memory. At present after the 5th block that's sent,
the subsequent sent blocks won't be added to the queue and hopefully not leaked
either.
The blocks are leaked because the map in Mailbox service keeps a reference
to the GrpcReceivingMailbox that keeps a reference to the
MailboxContentStreamObserver, which keeps a reference to the `_receivingBuffer`
queue.
This is based on my understanding.. lmk if there's something wrong
--
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]