Skip to content

KAFKA-13022: Optimize ClientQuotasImage#describe #19079

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

Open
wants to merge 5 commits into
base: trunk
Choose a base branch
from

Conversation

FrankYang0529
Copy link
Member

@FrankYang0529 FrankYang0529 commented Mar 3, 2025

In previous implementation, the ClientQuotasImage#describe goes through all ClientQuotaEntity and checks whether entity type and entity name are matched the filter. In this PR, it goes through all ClientQuotaEntity#entries in the constructor and build a new data structure Map<String, Map<String, Set<Entry<ClientQuotaEntity, ClientQuotaImage>>>>. The first layer is entity type. There are only three entity types: user, client-id, and ip. The second layer is entity name. The last layer is a set of matched entries. With this data structure, we can speed up describe function.

Correctness is covered by ClientQuotasRequestTest.

Benchmark result:

  • trunk

    Benchmark                                             (eachEntityCount)  Mode  Cnt      Score   Error  Units
    ClientQuotasImageDescribeBenchmark.describeDefault                   10  avgt    2    422.020          ns/op
    ClientQuotasImageDescribeBenchmark.describeDefault                  100  avgt    2   4380.829          ns/op
    ClientQuotasImageDescribeBenchmark.describeDefault                 1000  avgt    2  54567.929          ns/op
    ClientQuotasImageDescribeBenchmark.describeExact                     10  avgt    2    461.441          ns/op
    ClientQuotasImageDescribeBenchmark.describeExact                    100  avgt    2   4620.061          ns/op
    ClientQuotasImageDescribeBenchmark.describeExact                   1000  avgt    2  55275.026          ns/op
    ClientQuotasImageDescribeBenchmark.describeSpecified                 10  avgt    2    651.988          ns/op
    ClientQuotasImageDescribeBenchmark.describeSpecified                100  avgt    2   5477.147          ns/op
    ClientQuotasImageDescribeBenchmark.describeSpecified               1000  avgt    2  60935.943          ns/op
    
  • branch

    Benchmark                                             (eachEntityCount)  Mode  Cnt      Score   Error  Units
    ClientQuotasImageDescribeBenchmark.describeDefault                   10  avgt    2     10.666          ns/op
    ClientQuotasImageDescribeBenchmark.describeDefault                  100  avgt    2     11.317          ns/op
    ClientQuotasImageDescribeBenchmark.describeDefault                 1000  avgt    2     11.409          ns/op
    ClientQuotasImageDescribeBenchmark.describeExact                     10  avgt    2     68.551          ns/op
    ClientQuotasImageDescribeBenchmark.describeExact                    100  avgt    2     61.202          ns/op
    ClientQuotasImageDescribeBenchmark.describeExact                   1000  avgt    2     65.592          ns/op
    ClientQuotasImageDescribeBenchmark.describeSpecified                 10  avgt    2    366.921          ns/op
    ClientQuotasImageDescribeBenchmark.describeSpecified                100  avgt    2   4135.522          ns/op
    ClientQuotasImageDescribeBenchmark.describeSpecified               1000  avgt    2  52720.811          ns/op
    

@github-actions github-actions bot added kraft small Small PRs labels Mar 3, 2025
@FrankYang0529 FrankYang0529 force-pushed the KAFKA-13022 branch 2 times, most recently from a106160 to 91841c5 Compare March 4, 2025 13:42
@github-actions github-actions bot added performance and removed small Small PRs labels Mar 4, 2025
@FrankYang0529 FrankYang0529 force-pushed the KAFKA-13022 branch 2 times, most recently from 973ed33 to 9ebfa97 Compare March 5, 2025 02:33
@FrankYang0529 FrankYang0529 changed the title KAFKA-13022: Optimize ClientQuotasImage#describe (wip) KAFKA-13022: Optimize ClientQuotasImage#describe Mar 5, 2025
Copy link
Contributor

@ahuang98 ahuang98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

personally this logic seems really hard to follow, especially with all the stacked for loops. I think some code comments would really help!

Comment on lines +151 to +152
entitiesByType.get(exactMatchEntry.getKey()).containsKey(exactMatchEntry.getValue())) {
for (Entry<ClientQuotaEntity, ClientQuotaImage> entry : entitiesByType.get(exactMatchEntry.getKey()).get(exactMatchEntry.getValue())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: local vars to reduce redundant gets and improve readability?

if (entitiesByType.containsKey(exactMatchEntry.getKey()) &&
entitiesByType.get(exactMatchEntry.getKey()).containsKey(exactMatchEntry.getValue())) {
for (Entry<ClientQuotaEntity, ClientQuotaImage> entry : entitiesByType.get(exactMatchEntry.getKey()).get(exactMatchEntry.getValue())) {
if (request.strict() && !entry.getKey().entries().equals(exactMatch)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this have to equal exactMatch as long as entry.getKey().entries() are encompassed by both typeMatch and exactMatch?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants