Fix: intra.broker.goals cannot be configured as default.goals #2221
Conversation
|
@CCisGG would you review this please if you have some time? |
|
? |
Hi @imans777, |
| // Ensure that default goals are supported inter-broker goals. | ||
| if (defaultGoalNames.stream().anyMatch(g -> !interBrokerGoalNames.contains(g))) { | ||
| // Ensure that default goals are supported inter-broker or intra-broker goals. | ||
| if (defaultGoalNames.stream().anyMatch(g -> !interBrokerGoalNames.contains(g) && !intraBrokerGoalNames.contains(g))) { | ||
| throw new ConfigException(String.format("Attempt to configure default goals with unsupported goals (%s:%s and %s:%s).", | ||
| AnalyzerConfig.DEFAULT_GOALS_CONFIG, defaultGoalNames, | ||
| AnalyzerConfig.GOALS_CONFIG, interBrokerGoalNames)); |
There was a problem hiding this comment.
Looks like you also need to update the exception logs to make it more clear.
|
To me, allowing intra broker goals to be added into default goals is a valid intention. I'm just trying to make sure this won't break users who don't use intra broker goals. From the code change, looks like it won't. |
@CCisGG thanks for your feedback. But now it is not possible, just with the mentioned workaround. |
Hey there, I just wanted to pop up this change to be reviewed and merged sooner :) |
Ahh, got it, THX! |
|
Thank you for the approval @CCisGG! Do you have write access to merge this PR? |
Summary
default.goalsExpected Behavior
I can add (enable) goals to the
default.goalslist from both the intra and inter broker goalsActual Behavior
Sanity checks ensure the following rules:
goalsand theintra.broker.goalsat the same time)default.goalsshould be a subset ofgoals(As a result of this, we could not add intra broker goal to thedefault.goals)intra.broker.goalscant be empty (you could not move all the intra broker roles to the goals)We could not add intra goal to the
default.goalswithout a trick (see in the workaround section)Steps to Reproduce
Try to enable an intra broker goal by adding it to the
default. goalsKnown Workarounds
You can remove 1 intra broker goal from the
intra.broker.goalsconfig and add that to thegoals(keep in mind that the other one should be in theintra.broker.goalsas the list cant be empty) this way you can add that 1 goal to thedefault.goalsand enable itNote that this way we still cant enable both of the currently available intra broker goals as 1 have to be in the intra.broker.goals list to avoid hitting the mentioned sanity check. So only 1 intra goal at a time.
Categorization