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