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.

Sign up for GitHub

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

Jump to bottom

PIP-263: Just auto-create no-partitioned DLQ And Prevent auto-create a DLQ for a DLQ #20033

Closed
poorbarcode opened this issue Apr 6, 2023 · 2 comments
Closed

PIP-263: Just auto-create no-partitioned DLQ And Prevent auto-create a DLQ for a DLQ #20033

poorbarcode opened this issue Apr 6, 2023 · 2 comments
Labels
Stale type/PIP

Comments

@poorbarcode
Copy link
Contributor

poorbarcode commented Apr 6, 2023 •
edited

Discussion: https://lists.apache.org/thread/tp25lom2ggztljlo76krsldo270f293j

Motivation

Just auto-create no-partitioned DLQ/Retry Topic

If enabled the config allowAutoTopicCreation, Pulsar will auto-create a topic when the client loads it; After setting config allowAutoTopicCreationType=partitioned, defaultNumPartitions=2, Pulsar will auto-create a partitioned topic(which have two partitions) when the client loads it.

After the above, if using the feature Retry Topic and DLQ enable topic auto-creation, we will get a partitioned DLQ and a partitioned Retry Topic like this:

  • {primary_topic_name}-{sub_name}-DLQ
    -{primary_topic_name}-{sub_name}-DLQ-partition-0
    -{primary_topic_name}-{sub_name}-DLQ-partition-1
  • {primary_topic_name}-{sub_name}-RETRY
    -{primary_topic_name}-{sub_name}-RETRY-partition-0
    -{primary_topic_name}-{sub_name}-RETRY-partition-1

I feel that almost all users will not use the multi-partitioned DLQ or multi-partitioned Retry topic because there is a bug that causes the above behavior to be incorrect, but we have yet to receive any issues about it. This bug causes the above behavior to look like this: When the partitioned DLQ is auto-created for the topic tp1-partition-0, Pulsar will create a partitioned topic meta which has two partitioned but only create a topic named {primary_topic_name}-{sub_name}-DLQ, there is no topic named {primary_topic_name}-{sub_name}-DLQ-partition-x. Please look at this PR for a detailed bug description.

So I want to change the behavior to Just auto-create no-partitioned DLQ/Retry Topic.

Prevent auto-create the DLQ for a DLQ

If we use regex-topic(not filter out the Retry topics) consumer and enable retry, and after several times restart the client. it is possible to create such a topic persistent://public/default/tp1-sub1-RETRY-sub2-RETRY-sub3-RETRY.....
Please look at this Discussion for the detail.

Goal

  • Just auto-create no-partitioned DLQ/Retry Topic(with the other words: prevent auto-create partitioned DLQ)
  • DLQ/Retry topic should not create for a DLQ/Retry Topic
    • roles:
      • DLQ will not auto-create for a DLQ
      • Retry Topic will not auto-create for a Retry Topic
      • DLQ will not auto-create for a Retry Topic
      • Retry Topic will not auto-create for a DLQ
    • client changes: Clients will not create a DLQ for a DLQ
    • broker changes: rejected the request which wants to auto-create a DLQ for a DLQ

API Changes

CommandSubscribe.java

/**
  * This is an enumeration value with tree options: "standard", "dead letter", "retry letter".
  */
private String topicPurpose;

Properties of Topic

"purposeOfAutoCreatedTopic": value with tree options: "standard", "dead letter", "retry letter"

Why not use two properties: isAutoCreated and topicPurpose?
Because there is a scenario like this: auto-create a topic, use it as a DLQ after a few days, and not use it as a DLQ after a few days, this Topic will be allowed to have DLQ/Retry Topic. We only mark the topics created for DLQ/Retry purposes.
@sijie sijie mentioned this issue Apr 6, 2023
Open
@poorbarcode poorbarcode mentioned this issue Apr 7, 2023
Merged
4 tasks
@poorbarcode poorbarcode mentioned this issue Apr 18, 2023
Merged
4 tasks
@github-actions
Copy link

github-actions bot commented May 8, 2023

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

@github-actions github-actions bot added the Stale label May 8, 2023
@Technoboy- Technoboy- mentioned this issue May 31, 2023
Closed
2 tasks
@poorbarcode
Copy link
Contributor Author

instead of #20445

