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

[improve][broker] Improve efficiency of checking message deletion #20490

Merged

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented Jun 5, 2023

Motivation

Currently deleted messages are checked in the order that individual messages are checked first and then comparing the mark delete position after this. Since the check for the mark delete position is a simple check, it's more efficient to check that first.

Modifications

  • change order of condition evaluation
  • omit creation of PositionImpl instances in trimDeletedEntries

Documentation

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

- change order of condition evaluation
  - checking markDeletePosition is fast
- omit creation of PositionImpl instances in trimDeletedEntries
@lhotari lhotari added type/cleanup Code or doc cleanups e.g. remove the outdated documentation or remove the code no longer in use ready-to-test labels Jun 5, 2023
@lhotari lhotari self-assigned this Jun 5, 2023
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jun 5, 2023
@tisonkun
Copy link
Member

tisonkun commented Jun 6, 2023

Merging...

Thank you!

@tisonkun tisonkun merged commit fff5e39 into apache:master Jun 6, 2023
@Technoboy- Technoboy- added this to the 3.1.0 milestone Jun 12, 2023
lhotari added a commit that referenced this pull request Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-picked/branch-3.0 doc-not-needed Your PR changes do not impact docs ready-to-test release/3.0.6 type/cleanup Code or doc cleanups e.g. remove the outdated documentation or remove the code no longer in use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants