Jackie-Jiang commented on code in PR #17786:
URL: https://github.com/apache/pinot/pull/17786#discussion_r2866903863
##########
pinot-broker/src/main/java/org/apache/pinot/broker/broker/helix/BaseBrokerStarter.java:
##########
@@ -684,6 +683,28 @@ private void
updateInstanceConfigAndBrokerResourceIfNeeded() {
}
}
+ /**
+ * Validates that broker instance tags are configured when enforcement is
enabled. Throws
+ * {@link IllegalStateException} if enforcement is on but tags are missing.
+ */
+ @VisibleForTesting
+ static void validateInstanceTagsConfiguration(PinotConfiguration brokerConf)
{
Review Comment:
This is not even used in production code. Let's not add it. Given the change
is simple enough, no need to add a test
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java:
##########
@@ -385,6 +385,11 @@ public static class Broker {
public static final long DEFAULT_EXTRA_PASSIVE_TIMEOUT_MS = 100L;
public static final String CONFIG_OF_BROKER_ID =
"pinot.broker.instance.id";
public static final String CONFIG_OF_BROKER_INSTANCE_TAGS =
"pinot.broker.instance.tags";
+ // Cluster-level enforcement flag: when enabled, brokers must have
pinot.broker.instance.tags configured
+ // to start. Prevents misconfigured brokers from joining multi-tenant
clusters without tenant tags.
+ public static final String CONFIG_OF_BROKER_INSTANCE_TAGS_ENFORCE_ENABLED =
+ "pinot.cluster.broker.instance.tags.enforce.enabled";
Review Comment:
```suggestion
public static final String CONFIG_OF_BROKER_ENFORCE_INSTANCE_TAGS =
"pinot.broker.enforce.instance.tags";
```
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java:
##########
@@ -385,6 +385,11 @@ public static class Broker {
public static final long DEFAULT_EXTRA_PASSIVE_TIMEOUT_MS = 100L;
public static final String CONFIG_OF_BROKER_ID =
"pinot.broker.instance.id";
public static final String CONFIG_OF_BROKER_INSTANCE_TAGS =
"pinot.broker.instance.tags";
+ // Cluster-level enforcement flag: when enabled, brokers must have
pinot.broker.instance.tags configured
Review Comment:
This is still an instance level flag. We can enable it by setting it at
cluster level
--
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]
