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

The README documentation is wrong/unclear #900

Open
lewisdiamond opened this issue Mar 15, 2018 · 3 comments · May be fixed by #1370
Open

The README documentation is wrong/unclear #900

lewisdiamond opened this issue Mar 15, 2018 · 3 comments · May be fixed by #1370

Comments

@lewisdiamond
Copy link

Bug Report

The following implies that calling producer.createTopics(['x']. false) blocks the execution until the Kafka's topic is created.

When async is set to false, this method does not return until all topics are created

Which does not seem to be the case according to https://github.com/SOHU-Co/kafka-node/blob/master/lib/client.js#L369

The function uses retry which uses setTimout. It simply calls cb(null), returns and keeps retrying asynchronously.

createTopics(topics, async, cb)
This method is used to create topics on the Kafka server. It only works when auto.create.topics.enable, on the Kafka server, is set to true. Our client simply sends a metadata request to the server which will auto create topics. When async is set to false, this method does not return until all topics are created, otherwise it returns immediately.

topics: Array, array of topics
async: Boolean, async or sync
cb: Function, the callback
Example:

var kafka = require('kafka-node'),
    Producer = kafka.Producer,
    client = new kafka.Client(),
    producer = new Producer(client);
// Create topics sync
producer.createTopics(['t','t1'], false, function (err, data) {
    console.log(data);
});
// Create topics async
producer.createTopics(['t'], true, function (err, data) {});
producer.createTopics(['t'], function (err, data) {});// Simply omit 2nd arg

Environment

  • Node version:
  • Kafka-node version:
  • Kafka version:

For specific cases also provide

  • Number of Brokers:
  • Number partitions for topic:

Include Sample Code to reproduce behavior

// include code here

Include output with Debug turned on

Thanks for your contribution!

@Wenzil
Copy link

Wenzil commented Mar 15, 2018

Indeed the behaviour seems to be exactly the opposite of what the doc says (assuming that by "return" they mean "invoke the callback")

@aikar
Copy link
Contributor

aikar commented Mar 15, 2018

im not really sure why this isAsync even exists.

I ran into same issue on committing of a callback not being a guarantee of successful commit, and I have a PR open to fix that.

Callback shouldn't be executed until its actually done, or fatally errors.

isAsync=false seems to only report if you passed an illegal topic configuration or not.

@lamweili lamweili linked a pull request Jan 4, 2020 that will close this issue
@lamweili
Copy link

lamweili commented Jan 4, 2020

Documentation is correct.
The variable naming (isAsync) in the source code is correct as well.

However, there was a typo is the source code to use !isAsync instead.
So the logic was reversed.

I have fixed it 5fd3764 and raised a PR #1370.

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.

4 participants