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] [broker] [break change] Do not create partitioned DLQ/Retry topic automatically #22705

Merged
merged 2 commits into from
May 15, 2024

Conversation

poorbarcode
Copy link
Contributor

@poorbarcode poorbarcode commented May 13, 2024

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

Motivation

After you set defaultNumPartitions to 16, you will get 16 partitions per topic, and 16*16 DLQ partitions per subscription, you will get a huge number of DLQs if you have more than one subscription under one topic. For example:

  • create topic t1 with 16 partitions
  • you will get t1-partition-0, t1-partition-1...tp-partition-15.
  • after you enable DLQ, you will get the following 16*16 DLQs per subscription
    • t1-partition-0-{subscription}-DLQ-partition-0
    • t1-partition-0-{subscription}-DLQ-partition-1
    • ...
    • t1-partition-15-{subscription}-DLQ-partition-15
    • t1-partition-1-{subscription}-DLQ-partition-0
    • t1-partition-1-{subscription}-DLQ-partition-1
    • ...
    • ...
    • ...
    • t1-partition-15-{subscription}-partition-15

Issue both partitioned DLQ and non-partitioned DLQ were created automatically.

  • Background
    • Broker settings: {allowAutoTopicCreation=true, defaultNumPartitions=1, allowAutoTopicCreationType=partitioned}
    • client version < 3.1
  • Flows of creating DLQ.
    • Trigger the DLQ creation.
    • Get partitioned metadata of DLQ: partitions: 1.
      • Broker created the partitioned metadata at this step.
    • Before 3.1, the client mistakenly assumed {tenant}/{namespace}/{topic}-partition-0-{subscription}-DLQ-partition-0 is {tenant}/{namespace}/{topic}-partition-0-{subscription}-DLQ due to the bug that was fixed by [fix] [broker] Fix wrong logic of method TopicName.getPartition(int index) #19841.
    • Start a dead-letter producer under {tenant}/{namespace}/{topic}-partition-0-{subscription}-DLQ
      • Triggered a non-partitioned topic creation.

Eventually, you will get both partitioned and non-partitioned DLQ.

> bin/pulsar-admin topics list-partitioned-topics {tenant}/{namespace}
persistent://{tenant}/{namespace}/{topic}-partition-0-{subscription}-DLQ

> bin/pulsar-admin topics list {tenant}/{namespace}                  
persistent://{tenant}/{namespace}/{topic}-partition-0-{subscription}-DLQ

Modifications

  • Do not create partitioned DLQs/Retry topics automatically.
  • Users can also create partitioned DLQ manually if they need it.

Documentation

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

Matching PR in forked repository

PR in forked repository: x

@poorbarcode poorbarcode added this to the 3.4.0 milestone May 13, 2024
@poorbarcode poorbarcode self-assigned this May 13, 2024
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label May 13, 2024
@poorbarcode poorbarcode changed the title [improve] [broker] Do not create partitioned DLQ/Retry topic automatically [improve] [broker] [break change] Do not create partitioned DLQ/Retry topic automatically May 14, 2024
@mattisonchao
Copy link
Member

cc @shibd, I remember we discussed this place.

@poorbarcode
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

dao-jun
dao-jun previously approved these changes May 14, 2024
@dao-jun dao-jun dismissed their stale review May 14, 2024 17:22

dismiss the approve, need more discuss

@dao-jun
Copy link
Member

dao-jun commented May 14, 2024

I sent a comment to the mail list, PTAL https://lists.apache.org/thread/x5x5kyxwypothwsmrb7x3fflhklrdzb9

@poorbarcode poorbarcode requested a review from dao-jun May 14, 2024 17:53
@dao-jun dao-jun added the release/important-notice The changes which are important should be mentioned in the release note label May 15, 2024
@poorbarcode poorbarcode merged commit f07b3a0 into apache:master May 15, 2024
55 of 56 checks passed
poorbarcode added a commit that referenced this pull request May 15, 2024
… topic automatically (#22705)

(cherry picked from commit f07b3a0)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request May 31, 2024
… topic automatically (apache#22705)

(cherry picked from commit f07b3a0)
(cherry picked from commit 6ec15d8)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 4, 2024
… topic automatically (apache#22705)

(cherry picked from commit f07b3a0)
(cherry picked from commit 6ec15d8)
mukesh154 pushed a commit to cognitree/ds_pulsar that referenced this pull request Jun 4, 2024
… topic automatically (apache#22705)

(cherry picked from commit f07b3a0)
(cherry picked from commit 6ec15d8)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 7, 2024
… topic automatically (apache#22705)

(cherry picked from commit f07b3a0)
(cherry picked from commit 6ec15d8)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-picked/branch-3.0 doc-not-needed Your PR changes do not impact docs ready-to-test release/important-notice The changes which are important should be mentioned in the release note release/3.0.6
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants