Skip to content

[fix][broker]fix the publish latency spike issue with large number of producers#20607

Merged
Technoboy- merged 2 commits into
apache:masterfrom
codelipenghui:penghui/fix-producer-count
Jun 20, 2023
Merged

[fix][broker]fix the publish latency spike issue with large number of producers#20607
Technoboy- merged 2 commits into
apache:masterfrom
codelipenghui:penghui/fix-producer-count

Conversation

@codelipenghui

@codelipenghui codelipenghui commented Jun 19, 2023

Copy link
Copy Markdown
Contributor

Motivation

The issue should only happen on the topic with many producers, and the max producer limitation is enabled. When a new producer is added, the broker will check all the existing producers and filter out the remote producer (from the replicator), which will run out of the CPU resource for a short period of time.

image

profiler.html.txt

Modifications

A counter will be much more efficient instead of going through all the producers. So a new userCreatedProducerCount is added to the topic.

Verifying this change

The existing tests already covered the max producer limitation. The change will not affect the correctness.

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

@codelipenghui

Copy link
Copy Markdown
Contributor Author

/pulsarbot run-failure-checks

@codelipenghui

Copy link
Copy Markdown
Contributor Author

/pulsarbot run-failure-checks

@codecov-commenter

codecov-commenter commented Jun 20, 2023

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 73.05%. Comparing base (8125d1f) to head (f601751).
⚠️ Report is 2389 commits behind head on master.

Files with missing lines Patch % Lines
...rg/apache/pulsar/broker/service/AbstractTopic.java 90.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #20607      +/-   ##
============================================
+ Coverage     72.29%   73.05%   +0.76%     
+ Complexity    31951    31889      -62     
============================================
  Files          1854     1867      +13     
  Lines        138462   138663     +201     
  Branches      15221    15237      +16     
============================================
+ Hits         100095   101302    +1207     
+ Misses        30384    29330    -1054     
- Partials       7983     8031      +48     
Flag Coverage Δ
inttests 24.19% <80.00%> (?)
systests 25.00% <60.00%> (?)
unittests 72.34% <90.00%> (+0.05%) ⬆️

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

Files with missing lines Coverage Δ
...rg/apache/pulsar/broker/service/AbstractTopic.java 88.72% <90.00%> (+0.66%) ⬆️

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

@Technoboy- Technoboy- merged commit 084d2ab into apache:master Jun 20, 2023
@codelipenghui codelipenghui deleted the penghui/fix-producer-count branch July 3, 2023 00:24
codelipenghui added a commit that referenced this pull request Jul 3, 2023
codelipenghui added a commit that referenced this pull request Jul 3, 2023
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Jul 3, 2023
… producers (apache#20607)

(cherry picked from commit 084d2ab)
(cherry picked from commit 9fa56fc)
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.

7 participants