You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/08/17 11:06:08 UTC
[GitHub] [pulsar] Nicklee007 opened a new pull request #11686: fix the bug, can not update topic when the update topicName is contained by an existed topic as a part
Nicklee007 opened a new pull request #11686:
URL: https://github.com/apache/pulsar/pull/11686
Fixes #11685
validatePartitionTopicUpdate use contain to check if there has a exist topic will cause conflict, which will cause a failed when exist a topic which contain the new topic's prefix and we want to update the new topic partition;
we have a those topic
"persistent://public/default/113p-partition-0"
"persistent://public/default/113p-partition-1"
"persistent://public/default/113p-partition-2"
"persistent://public/default/3p-partition-0"
when we want to update topic 3p to more partitions ,we failed because "persistent://public/default/113p-partition-0" contain "3p-partition"
### Modifications
use the startwith to check if exist the same topic.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Anonymitaet commented on pull request #11686: fix the bug, can not update topic when the update topicName is contained by an existed topic as a part
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on pull request #11686:
URL: https://github.com/apache/pulsar/pull/11686#issuecomment-900740552
Thanks for your contribution. For this PR, do we need to update docs?
(The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Nicklee007 commented on pull request #11686: fix the bug, can not update topic when the update topicName is contained by an existed topic as a part
Posted by GitBox <gi...@apache.org>.
Nicklee007 commented on pull request #11686:
URL: https://github.com/apache/pulsar/pull/11686#issuecomment-900756328
> Thanks for your contribution. For this PR, do we need to update docs?
>
> (The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
It's fix a bug and don't change any system function, I think not need to update docs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on a change in pull request #11686: fix the bug, can not update topic when the update topicName is contained by an existed topic as a part
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on a change in pull request #11686:
URL: https://github.com/apache/pulsar/pull/11686#discussion_r691821691
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -3773,9 +3773,9 @@ private void validatePartitionTopicUpdate(String topicName, int numberOfPartitio
TopicName partitionTopicName = TopicName.get(domain(), namespaceName, topicName);
PartitionedTopicMetadata metadata = getPartitionedTopicMetadata(partitionTopicName, false, false);
int oldPartition = metadata.partitions;
- String prefix = topicName + TopicName.PARTITIONED_TOPIC_SUFFIX;
+ String prefix = partitionTopicName.getPartitionedTopicName() + TopicName.PARTITIONED_TOPIC_SUFFIX;
for (String exsitingTopicName : existingTopicList) {
- if (exsitingTopicName.contains(prefix)) {
+ if (exsitingTopicName.startsWith(prefix)) {
Review comment:
@lhotari I don't think this is the same issue as #11148 wants to fix. This is a bug here, #11148 is trying to improve the topic creation logic.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Nicklee007 commented on pull request #11686: fix the bug, can not update topic when the update topicName is contained by an existed topic as a part
Posted by GitBox <gi...@apache.org>.
Nicklee007 commented on pull request #11686:
URL: https://github.com/apache/pulsar/pull/11686#issuecomment-900756328
> Thanks for your contribution. For this PR, do we need to update docs?
>
> (The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
It's fix a bug and don't change any system function, I think not need to update docs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Nicklee007 commented on pull request #11686: fix the bug, can not update topic when the update topicName is contained by an existed topic as a part
Posted by GitBox <gi...@apache.org>.
Nicklee007 commented on pull request #11686:
URL: https://github.com/apache/pulsar/pull/11686#issuecomment-902388777
> The change looks good to me, @Nicklee007 Could you please add some tests for the new change?
@codelipenghui thanks, I have added the test for update the partitioned topic which a part topicName is contained in another old topicName.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Nicklee007 commented on a change in pull request #11686: fix the bug, can not update topic when the update topicName is contained by an existed topic as a part
Posted by GitBox <gi...@apache.org>.
Nicklee007 commented on a change in pull request #11686:
URL: https://github.com/apache/pulsar/pull/11686#discussion_r690845275
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -3773,9 +3773,9 @@ private void validatePartitionTopicUpdate(String topicName, int numberOfPartitio
TopicName partitionTopicName = TopicName.get(domain(), namespaceName, topicName);
PartitionedTopicMetadata metadata = getPartitionedTopicMetadata(partitionTopicName, false, false);
int oldPartition = metadata.partitions;
- String prefix = topicName + TopicName.PARTITIONED_TOPIC_SUFFIX;
+ String prefix = partitionTopicName.getPartitionedTopicName() + TopicName.PARTITIONED_TOPIC_SUFFIX;
for (String exsitingTopicName : existingTopicList) {
- if (exsitingTopicName.contains(prefix)) {
+ if (exsitingTopicName.startsWith(prefix)) {
Review comment:
> This change won't work properly. The topic name will be in format `persistent://tenant/cluster/namespace/topicName` .
changed to the prefix = partitionTopicName.getPartitionedTopicName() + TopicName.PARTITIONED_TOPIC_SUFFIX , it's the full topicName contain 'persistent://tenant/cluster/namespace/topicName';
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Nicklee007 commented on pull request #11686: fix the bug, can not update topic when the update topicName is contained by an existed topic as a part
Posted by GitBox <gi...@apache.org>.
Nicklee007 commented on pull request #11686:
URL: https://github.com/apache/pulsar/pull/11686#issuecomment-902387839
> The change looks good to me, @Nicklee007 Could you please add some tests for the new change?
> new
thanks, I have added the test for update the partitioned topic which a part is contained in old topicName.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Nicklee007 removed a comment on pull request #11686: fix the bug, can not update topic when the update topicName is contained by an existed topic as a part
Posted by GitBox <gi...@apache.org>.
Nicklee007 removed a comment on pull request #11686:
URL: https://github.com/apache/pulsar/pull/11686#issuecomment-902387839
> The change looks good to me, @Nicklee007 Could you please add some tests for the new change?
> new
thanks, I have added the test for update the partitioned topic which a part is contained in old topicName.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] hangc0276 commented on pull request #11686: fix the bug, can not update topic when the update topicName is contained by an existed topic as a part
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on pull request #11686:
URL: https://github.com/apache/pulsar/pull/11686#issuecomment-905151924
@lhotari Would you please review this PR again? I am cutting 2.8.1, i'd like to include this pr in 2.8.1, thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on a change in pull request #11686: fix the bug, can not update topic when the update topicName is contained by an existed topic as a part
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on a change in pull request #11686:
URL: https://github.com/apache/pulsar/pull/11686#discussion_r690845106
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -3773,9 +3773,9 @@ private void validatePartitionTopicUpdate(String topicName, int numberOfPartitio
TopicName partitionTopicName = TopicName.get(domain(), namespaceName, topicName);
PartitionedTopicMetadata metadata = getPartitionedTopicMetadata(partitionTopicName, false, false);
int oldPartition = metadata.partitions;
- String prefix = topicName + TopicName.PARTITIONED_TOPIC_SUFFIX;
+ String prefix = partitionTopicName.getPartitionedTopicName() + TopicName.PARTITIONED_TOPIC_SUFFIX;
for (String exsitingTopicName : existingTopicList) {
- if (exsitingTopicName.contains(prefix)) {
+ if (exsitingTopicName.startsWith(prefix)) {
Review comment:
> There is a PR #11148 in progress in this same area. I'd suggest contributing to #11148 to fix this issue #11685. One form of contributing is making sure that this problem gets fixed with the refactored implementation for checking topic existence.
it's full name `String prefix = partitionTopicName.getPartitionedTopicName() + TopicName.PARTITIONED_TOPIC_SUFFIX`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Nicklee007 commented on a change in pull request #11686: fix the bug, can not update topic when the update topicName is contained by an existed topic as a part
Posted by GitBox <gi...@apache.org>.
Nicklee007 commented on a change in pull request #11686:
URL: https://github.com/apache/pulsar/pull/11686#discussion_r690830592
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -3773,9 +3773,9 @@ private void validatePartitionTopicUpdate(String topicName, int numberOfPartitio
TopicName partitionTopicName = TopicName.get(domain(), namespaceName, topicName);
PartitionedTopicMetadata metadata = getPartitionedTopicMetadata(partitionTopicName, false, false);
int oldPartition = metadata.partitions;
- String prefix = topicName + TopicName.PARTITIONED_TOPIC_SUFFIX;
+ String prefix = partitionTopicName.getPartitionedTopicName() + TopicName.PARTITIONED_TOPIC_SUFFIX;
for (String exsitingTopicName : existingTopicList) {
- if (exsitingTopicName.contains(prefix)) {
+ if (exsitingTopicName.startsWith(prefix)) {
Review comment:
> This change won't work properly. The topic name will be in format `persistent://tenant/cluster/namespace/topicName` .
yes I have considered that. in this PR，I changed to the prefix = topicName + TopicName.PARTITIONED_TOPIC_SUFFIX, it's the full topicName contain 'persistent://tenant/cluster/namespace/topicName';
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -3773,9 +3773,9 @@ private void validatePartitionTopicUpdate(String topicName, int numberOfPartitio
TopicName partitionTopicName = TopicName.get(domain(), namespaceName, topicName);
PartitionedTopicMetadata metadata = getPartitionedTopicMetadata(partitionTopicName, false, false);
int oldPartition = metadata.partitions;
- String prefix = topicName + TopicName.PARTITIONED_TOPIC_SUFFIX;
+ String prefix = partitionTopicName.getPartitionedTopicName() + TopicName.PARTITIONED_TOPIC_SUFFIX;
for (String exsitingTopicName : existingTopicList) {
- if (exsitingTopicName.contains(prefix)) {
+ if (exsitingTopicName.startsWith(prefix)) {
Review comment:
> This change won't work properly. The topic name will be in format `persistent://tenant/cluster/namespace/topicName` .
changed to the prefix = partitionTopicName.getPartitionedTopicName() + TopicName.PARTITIONED_TOPIC_SUFFIX , it's the full topicName contain 'persistent://tenant/cluster/namespace/topicName';
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Anonymitaet commented on pull request #11686: fix the bug, can not update topic when the update topicName is contained by an existed topic as a part
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on pull request #11686:
URL: https://github.com/apache/pulsar/pull/11686#issuecomment-900740552
Thanks for your contribution. For this PR, do we need to update docs?
(The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on a change in pull request #11686: fix the bug, can not update topic when the update topicName is contained by an existed topic as a part
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on a change in pull request #11686:
URL: https://github.com/apache/pulsar/pull/11686#discussion_r690845106
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -3773,9 +3773,9 @@ private void validatePartitionTopicUpdate(String topicName, int numberOfPartitio
TopicName partitionTopicName = TopicName.get(domain(), namespaceName, topicName);
PartitionedTopicMetadata metadata = getPartitionedTopicMetadata(partitionTopicName, false, false);
int oldPartition = metadata.partitions;
- String prefix = topicName + TopicName.PARTITIONED_TOPIC_SUFFIX;
+ String prefix = partitionTopicName.getPartitionedTopicName() + TopicName.PARTITIONED_TOPIC_SUFFIX;
for (String exsitingTopicName : existingTopicList) {
- if (exsitingTopicName.contains(prefix)) {
+ if (exsitingTopicName.startsWith(prefix)) {
Review comment:
> There is a PR #11148 in progress in this same area. I'd suggest contributing to #11148 to fix this issue #11685. One form of contributing is making sure that this problem gets fixed with the refactored implementation for checking topic existence.
it's full name `String prefix = partitionTopicName.getPartitionedTopicName() + TopicName.PARTITIONED_TOPIC_SUFFIX`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a change in pull request #11686: fix the bug, can not update topic when the update topicName is contained by an existed topic as a part
Posted by GitBox <gi...@apache.org>.
lhotari commented on a change in pull request #11686:
URL: https://github.com/apache/pulsar/pull/11686#discussion_r690464641
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -3773,9 +3773,9 @@ private void validatePartitionTopicUpdate(String topicName, int numberOfPartitio
TopicName partitionTopicName = TopicName.get(domain(), namespaceName, topicName);
PartitionedTopicMetadata metadata = getPartitionedTopicMetadata(partitionTopicName, false, false);
int oldPartition = metadata.partitions;
- String prefix = topicName + TopicName.PARTITIONED_TOPIC_SUFFIX;
+ String prefix = partitionTopicName.getPartitionedTopicName() + TopicName.PARTITIONED_TOPIC_SUFFIX;
for (String exsitingTopicName : existingTopicList) {
- if (exsitingTopicName.contains(prefix)) {
+ if (exsitingTopicName.startsWith(prefix)) {
Review comment:
There is a PR #11148 in progress in this same area. I'd suggest contributing to #11148 to fix this issue #11685. One form of contributing is making sure that this problem gets fixed with the refactored implementation for checking topic existence.
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -3773,9 +3773,9 @@ private void validatePartitionTopicUpdate(String topicName, int numberOfPartitio
TopicName partitionTopicName = TopicName.get(domain(), namespaceName, topicName);
PartitionedTopicMetadata metadata = getPartitionedTopicMetadata(partitionTopicName, false, false);
int oldPartition = metadata.partitions;
- String prefix = topicName + TopicName.PARTITIONED_TOPIC_SUFFIX;
+ String prefix = partitionTopicName.getPartitionedTopicName() + TopicName.PARTITIONED_TOPIC_SUFFIX;
for (String exsitingTopicName : existingTopicList) {
- if (exsitingTopicName.contains(prefix)) {
+ if (exsitingTopicName.startsWith(prefix)) {
Review comment:
This change won't work properly. The topic name will be in format `persistent://tenant/cluster/namespace/topicName` .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] hangc0276 merged pull request #11686: fix the bug, can not update topic when the update topicName is contained by an existed topic as a part
Posted by GitBox <gi...@apache.org>.
hangc0276 merged pull request #11686:
URL: https://github.com/apache/pulsar/pull/11686
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on a change in pull request #11686: fix the bug, can not update topic when the update topicName is contained by an existed topic as a part
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on a change in pull request #11686:
URL: https://github.com/apache/pulsar/pull/11686#discussion_r690845106
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -3773,9 +3773,9 @@ private void validatePartitionTopicUpdate(String topicName, int numberOfPartitio
TopicName partitionTopicName = TopicName.get(domain(), namespaceName, topicName);
PartitionedTopicMetadata metadata = getPartitionedTopicMetadata(partitionTopicName, false, false);
int oldPartition = metadata.partitions;
- String prefix = topicName + TopicName.PARTITIONED_TOPIC_SUFFIX;
+ String prefix = partitionTopicName.getPartitionedTopicName() + TopicName.PARTITIONED_TOPIC_SUFFIX;
for (String exsitingTopicName : existingTopicList) {
- if (exsitingTopicName.contains(prefix)) {
+ if (exsitingTopicName.startsWith(prefix)) {
Review comment:
> There is a PR #11148 in progress in this same area. I'd suggest contributing to #11148 to fix this issue #11685. One form of contributing is making sure that this problem gets fixed with the refactored implementation for checking topic existence.
it's full name `String prefix = partitionTopicName.getPartitionedTopicName() + TopicName.PARTITIONED_TOPIC_SUFFIX`
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -3773,9 +3773,9 @@ private void validatePartitionTopicUpdate(String topicName, int numberOfPartitio
TopicName partitionTopicName = TopicName.get(domain(), namespaceName, topicName);
PartitionedTopicMetadata metadata = getPartitionedTopicMetadata(partitionTopicName, false, false);
int oldPartition = metadata.partitions;
- String prefix = topicName + TopicName.PARTITIONED_TOPIC_SUFFIX;
+ String prefix = partitionTopicName.getPartitionedTopicName() + TopicName.PARTITIONED_TOPIC_SUFFIX;
for (String exsitingTopicName : existingTopicList) {
- if (exsitingTopicName.contains(prefix)) {
+ if (exsitingTopicName.startsWith(prefix)) {
Review comment:
> There is a PR #11148 in progress in this same area. I'd suggest contributing to #11148 to fix this issue #11685. One form of contributing is making sure that this problem gets fixed with the refactored implementation for checking topic existence.
it's full name `String prefix = partitionTopicName.getPartitionedTopicName() + TopicName.PARTITIONED_TOPIC_SUFFIX`
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -3773,9 +3773,9 @@ private void validatePartitionTopicUpdate(String topicName, int numberOfPartitio
TopicName partitionTopicName = TopicName.get(domain(), namespaceName, topicName);
PartitionedTopicMetadata metadata = getPartitionedTopicMetadata(partitionTopicName, false, false);
int oldPartition = metadata.partitions;
- String prefix = topicName + TopicName.PARTITIONED_TOPIC_SUFFIX;
+ String prefix = partitionTopicName.getPartitionedTopicName() + TopicName.PARTITIONED_TOPIC_SUFFIX;
for (String exsitingTopicName : existingTopicList) {
- if (exsitingTopicName.contains(prefix)) {
+ if (exsitingTopicName.startsWith(prefix)) {
Review comment:
> same are
I don't think it's the same area, `validatePartitionTopicUpdate` only used when updating partition of topic. it's not the same logic.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on a change in pull request #11686: fix the bug, can not update topic when the update topicName is contained by an existed topic as a part
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on a change in pull request #11686:
URL: https://github.com/apache/pulsar/pull/11686#discussion_r690926200
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -3773,9 +3773,9 @@ private void validatePartitionTopicUpdate(String topicName, int numberOfPartitio
TopicName partitionTopicName = TopicName.get(domain(), namespaceName, topicName);
PartitionedTopicMetadata metadata = getPartitionedTopicMetadata(partitionTopicName, false, false);
int oldPartition = metadata.partitions;
- String prefix = topicName + TopicName.PARTITIONED_TOPIC_SUFFIX;
+ String prefix = partitionTopicName.getPartitionedTopicName() + TopicName.PARTITIONED_TOPIC_SUFFIX;
for (String exsitingTopicName : existingTopicList) {
- if (exsitingTopicName.contains(prefix)) {
+ if (exsitingTopicName.startsWith(prefix)) {
Review comment:
> same are
I don't think it's the same area, `validatePartitionTopicUpdate` only used when updating partition of topic. it's not the same logic.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Nicklee007 commented on a change in pull request #11686: fix the bug, can not update topic when the update topicName is contained by an existed topic as a part
Posted by GitBox <gi...@apache.org>.
Nicklee007 commented on a change in pull request #11686:
URL: https://github.com/apache/pulsar/pull/11686#discussion_r690830592
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -3773,9 +3773,9 @@ private void validatePartitionTopicUpdate(String topicName, int numberOfPartitio
TopicName partitionTopicName = TopicName.get(domain(), namespaceName, topicName);
PartitionedTopicMetadata metadata = getPartitionedTopicMetadata(partitionTopicName, false, false);
int oldPartition = metadata.partitions;
- String prefix = topicName + TopicName.PARTITIONED_TOPIC_SUFFIX;
+ String prefix = partitionTopicName.getPartitionedTopicName() + TopicName.PARTITIONED_TOPIC_SUFFIX;
for (String exsitingTopicName : existingTopicList) {
- if (exsitingTopicName.contains(prefix)) {
+ if (exsitingTopicName.startsWith(prefix)) {
Review comment:
> This change won't work properly. The topic name will be in format `persistent://tenant/cluster/namespace/topicName` .
yes I have considered that. in this PR，I changed to the prefix = topicName + TopicName.PARTITIONED_TOPIC_SUFFIX, it's the full topicName contain 'persistent://tenant/cluster/namespace/topicName';
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org