albertobastos commented on code in PR #15571:
URL: https://github.com/apache/pinot/pull/15571#discussion_r2067999120


##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/mailbox/GrpcSendingMailbox.java:
##########
@@ -52,24 +58,32 @@
 public class GrpcSendingMailbox implements SendingMailbox {
   private static final Logger LOGGER = 
LoggerFactory.getLogger(GrpcSendingMailbox.class);
 
+  private static final List<ByteString> EMPTY_BYTEBUFFER_LIST = 
Collections.emptyList();
   private final String _id;
   private final ChannelManager _channelManager;
   private final String _hostname;
   private final int _port;
   private final long _deadlineMs;
   private final StatMap<MailboxSendOperator.StatKey> _statMap;
   private final MailboxStatusObserver _statusObserver = new 
MailboxStatusObserver();
+  private final int _maxByteStringSize;
 
   private StreamObserver<MailboxContent> _contentObserver;
 
-  public GrpcSendingMailbox(String id, ChannelManager channelManager, String 
hostname, int port, long deadlineMs,
+  public GrpcSendingMailbox(
+      PinotConfiguration config, String id, ChannelManager channelManager, 
String hostname, int port, long deadlineMs,
       StatMap<MailboxSendOperator.StatKey> statMap) {
     _id = id;
     _channelManager = channelManager;
     _hostname = hostname;
     _port = port;
     _deadlineMs = deadlineMs;
     _statMap = statMap;
+    // so far we ensure payload is not bigger than maxBlockSize/2, we can fine 
tune this later
+    _maxByteStringSize = Math.max(config.getProperty(
+        
CommonConstants.MultiStageQueryRunner.KEY_OF_MAX_INBOUND_QUERY_DATA_BLOCK_SIZE_BYTES,
+        
CommonConstants.MultiStageQueryRunner.DEFAULT_MAX_INBOUND_QUERY_DATA_BLOCK_SIZE_BYTES
+    ) / 2, 1);

Review Comment:
   Each message sent through GRPC includes some metadata in addition to the 
payload (which is what we are splitting here). The ideal case would be to know 
the size of that metadata, so we could split the payload in chunks as big as 
possible. But for now, we just go with using "half of the established limit" 
assuming the rest of metadata will never exceed the other half.
   
   The `Math.max(N, 1)` is just protecting against the case where the limit is 
set to 1. Should never happen in production, but we are checking it during 
tests.



-- 
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