gortiz commented on code in PR #14507:
URL: https://github.com/apache/pinot/pull/14507#discussion_r1895401883


##########
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:
   We would need some consistency here. We don't use it in 
`"pinot.query.multistage.explain.include.segment.plan"`, neither in 
`pinot.query.join.max.rows`. I think the only place we use it is in 
`"pinot.server.mmap.advice.default"`. 
   
   I think it is understandable that some configs defined here could be 
overriden at query time. Also, adding `default` in the name has some lateral 
impacts. For example, imagine a config that initially wasn't allowed to be 
changed at query time but in a change we allow that. The original name would 
not include `default` because it wasn't originally planned to be changed at 
query time. But once we allow that... what should we do? If we change the name 
of the property users will need to change their config files once they upgrade 
Pinot. If we don't change the name of the property we would break the 
assumption that only properties with `default` in the name can be override at 
query time.



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