KAFKA-18399 Remove ZooKeeper from KafkaApis (9/N): ALTER_CLIENT_QUOTAS and ALLOCATE_PRODUCER_IDS#18465
Conversation
github-actions
Bot
added
triage
| requestHelper.sendResponseMaybeThrottle(request, throttleTimeMs => | ||
| new AllocateProducerIdsResponse(producerIdsResponse.setThrottleTimeMs(throttleTimeMs))) | ||
| ) | ||
| throw KafkaApis.shouldNeverReceive(request) |
There was a problem hiding this comment.
I think we shouldn't handle it like this - this is one of the request types that is only handled by the controller listener (now that zk is gone):
"listeners": ["zkBroker", "controller"]
So, we should have generic logic that handles that automatically before it gets here. We can probably just delete this whole method.
There was a problem hiding this comment.
Same for handleAlterClientQuotasRequest.
There was a problem hiding this comment.
So, we should have generic logic that handles that automatically before it gets here. We can probably just delete this whole method.
totally agree that we should have a generic logic for those zk-related handler. The socket server can reject the zk-related requests automatically, and we can rewrite maybeForwardToController to throw KafkaApis.shouldAlwaysForward automatically. With those change, those zk-related handlers can be deleted directly.
In short, the line case ApiKeys.ALLOCATE_PRODUCER_IDS => handleAllocateProducerIdsRequest(request) can be removed.
There was a problem hiding this comment.
Yes, exactly. We can also remove the zk listener from the json files and the socket server will automatically do the right thing, I believe (i.e. it already has logic for that).
There was a problem hiding this comment.
We can also remove the zk listener from the json files
|
@mingdaoy please fix the conflicts |
|
@mingdaoy please fix the conflicts, thanks! |
…dleAllocateProducerIdsRequest
| @@ -2580,34 +2580,7 @@ class KafkaApis(val requestChannel: RequestChannel, | |||
| } | |||
|
|
|||
| def handleAlterClientQuotasRequest(request: RequestChannel.Request): Unit = { | |||
There was a problem hiding this comment.
please remove this unused method and its related test
| @@ -2813,19 +2782,7 @@ class KafkaApis(val requestChannel: RequestChannel, | |||
| } | |||
|
|
|||
| def handleAllocateProducerIdsRequest(request: RequestChannel.Request): Unit = { | |||
There was a problem hiding this comment.
please remove this handle also
|
I observed that flaky test failures have been reported. |
chia7712
left a comment
chia7712
left a comment
There was a problem hiding this comment.
LGTM and the failed tests pass on my local
chia7712
changed the title
…S and ALLOCATE_PRODUCER_IDS (#18465) Reviewers: Ismael Juma <ismael@juma.me.uk>, Chia-Ping Tsai <chia7712@gmail.com>
…S and ALLOCATE_PRODUCER_IDS (apache#18465) Reviewers: Ismael Juma <ismael@juma.me.uk>, Chia-Ping Tsai <chia7712@gmail.com>
…S and ALLOCATE_PRODUCER_IDS (apache#18465) Reviewers: Ismael Juma <ismael@juma.me.uk>, Chia-Ping Tsai <chia7712@gmail.com>
…S and ALLOCATE_PRODUCER_IDS (apache#18465) Reviewers: Ismael Juma <ismael@juma.me.uk>, Chia-Ping Tsai <chia7712@gmail.com>
3 participants
https://issues.apache.org/jira/browse/KAFKA-18399
Committer Checklist (excluded from commit message)