Skip to content

Issue 1067: Problems with Partitioned Topics which name contains -partition-N#2342

Merged
sijie merged 4 commits intoapache:masterfrom
sijie:dont_allow_partition_name
Aug 11, 2018
Merged

Issue 1067: Problems with Partitioned Topics which name contains -partition-N#2342
sijie merged 4 commits intoapache:masterfrom
sijie:dont_allow_partition_name

Conversation

@sijie
Copy link
Copy Markdown
Member

@sijie sijie commented Aug 8, 2018

Motivation

Fixes #1067.

Someone accidentally created a partitioned topic in one of our cluster with a name which contains -partition-2.
This raised all sorts of issues, it would seem that only one partition was created for this topic, but metadata exists saying that it has 10 partitions.

Changes

Disallow creating a partitioned topic contains '-partition-' in the name.

…tition-N

 ### Motivation

Fixes apache#1067.

Someone accidentally created a partitioned topic in one of our cluster with a name which contains -partition-2.
This raised all sorts of issues, it would seem that only one partition was created for this topic, but metadata exists saying that it has 10 partitions.

 ### Changes

Disallow creating a partitioned topic contains '-partition-' in the name.
@sijie sijie added type/bug The PR fixed a bug or issue reported a bug area/broker labels Aug 8, 2018
@sijie sijie added this to the 2.2.0-incubating milestone Aug 8, 2018
@sijie sijie self-assigned this Aug 8, 2018
@srkukarni
Copy link
Copy Markdown
Contributor

srkukarni commented Aug 8, 2018

rerun java8 tests

1 similar comment
@sijie
Copy link
Copy Markdown
Member Author

sijie commented Aug 8, 2018

rerun java8 tests

Copy link
Copy Markdown
Contributor

@merlimat merlimat left a comment

Choose a reason for hiding this comment

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

👍

// first, it has to be a validate topic name
validateTopicName(tenant, namespace, encodedTopic);
// second, "-partition-" is not allowed
if (encodedTopic.contains("-partition-")) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we use TopicName.PARTITIONED_TOPIC_SUFFIX instead a plain string?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

changed it to use the constant

@sijie sijie merged commit cb16f55 into apache:master Aug 11, 2018
@sijie sijie deleted the dont_allow_partition_name branch August 11, 2018 02:40
sijie added a commit that referenced this pull request Aug 27, 2018
…tition-N (#2342)

* Issue 1067: Problems with Partitioned Topics which name contains -partition-N

 ### Motivation

Fixes #1067.

Someone accidentally created a partitioned topic in one of our cluster with a name which contains -partition-2.
This raised all sorts of issues, it would seem that only one partition was created for this topic, but metadata exists saying that it has 10 partitions.

 ### Changes

Disallow creating a partitioned topic contains '-partition-' in the name.

* only validate partitioned topic name when create/delete/update partitioned topics

* Changed to use a CONSTANT
@sijie
Copy link
Copy Markdown
Member Author

sijie commented Aug 27, 2018

merged as
05488a0 in branch-2.1

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

Labels

area/broker 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.

5 participants