oleksiyk / kafka

Apache Kafka 0.9 client for Node
MIT License
297 stars 85 forks source link

Schedule fetches as partition processing completes. #217

Closed thomaslee closed 3 years ago

thomaslee commented 6 years ago

The way we were using Promise.map(...) in BaseConsumer._fetch can unnecessarily restrict throughput when handlers are relatively slow, since we need to process all partitions before we can start fetching more partition data.

The intent of this change is to schedule requests to fetch additional data as soon as we can, using an async.queue to throttle processing parallelism in place of Promise.map(...)'s concurrency option.


This change isn't quite so graceful as the code that came before it (JS isn't my strong suit!) but figured a proof of concept might be useful starting point. In particular I find all the stopping/tryStop noise a bit irritating & I'm sure there's a more graceful way to tackle it, but it's getting late. 😄 Happy to clean this up if you feel the change is worth the extra complexity -- and open to ideas for doing so.

oleksiyk commented 6 years ago

This makes sense but definitely needs more time. I'll see if I can make something myself.

thomaslee commented 6 years ago

Even better -- thanks!

thomaslee commented 6 years ago

See also #218 -- fixing this might be a good opportunity to tackle that as well.