[fix][client] Make DeadLetterPolicy & KeySharedPolicy serializable

[AnuragReddy2000]

Fixes apache#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 the PulsarSpout seems to be lost when the topology is started. Upon investigation, we found that this is due to the deadLetterPolicy attribute in ConsumerConfigurationData 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.

Verifying this change

• Make sure that the change passes the CI checks.

This change is already covered by existing tests, such ClientConfigurationDataTest.java

Does this pull request potentially affect one of the following parts:

• 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

Matching PR in forked repository

PR in forked repository:

[lhotari]

Add private static final long serialVersionUID = 1L; to the implementation classes of the abstract KeySharedPolicy class instead of adding it to the abstract class where it is unnecessary.

[AnuragReddy2000]

Yes, my bad! Made these changes.

[lhotari]

Since this change is also included in this PR, I guess the transient definition for the keyHashRanges field in ReaderConfigurationData could be removed.

[AnuragReddy2000]

Agreed. Removed the transient keyword there too.

[lhotari]

This change is already covered by existing tests, such ClientConfigurationDataTest.java

I think it would be useful to reproduce the serialization problem that you explained in the issue apache#23704

[lhotari]

btw. I didn't notice until now that this is the PR in your fork to run Pulsar CI. Once you are ready with the initial pass, you can go ahead and open the PR at apache/pulsar for this PR branch. I can continue the review on that PR. I usually don't add review comments in the shadowed PR in the fork (which is for having full control of Pulsar CI and getting that feedback while working on the PR).

[AnuragReddy2000]

btw. I didn't notice until now that this is the PR in your fork to run Pulsar CI. Once you are ready with the initial pass, you can go ahead and open the PR at apache/pulsar for this PR branch. I can continue the review on that PR. I usually don't add review comments in the shadowed PR in the fork (which is for having full control of Pulsar CI and getting that feedback while working on the PR).

Sure. I just need one clarification though. When you said that it would be useful to reproduce the serialization problem, Did you mean for me to enhance the tests for that class to include checks on these fields?

[lhotari]

Sure. I just need one clarification though. When you said that it would be useful to reproduce the serialization problem, Did you mean for me to enhance the tests for that class to include checks on these fields?

I meant that there should be a test case what you described in the issue itself: "Create an instance of ConsumerConfigurationData class and set a deadLetterPolicy on it. Serialize it and de-serialize it back to get a new instance of the same. The deadLetterPolicy previously set should not be present after deserialization.".

[AnuragReddy2000]

@lhotari All the CI actions, (except the code coverage upload actions) have run successfully. I have raised a PR against the main repo master branch (apache#23718). Please do let me know if anything else is needed from my end.

[AnuragReddy2000]

Closing this PR as it is merged on the main repo (apache#23718)