yashmayya commented on code in PR #14507:
URL: https://github.com/apache/pinot/pull/14507#discussion_r1895761098
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/QueryEnvironment.java:
##########
@@ -484,6 +484,18 @@ default boolean defaultInferPartitionHint() {
return CommonConstants.Broker.DEFAULT_INFER_PARTITION_HINT;
}
+ /**
+ * Whether to use spools or not.
+ *
+ * This is treated as the default value for the broker and it is expected
to be obtained from a Pinot configuration.
+ * This default value can be always overridden at query level by the query
option
+ * {@link CommonConstants.Broker.Request.QueryOptionKey#USE_SPOOLS}.
+ */
+ @Value.Default
+ default boolean defaultUseSpools() {
+ return CommonConstants.Broker.DEFAULT_OF_SPOOLS;
+ }
Review Comment:
I see, thanks for the pointer.
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java:
##########
@@ -368,6 +368,13 @@ public static class Broker {
public static final String CONFIG_OF_INFER_PARTITION_HINT =
"pinot.broker.multistage.infer.partition.hint";
public static final boolean DEFAULT_INFER_PARTITION_HINT = false;
+ /**
+ * Whether to use spools in multistage query engine by default.
+ * This value can always be overridden by {@link
Request.QueryOptionKey#USE_SPOOLS} query option
+ */
+ public static final String CONFIG_OF_SPOOLS =
"pinot.broker.multistage.spools";
Review Comment:
Fair enough, I'm convinced 😄
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/exchange/BlockExchange.java:
##########
@@ -84,8 +92,19 @@ public boolean send(TransferableBlock block)
if (block.isSuccessfulEndOfStreamBlock()) {
// Send metadata to only one randomly picked mailbox, and empty EOS
block to other mailboxes
int numMailboxes = _sendingMailboxes.size();
- int mailboxIdToSendMetadata =
ThreadLocalRandom.current().nextInt(numMailboxes);
- assert block.getQueryStats() != null;
+ int mailboxIdToSendMetadata;
+ if (block.getQueryStats() != null) {
+ mailboxIdToSendMetadata =
ThreadLocalRandom.current().nextInt(numMailboxes);
+ if (LOGGER.isTraceEnabled()) {
+ LOGGER.trace("Sending EOS metadata. Only mailbox #{} will get
stats", mailboxIdToSendMetadata);
+ }
+ } else {
+ if (LOGGER.isTraceEnabled()) {
+ LOGGER.trace("Sending EOS metadata. No stat will be sent");
+ }
+ // this may happen when the block exchange is itself used as a sending
mailbox, like when using spools
+ mailboxIdToSendMetadata = -1;
Review Comment:
Wow thanks, this makes things very clear 😄
Would be great to include this in the dev docs if / when we have that!
--
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
