pingles / clj-kafka

Wrapper to the Java API for interacting with Kafka
Eclipse Public License 1.0
211 stars 77 forks source link

Add more options to `messages` method for simple consumer #69

Open vincentbernat opened 8 years ago

vincentbernat commented 8 years ago

Currently, it is not possible to set maxWait and minBytes for the request to be built. Therefore, it defaults to 0 for both values.

It means that if there is no message available, messages will just return without blocking and therefore, if we try to wait for new messages, we enter a tight loop except if we sleep between each requests (but in this case, latency is increased).

Being able to specify those values would be useful.

pingles commented 8 years ago

Hi,

I'm happy to merge in a pull request for it :)

I guess that the reason I'd not added it already, or had people submit patches, is that the ZooKeeper consumer seems to be the preferred API, I added the simple consumer in the early days just because it was easier to test out using Kafka.

vincentbernat commented 8 years ago

I'll do a PR. I am using the simple consumer because I want to manage offsets myself. Unfortunately, the simple consumer is quite too basic and the ZK consumer is not flexible at all (you need to let it handle everything). Hopefully, Kafka 0.9 comes with a new consumer API that will unify everything. The API is already available in 0.8 but not fully implemented (rebalancing and stuff like that, I am not quite sure, but they rely on stuff that will only be available in 0.9).

pingles commented 8 years ago

We've used the ZK consumer and managed offsets ourselves (if it helps?). We just track the :offset of messages we consume and periodically call set-offset! (https://github.com/pingles/clj-kafka/blob/master/src/clj_kafka/zk.clj#L75).

Still- a PR would be appreciated, thank you!

vincentbernat commented 8 years ago

Humm, good idea. I wish I would have thought about that earlier. :) But as Kafka 0.9 is almost here, I'll just use the new consumer API not relying on ZK. The PR is on its way.