[improve][ml] Filter out deleted entries before read entries from ledger. #21739

dao-jun · 2023-12-16T21:57:54Z

PIP: I'm not sure if the PR needs a PIP

Motivation

In #19035 we introduced skipCondition before read entries from ledger.

It filters out delay-messages for the purpose of optimizing performance: to avoid reading unnecessary entries to save memory usage, network usage and reduce latency of reading entries.

As the same purpose, this PR wants to filter-out deleted entries before reading entries from ledger to optimize performance.

This PR won't change the semantics of readEntries , it just made some enhancements.

It may brings huge performance optimization in the individualDelete mode, especially when bookie client metadata loss, bookie auto-recover disabled and some bookie nodes down.

Modifications

Verifying this change

Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

Added integration tests for end-to-end deployment with large payloads (10MB)
Extended integration test for recovery after broker failure

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

Documentation

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

Matching PR in forked repository

PR in forked repository: dao-jun#4

github-actions · 2023-12-16T21:58:23Z

@dao-jun Please add the following content to your PR description and select a checkbox:

- [ ] `doc` <!-- Your PR contains doc changes -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpReadEntry.java

dao-jun · 2023-12-19T15:07:07Z

/pulsarbot run-failure-checks

codelipenghui · 2023-12-25T03:43:18Z

It may brings huge performance optimization in the individualDelete mode, especially when bookie client metadata loss, bookie auto-recover disabled and some bookie nodes down.

Could you please explain more about this line? I don't understand why it pertains to bookie client metadata loss, the disabled auto-recovery and bookies unavailability.

dao-jun · 2024-01-05T08:08:10Z

@codelipenghui @coderzc PTAL

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java

codecov-commenter · 2024-01-18T20:59:14Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.58%. Comparing base (22838ea) to head (276358b).
Report is 1211 commits behind head on master.

Additional details and impacted files

@@              Coverage Diff              @@
##             master   #21739       +/-   ##
=============================================
+ Coverage     36.46%   73.58%   +37.11%     
- Complexity    12390    32381    +19991     
=============================================
  Files          1725     1861      +136     
  Lines        131701   138596     +6895     
  Branches      14401    15187      +786     
=============================================
+ Hits          48027   101985    +53958     
+ Misses        77254    28700    -48554     
- Partials       6420     7911     +1491

