Skip to content
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

[improve][client] Make replicateSubscriptionState nullable #23757

Merged
merged 2 commits into from
Dec 20, 2024

Conversation

nodece
Copy link
Member

@nodece nodece commented Dec 19, 2024

Motivation

The current implementation of subscription replication relies on consumer control with the following configuration:

boolean replicateSubscriptionState = false;
// boolean replicateSubscriptionState = true;
pulsarClient.newConsumer(Schema.STRING)
                .replicateSubscriptionState(replicateSubscriptionState)

By default, the replicateSubscriptionState is set to false in the ConsumerConfigurationData.java, which creates a mismatch with the CommandSubscribe definition, where replicate_subscription_state is optional. To address this, I propose changing the default value of replicateSubscriptionState to null, aligning it with the optional nature of the field in CommandSubscribe.

This adjustment would enhance flexibility in managing subscription replication policies, such as applying the policy that comes from the namespace or topic level, which streamlines the configuration of subscription replication behavior. This feature is in progress.

These changes are compatible with both the old and new versions, there won't be any breaking changes.

Modifications

  • The replicateSubscriptionState type is changed from boolean to Boolean and the default value is null in theConsumerConfigurationData.java. When it is null, it won't set the value to the replicate_subscription_state of CommandSubscribe.
  • In the broker, use the Boolean instead of boolean to match the replicate_subscription_state of CommandSubscribe.

Documentation

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

@nodece nodece requested review from lhotari and dao-jun December 19, 2024 09:08
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Dec 19, 2024
@nodece nodece requested a review from crossoverJie December 19, 2024 09:23
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

This adjustment would enhance flexibility in managing subscription replication policies, such as applying the policy that comes from the namespace or topic level, which streamlines the configuration of subscription replication behavior. This feature is in progress.

This will be a useful feature.

@lhotari lhotari requested a review from merlimat December 20, 2024 06:59
@lhotari lhotari added this to the 4.1.0 milestone Dec 20, 2024
@nodece nodece force-pushed the make-replicateSubscriptionState-nullable branch from b388bc2 to 1f9d5c8 Compare December 20, 2024 07:49
@nodece nodece requested a review from liudezhi2098 December 20, 2024 07:53
@nodece nodece force-pushed the make-replicateSubscriptionState-nullable branch from f56aad3 to e2b4c99 Compare December 20, 2024 08:21
@nodece nodece self-assigned this Dec 20, 2024
@nodece nodece merged commit 3fce309 into apache:master Dec 20, 2024
51 of 52 checks passed
@lhotari
Copy link
Member

lhotari commented Dec 23, 2024

This adjustment would enhance flexibility in managing subscription replication policies, such as applying the policy that comes from the namespace or topic level, which streamlines the configuration of subscription replication behavior. This feature is in progress.

This will be a useful feature.

@nodece Please check #23769, it seems to be a related new feature.

@nodece nodece deleted the make-replicateSubscriptionState-nullable branch December 23, 2024 08:18
@nodece
Copy link
Member Author

nodece commented Dec 23, 2024

@lhotari Sure, go to #23769 and #23770.

nodece added a commit to ascentstream/pulsar that referenced this pull request Dec 25, 2024
)

Signed-off-by: Zixuan Liu <[email protected]>

(cherry picked from commit 3fce309)
Signed-off-by: Zixuan Liu <[email protected]>
nodece added a commit that referenced this pull request Dec 25, 2024
nodece added a commit that referenced this pull request Dec 30, 2024
nodece added a commit that referenced this pull request Dec 30, 2024
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Dec 31, 2024
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Dec 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants