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

DescribeConfigs for brokers not working with multiple brokers #1172

Closed
jlandersen opened this issue Jan 16, 2019 · 4 comments · Fixed by #1280
Closed

DescribeConfigs for brokers not working with multiple brokers #1172

jlandersen opened this issue Jan 16, 2019 · 4 comments · Fixed by #1280

Comments

@jlandersen
Copy link
Contributor

jlandersen commented Jan 16, 2019

describeConfigs does not work when requesting broker configs and the cluster has multiple brokers. The issue is that the implementation sends a describe config request to all brokers and merges the results. As a result requesting configs for broker X will result in errors from all other brokers than X (see [1]).

I see a couple of things:

  1. There should not be a need to send describeConfigs to all brokers. Topic requests can be handled by any broker even if that is not hosting any partitions for that topic.

  2. describeConfigs for brokers should send only to the requested brokers. I recently added a sendRequestToBroker as part of Add config entries and replica assignment to create topic #1157 which can be used to send a request to a specific broker in a safe way (handling connection setup if not ready etc.). I also started a PR for describeConfigs in Add describe config to kafkaclient #1142 where this logic was implemented - this might be inspiration for a fix.

[1]
https://github.com/apache/kafka/blob/1.0/core/src/main/scala/kafka/server/AdminManager.scala#L321

Environment

  • Node version: any
  • Kafka-node version: 4.0.0+
  • Kafka version: 0.11.0-2.0

For specific cases also provide

  • Number of Brokers: > 1
  • Number partitions for topic: any

Include Sample Code to reproduce behavior

const kafka = require("kafka-node");

const client = new kafka.KafkaClient({
    autoConnect: true,
    connectRetryOptions: {
        retries: 1,
    },
    connectTimeout: 3000,
    kafkaHost: '' // any host in a multi-broker cluster
});

const adminClient = new kafka.Admin(client);


client.on("ready", () => {
    const resource = {
        resourceType: adminClient.RESOURCE_TYPES.broker,
        resourceName: '1', // any id in the multi-broker cluster
        configNames: []
    };

    const payload = {
        resources: [resource],
        includeSynonyms: false,
    };

    adminClient.describeConfigs(payload,
        (err, res) => {
        // Error: InvalidRequest: Unexpected broker id, expected 2, but received 1
        console.log(err);
    });
});
@jlandersen
Copy link
Contributor Author

cc @ahmedlafta
do you wanna take a look otherwise I can take a stab at some point :)

@ahmedlafta
Copy link
Contributor

@jlandersen Oh interesting! Thanks for the tag and helpful explanation! I will try and look into it this weekend 😃

@jlandersen
Copy link
Contributor Author

@ahmedlafta sorry to poke you, but wanted to check if you had any rough estimates on this? I would also be able to take a look if you want :-)

@ahmedlafta
Copy link
Contributor

hey @jlandersen i apologize for the huge delay. I haven't been able to get to this so please feel free to jump on it if you'd like. let me know if you run into any issues!

jlandersen added a commit to jlandersen/kafka-node that referenced this issue Jun 6, 2019
Fixes describe configs when requesting broker configs for clusters with more than 1 broker.

Instead of sending all requests to all brokers, this now sends requests for broker configs to the specific broker. Topic config requests can go to any broker.
This follows the logic of the Java SDK.

Fixes SOHU-Co#1172.
jlandersen added a commit to jlandersen/kafka-node that referenced this issue Jun 6, 2019
… describe configs when requesting broker configs for clusters with more than 1 broker.Instead of sending all requests to all brokers, this now sends requests for broker configs to the specific broker. Topic config requests can go to any broker.This follows the logic of the Java SDK.Fixes SOHU-Co#1172.
jlandersen added a commit to jlandersen/kafka-node that referenced this issue Jun 6, 2019
Fixes describe configs when requesting broker configs for clusters with more than 1 broker.Instead of sending all requests to all brokers, this now sends requests for broker configs to the specific broker. Topic config requests can go to any broker.This follows the logic of the Java SDK.
Also reworked a bit to fit better into api support functionality.

Fixes SOHU-Co#1172.
jlandersen added a commit to jlandersen/kafka-node that referenced this issue Jun 6, 2019
… describe configs when requesting broker configs for clusters with more than 1 broker.Instead of sending all requests to all brokers, this now sends requests for broker configs to the specific broker. Topic config requests can go to any broker.This follows the logic of the Java SDK.Also reworked a bit to fit better into api support functionality.Fixes SOHU-Co#1172.
jlandersen added a commit to jlandersen/kafka-node that referenced this issue Jun 6, 2019
…s fixes describe configs when requesting broker configs for clusters with more than 1 broker. Instead of sending all requests to all brokers, requests for broker configs are sent to the specific broker. Topic config requests can go to any broker (follows the logic of the Java SDK). Also reworked a bit to fit better into api support functionality. Fixes SOHU-Co#1172.
jlandersen added a commit to jlandersen/kafka-node that referenced this issue Jun 6, 2019
This fixes describe configs when requesting broker configs for clusters with  more than 1 broker.
Instead of sending all requests to all brokers, requests for broker configs are sent to the specific broker. T
opic config requests can go to any broker (follows the logic of the Java SDK).

Also reworked a bit to fit better into api support functionality.

Fixes SOHU-Co#1172.
hyperlink pushed a commit that referenced this issue Jun 13, 2019
This fixes describe configs when requesting broker configs for clusters with more than 1 broker.
Instead of sending all requests to all brokers, requests for broker configs are sent to the specific broker. Topic config requests can go to any broker (follows the logic of the Java SDK).

Also reworked a bit to fit better into api support functionality.

Fixes #1172.
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 a pull request may close this issue.

2 participants