Flag	Coverage Δ
inttests	`24.14% <0.00%> (+0.02%)`	⬆️
systests	`23.71% <0.00%> (+0.04%)`	⬆️
unittests	`72.86% <100.00%> (+40.81%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...che/bookkeeper/mledger/impl/ManagedCursorImpl.java	`79.22% <100.00%> (+30.88%)`	⬆️
...che/bookkeeper/mledger/impl/ManagedLedgerImpl.java	`80.53% <ø> (+32.69%)`	⬆️
...he/bookkeeper/mledger/impl/ReadOnlyCursorImpl.java	`95.00% <100.00%> (+95.00%)`	⬆️

... and 1435 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

coderzc · 2024-01-20T01:23:28Z

approve

codelipenghui · 2024-02-08T12:44:12Z

I have added the cherry-pick labels since this improvement fixed the issue which can impact the bookies if the cursor rewind to the earlier position (the mark delete position can't move forward due to individual acks).

…ger. (#21739) (cherry picked from commit c66167b)

…ger. (apache#21739) (cherry picked from commit c66167b) (cherry picked from commit 84ed73e)

dao-jun added 2 commits December 17, 2023 05:42

Filter out deleted entries before read entries from bookie.

68c4374

Filter out deleted entries before read entries from bookie.

4d42fd5

dao-jun self-assigned this Dec 16, 2023

dao-jun requested review from codelipenghui and coderzc December 16, 2023 21:58

github-actions bot added the doc-label-missing label Dec 16, 2023

dao-jun requested review from merlimat and rdhabalia December 16, 2023 21:58

github-actions bot added doc-not-needed Your PR changes do not impact docs and removed doc-label-missing labels Dec 16, 2023

Filter out deleted entries before read entries from bookie.

81de6c1

dao-jun mentioned this pull request Dec 17, 2023

[improve][broker] Filter out deleted entries before read entries from ledger. dao-jun/pulsar#4

Closed

dao-jun added 5 commits December 17, 2023 22:25

Fix ManagedCursorTest code format

c3e1ad4

Fix ReadonlyCursor NPE

2f4a17e

Don't filter out deleted entries if the cursor is ReadonlyCursor

d5a8362

Fix code.

093a0c1

Fix imports

1566114

coderzc reviewed Dec 18, 2023

View reviewed changes

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java Outdated Show resolved Hide resolved

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpReadEntry.java Outdated Show resolved Hide resolved

dao-jun added 3 commits December 18, 2023 18:43

Fix code

27d2e0e

Fix imports

850f8b5

Fix code

5dd57fd

dao-jun added category/performance Performance issues fix or improvements ready-to-test area/ML labels Dec 19, 2023

dao-jun changed the title ~~[improve][broker] Filter out deleted entries before read entries from ledger.~~ [improve][ml] Filter out deleted entries before read entries from ledger. Dec 19, 2023

dao-jun requested a review from coderzc December 20, 2023 05:57

Technoboy- added this to the 3.3.0 milestone Dec 22, 2023

dao-jun removed request for merlimat and rdhabalia January 5, 2024 08:07

codelipenghui approved these changes Jan 18, 2024

View reviewed changes

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java Outdated Show resolved Hide resolved

dao-jun added 2 commits January 18, 2024 17:56

Merge branch 'master' into dev/skip_deleted_entries

60bb406

address comment.

276358b

liangyepianzhou approved these changes Jan 19, 2024

View reviewed changes

dao-jun merged commit c66167b into apache:master Jan 19, 2024

codelipenghui added release/2.10.6 release/3.0.3 release/2.11.4 release/3.1.3 release/3.2.1 and removed release/2.10.6 release/2.11.4 labels Feb 8, 2024

codelipenghui pushed a commit that referenced this pull request Feb 8, 2024

[improve][ml] Filter out deleted entries before read entries from led…

0eb4d5e

…ger. (#21739) (cherry picked from commit c66167b)

codelipenghui pushed a commit that referenced this pull request Feb 8, 2024

[improve][ml] Filter out deleted entries before read entries from led…

84ed73e

…ger. (#21739) (cherry picked from commit c66167b)

codelipenghui pushed a commit that referenced this pull request Feb 8, 2024

[improve][ml] Filter out deleted entries before read entries from led…

e6cd005

…ger. (#21739) (cherry picked from commit c66167b)

codelipenghui added cherry-picked/branch-3.0 cherry-picked/branch-3.1 cherry-picked/branch-3.2 labels Feb 8, 2024

dao-jun deleted the dev/skip_deleted_entries branch February 24, 2024 07:00

mukesh-ctds pushed a commit to datastax/pulsar that referenced this pull request Mar 1, 2024

[improve][ml] Filter out deleted entries before read entries from led…

9bb8989

…ger. (apache#21739) (cherry picked from commit c66167b) (cherry picked from commit 84ed73e)

mukesh-ctds pushed a commit to datastax/pulsar that referenced this pull request Mar 6, 2024

[improve][ml] Filter out deleted entries before read entries from led…

43f0217

…ger. (apache#21739) (cherry picked from commit c66167b) (cherry picked from commit 84ed73e)

dao-jun mentioned this pull request Apr 23, 2024

[improve][ml] Optimize read entries with skipCondition #22560

Open

15 tasks

lhotari mentioned this pull request Aug 20, 2024

[feat] Add topic stats and metrics for observing message replay behavior and Key_Shared filtering/blocking behavior #23205

Closed

2 tasks

coderzc mentioned this pull request Aug 21, 2025

[improve][ml] Optimize ledger opening by skipping fully acknowledged ledgers #24655

Merged

15 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[improve][ml] Filter out deleted entries before read entries from ledger. #21739

[improve][ml] Filter out deleted entries before read entries from ledger. #21739

Uh oh!

dao-jun commented Dec 16, 2023 •

edited

Loading

Uh oh!

github-actions bot commented Dec 16, 2023

Uh oh!

Uh oh!

Uh oh!

dao-jun commented Dec 19, 2023

Uh oh!

codelipenghui commented Dec 25, 2023

Uh oh!

dao-jun commented Jan 5, 2024

Uh oh!

Uh oh!

codecov-commenter commented Jan 18, 2024 •

edited

Loading

Uh oh!

coderzc commented Jan 20, 2024

Uh oh!

codelipenghui commented Feb 8, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

[improve][ml] Filter out deleted entries before read entries from ledger. #21739

[improve][ml] Filter out deleted entries before read entries from ledger. #21739

Uh oh!

Conversation

dao-jun commented Dec 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Modifications

Verifying this change

Does this pull request potentially affect one of the following parts:

Documentation

Matching PR in forked repository

Uh oh!

github-actions bot commented Dec 16, 2023

Uh oh!

Uh oh!

Uh oh!

dao-jun commented Dec 19, 2023

Uh oh!

codelipenghui commented Dec 25, 2023

Uh oh!

dao-jun commented Jan 5, 2024

Uh oh!

Uh oh!

codecov-commenter commented Jan 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderzc commented Jan 20, 2024

Uh oh!

codelipenghui commented Feb 8, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

dao-jun commented Dec 16, 2023 •

edited

Loading

codecov-commenter commented Jan 18, 2024 •

edited

Loading