-
-
Notifications
You must be signed in to change notification settings - Fork 131
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
add support of entity-default kafka quotas #267
base: main
Are you sure you want to change the base?
Conversation
8191609
to
135101e
Compare
135101e
to
846e6a2
Compare
@luisfmcalado @Mongey could you review please? and how to trigger pipeline with github actions here? |
@azhurbilo Thanks for the PR. I wonder if we should change this so that |
in this case existed code/tests about "quotas" should be refactored and be more complex as resource ID depends on it and most of functions have "entity_name" param. |
@luisfmcalado UP, do you agree? @Mongey or maybe you can implement omitted |
@azhurbilo analogous to kafka scripts: kafka-configs \
--bootstrap-server localhost:9092 --alter \
--add-config 'producer_byte_rate=1024' \
--entity-type users --entity-name alice \
--entity-type clients --entity-name my-app
kafka-configs \
--bootstrap-server localhost:9092 --alter \
--add-config 'producer_byte_rate=1024' \
--entity-type users --entity-name alice \
--entity-type clients --entity-default |
@palson not sure that it's good idea implement support for both entity-type at the same time. There is no problem with this PR to create 2 separate resources to control quotas for "client" and "users" without any conflict, do you agree?
|
@azhurbilo I know that implementing two features in the same PR is not the best approach, but since you've already reviewed the quota logic, this would be a good chance to add support for both types as well. As for your example - it won't work. According to the quota precedense order the quota rule {specific user, <default client>} matches requests first and the client's quouta will never work. Because the first one resource creates the |
@palson yes, got it. But If we want support "/config/users/*/clients/*" path we need brake compatibility of kafka_quota resource as now it supports only "/config/users/*" or "/config/clients/*" and I can not imagine how to add support for both without breaking changes |
Hello @Mongey, it's been a while since the last activity on this subject. |
@azhurbilo I'm not sure about the details of the breaking changes. In regards to the resource format, what about adding a new resource "kafka_quota" "users_default" {
entity_tuple = {
client_id = "<clientname>"
user = "<username>"
}
config = {
"consumer_byte_rate" = 10000
"producer_byte_rate" = 10000
}
} I think the |
@Dugong42 absolutely agree with your last message proposal! but I stopped working in company where we use Kafka, maybe someone else can implement "kafka_quota_tuple" or "kafka_quotas" to support |
@dstrates I would also want this feature, it seems the author want it implemented as |
previously there is no ability to create default "kafka_quotas" for any user/client
https://kafka.apache.org/26/documentation.html#quotas
https://cwiki.apache.org/confluence/display/KAFKA/KIP-257+-+Configurable+Quota+Management