Blizzard / node-rdkafka

Node.js bindings for librdkafka
MIT License
2.1k stars 390 forks source link

broken fastify-kafka implementation when using rdkafka bigger than 2.14.0 #1016

Closed Uzlopak closed 1 year ago

Uzlopak commented 1 year ago

See https://github.com/fastify/fastify-kafka/pull/99

We had to revert to version 2.14.0 to ensure everything works properly. I assume it is because of #982 by @GaryWilber (no blame on him). It seems that the connection is not disconnected properly and so the test runner hangs.

whackenb commented 1 year ago

I could confirm with why-is-node-running that two threads introduced with 2.14.0 are not terminated. As only our tests were affected, we currently explicitly exit the process as a workaround.

GaryWilber commented 1 year ago

Thanks for reporting this. I have not seen this behavior myself since my service calls process.exit when exiting.

I made a fix in #1017. Would one of you be able to try that change out and verify it fixes the issue?

whackenb commented 1 year ago

Thanks a lot for the quick fix. :)

I could verify that our integration tests terminate when using version 2.16.1 build from branch 'user/garywilb/consume_loop_disconnect', while the process keeps running with version 2.15.0.

We shut down our consumer and producers by publishing a shutdown event:

process.on('shutdown', () => {
    if (consumer.isConnected()) {
        logger.info('Disconnecting from cluster...');
        consumer.disconnect(handleKafkaError);
    }
});

consumer.on('disconnected', () => {
    logger.info('Disconnected from cluster');
});

No process.exit is present, and test framework mocha is called without the --exit flag. The disconnect log messages are present and all expected messages are published/consumed, so I think the issue is resolved by the change.

Uzlopak commented 1 year ago

@GaryWilber

Thank you for the fast fix ;).