Jackie-Jiang commented on code in PR #13131: URL: https://github.com/apache/pinot/pull/13131#discussion_r1625165770
########## pinot-controller/src/main/java/org/apache/pinot/controller/ControllerConf.java: ########## @@ -316,6 +316,9 @@ private static long getRandomInitialDelayInSeconds() { public static final String DISABLE_GROOVY = "controller.disable.ingestion.groovy"; public static final boolean DEFAULT_DISABLE_GROOVY = true; + public static final String INSTANCE_POOL_AND_REPLICA_GROUP_CHECK_KEY = "instance.pool.and.replica.group.check"; Review Comment: I feel it is more user friendly if we call it `enforce.pool.based.assignment` ########## pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java: ########## @@ -128,10 +128,10 @@ private TableConfigUtils() { RoutingConfig.MULTI_STAGE_REPLICA_GROUP_SELECTOR_TYPE); /** - * @see TableConfigUtils#validate(TableConfig, Schema, String, boolean) + * @see TableConfigUtils#validate(TableConfig, Schema, String, boolean, boolean) */ public static void validate(TableConfig tableConfig, @Nullable Schema schema) { - validate(tableConfig, schema, null, false); + validate(tableConfig, schema, null, false, false); Review Comment: Let's not keep adding booleans for each check type because it is hard to maintain. Suggest making them static, and set them up when starting the controller. `validate()` only takes `tableConfig`, `schema` and optional `typesToSkip` -- 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