SOHU-Co / kafka-node

Node.js client for Apache Kafka 0.8 and later.
MIT License
2.66k stars 627 forks source link

ConsumerGroup crazy very slow, if pass topic as string #880

Open darky opened 6 years ago

darky commented 6 years ago

Bug Report

Environment

Include Sample Code to reproduce behavior

// All ok
var consumerGroup = new ConsumerGroup(options, ['RebalanceTopic');

// Crazy very slow
var consumerGroup = new ConsumerGroup(options, 'RebalanceTopic');
aikar commented 6 years ago

There is something else at play and not what you think it is.

lodash's isString is not going to be slow, nor would it have a long running impact on the overall runtime.

This code is only ran during consumer creation.

darky commented 6 years ago

Sorry, I mean receiving messages process.

var consumerGroup = new ConsumerGroup(options, ['RebalanceTopic']);
consumerGroup.on("message", m => {
  // this call so fast
});

var consumerGroup = new ConsumerGroup(options, 'RebalanceTopic');
consumerGroup.on("message", m => {
  // this almost not call, very poor performance
})
davidqhr commented 6 years ago

I met this. Thanks @darky

hyperlink commented 6 years ago

The string gets turned into an array that's the only difference between the two. https://github.com/SOHU-Co/kafka-node/blob/master/lib/consumerGroup.js#L95

Do you have some sample code to demonstrate this issue?

darky commented 6 years ago

I don't understand, how it occured, but code smell bad little bit. No need to mutate parameters, otherwise need directly assign:

this.topics = _.isString(topics) ? [topics] : topics;
aikar commented 6 years ago

The only way this code can have a problem is if you found a performance bug in v8, which I doubt that such a simple operation of wrapping a string in an array would be an issue. It would be chaos on the internet if there was since this is an extremely common pattern.

aikar commented 6 years ago

Your issue has nothing to do with string vs array. You're going to need to give hyperlink more code / details / debug logs.