Blizzard / node-rdkafka

Node.js bindings for librdkafka
MIT License
2.11k stars 396 forks source link

Run consume logic in a separate thread #982

Closed GaryWilber closed 1 year ago

GaryWilber commented 2 years ago

The current consume method works like this:

  1. Schedules work on an existing node.js worker thread
  2. Runs an infinite loop on that thread until the consumer is closed
  3. Sleeps when it hits EOF and this is not configurable

That logic is not great. The while() loop is essentially blocking one of the 4 node.js worker threads from doing any other work because it took over with its infinite loop. You can confirm this yourself by starting an HTTP server & then calling the consume method. The HTTP server will not process any requests because it’s unable to schedule the work since that node.js worker thread is blocked – this repro’s 100% of the time for me on node.js 16 with a fastify webserver.

Some reading regarding node.js & it’s worker threads / thread pool to learn more:

This PR does the following:

  1. Create a separate thread for running the consume logic. This makes it so the consume while loop will not block any other node.js work, such as processing network requests
  2. Removed the EOF sleeps - those were slowing things down and the sleeps seemed unnecessary
  3. Allow setting consume timeout to 0. Before, setting it to 0 would end up making it set to the default time instead.

These changes result in a 30% latency reduction for my scenario. My application has been running with these changes for a while now and it has been stable.

This PR should be checked in as a squash commit due to the amount of commits.

sharmakhushboo commented 1 year ago

Ping @webmakersteve , @iradul, @eandersson , @BlizzTom

RomainDECOSTER commented 1 year ago

Hello,

I am currently experiencing the same issue. I have 4 consumers listening and I am not longer able to use my express API.

Is it still planned to merge this PR?

Thank you in advance and thank you for the work you are doing.

Regards.