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: [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]
