ankitsultana commented on code in PR #15762:
URL: https://github.com/apache/pinot/pull/15762#discussion_r2085347767
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/mailbox/channel/MailboxContentObserver.java:
##########
@@ -49,19 +49,24 @@ public class MailboxContentObserver implements
StreamObserver<MailboxContent> {
private final StreamObserver<MailboxStatus> _responseObserver;
private final List<ByteBuffer> _mailboxBuffers;
+ private transient String _mailboxId;
private transient ReceivingMailbox _mailbox;
- public MailboxContentObserver(MailboxService mailboxService,
StreamObserver<MailboxStatus> responseObserver) {
+ public MailboxContentObserver(
+ MailboxService mailboxService, String mailboxId,
StreamObserver<MailboxStatus> responseObserver) {
_mailboxService = mailboxService;
+ _mailboxId = mailboxId;
Review Comment:
Would it be cleaner if we initialize the `_mailbox` here if the mailboxId is
not empty/null? The onNext and onError methods would remain unchanged then.
That would also mean that as soon as the sender makes the connection, the
receiver MailboxService is immediately signaled about the existence of this
mailbox, which is ideal in my opinion.
Right now we have null checks for both mailboxId and mailbox which is
slightly complicated. Their initialization is linked anyways, so might be best
to init it in the ctor itself
--
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]
