Skip to content

[improve][admin] Add client side looping to analyze-backlog in Topics to avoid potential HTTP call timeout#25127

Open
oneby-wang wants to merge 15 commits intoapache:masterfrom
oneby-wang:pulsar_admin_client_side_analyze_backlog
Open

[improve][admin] Add client side looping to analyze-backlog in Topics to avoid potential HTTP call timeout#25127
oneby-wang wants to merge 15 commits intoapache:masterfrom
oneby-wang:pulsar_admin_client_side_analyze_backlog

Conversation

@oneby-wang
Copy link
Contributor

Fixes #25083

Motivation

Use client-side looping instead of increasing broker settings to avoid potential HTTP call timeout in analyze-backlog method of Topics.

Modifications

Add client-side looping, add test.

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#22

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jan 6, 2026
@oneby-wang oneby-wang marked this pull request as draft January 6, 2026 04:53
@oneby-wang oneby-wang marked this pull request as ready for review January 6, 2026 06:45
@BewareMyPower BewareMyPower requested a review from Copilot January 8, 2026 09:06
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds client-side looping to the analyzeSubscriptionBacklog method to avoid HTTP call timeouts when analyzing large backlogs. Instead of relying solely on broker-side configuration limits, the client can now iteratively call the analyze-backlog endpoint and merge results until completion or a specified entry limit is reached.

  • Adds new overloaded methods with a backlogScanMaxEntries parameter to control client-side loop termination
  • Implements iterative scanning logic that merges results from multiple server calls
  • Includes comprehensive integration tests covering various scenarios
  • Modifies OpScan behavior by removing individual deleted entry filtering

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.

File Description
pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/Topics.java Adds new API methods with backlogScanMaxEntries parameter and detailed JavaDoc documentation
pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/TopicsImpl.java Implements client-side looping logic with result merging and next position calculation
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AnalyzeBacklogSubscriptionTest.java Adds comprehensive integration tests for the new looping behavior
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpScan.java Removes filterReadEntries call and adjusts entry counting logic

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@oneby-wang oneby-wang force-pushed the pulsar_admin_client_side_analyze_backlog branch from 7848fcf to bd68a5b Compare January 21, 2026 00:44
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.

A few comments. I'm sorry for the delay in the review.

@oneby-wang oneby-wang force-pushed the pulsar_admin_client_side_analyze_backlog branch 2 times, most recently from 215bf27 to cba3498 Compare February 5, 2026 05:49
@oneby-wang oneby-wang force-pushed the pulsar_admin_client_side_analyze_backlog branch from cba3498 to da82b1b Compare February 5, 2026 07:33
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.

A few comments about the existing log messages. I think it makes sense to change them in this PR since after this PR, it would be an ordinary use case to scan larger backlogs.

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. Great work @oneby-wang

@lhotari lhotari added this to the 4.2.0 milestone Feb 5, 2026
@oneby-wang
Copy link
Contributor Author

Thanks for your great suggestions! @lhotari

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.1.3

Projects

None yet

2 participants