ah- / rdkafka-dotnet

C# Apache Kafka client
Other
242 stars 73 forks source link

Ignore NO_OFFSET condition while commiting #108

Open soumyajit-sahu opened 7 years ago

soumyajit-sahu commented 7 years ago

While committing, we should ignore the NO_OFFSET error code. There is a note in the comments in the rdkafka.h file as follows, which corroborates this change: If no partitions had valid offsets to commit this callback will be called with \p err == RD_KAFKA_RESP_ERR__NO_OFFSET which is not to be considered an error.

edenhill commented 7 years ago

The reason for this behaviour is state synchronization: it allows the application to know when an async commit() operation has finished, regardless if it actually had to commit offsets or the requested offsets were only commited (NO_OFFSET).

soumyajit-sahu commented 7 years ago

We are calling the Consumer.Commit() synchronously to guarantee at-least once in our logic. I believe that a NO_OFFSET exception is raised when we don't have any new data in any partition. The Consumer.Commit() call should pass in this case too.

@edenhill if we shouldn't ignore the error in the SafeKafkaHandle.Commit(), then would you suggest to: 1) ignore it in the Consumer.Commit() or, 2) have the caller ignore it. In this case we can put a comment for Consumer.Commit() to document it just like it is done in rdkafka.h.

I would prefer the later to keep it alike to the c lib.

edenhill commented 7 years ago

Yep, I would go with option 2.

soumyajit-sahu commented 7 years ago

Thanks @edenhill . I have added a comment, and reverted the previous change.