Skip to content

[improve][broker] PIP-406: Introduce metrics related to dispatch throttled events #23946

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.

Sign up for GitHub

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

Jump to bottom
Merged
shibd merged 2 commits into from
Mar 24, 2025
Merged

[improve][broker] PIP-406: Introduce metrics related to dispatch throttled events #23946

shibd merged 2 commits into from
Mar 24, 2025

Conversation

@shibd
Copy link
Member

@shibd shibd commented Feb 7, 2025 •
edited
Loading

Motivation

Refer to #23945

Modifications

  1. Maintain rate limit event fields in AbstractBaseDispatcher.
  2. Output these fields when retrieving topic stats and metrics.

Verifying this change

  • Add testSubscriptionStatsDispatchThrottled, testBrokerDispatchThrottledMetrics , testTopicDispatchThrottledMetrics to cover it

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

@shibd shibd added type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages area/metrics ready-to-test labels Feb 7, 2025
@shibd shibd self-assigned this Feb 7, 2025
@shibd shibd mentioned this pull request Feb 7, 2025
Merged
4 tasks
@github-actions github-actions bot added the doc-required Your PR changes impact docs and you will update later. label Feb 7, 2025
@shibd shibd force-pushed the add_throttle_count branch from 13a97c0 to a51bb86 Compare February 18, 2025 09:05
@shibd shibd changed the title [improve][broker] PIP-406: Introduce pulsar_subscription_dispatch_throttled_msgs and bytes metrics [improve][broker] PIP-406: Introduce metrics related to dispatch_throttled_count Feb 18, 2025
@shibd shibd force-pushed the add_throttle_count branch from a51bb86 to 4b4915b Compare February 28, 2025 07:16
@shibd shibd force-pushed the add_throttle_count branch from 4488dc2 to 216e885 Compare March 13, 2025 12:00
@shibd shibd changed the title [improve][broker] PIP-406: Introduce metrics related to dispatch_throttled_count [improve][broker] PIP-406: Introduce metrics related to dispatch throttled events Mar 13, 2025
@shibd shibd marked this pull request as ready for review March 17, 2025 13:05
@shibd shibd force-pushed the add_throttle_count branch from 216e885 to 19df20e Compare March 17, 2025 13:05
@shibd shibd requested a review from lhotari March 17, 2025 13:07
@shibd
Copy link
Member Author

shibd commented Mar 17, 2025

@RobertIndie @lhotari @poorbarcode Could help review this PR, thanks.

@codecov-commenter
Copy link

codecov-commenter commented Mar 18, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 91.11111% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.27%. Comparing base (bbc6224) to head (a7c3f3a).
⚠️ Report is 1269 commits behind head on master.

Files with missing lines Patch % Lines
...a/org/apache/pulsar/broker/service/Dispatcher.java 0.00% 6 Missing ⚠️
.../pulsar/broker/service/AbstractBaseDispatcher.java 91.66% 0 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #23946      +/-   ##
============================================
+ Coverage     73.57%   74.27%   +0.69%     
+ Complexity    32624    32087     -537     
============================================
  Files          1877     1863      -14     
  Lines        139502   144403    +4901     
  Branches      15299    16461    +1162     
============================================
+ Hits         102638   107254    +4616     
+ Misses        28908    28688     -220     
- Partials       7956     8461     +505
Flag Coverage Δ
inttests 26.73% <42.22%> (+2.14%) ⬆️
systests 23.23% <28.88%> (-1.10%) ⬇️
unittests 73.76% <91.11%> (+0.92%) ⬆️

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

Files with missing lines Coverage Δ
...rvice/nonpersistent/NonPersistentSubscription.java 57.89% <100.00%> (+4.62%) ⬆️
...ker/service/persistent/PersistentSubscription.java 77.04% <100.00%> (+0.35%) ⬆️
...ker/stats/prometheus/AggregatedNamespaceStats.java 100.00% <100.00%> (ø)
.../stats/prometheus/AggregatedSubscriptionStats.java 100.00% <ø> (ø)
...ker/stats/prometheus/NamespaceStatsAggregator.java 94.33% <100.00%> (+3.93%) ⬆️
...che/pulsar/broker/stats/prometheus/TopicStats.java 98.48% <100.00%> (+4.28%) ⬆️
...mon/policies/data/stats/SubscriptionStatsImpl.java 94.05% <100.00%> (-0.06%) ⬇️
.../pulsar/broker/service/AbstractBaseDispatcher.java 91.70% <91.66%> (-1.60%) ⬇️
...a/org/apache/pulsar/broker/service/Dispatcher.java 50.00% <0.00%> (-10.00%) ⬇️

... and 1055 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 requested a review from lhotari March 18, 2025 08:29
@lhotari lhotari added this to the 4.1.0 milestone Mar 19, 2025
lhotari
lhotari approved these changes Mar 19, 2025
View reviewed changes
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, good work @shibd

@lhotari
Copy link
Member

lhotari commented Mar 19, 2025

@shibd It would be great to have this metric available in 4.0.x. Any plans for that?

@shibd
Copy link
Member Author

shibd commented Mar 19, 2025

@shibd It would be great to have this metric available in 4.0.x. Any plans for that?

I think we can cherry-picked to 4.0. Once this PR is merged, I'll submit a separate PR and then vote on it via email.

@shibd shibd merged commit 4f9031b into apache:master Mar 24, 2025
52 checks passed
shibd added a commit to shibd/pulsar that referenced this pull request Mar 24, 2025
@shibd
[improve][broker] PIP-406: Introduce metrics related to dispatch thro…
6a1732a 
…ttled events (apache#23946)

(cherry picked from commit 4f9031b)
@shibd shibd mentioned this pull request Mar 24, 2025
Merged
4 tasks
walkinggo pushed a commit to walkinggo/pulsar that referenced this pull request Oct 8, 2025
@shibd @walkinggo
[improve][broker] PIP-406: Introduce metrics related to dispatch thro…
d0ea8ad 
…ttled events (apache#23946)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lhotari lhotari lhotari approved these changes

@Technoboy- Technoboy- Awaiting requested review from Technoboy-

@codelipenghui codelipenghui Awaiting requested review from codelipenghui

@gaoran10 gaoran10 Awaiting requested review from gaoran10

@RobertIndie RobertIndie Awaiting requested review from RobertIndie

@poorbarcode poorbarcode Awaiting requested review from poorbarcode

@coderzc coderzc Awaiting requested review from coderzc

Assignees

@shibd shibd

Labels

area/metrics doc-required Your PR changes impact docs and you will update later. ready-to-test type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages

Projects

None yet

Milestone

4.1.0

Development

Successfully merging this pull request may close these issues.

3 participants

@shibd @codecov-commenter @lhotari