ankitsultana commented on code in PR #15562:
URL: https://github.com/apache/pinot/pull/15562#discussion_r2047732275
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/exchange/SingletonExchange.java:
##########
@@ -38,9 +37,7 @@ class SingletonExchange extends BlockExchange {
SingletonExchange(List<SendingMailbox> sendingMailboxes, BlockSplitter
splitter,
Function<List<SendingMailbox>, Integer> statsIndexChooser) {
super(sendingMailboxes, splitter, statsIndexChooser);
- Preconditions.checkArgument(
- sendingMailboxes.size() == 1 && sendingMailboxes.get(0) instanceof
InMemorySendingMailbox,
- "Expect single InMemorySendingMailbox for SingletonExchange");
+ Preconditions.checkArgument(sendingMailboxes.size() == 1, "Expect single
mailbox in Singleton Exchange");
Review Comment:
Singleton from a definition perspective means that there's a single
receiver, and hence there should be only 1 sending mailbox. It doesn't really
mean local. Data could be sent across different instances too (e.g. in lite
mode we may send data to the broker from all of the servers, and all of the
servers in that case would be using the SingletonExchange)
Our current Runtime is already **not** tying SingletonExchange with
InMemorySendingMailbox. We have tied Singleton with Local only in the planner.
With my changes we'll get rid of that too and use Singleton correctly.
--
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]
