Skip to content

Conversation

@mattisonchao
Copy link
Member

@mattisonchao mattisonchao commented Mar 29, 2022

Motivation

When the user set config as follow:

{
    allowAutoTopicCreation: true,
    allowAutoTopicCreationType: "partitioned",
    defaultNumPartitions: 1
}

they can create partitioned topic success with topic name persistent://prop/autoNs/failedcreate-partition-abcde.

run get-partitioned-topic-metadata

{
  "partitions" : 1
}

run admin.topics().deletePartitionedTopic(topicName);

Partitioned Topic Name should not contain '-partition-'

run admin.topics().delete(topicName)

this operation can success delete the topic, but metadata still exists.

Modifications

Describe the modifications you've done.

Verifying this change

  • Make sure that the change passes the CI checks.

Add TCP/HTTP client test for this change.

Documentation

  • no-need-doc

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Mar 29, 2022
@mattisonchao mattisonchao changed the title [fix][broker] Fix auto create partitioned topic when topic name contains -partition- [fix][broker] Reject auto create partitioned topic when topic name contains -partition- Mar 29, 2022
@mattisonchao
Copy link
Member Author

@Technoboy- @AnonHxy PTAL :)

@AnonHxy
Copy link
Contributor

AnonHxy commented Mar 29, 2022

LGTM

@RobertIndie RobertIndie added type/bug The PR fixed a bug or issue reported a bug area/broker labels Mar 30, 2022
Copy link
Contributor

@Jason918 Jason918 left a comment

Choose a reason for hiding this comment

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

I think it's better to narrow the limitation to ends with -partition-\d+

@mattisonchao mattisonchao reopened this Apr 1, 2022
@Jason918
Copy link
Contributor

Jason918 commented Apr 1, 2022

I think it's better to narrow the limitation to ends with -partition-\d+

@mattisonchao What do you think about this?

@mattisonchao
Copy link
Member Author

I think it's better to narrow the limitation to ends with -partition-\d+

@mattisonchao What do you think about this?

@Jason918
I'm sorry for omitting this message, I think it's a great idea.

@mattisonchao
Copy link
Member Author

Hi, @Jason918
I use topicName.getLocalName() to solve the problem you mentioned. PTAL:)

Copy link
Contributor

@Jason918 Jason918 left a comment

Choose a reason for hiding this comment

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

LGTM

@codelipenghui codelipenghui added the release/important-notice The changes which are important should be mentioned in the release note label Apr 6, 2022
@codelipenghui codelipenghui added this to the 2.11.0 milestone Apr 6, 2022
@mattisonchao
Copy link
Member Author

/pulsarbot rerun-failure-checks

@mattisonchao mattisonchao requested a review from Technoboy- April 7, 2022 08:34
@mattisonchao
Copy link
Member Author

/pulsarbot rerun-failure-checks

@Technoboy- Technoboy- merged commit 8add5db into apache:master Apr 8, 2022
aparajita89 pushed a commit to aparajita89/pulsar that referenced this pull request Apr 18, 2022
Nicklee007 pushed a commit to Nicklee007/pulsar that referenced this pull request Apr 20, 2022
@codelipenghui
Copy link
Contributor

@mattisonchao It might introduce a breaking change for DLQ, please help check the topic auto-creation with partitioned topic can work with DLQ with this change.

@mattisonchao
Copy link
Member Author

Yes, We have to revert this PR and start a discussion in the mail list to find another graceful method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/broker doc-not-needed Your PR changes do not impact docs release/important-notice The changes which are important should be mentioned in the release note type/bug The PR fixed a bug or issue reported a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants