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

Producer core dump #49

Closed
ottomata opened this issue Oct 24, 2016 · 11 comments
Closed

Producer core dump #49

ottomata opened this issue Oct 24, 2016 · 11 comments
Labels

Comments

@ottomata
Copy link

Am in the process of testing PR #42 as a fix for Issue #5, and ran into this:

const Kafka = require('node-rdkafka');
const producer = new Kafka.Producer({
    'metadata.broker.list': 'localhost:9092',
});
producer.connect(undefined, () => {
    producer.produce({
        topic: 'test',
        message: '{"f": 1}'
    });
});
nodejs: ../node_modules/nan/nan_object_wrap.h:33: static T* Nan::ObjectWrap::Unwrap(v8::Local<v8::Object>) [with T = NodeKafka::Topic]: Assertion `object->InternalFieldCount() > 0' failed.
Aborted (core dumped)

node-rdkafka compiled at commit 62e53c22cafe93b9697fc9ff6fc0daa7c2670da2.

@webmakersteve
Copy link
Contributor

It looks like it didn't properly create the topic. I can test this later, but just in case this helps, can you try using a different topic name?

If that doesn't work, instead of using a string in that method, can you try using new producer.Topic('test', {}); and see if that makes the issue go away?

I don't normally recommend doing it that way as it interferes with graceful shutdown, but it may help isolate the bug. But like I said, I'll take a look at this later today otherwise :)

@ottomata
Copy link
Author

Topic 'test' does exist. I also tried with another existent topic and a non-existent one, and get the same behavior. Switching to new Topic like this:

const Kafka = require('node-rdkafka');
const producer = new Kafka.Producer({
    'metadata.broker.list': 'localhost:9092',
});
producer.connect(undefined, () => {
    producer.produce({
        topic: new producer.Topic('test', {}),
        message: '{"f": 1}'
    });
});

Gives

/vagrant/es/kasocki/node_modules/node-rdkafka.latest/lib/producer.js:165
    this.emit('error', LibrdKafkaError.create(err));
         ^

TypeError: this.emit is not a function
    at new Producer.Topic (/vagrant/es/kasocki/node_modules/node-rdkafka.latest/lib/producer.js:165:10)
    at /vagrant/es/kasocki/scr/producer.js:8:16
    at next (/vagrant/es/kasocki/node_modules/node-rdkafka.latest/lib/client.js:132:7)
    at /vagrant/es/kasocki/node_modules/node-rdkafka.latest/lib/client.js:209:7
    at /vagrant/es/kasocki/node_modules/node-rdkafka.latest/lib/client.js:335:7`

@webmakersteve
Copy link
Contributor

webmakersteve commented Oct 24, 2016

Sorry. Should be topic: producer.Topic('test', {}); And make sure to add an on('error') listener so we can see why the RdKafka::Topic creation is failing.

I'll update this to fail a little bit better than it does. It should throw when you try to produce to a failed topic, rather than core dump due to an assertion.

@ottomata
Copy link
Author

topic: producer.Topic('test', {}) also assert and dumps:

nodejs: ../node_modules/nan/nan_object_wrap.h:33: static T* Nan::ObjectWrap::Unwrap(v8::Local<v8::Object>) [with T = NodeKafka::Topic]: Assertion `object->InternalFieldCount() > 0' failed.
Aborted (core dumped)

I had an on('error' handler, but it never was called. I just added it back, and it isn't called before the assert.

@webmakersteve
Copy link
Contributor

d'oh. Today is definitely a Monday. This is a breaking change. it should be:

producer.produce('test', null, new Buffer('{"f": 1}'), null);

I am going to add deprecation messages across the board for the old usage before release.

@ottomata
Copy link
Author

Oh hm. Ok. Can you keep the callback parameter in there too? We promisify() the producer/consumer, and having a node-style callback is needed to get a promisified produceAsync() method added.

@webmakersteve
Copy link
Contributor

webmakersteve commented Oct 24, 2016

The deprecated version of the function will still honor the callback, even though it will just setImmediate to it with the error if one was thrown. This method is no longer async, as it only adds a message to the queue of the internal librdkafka producer buffer. It is already non-blocking, so the update avoids wasting time queueing up the work.

@webmakersteve
Copy link
Contributor

f9070c6 you can see how it will work here

@ottomata
Copy link
Author

Hm ok. Does this mean it will no longer be possible to know if a specific message is ACKed by Kafka via callback?

@webmakersteve
Copy link
Contributor

The callback wouldn't tell you if the message was acked unfortunately: https://github.com/edenhill/librdkafka/blob/master/src-cpp/rdkafkacpp.h#L1635

If you require acks, you can use the delivery report as an authoritative indicator that your message has been written to the ISRs/lead broker. But acks do not block produce

The library has the ability to take an opaque to identify the message later in the delivery report, but it is not currently implemented on the node side. However, it isn't out of the realm of possibility if people got some use out of it.

I think generally, you need to trust librdkafka to retry until it gets the acks you specify you need for that message.

@webmakersteve
Copy link
Contributor

Gracefully deprecated with #55

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants