Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable users to monitor the min.insync.replicas of all topics #1622

Merged
merged 2 commits into from
Jul 20, 2021

Conversation

efeg
Copy link
Collaborator

@efeg efeg commented Jul 20, 2021

This PR resolves #1621.

  • Enable users to monitor the min.insync.replicas of all topics via kafka_cluster_state endpoint.

  • Update the default value of topic.config.provider.class config to switch from ZK-based client to AdminClient by default (see earlier PR that introduced admin client-based topic config provider).

  • Make KafkaAdminTopicConfigProvider retrieve cluster configs via admin client rather than reading them from a file.

  • Make private variables / methods of KafkaTopicConfigProvider and KafkaAdminTopicConfigProvider to let them be extended.

  • This patch has been tested on a Kafka cluster containing topics that (1) override the cluster default and (2) use the cluster default. The response has been verified for correctness of the corresponding value for each topic with and without json=true parameter. Excerpt (i.e. showing only selected partition states) from the example plaintext response with verbose=true:

Summary: The cluster has XXX brokers, ...
...

All Partitions in the Cluster (verbose):
                                                         TOPIC PARTITION    LEADER                      REPLICAS                       IN-SYNC              OUT-OF-SYNC                  OFFLINE   MIN-ISR
Offline Partitions:
Partitions with Offline Replicas:
Under Replicated Partitions:
Under MinIsr Partitions:
Other Partitions:
                                ExampleTopicWithClusterDefault         0     10000           [10000, 2000, 1000]           [10000, 1000, 2000]                       []                       []         1
                                ExampleTopicWithClusterDefault         1     10000           [10000, 1000, 2000]           [10000, 1000, 2000]                       []                       []         1
                                 ...
                                  TopicThatOverridesTheDefault         0      1000           [1000, 2000, 30000]           [30000, 2000, 1000]                       []                       []         2

@efeg efeg requested a review from zornhsu July 20, 2021 02:15
Copy link
Contributor

@zornhsu zornhsu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for the deprecating more ZK clients, Efe! I left some comment below.

@efeg efeg merged commit e847de5 into linkedin:master Jul 20, 2021
efeg added a commit to efeg/cruise-control that referenced this pull request Jul 20, 2021
efeg added a commit to efeg/cruise-control that referenced this pull request Jul 20, 2021
@efeg efeg deleted the feat/minIsrMonitoring branch July 20, 2021 20:34
fvaleri added a commit to fvaleri/strimzi-kafka-operator that referenced this pull request Dec 22, 2023
The cluster.configs.file configuration property is never used.
The KafkaTopicConfigProvider is now deprecated, and KafkaAdminTopicConfigProvider is used by default.
The KafkaAdminTopicConfigProvider retrieves cluster configs via the Kafka admin client rather than reading them from a file.

linkedin/cruise-control#1622

Signed-off-by: Federico Valeri <[email protected]>
fvaleri added a commit to fvaleri/strimzi-kafka-operator that referenced this pull request Jan 3, 2024
The Cruise Control's cluster.configs.file configuration property is never used.
The KafkaTopicConfigProvider is now deprecated in favor of KafkaAdminTopicConfigProvider, which is used by default.
The KafkaAdminTopicConfigProvider retrieves cluster configs via the Kafka admin client rather than reading them from a file.
For these reasons, we can get rid of this configuration and related code.

linkedin/cruise-control#1622

Signed-off-by: Federico Valeri <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable users to monitor the min.insync.replicas of all topics
2 participants