Skip to content

[improve][broker] Reduce unnecessary MessageMetadata parsing by caching the parsed instance in the broker cache #24682

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
lhotari merged 10 commits into from
Sep 9, 2025
Merged

[improve][broker] Reduce unnecessary MessageMetadata parsing by caching the parsed instance in the broker cache #24682

lhotari merged 10 commits into from
Sep 9, 2025

Conversation

@lhotari
Copy link
Member

@lhotari lhotari commented Aug 28, 2025 •
edited
Loading

Fixes #24644

Motivation

See #24644. Currently in Pulsar, MessageMetadata isn't cached in the broker entry cache and parsing of MessageMetata also happens multiple times during dispatching. When there's a large fan-out, each dispatcher for each subscription will be also doing the parsing. Most of this could be avoided.

Modifications

  • add MessageMetadata caching
  • cleanup and optimize BrokerEntryMetadata parsing

Documentation

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

@lhotari lhotari added this to the 4.1.0 milestone Aug 28, 2025
@lhotari lhotari self-assigned this Aug 28, 2025
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Aug 28, 2025
@lhotari lhotari changed the title [improve][broker] Reduce [improve][broker] Reduce unnecessary MessageMetadata parsing by caching the parsed instance in the broker cache Aug 28, 2025
@coderzc coderzc modified the milestones: 4.1.0, 4.2.0 Sep 1, 2025
@coderzc coderzc requested a review from Technoboy- September 5, 2025 09:27
@Technoboy- Technoboy- closed this Sep 9, 2025
@Technoboy- Technoboy- reopened this Sep 9, 2025
@codecov-commenter
Copy link

codecov-commenter commented Sep 9, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 74.28571% with 45 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.10%. Comparing base (a2b69cc) to head (36a3f18).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...sar/broker/service/persistent/PersistentTopic.java 38.46% 5 Missing and 3 partials ⚠️
.../pulsar/broker/service/AbstractBaseDispatcher.java 56.25% 3 Missing and 4 partials ⚠️
...per/mledger/impl/cache/RangeCacheEntryWrapper.java 50.00% 1 Missing and 5 partials ⚠️
...pulsar/broker/admin/impl/PersistentTopicsBase.java 50.00% 3 Missing and 3 partials ⚠️
...main/java/org/apache/bookkeeper/mledger/Entry.java 50.00% 2 Missing and 2 partials ⚠️
...ker/service/persistent/PersistentSubscription.java 40.00% 2 Missing and 1 partial ⚠️
...va/org/apache/pulsar/common/protocol/Commands.java 93.18% 2 Missing and 1 partial ⚠️
...apache/pulsar/broker/service/EntryAndMetadata.java 75.00% 1 Missing and 1 partial ⚠️
...va/org/apache/pulsar/broker/service/ServerCnx.java 33.33% 1 Missing and 1 partial ⚠️
...ransaction/buffer/impl/TopicTransactionBuffer.java 33.33% 1 Missing and 1 partial ⚠️
... and 2 more
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #24682      +/-   ##
============================================
- Coverage     74.17%   74.10%   -0.07%     
- Complexity    33470    33480      +10     
============================================
  Files          1895     1896       +1     
  Lines        148029   148109      +80     
  Branches      17142    17163      +21     
============================================
- Hits         109796   109762      -34     
- Misses        29479    29564      +85     
- Partials       8754     8783      +29
Flag Coverage Δ
inttests 26.39% <30.28%> (+0.14%) ⬆️
systests 22.65% <26.85%> (-0.01%) ⬇️
unittests 73.64% <74.28%> (-0.06%) ⬇️

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

Files with missing lines Coverage Δ
.../org/apache/bookkeeper/mledger/impl/EntryImpl.java 90.27% <100.00%> (+0.88%) ⬆️
...org/apache/bookkeeper/mledger/impl/OpAddEntry.java 77.35% <100.00%> (-0.11%) ⬇️
...okkeeper/mledger/impl/ShadowManagedLedgerImpl.java 59.04% <100.00%> (+0.19%) ⬆️
...kkeeper/mledger/impl/cache/EntryCacheDisabled.java 76.59% <100.00%> (+1.04%) ⬆️
...ache/bookkeeper/mledger/impl/cache/RangeCache.java 76.28% <100.00%> (-0.06%) ⬇️
...keeper/mledger/impl/cache/RangeEntryCacheImpl.java 70.95% <100.00%> (+0.10%) ⬆️
...tent/NonPersistentDispatcherMultipleConsumers.java 67.46% <100.00%> (ø)
...t/NonPersistentDispatcherSingleActiveConsumer.java 87.50% <100.00%> (ø)
...ersistentStickyKeyDispatcherMultipleConsumers.java 75.28% <100.00%> (ø)
...sistent/PersistentDispatcherMultipleConsumers.java 75.53% <100.00%> (+0.03%) ⬆️
... and 17 more

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

@lhotari lhotari merged commit 53bbd4a into apache:master Sep 9, 2025
156 of 162 checks passed
Technoboy- pushed a commit that referenced this pull request Sep 10, 2025
@lhotari @Technoboy-
[improve][broker] Reduce unnecessary MessageMetadata parsing by cachi…
6509ee3 
…ng the parsed instance in the broker cache (#24682)
KannarFr pushed a commit to CleverCloud/pulsar that referenced this pull request Sep 22, 2025
@lhotari @KannarFr
[improve][broker] Reduce unnecessary MessageMetadata parsing by cachi…
ee9583f 
…ng the parsed instance in the broker cache (apache#24682)
walkinggo pushed a commit to walkinggo/pulsar that referenced this pull request Oct 8, 2025
@lhotari @walkinggo
[improve][broker] Reduce unnecessary MessageMetadata parsing by cachi…
b37b46e 
…ng the parsed instance in the broker cache (apache#24682)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderzc coderzc coderzc approved these changes

@Technoboy- Technoboy- Awaiting requested review from Technoboy-

Assignees

@lhotari lhotari

Labels

cherry-picked/branch-4.1 doc-not-needed Your PR changes do not impact docs ready-to-test release/4.1.1

Projects

None yet

Milestone

4.2.0

Development

Successfully merging this pull request may close these issues.

[Bug] Performance improvement opportunity in caching parsed message metadata

4 participants

@lhotari @codecov-commenter @coderzc @Technoboy-