Skip to content

Conversation

@zhouyifan279
Copy link
Contributor

@zhouyifan279 zhouyifan279 commented Sep 14, 2025

Fixes #24737

Motivation

Pulsar ProtocolHandler, such as KoP, uses ManagedLedgerImpl#asyncFindPosition to find startCursorPosition and create a NonDurableCursor.
When a topic's ledger is trimmed and is empty, startCursorPosition will be (currentLedgerId, -1).
However, the created NonDurableCursor's readPosition is not stable if Topic ledger is adding a new entry simultaneously.
It may be (currentLedgerId, 0) or (currentLedgerId, 2).

We expect the created NonDurableCursor readPosition to always be (currentLedgerId, 0).

Modifications

In NonDurableCursorImpl constructor, we declare a reference to ManagedLedgerImpl#lastConfirmedEntry and use it to compare with startCursorPosition and to initializeCursorPosition.

Verifying this change

Run new test NonDurableCursorTest#testOpenNonDurableCursorWhileLedgerIsAddingFirstEntryAfterTrimmed

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

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

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

Matching PR in forked repository

PR in forked repository: zhouyifan279#2

@github-actions
Copy link

@zhouyifan279 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 -->

@github-actions github-actions bot added doc-not-needed Your PR changes do not impact docs and removed doc-label-missing labels Sep 14, 2025
@zhouyifan279 zhouyifan279 changed the title [fix][broker] First entry will be skipped if opening NonDurableCursor while ledger is adding first entry after trimmed. [fix][broker] First entry will be skipped if opening NonDurableCursor while trimmed ledger is adding first entry. Sep 14, 2025
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

LGTM

…l/NonDurableCursorTest.java

Co-authored-by: Lari Hotari <lhotari@users.noreply.github.com>
@codecov-commenter
Copy link

codecov-commenter commented Sep 18, 2025

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 74.18%. Comparing base (80beab6) to head (4aee598).
⚠️ Report is 145 commits behind head on master.

Files with missing lines Patch % Lines
.../bookkeeper/mledger/impl/NonDurableCursorImpl.java 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #24738      +/-   ##
============================================
- Coverage     74.28%   74.18%   -0.10%     
- Complexity    33256    33605     +349     
============================================
  Files          1900     1901       +1     
  Lines        148398   148366      -32     
  Branches      17208    17200       -8     
============================================
- Hits         110231   110072     -159     
- Misses        29392    29522     +130     
+ Partials       8775     8772       -3     
Flag Coverage Δ
inttests 26.28% <66.66%> (-0.53%) ⬇️
systests 22.77% <33.33%> (-0.17%) ⬇️
unittests 73.71% <66.66%> (-0.07%) ⬇️

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

Files with missing lines Coverage Δ
.../bookkeeper/mledger/impl/NonDurableCursorImpl.java 81.13% <66.66%> (+0.36%) ⬆️

... and 104 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.

@lhotari lhotari merged commit 7a120f3 into apache:master Sep 18, 2025
51 checks passed
lhotari added a commit that referenced this pull request Sep 22, 2025
… while trimmed ledger is adding first entry. (#24738)

Co-authored-by: Yunze Xu <xyzinfernity@163.com>
Co-authored-by: Lari Hotari <lhotari@users.noreply.github.com>
(cherry picked from commit 7a120f3)
KannarFr pushed a commit to CleverCloud/pulsar that referenced this pull request Sep 22, 2025
… while trimmed ledger is adding first entry. (apache#24738)

Co-authored-by: Yunze Xu <xyzinfernity@163.com>
Co-authored-by: Lari Hotari <lhotari@users.noreply.github.com>
lhotari added a commit that referenced this pull request Sep 23, 2025
… while trimmed ledger is adding first entry. (#24738)

Co-authored-by: Yunze Xu <xyzinfernity@163.com>
Co-authored-by: Lari Hotari <lhotari@users.noreply.github.com>
(cherry picked from commit 7a120f3)
lhotari added a commit that referenced this pull request Sep 23, 2025
… while trimmed ledger is adding first entry. (#24738)

Co-authored-by: Yunze Xu <xyzinfernity@163.com>
Co-authored-by: Lari Hotari <lhotari@users.noreply.github.com>
(cherry picked from commit 7a120f3)
lhotari added a commit that referenced this pull request Sep 23, 2025
… while trimmed ledger is adding first entry. (#24738)

Co-authored-by: Yunze Xu <xyzinfernity@163.com>
Co-authored-by: Lari Hotari <lhotari@users.noreply.github.com>
(cherry picked from commit 7a120f3)
manas-ctds pushed a commit to datastax/pulsar that referenced this pull request Sep 26, 2025
… while trimmed ledger is adding first entry. (apache#24738)

Co-authored-by: Yunze Xu <xyzinfernity@163.com>
Co-authored-by: Lari Hotari <lhotari@users.noreply.github.com>
(cherry picked from commit 7a120f3)
(cherry picked from commit 0e4e54b)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Sep 26, 2025
… while trimmed ledger is adding first entry. (apache#24738)

Co-authored-by: Yunze Xu <xyzinfernity@163.com>
Co-authored-by: Lari Hotari <lhotari@users.noreply.github.com>
(cherry picked from commit 7a120f3)
(cherry picked from commit 0e4e54b)
@zhouyifan279 zhouyifan279 deleted the non-durable-cursor branch September 27, 2025 01:32
manas-ctds pushed a commit to datastax/pulsar that referenced this pull request Sep 29, 2025
… while trimmed ledger is adding first entry. (apache#24738)

Co-authored-by: Yunze Xu <xyzinfernity@163.com>
Co-authored-by: Lari Hotari <lhotari@users.noreply.github.com>
(cherry picked from commit 7a120f3)
(cherry picked from commit f427b97)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Sep 29, 2025
… while trimmed ledger is adding first entry. (apache#24738)

Co-authored-by: Yunze Xu <xyzinfernity@163.com>
Co-authored-by: Lari Hotari <lhotari@users.noreply.github.com>
(cherry picked from commit 7a120f3)
(cherry picked from commit f427b97)
walkinggo pushed a commit to walkinggo/pulsar that referenced this pull request Oct 8, 2025
… while trimmed ledger is adding first entry. (apache#24738)

Co-authored-by: Yunze Xu <xyzinfernity@163.com>
Co-authored-by: Lari Hotari <lhotari@users.noreply.github.com>
nodece pushed a commit to ascentstream/pulsar that referenced this pull request Oct 30, 2025
… while trimmed ledger is adding first entry. (apache#24738)

Co-authored-by: Yunze Xu <xyzinfernity@163.com>
Co-authored-by: Lari Hotari <lhotari@users.noreply.github.com>
(cherry picked from commit 7a120f3)
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.

[Bug] First entry will be skipped if opening NonDurableCursor while ledger is adding first entry after trimmed.

5 participants