Skip to content
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

[fix][broker] Fix ledger cachemiss size metric #20257

Merged
merged 1 commit into from May 29, 2023

Conversation

AnonHxy
Copy link
Contributor

@AnonHxy AnonHxy commented May 8, 2023

Motivation

ledger cache miss rate should be same as ledgerFactory cache miss rate. For details, the major fix is that the count always increace 1 when entriesToReturn.size() is greater thant one, e.g., line453 at RangeEntryCacheImpl.java

Modifications

ledger cache miss rate should be same as ledgerFactory cache miss rate

Verifying this change

  • Make sure that the change passes the CI checks.

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: AnonHxy#38

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label May 8, 2023
@AnonHxy AnonHxy added this to the 3.1.0 milestone May 8, 2023
@AnonHxy AnonHxy added the type/bug The PR fixed a bug or issue reported a bug label May 8, 2023
@AnonHxy AnonHxy self-assigned this May 8, 2023
@AnonHxy AnonHxy changed the title [fix][broker] Fix ledger cachemiss size [fix][broker] Fix ledger cachemiss size metric May 8, 2023
@AnonHxy AnonHxy requested a review from HQebupt May 11, 2023 11:22
Copy link
Contributor

@StevenLuMT StevenLuMT left a comment

Choose a reason for hiding this comment

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

LGTM

@AnonHxy AnonHxy removed this from the 3.1.0 milestone May 11, 2023
Copy link
Contributor

@HQebupt HQebupt left a comment

Choose a reason for hiding this comment

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

LGTM

@Technoboy- Technoboy- added this to the 3.1.0 milestone May 15, 2023
@Technoboy-
Copy link
Contributor

We don't need valueRate, so it seems no need to fix.

@AnonHxy
Copy link
Contributor Author

AnonHxy commented May 23, 2023

seems no need to fix.

The major fix is not the valueRate. It's that the count always increace 1 when entriesToReturn.size() is greater thant one, e.g., line453 at RangeEntryCacheImpl.java @Technoboy-

@AnonHxy
Copy link
Contributor Author

AnonHxy commented May 29, 2023

seems no need to fix.

The major fix is not the valueRate. It's that the count always increace 1 when entriesToReturn.size() is greater thant one, e.g., line453 at RangeEntryCacheImpl.java @Technoboy-

Hi @Technoboy- Do you have any other comments:) thanks

@Technoboy-
Copy link
Contributor

seems no need to fix.

The major fix is not the valueRate. It's that the count always increace 1 when entriesToReturn.size() is greater thant one, e.g., line453 at RangeEntryCacheImpl.java @Technoboy-

Hi @Technoboy- Do you have any other comments:) thanks

Make sense. So we'd better update the motivation.

@AnonHxy
Copy link
Contributor Author

AnonHxy commented May 29, 2023

Make sense. So we'd better update the motivation.

OK. Updated

@Technoboy- Technoboy- merged commit 1c813fd into apache:master May 29, 2023
55 of 56 checks passed
poorbarcode pushed a commit that referenced this pull request May 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/broker cherry-picked/branch-3.0 doc-not-needed Your PR changes do not impact docs ready-to-test release/3.0.1 type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants