search

linkedin / cruise-control

Cruise-control is the first of its kind to fully automate the dynamic workload rebalance and self-healing of a Kafka cluster. It provides great value to Kafka users by simplifying the operation of Kafka clusters.
https://github.com/linkedin/cruise-control/tags
BSD 2-Clause "Simplified" License
2.74k stars 587 forks source link

BrokerSetResolver throwing exception if no brokerSets.json is provided #1911

Closed CCisGG closed 2 years ago

CCisGG commented 2 years ago

Since BrokerSetResolver is the default implementation, but brokerSet.json is not embedded in the codebase by default, user can easily get NoSuchFileException if they run the BrokerSetAwareGoal or query KafkaClusterState.

I think we need to find some way to tolerate the case when the brokerSet.json is not found. E.g. embed the file, or just throw a warning message when the file is not found.

However, we do want to throw the exception if the user indeed has configured their own brokerSet file path but the file was not there.

@mohitpali would like to hear your thoughts.

mohitpali commented 2 years ago

Is it just for the stats and cluster state ?

For Stats screen we can not populate the brokerSets if the file is not available.

We can take a leaf from how BrokerCapacityFileResolver is implemented. One option is to have a default brokerSets.json passed that has brokerSetId as -1 and empty broker list. We would have to make changes in the code also to see if -1 brokerSetId is passed then all the Brokers are in a single broker set. That way BrokerSetAwareGoal becomes ineffective.

Thinking deep about it and to simplify it, since this is not a default goal and if the user decides to include BrokerSetAwareGoal, they are also supposed to pass a file by default if the BrokerSetFileResolver is used by default.

A validation can fulfill this purpose.

If BrokerSetAwareGoal is included in the goals list and no specific BrokerSetResolver is passed (BrokerSetFileResolved is used by default), then the brokerSet file config should be present.

CCisGG commented 2 years ago

If BrokerSetAwareGoal is included in the goals list and no specific BrokerSetResolver is passed (BrokerSetFileResolved is used by default), then the brokerSet file config should be present.

@mohitpali This might not be the best option. Today BrokerSetAwareGoal is already in the goals list by default. It is just not in the default.goals. This case if we force the file existence by validation, then most of CC users will be affected as they don't need this goal.

mohitpali commented 2 years ago

As discussed, just like BrokerCapacityFileResolver we can add the default file location in the cruisecontrol.properties file.

CCisGG commented 2 years ago

Actually I just realized there is already a default location embedded. I'm trying to figure out why it was still throwing that FileNotFoundException... 

public static final String BROKER_SET_CONFIG_FILE_CONFIG = "broker.set.config.file";
public static final String DEFAULT_BROKER_SET_CONFIG_FILE = "config/brokerSets.json";
CCisGG commented 2 years ago

Ok, upon recheck the exception, it looks like not a FileNotFoundException:

2022/09/29 17:52:43.139 WARN [ClusterBrokerState] [qtp1134202713-25] [kafka-cruise-control] [] Failed to resolve broker set with exception: com.linkedin.kafka.cruisecontrol.exception.BrokerSetResolutionException: config/brokerSets.json

The exception message is sort of vague. It may be a json file parse exception or other exceptions. I'll further dig through that

CCisGG commented 2 years ago

Verified it is NoSuchFileException. It's unknown why CC can't find that file even though it should be there by default.

CCisGG commented 2 years ago

Just figured out this is an internal issue. Closing this issue. Thanks @mohitpali and sorry for the confusion! This issue should not exist if users use open source cc.