-
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][client] Make DeadLetterPolicy & KeySharedPolicy serializable #23718
[fix][client] Make DeadLetterPolicy & KeySharedPolicy serializable #23718
Conversation
oh, that is a unconventional use case I've been never seen. @lhotari Do you think is this change makes sense? |
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.
LGTM, good work @AnuragReddy2000
/pulsarbot rerun-failure-checks |
@lhotari A test is failing on this PR which did not fail on the PR that raised in my fork. I checked the test, I'm not sure how my changes may cause it to fail. What do you suggest I do here? |
@AnuragReddy2000 there are quite a few flaky tests. Reporting ones that haven't already been reported and retrying is the way to handle it. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #23718 +/- ##
============================================
+ Coverage 73.57% 74.41% +0.83%
- Complexity 32624 34588 +1964
============================================
Files 1877 1945 +68
Lines 139502 147480 +7978
Branches 15299 16277 +978
============================================
+ Hits 102638 109742 +7104
- Misses 28908 29278 +370
- Partials 7956 8460 +504
Flags with carried forward coverage won't be shown. Click here to find out more.
|
…23718) Co-authored-by: anurag.reddy <[email protected]> (cherry picked from commit 14129e3)
…23718) Co-authored-by: anurag.reddy <[email protected]> (cherry picked from commit 14129e3)
…23718) Co-authored-by: anurag.reddy <[email protected]> (cherry picked from commit 14129e3)
…pache#23718) Co-authored-by: anurag.reddy <[email protected]> (cherry picked from commit 14129e3) (cherry picked from commit b9ce087)
…pache#23718) Co-authored-by: anurag.reddy <[email protected]> (cherry picked from commit 14129e3) (cherry picked from commit b9ce087)
Fixes #23704
Motivation
We use the pulsar client to consume messages from a Pulsar broker in a Storm topology. Recently, we encountered an issue where the
deadLetterPolicy
that we set on thePulsarSpout
seems to be lost when the topology is started. Upon investigation, we found that this is due to thedeadLetterPolicy
attribute inConsumerConfigurationData
being marked as transient. This prevents us from utilising the dead letter queue feature in our topology.Modifications
Made DeadLetterPolicy, KeySharedPolicy and Range classes implement the java.io.Serializable interface and removed the transient keyword from the corresponding fields in the ConsumerConfigurationData class. In addition, two tests were also modified / updated.
Verifying this change
This change added tests and can be verified as follows:
Does this pull request potentially affect one of the following parts:
Documentation
doc
doc-required
doc-not-needed
doc-complete
Matching PR in forked repository
PR in forked repository: AnuragReddy2000#1