Skip to content

Conversation

@michaeljmarshall
Copy link
Member

Motivation

Builds on #18997. Essentially, we shouldn't get the consumer future when there isn't a broker interceptor.

Modifications

  • Move the conditional check for brokerInterceptor != null earlier to prevent unnecessary work.

Verifying this change

This is a trivial change.

Documentation

  • doc-not-needed

@michaeljmarshall michaeljmarshall added type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages area/broker doc-not-needed Your PR changes do not impact docs ready-to-test labels Dec 21, 2022
@michaeljmarshall michaeljmarshall added this to the 2.12.0 milestone Dec 21, 2022
@michaeljmarshall michaeljmarshall self-assigned this Dec 21, 2022
@codecov-commenter
Copy link

codecov-commenter commented Dec 21, 2022

Codecov Report

Merging #19022 (af2af2f) into master (08591d9) will decrease coverage by 2.73%.
The diff coverage is 58.82%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #19022      +/-   ##
============================================
- Coverage     49.85%   47.12%   -2.74%     
- Complexity     8658    10620    +1962     
============================================
  Files           500      709     +209     
  Lines         54930    69352   +14422     
  Branches       5867     7440    +1573     
============================================
+ Hits          27386    32681    +5295     
- Misses        24464    33002    +8538     
- Partials       3080     3669     +589     
Flag Coverage Δ
unittests 47.12% <58.82%> (-2.74%) ⬇️

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

Impacted Files Coverage Δ
...g/apache/pulsar/broker/lookup/TopicLookupBase.java 55.17% <ø> (+4.33%) ⬆️
...rg/apache/pulsar/broker/web/PulsarWebResource.java 58.34% <ø> (+2.68%) ⬆️
...va/org/apache/pulsar/broker/service/ServerCnx.java 48.34% <33.33%> (+0.53%) ⬆️
...pulsar/broker/admin/impl/PersistentTopicsBase.java 59.44% <72.72%> (+0.72%) ⬆️
...rvice/schema/KeyValueSchemaCompatibilityCheck.java 21.62% <0.00%> (-45.95%) ⬇️
...lsar/broker/loadbalance/impl/ThresholdShedder.java 3.27% <0.00%> (-27.05%) ⬇️
.../apache/pulsar/broker/loadbalance/LoadManager.java 61.11% <0.00%> (-16.67%) ⬇️
...tent/NonPersistentDispatcherMultipleConsumers.java 40.74% <0.00%> (-12.35%) ⬇️
...apache/pulsar/broker/service/TopicListService.java 40.80% <0.00%> (-12.00%) ⬇️
...pulsar/broker/service/PulsarCommandSenderImpl.java 66.32% <0.00%> (-9.19%) ⬇️
... and 250 more

Copy link
Contributor

@nicoloboschi nicoloboschi left a comment

Choose a reason for hiding this comment

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

Good catch!

@codelipenghui codelipenghui merged commit 74017d5 into apache:master Dec 22, 2022
@michaeljmarshall michaeljmarshall deleted the optimize-call-to-broker-interceptor branch December 22, 2022 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/broker doc-not-needed Your PR changes do not impact docs 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

Development

Successfully merging this pull request may close these issues.

5 participants