MINOR: Support KRaft mode in RequestQuotaTest #12614
Conversation
There was a problem hiding this comment.
Hello @hachikuji @dajac , please take a look at the problem here, BrokerMetadataPublisher will publish ClientQuotasDelta to brokers, but not to controllers, I'm not sure if it was deliberately designed like this.
| @ValueSource(strings = Array("zk", "kraft")) | ||
| def testResponseThrottleTime(quorum: String): Unit = { | ||
| val apiKeys = if (isKRaftTest()) { | ||
| (clientActions ++ clusterActionsWithThrottle).filterNot(_.forwardable) |
There was a problem hiding this comment.
Currently, ClientQuotasDelta is only published to brokers whereas ControllerApis.QuotaManagers is not updated after we update client quota configs, so all request is not throttled in the controller, so we need to filter forwardable ApiKeys here.
| RequestQuotaTest.principal = RequestQuotaTest.UnauthorizedPrincipal | ||
|
|
||
| for (apiKey <- ApiKeys.zkBrokerApis.asScala) { | ||
| val apiKeys = if (isKRaftTest()) ApiKeys.kraftBrokerApis().asScala.filterNot(_.forwardable) else ApiKeys.zkBrokerApis.asScala |
There was a problem hiding this comment.
Only throttling in brokers, not controllers did sound like a bug. The changes make sense to me. Could we fix the build issue first? All 3 builds terminated in core:unittest, which looks like it is caused by this PR. I had a look, but don't have any clue. Do you have any idea?
I triggered the build again, let's see if it is still the same.
https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-12614/2/
There was a problem hiding this comment.
Had a look at https://cwiki.apache.org/confluence/display/KAFKA/KIP-124+-+Request+rate+quotas, looks like the request rate is to throttle the request from client. So, if the client won't sent requests directly to controller, maybe we don't worry about controller here?
dengziming
force-pushed
the
RequestQuotaTest-kraft
branch
from
0a1852e
to
6f461c8
Compare
dengziming
force-pushed
the
RequestQuotaTest-kraft
branch
from
6f461c8
to
5c93a2c
Compare
|
Fixed in #14201 |
More detailed description of your change
Support kraft mode in RequestQuotaTest
Summary of testing strategy (including rationale)
QA
Committer Checklist (excluded from commit message)