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: commits-unsubscr...@pinot.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to