jtuglu1 commented on code in PR #19595:
URL: https://github.com/apache/druid/pull/19595#discussion_r3430919758
##########
server/src/main/java/org/apache/druid/server/coordinator/CoordinatorDynamicConfig.java:
##########
@@ -184,14 +192,41 @@ public CoordinatorDynamicConfig(
this.historicalTierAliases = Configs.valueOrDefault(historicalTierAliases,
Map.of());
final Set<String> aliasKeys = this.historicalTierAliases.keySet();
+ final Set<String> seenTiers = new HashSet<>();
for (Set<String> mappedTiers : this.historicalTierAliases.values()) {
if (!Sets.intersection(mappedTiers, aliasKeys).isEmpty()) {
throw InvalidInput.exception(
"historicalTierAliases [%s] is invalid. A virtual tier alias
cannot be a physical tier.",
this.historicalTierAliases
);
}
+ final Set<String> duplicateTiers = Sets.intersection(mappedTiers,
seenTiers);
+ if (!duplicateTiers.isEmpty()) {
+ throw InvalidInput.exception(
+ "historicalTierAliases [%s] is invalid. Physical tier%s %s cannot
belong to more than one alias.",
+ this.historicalTierAliases,
+ duplicateTiers.size() > 1 ? "s" : "",
Review Comment:
can we just do something like:
`Physical tier(s) [%s] cannot...`
##########
server/src/main/java/org/apache/druid/server/coordinator/CoordinatorDynamicConfig.java:
##########
@@ -184,14 +192,41 @@ public CoordinatorDynamicConfig(
this.historicalTierAliases = Configs.valueOrDefault(historicalTierAliases,
Map.of());
final Set<String> aliasKeys = this.historicalTierAliases.keySet();
+ final Set<String> seenTiers = new HashSet<>();
for (Set<String> mappedTiers : this.historicalTierAliases.values()) {
if (!Sets.intersection(mappedTiers, aliasKeys).isEmpty()) {
throw InvalidInput.exception(
"historicalTierAliases [%s] is invalid. A virtual tier alias
cannot be a physical tier.",
this.historicalTierAliases
);
}
+ final Set<String> duplicateTiers = Sets.intersection(mappedTiers,
seenTiers);
+ if (!duplicateTiers.isEmpty()) {
+ throw InvalidInput.exception(
+ "historicalTierAliases [%s] is invalid. Physical tier%s %s cannot
belong to more than one alias.",
+ this.historicalTierAliases,
+ duplicateTiers.size() > 1 ? "s" : "",
Review Comment:
nit: can we just do something like:
`Physical tier(s) [%s] cannot...`
--
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]
