kfaraz commented on pull request #11766: URL: https://github.com/apache/druid/pull/11766#issuecomment-934018421
Thanks a lot for the review, @capistrant ! Please consider the alternatives below. __Alternative 1:__ The configurations `watchedTiers` and `ignoredTiers` make most sense when the other is not specified at all. So at start-up, we could just verify that only one is non-null and fail the start-up. Otherwise, all the cases mentioned later here would add to the confusion. I feel failing the broker start-up might be a little heavy-handed but the next alternative is more of a silent failure, which is worse 😅 . __Alternative 2:__ We could give priority to one of the lists over the other (right now, `ignoredTiers` is the higher priority one. so, if a tier is present in both `ignoredTiers` and `watchedTiers`, that tier would be __ignored__) and spit out a WARN log and call this out in the docs*. I do agree that this could create data inconsistencies for the user as their intent is not clear and would involve poring through the Broker logs to see what went wrong. __Possible Cases__ *From a docs point of view, I feel there are several cases that would need clarification now: `ignoredTiers` | `watchedTiers` | Behaviour -----------------|-----------------------|--------------- null (i.e. not specified in the config) | null | Watch ALL tiers empty array | null | Watch ALL tiers null | empty array | Watch NO tier empty array | empty array | Watch NO tier Tier A absent | Tier A absent | DO NOT watch Tier A Tier A absent | Tier A present | Watch Tier A Tier A present | Tier A absent | DO NOT watch Tier A Tier A present | Tier A present | Watch Tier A? __With Alternative 1__: We fail at start-up and don't need to clarify any of these scenarios (except `null`, `null`, which is the default setting) __With Alternative 2__ From the above table, I feel that the tie-breaker seems to rest with `watchedTiers` in the sense that if there is a tie, then we do what `watchedTiers` dictates. So I guess it would make sense for the case in discussion to watch the tier as it is after all present in the `watchedTiers` list. Please let me know what you think 😃 -- 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]