@poorbarcode poorbarcode mentioned this issue Jan 30, 2024
Merged
4 tasks
poorbarcode added a commit that referenced this issue Jan 31, 2024
@poorbarcode
[fix] [broker] [branch-3.0] Fast fix infinite HTTP call createSubscri…
4386401 
…ptions caused by wrong topicName (#21997)

Similar to: #20131

The master branch has fixed the issue by #19841 Since it will makes users can not receive the messages which created in mistake, we did not cherry-pick #19841 into other branches, see detail #19841)

### Motivation

#### Background of Admin API `PersistentTopics.createSubscription`
It works like this:
1. createSubscription( `tp1` )
2. is partitioned topic? 
  `no`: return subscriptions
  `yes`: createSubscription(`tp1-partition-0`)....createSubscription(`tp1-partition-n`)

--- 

#### Background of the issue of `TopicName.getPartition(int index)`
```java
String partitionedTopic = "tp1-partition-0-DLQ";

TopicName partition0 = partitionedTopic.getPartition(0);// Highlight: the partition0.toString() will be "tp1-partition-0-DLQ"(it is wrong).The correct value is "tp1-partition-0-DLQ-partition-0"
```


#### Issue
Therefore, if there has a partitioned topic named `tp1-partition-0-DLQ`, the method `PersistentTopics.createSubscription` will works like this:
1. call Admin API ``PersistentTopics.createSubscription("tp1-partition-0-DLQ")`
2. is partitioned topic? 
3. yes, call `TopicName.getPartition(0)` to get partition 0 and will get `tp1-partition-0-DLQ` , then loop to step-1.

Then the infinite HTTP call `PersistentTopics.createSubscription` makes the broker crash.

### Modifications

#### Quick fix(this PR does it)
If hits the issue which makes the topic name wrong, do not loop to step 1.

#### Long-term fix
The PR #19841 fixes the issue which makes the topic name wrong, and this PR will create unfriendly compatibility, and PIP 263 #20033 will make compatibility good.
mukesh-ctds pushed a commit to datastax/pulsar that referenced this issue Apr 15, 2024
@poorbarcode @mukesh-ctds
[fix] [broker] [branch-3.0] Fast fix infinite HTTP call createSubscri…
58a421a 
…ptions caused by wrong topicName (apache#21997)

Similar to: apache#20131

The master branch has fixed the issue by apache#19841 Since it will makes users can not receive the messages which created in mistake, we did not cherry-pick apache#19841 into other branches, see detail apache#19841)

It works like this:
1. createSubscription( `tp1` )
2. is partitioned topic?
  `no`: return subscriptions
  `yes`: createSubscription(`tp1-partition-0`)....createSubscription(`tp1-partition-n`)

---

```java
String partitionedTopic = "tp1-partition-0-DLQ";

TopicName partition0 = partitionedTopic.getPartition(0);// Highlight: the partition0.toString() will be "tp1-partition-0-DLQ"(it is wrong).The correct value is "tp1-partition-0-DLQ-partition-0"
```

Therefore, if there has a partitioned topic named `tp1-partition-0-DLQ`, the method `PersistentTopics.createSubscription` will works like this:
1. call Admin API ``PersistentTopics.createSubscription("tp1-partition-0-DLQ")`
2. is partitioned topic?
3. yes, call `TopicName.getPartition(0)` to get partition 0 and will get `tp1-partition-0-DLQ` , then loop to step-1.

Then the infinite HTTP call `PersistentTopics.createSubscription` makes the broker crash.

If hits the issue which makes the topic name wrong, do not loop to step 1.

The PR apache#19841 fixes the issue which makes the topic name wrong, and this PR will create unfriendly compatibility, and PIP 263 apache#20033 will make compatibility good.

(cherry picked from commit 4386401)
mukesh-ctds pushed a commit to datastax/pulsar that referenced this issue Apr 17, 2024
@poorbarcode @mukesh-ctds
[fix] [broker] [branch-3.0] Fast fix infinite HTTP call createSubscri…
2cea505 
…ptions caused by wrong topicName (apache#21997)

Similar to: apache#20131

The master branch has fixed the issue by apache#19841 Since it will makes users can not receive the messages which created in mistake, we did not cherry-pick apache#19841 into other branches, see detail apache#19841)

It works like this:
1. createSubscription( `tp1` )
2. is partitioned topic?
  `no`: return subscriptions
  `yes`: createSubscription(`tp1-partition-0`)....createSubscription(`tp1-partition-n`)

---

```java
String partitionedTopic = "tp1-partition-0-DLQ";

TopicName partition0 = partitionedTopic.getPartition(0);// Highlight: the partition0.toString() will be "tp1-partition-0-DLQ"(it is wrong).The correct value is "tp1-partition-0-DLQ-partition-0"
```

Therefore, if there has a partitioned topic named `tp1-partition-0-DLQ`, the method `PersistentTopics.createSubscription` will works like this:
1. call Admin API ``PersistentTopics.createSubscription("tp1-partition-0-DLQ")`
2. is partitioned topic?
3. yes, call `TopicName.getPartition(0)` to get partition 0 and will get `tp1-partition-0-DLQ` , then loop to step-1.

Then the infinite HTTP call `PersistentTopics.createSubscription` makes the broker crash.

If hits the issue which makes the topic name wrong, do not loop to step 1.

The PR apache#19841 fixes the issue which makes the topic name wrong, and this PR will create unfriendly compatibility, and PIP 263 apache#20033 will make compatibility good.

(cherry picked from commit 4386401)
mukesh-ctds pushed a commit to datastax/pulsar that referenced this issue Apr 17, 2024
@poorbarcode @mukesh-ctds
[fix] [broker] [branch-3.0] Fast fix infinite HTTP call createSubscri…
af16ff6 
…ptions caused by wrong topicName (apache#21997)

Similar to: apache#20131

The master branch has fixed the issue by apache#19841 Since it will makes users can not receive the messages which created in mistake, we did not cherry-pick apache#19841 into other branches, see detail apache#19841)

It works like this:
1. createSubscription( `tp1` )
2. is partitioned topic?
  `no`: return subscriptions
  `yes`: createSubscription(`tp1-partition-0`)....createSubscription(`tp1-partition-n`)

---

```java
String partitionedTopic = "tp1-partition-0-DLQ";

TopicName partition0 = partitionedTopic.getPartition(0);// Highlight: the partition0.toString() will be "tp1-partition-0-DLQ"(it is wrong).The correct value is "tp1-partition-0-DLQ-partition-0"
```

Therefore, if there has a partitioned topic named `tp1-partition-0-DLQ`, the method `PersistentTopics.createSubscription` will works like this:
1. call Admin API ``PersistentTopics.createSubscription("tp1-partition-0-DLQ")`
2. is partitioned topic?
3. yes, call `TopicName.getPartition(0)` to get partition 0 and will get `tp1-partition-0-DLQ` , then loop to step-1.

Then the infinite HTTP call `PersistentTopics.createSubscription` makes the broker crash.

If hits the issue which makes the topic name wrong, do not loop to step 1.

The PR apache#19841 fixes the issue which makes the topic name wrong, and this PR will create unfriendly compatibility, and PIP 263 apache#20033 will make compatibility good.

(cherry picked from commit 4386401)
mukesh-ctds pushed a commit to datastax/pulsar that referenced this issue Apr 19, 2024
@poorbarcode @mukesh-ctds
[fix] [broker] [branch-3.0] Fast fix infinite HTTP call createSubscri…
5052803 
…ptions caused by wrong topicName (apache#21997)

Similar to: apache#20131

The master branch has fixed the issue by apache#19841 Since it will makes users can not receive the messages which created in mistake, we did not cherry-pick apache#19841 into other branches, see detail apache#19841)

It works like this:
1. createSubscription( `tp1` )
2. is partitioned topic?
  `no`: return subscriptions
  `yes`: createSubscription(`tp1-partition-0`)....createSubscription(`tp1-partition-n`)

---

```java
String partitionedTopic = "tp1-partition-0-DLQ";

TopicName partition0 = partitionedTopic.getPartition(0);// Highlight: the partition0.toString() will be "tp1-partition-0-DLQ"(it is wrong).The correct value is "tp1-partition-0-DLQ-partition-0"
```

Therefore, if there has a partitioned topic named `tp1-partition-0-DLQ`, the method `PersistentTopics.createSubscription` will works like this:
1. call Admin API ``PersistentTopics.createSubscription("tp1-partition-0-DLQ")`
2. is partitioned topic?
3. yes, call `TopicName.getPartition(0)` to get partition 0 and will get `tp1-partition-0-DLQ` , then loop to step-1.

Then the infinite HTTP call `PersistentTopics.createSubscription` makes the broker crash.

If hits the issue which makes the topic name wrong, do not loop to step 1.

The PR apache#19841 fixes the issue which makes the topic name wrong, and this PR will create unfriendly compatibility, and PIP 263 apache#20033 will make compatibility good.

(cherry picked from commit 4386401)
srinath-ctds pushed a commit to datastax/pulsar that referenced this issue Apr 23, 2024
@poorbarcode @srinath-ctds
[fix] [broker] [branch-3.0] Fast fix infinite HTTP call createSubscri…
0381bc4 
…ptions caused by wrong topicName (apache#21997)

Similar to: apache#20131

The master branch has fixed the issue by apache#19841 Since it will makes users can not receive the messages which created in mistake, we did not cherry-pick apache#19841 into other branches, see detail apache#19841)

It works like this:
1. createSubscription( `tp1` )
2. is partitioned topic?
  `no`: return subscriptions
  `yes`: createSubscription(`tp1-partition-0`)....createSubscription(`tp1-partition-n`)

---

```java
String partitionedTopic = "tp1-partition-0-DLQ";

TopicName partition0 = partitionedTopic.getPartition(0);// Highlight: the partition0.toString() will be "tp1-partition-0-DLQ"(it is wrong).The correct value is "tp1-partition-0-DLQ-partition-0"
```

Therefore, if there has a partitioned topic named `tp1-partition-0-DLQ`, the method `PersistentTopics.createSubscription` will works like this:
1. call Admin API ``PersistentTopics.createSubscription("tp1-partition-0-DLQ")`
2. is partitioned topic?
3. yes, call `TopicName.getPartition(0)` to get partition 0 and will get `tp1-partition-0-DLQ` , then loop to step-1.

Then the infinite HTTP call `PersistentTopics.createSubscription` makes the broker crash.

If hits the issue which makes the topic name wrong, do not loop to step 1.

The PR apache#19841 fixes the issue which makes the topic name wrong, and this PR will create unfriendly compatibility, and PIP 263 apache#20033 will make compatibility good.

(cherry picked from commit 4386401)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Stale type/PIP
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@poorbarcode