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