walterddr commented on code in PR #9773: URL: https://github.com/apache/pinot/pull/9773#discussion_r1022137874
########## pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/MailboxSendOperator.java: ########## @@ -49,57 +47,67 @@ */ public class MailboxSendOperator extends BaseOperator<TransferableBlock> { private static final Logger LOGGER = LoggerFactory.getLogger(MailboxSendOperator.class); + private static final String EXPLAIN_NAME = "MAILBOX_SEND"; private static final Set<RelDistribution.Type> SUPPORTED_EXCHANGE_TYPE = ImmutableSet.of(RelDistribution.Type.SINGLETON, RelDistribution.Type.RANDOM_DISTRIBUTED, RelDistribution.Type.BROADCAST_DISTRIBUTED, RelDistribution.Type.HASH_DISTRIBUTED); - private static final Random RANDOM = new Random(); - // TODO: Deduct this value via grpc config maximum byte size; and make it configurable with override. - // TODO: Max block size is a soft limit. only counts fixedSize datatable byte buffer - private static final int MAX_MAILBOX_CONTENT_SIZE_BYTES = 4 * 1024 * 1024; - - private final List<ServerInstance> _receivingStageInstances; - private final RelDistribution.Type _exchangeType; - private final KeySelector<Object[], Object[]> _keySelector; - private final String _serverHostName; - private final int _serverPort; - private final long _jobId; - private final int _stageId; - private final MailboxService<TransferableBlock> _mailboxService; - private final DataSchema _dataSchema; - private final boolean _isLeafStageSender; - private Operator<TransferableBlock> _dataTableBlockBaseOperator; - - public MailboxSendOperator(MailboxService<TransferableBlock> mailboxService, DataSchema dataSchema, + + private final Operator<TransferableBlock> _dataTableBlockBaseOperator; + private final BlockExchange _exchange; + + @VisibleForTesting + interface BlockExchangeFactory { + BlockExchange build(MailboxService<TransferableBlock> mailboxService, List<MailboxIdentifier> destinations, + RelDistribution.Type exchange, KeySelector<Object[], Object[]> selector, BlockSplitter splitter); + } + + @VisibleForTesting + interface MailboxIdGenerator { + MailboxIdentifier generate(ServerInstance server); + } + + public MailboxSendOperator(MailboxService<TransferableBlock> mailboxService, Operator<TransferableBlock> dataTableBlockBaseOperator, List<ServerInstance> receivingStageInstances, RelDistribution.Type exchangeType, KeySelector<Object[], Object[]> keySelector, String hostName, int port, long jobId, int stageId, boolean isLeafStageSender) { - _dataSchema = dataSchema; - _mailboxService = mailboxService; + this(mailboxService, dataTableBlockBaseOperator, receivingStageInstances, exchangeType, keySelector, + isLeafStageSender, server -> toMailboxId(server, jobId, stageId, hostName, port), BlockExchange::getExchange); + } + + @VisibleForTesting + MailboxSendOperator(MailboxService<TransferableBlock> mailboxService, + Operator<TransferableBlock> dataTableBlockBaseOperator, List<ServerInstance> receivingStageInstances, + RelDistribution.Type exchangeType, KeySelector<Object[], Object[]> keySelector, + boolean isLeafStageSender, MailboxIdGenerator mailboxIdGenerator, BlockExchangeFactory blockExchangeFactory) { _dataTableBlockBaseOperator = dataTableBlockBaseOperator; - _exchangeType = exchangeType; - if (_exchangeType == RelDistribution.Type.SINGLETON) { + + List<MailboxIdentifier> receivingMailboxes; + if (exchangeType == RelDistribution.Type.SINGLETON) { + // TODO: this logic should be moved into SingletonExchange Review Comment: good question. i guess it requires a separate clean up. -- 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