Skip to content

Conversation

@shibd
Copy link
Member

@shibd shibd commented Mar 21, 2025

Motivation

When EnableBrokerSideSubscriptionPatternEvaluation is set to false, the pattern subscription can't discover newly created topics.

This regression was introduced by PR #21853 and #22854, which removed pattern consumer regularity discovery topic logic:

https://github.com/apache/pulsar/pull/22854/files#diff-779a47f80b0048bab183870ed4c1ecd07ffd538006cf9358e494bee23405d924L154-L159

Modifications

  • Rollback to poll mode for topic discovery when watcherFuture returns an exception.
  • Remove recheckPatternTaskBackoff as it is unused and does not align with the user-side configuration patternAutoDiscoveryPeriod.
  • Remove test code like consumer.getRecheckPatternTimeout().task().run(consumer.getRecheckPatternTimeout()); from the unit tests, because these tests now operate under the premise of the broker actively pushing topic changes.

Verifying this change

  • To reproduce and verify this issue, you can run the test testSubscribePatterBrokerDisable.

Documentation

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

Matching PR in forked repository

PR in forked repository:

@shibd shibd self-assigned this Mar 21, 2025
@shibd shibd changed the title [fix][client] Pattern subscription discovery regression when broker-side evaluation is disabled [fix][client] Pattern subscription regression when broker-side evaluation is disabled Mar 21, 2025
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Mar 21, 2025
@shibd shibd requested a review from poorbarcode March 24, 2025 01:12
@codecov-commenter
Copy link

codecov-commenter commented Mar 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.27%. Comparing base (bbc6224) to head (2d5b6f8).
Report is 983 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #24104      +/-   ##
============================================
+ Coverage     73.57%   74.27%   +0.69%     
+ Complexity    32624    32484     -140     
============================================
  Files          1877     1864      -13     
  Lines        139502   144351    +4849     
  Branches      15299    16464    +1165     
============================================
+ Hits         102638   107212    +4574     
+ Misses        28908    28686     -222     
- Partials       7956     8453     +497     
Flag Coverage Δ
inttests 26.72% <0.00%> (+2.13%) ⬆️
systests 23.22% <20.00%> (-1.10%) ⬇️
unittests 73.77% <100.00%> (+0.92%) ⬆️

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

Files with missing lines Coverage Δ
...ar/client/impl/PatternMultiTopicsConsumerImpl.java 75.53% <100.00%> (-7.93%) ⬇️

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

@shibd shibd merged commit d7962a1 into apache:master Mar 24, 2025
52 checks passed
shibd added a commit that referenced this pull request Mar 24, 2025
shibd added a commit that referenced this pull request Mar 24, 2025
shibd added a commit that referenced this pull request Mar 24, 2025
nodece pushed a commit to nodece/pulsar that referenced this pull request Mar 27, 2025
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Mar 27, 2025
…tion is disabled (apache#24104)

(cherry picked from commit d7962a1)
(cherry picked from commit 734f79f)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Mar 28, 2025
…tion is disabled (apache#24104)

(cherry picked from commit d7962a1)
(cherry picked from commit 734f79f)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Mar 28, 2025
…tion is disabled (apache#24104)

(cherry picked from commit d7962a1)
(cherry picked from commit 4db81d7)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Mar 29, 2025
…tion is disabled (apache#24104)

(cherry picked from commit d7962a1)
(cherry picked from commit 4db81d7)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Mar 31, 2025
…tion is disabled (apache#24104)

(cherry picked from commit d7962a1)
(cherry picked from commit 4db81d7)
walkinggo pushed a commit to walkinggo/pulsar that referenced this pull request Oct 8, 2025
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.

3 participants