-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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] do not serialize field bundleStats #18150
[fix][broker] do not serialize field bundleStats #18150
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch!
Could you please help add an unit test to make sure we will not break it again in the future?
pulsar-broker/src/test/java/org/apache/pulsar/broker/loadbalance/LocalBrokerDataTest.java
Outdated
Show resolved
Hide resolved
pulsar-broker/src/test/java/org/apache/pulsar/broker/loadbalance/LocalBrokerDataTest.java
Outdated
Show resolved
Hide resolved
pulsar-broker/src/test/java/org/apache/pulsar/broker/loadbalance/LocalBrokerDataTest.java
Outdated
Show resolved
Hide resolved
/* | ||
Ensuring that there no bundleStats field in the json string serialized from LocalBrokerData. | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/* | |
Ensuring that there no bundleStats field in the json string serialized from LocalBrokerData. | |
*/ | |
/** | |
* Ensure that there is no bundleStats field in the json string serialized from LocalBrokerData. | |
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i found another LocalBrokerDataTest class, so i move the implementation into it.
Codecov Report
@@ Coverage Diff @@
## master #18150 +/- ##
============================================
+ Coverage 34.91% 40.28% +5.36%
- Complexity 5707 15008 +9301
============================================
Files 607 1579 +972
Lines 53396 128710 +75314
Branches 5712 14152 +8440
============================================
+ Hits 18644 51852 +33208
- Misses 32119 71046 +38927
- Partials 2633 5812 +3179
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Fixes #17738
Modifications
because broker will do not use the field bundleStats, so we could do not serialize the field bundleStats.
Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
Documentation
doc
doc-required
doc-not-needed
doc-complete
Matching PR in forked repository
PR in forked repository:
thetumbled#3