KAFKA-10000: Add producer fencing API to admin client (KIP-618)

[C0urante]

Implements the new admin client API described in KIP-618.

Note that this API is not used by the Connect framework in this PR. It will be leveraged by Connect in downstream PRs.

[showuon]

@C0urante , I've made a pass. Left some comments. Thanks.

[showuon]

There's also an expected error: PRODUCER_FENCED which should be handled here.

[C0urante]

This shouldn't happen if we don't include a producer epoch/ID in the InitProducerIdRequest, right? If so, I could see there still being a case for adding coverage for PRODUCER_FENCED just in order to provide a better user-facing error message, but it's unclear what else we could really say beyond "unexpected error."

Thoughts?

[showuon]

I agree with you that since we always sent without producerID and epoch, the error should never happen. But I think there might be someone to extend the existing FenceProducerHandler for other purpose, the assumption will not be true. Or at least, we can add a comment here to mention we skip PRODUCER_FENCED on purpose.

[C0urante]

Ah, good point about extensibility. I'd personally opt to just leave a comment (and in fact, remove the existing cases for TRANSACTIONAL_ID_NOT_FOUND and INVALID_PRODUCER_EPOCH, which should also never happen) in order to highlight the fact that, should those errors surface, they are completely unexpected.

If we end up needing to leverage this class somewhere else in the future, those assumptions should still hold unless we also change the InitProducerIdRequest we send to the broker. I can make note of that in another comment in the buildSingleRequest method so that we know to update the error handling logic in that case.

[C0urante]

Thanks @showuon, appreciate it. I've addressed three of your comments and left a question about the remaining one.

[showuon]

LGTM! Thanks for the PR.

[showuon]

Looks good.

[showuon]

I'll wait for a while to see if there are other people want to have another review. Thanks.

[tombentley]

Is it correct to use Object::toString here, rather than Enum::name? If so, maybe the javadoc needs adjusting since it's not guaranteed to return the enum value name.

[C0urante]

I opted for toString instead of name since the former can be overridden, but the latter can't. The Javadoc does explicitly link to the toString method, but that's probably not enough to cancel out the assumption people might make that the text "name" refers to Enum::name.

I can tweak it to say "string representations", which is a little clunkier but shouldn't mislead people like "name" might. LMK if you have other thoughts.

[C0urante]

Thanks for taking a look, @tombentley. Ready for another pass when you have time.

[tombentley]

LGTM. Thanks @C0urante!

[tombentley]

Merging since failing tests appear to be unrelated.

[C0urante]

Thanks Tom!