Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix][broker] Fix ProducerBusy issue due to incorrect userCreatedProducerCount on non-persistent topic #22685

Merged
merged 2 commits into from
May 9, 2024

Conversation

hrzzzz
Copy link
Contributor

@hrzzzz hrzzzz commented May 9, 2024

Motivation

#20607 introduced the variable userCreatedProducerCount to track the number of producers created by users. Concurrently, the removeProducer method in AbstractTopic decreases the count of userCreatedProducerCount using USER_CREATED_PRODUCER_COUNTER_UPDATER.decrementAndGet(this) when a producer is removed. However, NonPersistentTopic overrides the removeProducer method of AbstractTopic, and the overridden removeProducer method in NonPersistentTopic does not decrease the count of userCreatedProducerCount. As a result, when maxProducersPerTopic is set, this incorrect value of userCreatedProducerCount can lead to a ProducerBusy error when creating a producer.

Modifications

Since the NonPersistentTopic class extends the AbstractTopic class and the implementation of the removeProducer method in NonPersistentTopic is identical to that in AbstractTopic, the removeProducer method in NonPersistentTopic can be safely removed.

Verifying this change

  • Make sure that the change passes the CI checks.

Added test in org.apache.pulsar.broker.service.nonpersistent.NonPersistentTopicTest#testRemoveProducerOnNonPersistentTopic

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: hrzzzz#5

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label May 9, 2024
@hrzzzz
Copy link
Contributor Author

hrzzzz commented May 9, 2024

/pulsarbot rerun-failure-checks

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.09%. Comparing base (bbc6224) to head (a3883a9).
Report is 250 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #22685      +/-   ##
============================================
- Coverage     73.57%   73.09%   -0.48%     
+ Complexity    32624    32465     -159     
============================================
  Files          1877     1888      +11     
  Lines        139502   141103    +1601     
  Branches      15299    15484     +185     
============================================
+ Hits         102638   103145     +507     
- Misses        28908    29971    +1063     
- Partials       7956     7987      +31     
Flag Coverage Δ
inttests 27.36% <ø> (+2.77%) ⬆️
systests 24.79% <ø> (+0.47%) ⬆️
unittests 72.11% <ø> (-0.73%) ⬇️

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

Files Coverage Δ
...oker/service/nonpersistent/NonPersistentTopic.java 71.07% <ø> (+1.60%) ⬆️

... and 334 files with indirect coverage changes

Copy link
Member

@dao-jun dao-jun left a comment

Choose a reason for hiding this comment

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

lgtm

@dao-jun dao-jun merged commit 253e650 into apache:master May 9, 2024
52 checks passed
lhotari pushed a commit that referenced this pull request May 14, 2024
…ucerCount on non-persistent topic (#22685)

Co-authored-by: ruihongzhou <ruihongzhou@tencent.com>
(cherry picked from commit 253e650)
lhotari pushed a commit that referenced this pull request May 14, 2024
…ucerCount on non-persistent topic (#22685)

Co-authored-by: ruihongzhou <ruihongzhou@tencent.com>
(cherry picked from commit 253e650)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request May 15, 2024
…ucerCount on non-persistent topic (apache#22685)

Co-authored-by: ruihongzhou <ruihongzhou@tencent.com>
(cherry picked from commit 253e650)
(cherry picked from commit 89b545e)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request May 16, 2024
…ucerCount on non-persistent topic (apache#22685)

Co-authored-by: ruihongzhou <ruihongzhou@tencent.com>
(cherry picked from commit 253e650)
(cherry picked from commit 89b545e)
coderzc pushed a commit that referenced this pull request Jun 5, 2024
…ucerCount on non-persistent topic (#22685)

Co-authored-by: ruihongzhou <ruihongzhou@tencent.com>
(cherry picked from commit 253e650)
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.

6 participants