Skip to content

Conversation

@oneby-wang
Copy link
Contributor

@oneby-wang oneby-wang commented Dec 18, 2025

Fixes #25082

Motivation

PersistentTopics.analyzeSubscriptionBacklog() rest api returns the counters of backlog, but the counters don't return the amount of marker messages.

Modifications

Add counter for marker messages in PersistentTopics.analyzeSubscriptionBacklog() rest api, add tests.

Verifying this change

  • Make sure that the change passes the CI checks.

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: oneby-wang#15

@github-actions
Copy link

@oneby-wang 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 -->

@oneby-wang oneby-wang marked this pull request as draft December 18, 2025 08:58
@github-actions github-actions bot added doc-not-needed Your PR changes do not impact docs and removed doc-label-missing labels Dec 18, 2025
@oneby-wang oneby-wang force-pushed the analyze_backlog_marker_messages_counter branch from 6478918 to 31340e1 Compare December 19, 2025 02:52
@oneby-wang oneby-wang force-pushed the analyze_backlog_marker_messages_counter branch from 72bdea7 to 0fda5ba Compare December 19, 2025 06:57
@oneby-wang oneby-wang marked this pull request as ready for review December 19, 2025 10:41
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

@oneby-wang
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

@codecov-commenter
Copy link

codecov-commenter commented Dec 22, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.47%. Comparing base (e041fab) to head (bf0ed12).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #25091       +/-   ##
=============================================
+ Coverage     39.20%   74.47%   +35.27%     
- Complexity    13498    33675    +20177     
=============================================
  Files          1842     1899       +57     
  Lines        145491   149660     +4169     
  Branches      16907    17394      +487     
=============================================
+ Hits          57038   111465    +54427     
+ Misses        80844    29315    -51529     
- Partials       7609     8880     +1271     
Flag Coverage Δ
inttests 26.35% <0.00%> (-0.32%) ⬇️
systests 23.05% <0.00%> (-0.08%) ⬇️
unittests 74.00% <100.00%> (+38.85%) ⬆️

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

Files with missing lines Coverage Δ
...pulsar/broker/admin/impl/PersistentTopicsBase.java 69.44% <100.00%> (+56.25%) ⬆️
...ker/service/persistent/PersistentSubscription.java 76.65% <100.00%> (+27.10%) ⬆️

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

@oneby-wang
Copy link
Contributor Author

Since uncommitted or aborted messages will not be delivered to consumer, counting them into numMessages ormarkerMessages seems all improper.

The aborted messages is filtered by AbstractBaseDispatcher.

} else if (((PersistentTopic) subscription.getTopic())
.isTxnAborted(new TxnID(msgMetadata.getTxnidMostBits(), msgMetadata.getTxnidLeastBits()),
entry.getPosition())) {
individualAcknowledgeMessageIfNeeded(Collections.singletonList(entry.getPosition()),
Collections.emptyMap());
entries.set(i, null);
entry.release();
continue;

The uncommitted messages will not be read and delivered before committed.

public Position getMaxReadPosition() {
return this.transactionBuffer.getMaxReadPosition();
}

Maybe we should modify OpScan scan params and scan logic and filter aborted messages in analyzeBacklog method, so the backlog would be consistent with consumer backlog.

if (cursor.hasMoreEntries(searchPosition)) {
OpReadEntry opReadEntry = OpReadEntry.create(cursor, searchPosition, batchSize,
this, OpScan.this.ctx, null, null, false);
ledger.asyncReadEntries(opReadEntry);

@lhotari WDYT?

@lhotari
Copy link
Member

lhotari commented Dec 23, 2025

Since uncommitted or aborted messages will not be delivered to consumer, counting them into numMessages ormarkerMessages seems all improper.

I agree that it would be useful to have a separate counter for such transactional messages that are part of aborted transactions. For the transaction markers themselves, it's useful to have them part of marker messages.
I think that the aborted transactional message counter could be handled in a separate PR.

@lhotari lhotari added this to the 4.2.0 milestone Dec 23, 2025
@lhotari lhotari assigned lhotari and oneby-wang and unassigned lhotari Dec 23, 2025
@oneby-wang
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

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

Labels

doc-not-needed Your PR changes do not impact docs release/4.0.9 release/4.1.3

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Enhancement] Add counter for marker messages to "pulsar-admin topics analyze-backlog"

3 participants