-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: prevent idempotent producer epoch exhaustion #2178
Conversation
0e48f14
to
41494f2
Compare
86c8dc0
to
b472f28
Compare
if err != nil { | ||
Logger.Println(err) | ||
return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, where we failed to send our InitProducerIDRequest to any broker and got ErrOutOfBrokers returned, do we need to do something more serious than just logging the error? Or are we happy just to leave p.txnmgr
as the existing transaction manager with epoch left at MaxInt16 and we think the producer will eventually recover itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ladislavmacoun I'm assuming the latter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dnwe Apologies for the late reply.
Looking at the Java AK client, the producer only transitions to an uninitialized state, I believe we will achieve the same thing with resetting the transitional manager. If it fails I don't think there is anything else we can do, and it should get initialised with another request eventually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ladislavmacoun no worries, thanks for your great contribution!
Add check for idempotent producer epoch overflow, as it is required by the KIP-360[1], the producer should request a new producer ID when the epoch is exhausted. Otherwise, the producer might get desynchronized with the broker. [1]: https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=89068820 Signed-off-by: Ladislav Macoun <ladislavmacoun@gmail.com>
b472f28
to
5c6b581
Compare
Add check for idempotent producer epoch overflow, as it is required by the
KIP-360, the producer should request a new producer ID when the epoch
is exhausted. Otherwise, the producer might get desynchronized with the
broker.
AK producer for reference
Similar issue to confluentinc/librdkafka#3720