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

PIP-245: Make subscriptions of NonPersistentTopic non-durable #19448

Closed
tjiuming opened this issue Feb 7, 2023 · 5 comments
Closed

PIP-245: Make subscriptions of NonPersistentTopic non-durable #19448

tjiuming opened this issue Feb 7, 2023 · 5 comments
Assignees

Comments

@tjiuming
Copy link
Contributor

tjiuming commented Feb 7, 2023

Motivation

There are two types of subscriptions for a topic: Durable and Non-durable.

We create a Consumer with a Durable subscription and a Reader with a Non-durable subscription.

But for NonPersistentTopic, creating a Durable subscription is meaningless, NonPersistentSubscription doesn't have a ManagedCursor to persistent its data. After its consumer disconnected, the subscription couldn't be removed automatically if we didn't set the value of subscriptionExpirationTimeMinutes greater than 0.

For subscriptionExpirationTimeMinutes, it controls the subscription expiration of NonPersistentTopic and PersistentTopic, if we set the value of subscriptionExpirationTimeMinutes greater than 0, it may lead to data loss(The durable subscriptions of PersistentTopic also can be removed).

And the Non-durable subscriptions will be removed automatically after all the consumers disconnected, it's the existing logic.

For the purpose of removing the subscriptions which have no active consumers of NonPersistentTopic and the above reasons, we can make all the subscriptions of a NonPersistentTopic Non-durable.

Goal

Make the subscriptions of NonPersistentTopic Non-durable.

API Changes

Implementation

1. Client-side:

Add the following logic to the constructor of ConsumerImpl before this.subscriptionMode=conf.getSubscriptionMode():

        TopicName topicName = TopicName.get(topic);
        if (!topicName.isPersistent() && conf.getSubscriptionMode().equals(SubscriptionMode.Durable)) {
            conf.setSubscriptionMode(SubscriptionMode.NonDurable);
            log.warn("[{}] Cannot create a [Durable] subscription for a NonPersistentTopic, " +
                    "will use [NonDurable] to subscribe. Subscription name: {}", topic, conf.getSubscriptionName());
        }

Check the topic type, if the topic is a NonPersistentTopic, change the value of subscriptionMode to NonDurable and print WARN log, Users don't need to change their code.

2. Broker-side:

Change the constructor of NonPersistentTopic from

   public NonPersistentSubscription(NonPersistentTopic topic, String subscriptionName, boolean isDurable, 
   Map<String, String> properties) {
       this.topic = topic;
       this.topicName = topic.getName();
       this.subName = subscriptionName;
       this.fullName = MoreObjects.toStringHelper(this).add("topic", topicName).add("name", subName).toString();
       IS_FENCED_UPDATER.set(this, FALSE);
       this.lastActive = System.currentTimeMillis();
       this.isDurable = isDurable;
       this.subscriptionProperties = properties != null
               ? Collections.unmodifiableMap(properties) : Collections.emptyMap();
   }

to

   public NonPersistentSubscription(NonPersistentTopic topic, String subscriptionName,
   Map<String, String> properties) {
       this.topic = topic;
       this.topicName = topic.getName();
       this.subName = subscriptionName;
       this.fullName = MoreObjects.toStringHelper(this).add("topic", topicName).add("name", subName).toString();
       IS_FENCED_UPDATER.set(this, FALSE);
       this.lastActive = System.currentTimeMillis();
       this.isDurable = false;
       this.subscriptionProperties = properties != null
               ? Collections.unmodifiableMap(properties) : Collections.emptyMap();
   }

Remove the constructor argument isDurable, and set its value to false.

Compatibility

The PIP will deprecate Durable subscriptions on NonPersistentTopic in 3.0.0, if users want to create a Durable subscription on NonPersistentTopic we will print WARN logs and change isDurable to false.

We will not be compatible with the next version after 3.0.1.

In the next release after 3.0.0, if users want to create Durable subscriptions on NonPersistentTopic, will throw an exception.

Alternatives

No response

Anything else?

No response

@tjiuming tjiuming changed the title PIP-245: Subscriptions expiration for NonPersistentTopic only [DISCUSS]PIP-245: Subscriptions expiration for NonPersistentTopic only Feb 7, 2023
@tjiuming
Copy link
Contributor Author

tjiuming commented Feb 7, 2023

@tjiuming tjiuming changed the title [DISCUSS]PIP-245: Subscriptions expiration for NonPersistentTopic only [DISCUSS]PIP-245: Make subscriptions of NonPersistentTopic NonDurable Feb 12, 2023
@tjiuming tjiuming changed the title [DISCUSS]PIP-245: Make subscriptions of NonPersistentTopic NonDurable [DISCUSS]PIP-245: Make subscriptions of NonPersistentTopic non-durable Feb 12, 2023
@codelipenghui
Copy link
Contributor

The default subscription mode is durable. So users will see the warning log if they upgrade to the new version client? Why not just change isDurable to false for the non-persistent topic subscriptions. And if users are explicitly changed to true, then see the warning log.

@tjiuming
Copy link
Contributor Author

The default subscription mode is durable. So users will see the warning log if they upgrade to the new version client?

Yes

Why not just change isDurable to false for the non-persistent topic subscriptions. And if users are explicitly changed to true, then see the warning log.

Because PersistentTopic and NonPersistentTopic subscriptions are using ConsumerConfigurationData,
if we change isDurable to false, when create consumers on PersistentTopic, we also need to add logics to change isDurable to true.

@codelipenghui

@dao-jun dao-jun changed the title [DISCUSS]PIP-245: Make subscriptions of NonPersistentTopic non-durable PIP-245: Make subscriptions of NonPersistentTopic non-durable Mar 7, 2023
@dao-jun
Copy link
Member

dao-jun commented Mar 7, 2023

@github-actions
Copy link

github-actions bot commented Apr 7, 2023

The issue had no activity for 30 days, mark with Stale label.

@github-actions github-actions bot added the Stale label Apr 7, 2023
@dao-jun dao-jun closed this as completed May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants