You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by mattison chao <ma...@apache.org> on 2022/06/16 03:20:19 UTC
[DISCUSS] Reject partitioned topic creation when the topic name contains the `partition` keyword.
Hello, everyone.
I want to start a discussion about *Reject partitioned topic creation when
the topic name contains the `-partition-` keyword.*
Please feel free to express your opinion.
*Background*
We first found this problem two months ago. When the user sets the
*allowAutoTopicCreation* policy as below:
> {
> allowAutoTopicCreation: true,
> allowAutoTopicCreationType: "partitioned",
> defaultNumPartitions: 1
> }
This policy will make brokers automatically create the partitioned topic
when the topic does not exist. But we don't check if users use topic
names like "persistent://public/default/XXXX-partition-YYY".
In our logic, this topic will pass the automatically created
partition topic metadata check. We will make the partitioned metadata like
this:
Z-NODE:
paritioned-topics/public/defualt/persistent/XXXX-partition-YYY
{"partitions": 1}
The things here stand at the broker's side. Everything looks like we
expect. Even the topic name includes "-partition-" keywords.
And then, we continue the process. The Pulsar Client(Java) will get this
metadata and then try to create separate topics with `-partition-{index}`.
But there have small trick things,
The client try to invoke
`TopicName.get(topicName).getPartition(partitionIndex).toString();` method
to get paritioned topic name. however, we have `partition` keywords check
in the method `TopicName.get(topicName).getPartition(partitionIndex)`[1].
If the topic name includes `-partition-` keywords, it will return the
original topic name directly. So, The client will use this topic name to
create the consumer.
Here we go. Let's summarize:
1. We make the topic `persistent://public/default/XXXX-partition-YYY` a
partitioned topic.
2. The client tries to create the topic using
`persistent://public/default/XXXX-partition-YYY` not
`persistent://public/default/XXXX-partition-YYY-partition-0`
In this condition, we have s problem for users. If they want to delete this
topic, awkward things happen.
- If the users want to use delete-partition-topic admin API, The broker
will return the error "Partitioned Topic Name should not contain
'-partition-'".
- If the users want to use delete-topic admin API, The broker will delete
this topic, but the broker doesn't remove the partition topic metadata.
So, maybe there have many methods to solve this problem. It looks like
removing the `partition` keywords check for the `delete-partition` admin
API. But it seems not to make sense. So, we chose to reject this topic
creation directly and push a PR for it[2].
Things are not over yet. In the past few days. After discussing with
Penghui Li and Baodi Shi. We found another problem that will affect the
dead letter queue(DLQ) If we reject topic creation directly. So, I push a
PR[3] to revert it soon. And then draft this discussion.
Please remember the phenomenon described above and take it into DLQ with
the partitioned topic.
When we want to create a Share model subscription, for example, that topic
name is "persistent://public/default/test-topic". And we enabled DLQ.
The client will try to create a partitioned topic that looks like
`persistent://public/default/test-topic-partition-0`. And then, when we
need to use DLQ, the client will use a new producer that topic name follows
the DLQ default naming format:
topicname-subscriptionname-DLQ =>
> persistent://public/default/test-topic-partition-0-test-sub-DLQ
With the allowAutoTopicCreation policy, we mentioned before. the topic
`persistent://public/default/test-topic-partition-0-test-sub-DLQ` will
create the partitioned topic metadata.
Z-NODE:
paritioned-topics/public/defualt/persistent/test-topic-partition-0-test-sub-DLQ
{"partitions": 1}
And the client will also do the same things we mentioned above.
So, there have some problems:
- The DLQ topic is partitioned, but the client doesn't use
`persistent://public/default/test-topic-partition-0-test-sub-DLQ-partition-0`
to create this topic.
- If the auto-creation policy `defaultNumPartitions` > 1, and the
subscription type is `Exclusive`. the error will occur. Because the
consumer already exists. Please check this logic.[4]
- If the auto-creation policy `defaultNumPartitions` > 1, and the
subscription type is `Shared`. we will create many consumers/producers to
the same topic. The number is the number of partitions.[4]
- Because this DLQ is not partitioned topic name in the broker and it has
partitioned topic metadata. So, If we have some checks in the broker, we
will get errors. Just like the replicator.[5]
*Problem*
After background explanation, we've got some problems to want to discuss:
1. Should we reject the partitioned topic metadata creation when the topic
name contains the `-partition-` keywords?
2. If you want to reject the partitioned topic metadata creation, how can
you handle DLQ?
3. If we don't let the client do the `-partition-` keyword check in
`TopicName.get(topicName).getPartition(partitionIndex).toString();`, is the
DLQ of the partition what we expect?
4. If we want to reject topic creation with `-partition-` keywords and make
DLQ a non-partitioned topic. could change the DLQ naming format from
`persistent://public/default/test-topic-partition-0-test-sub-DLQ` to
`persistent://public/default/test-topic-test-sub-DLQ-partition-0`? because
we will check it when creating partitioned topic metadata and then make it
a non-partitioned topic.[6]
Best,
Mattison
*Reference*
[1]
https://github.com/apache/pulsar/blob/7576c9303513ef8212452ff64a5a53ec7def6a5b/pulsar-common/src/main/java/org/apache/pulsar/common/naming/TopicName.java#L231-L237
[2] https://github.com/apache/pulsar/pull/14920
[3] https://github.com/apache/pulsar/pull/16066
[4]
https://github.com/apache/pulsar/blob/7576c9303513ef8212452ff64a5a53ec7def6a5b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/MultiTopicsConsumerImpl.java#L1049-L1072
[5]
https://github.com/apache/pulsar/blob/0239d417e054be524e1e3fda9a7937647720289e/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractReplicator.java#L247-L259
[6]
https://github.com/apache/pulsar/blob/1097dbdf4853218b73dec3be5c83a82c26fe8385/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java#L2618-L2667
Re: [DISCUSS] Reject partitioned topic creation when the topic name contains the `partition` keyword.
Posted by Anon Hxy <an...@gmail.com>.
Hi Mattison,
I did a quick reading and verify the PR on standalone, and I have a
question:
If the topic name contains `-partition-`, it is expected to reject to
create a partitioned topic. However it will auto-create a non-partition
topic with `-partition-`.
But our `allowAutoTopicCreationType` is `partitioned`, that will confuse
the users I think.
So should we also reject creating non-partition topics with `-partition-`.
Thanks,
Xiaoyu Hou
mattison chao <ma...@apache.org> 于2022年6月28日周二 20:33写道：
> I hope everyone can express their views. :)
>
> Best,
> Mattison
>
Re: [DISCUSS] Reject partitioned topic creation when the topic name contains the `partition` keyword.
Posted by Baodi Shi <ba...@icloud.com.INVALID>.
Hi,
I think the topic name of DLQ is a bit awkward now, should use `xxx-sub-name-DLQ-patition-0` instead of `xxx-sub-name-patition-0-DLQ`.
Thanks,
Baodi Shi
> On Jun 28, 2022, at 20:3314, mattison chao <ma...@apache.org> wrote:
>
> I hope everyone can express their views. :)
>
> Best,
> Mattison
Re: [DISCUSS] Reject partitioned topic creation when the topic name contains the `partition` keyword.
Posted by mattison chao <ma...@apache.org>.
I hope everyone can express their views. :)
Best,
Mattison
Re: [DISCUSS] Reject partitioned topic creation when the topic name contains the `partition` keyword.
Posted by mattison chao <ma...@apache.org>.
I pushed a pull request[1]. That can avoid creating partitioned metadata
when the topic local name includes `-partition-` keywords.
[1] https://github.com/apache/pulsar/pull/16104
Re: [DISCUSS] Reject partitioned topic creation when the topic name contains the `partition` keyword.
Posted by Enrico Olivelli <eo...@gmail.com>.
Il giorno mer 29 giu 2022 alle ore 15:58 Dave Maughan
<da...@streamnative.io.invalid> ha scritto:
>
> Hi Mattison,
>
> 1. Should we reject the partitioned topic metadata creation when the topic
> > name contains the `-partition-` keywords?
>
>
> I'm not familiar with all the details but my naive opinion would be that we
> should avoid placing constraints on users if we can. In this case that
> would mean removing any technical issues that arise due to `-partition-`
> being in the topic name, rather than rejecting topics with that string in
> the name.
I agree 100%
Enrico
>
> Regards,
> Dave
>
>
>
> On Thu, Jun 16, 2022 at 4:20 AM mattison chao <ma...@apache.org>
> wrote:
>
> > Hello, everyone.
> >
> > I want to start a discussion about *Reject partitioned topic creation when
> > the topic name contains the `-partition-` keyword.*
> > Please feel free to express your opinion.
> >
> > *Background*
> >
> > We first found this problem two months ago. When the user sets the
> > *allowAutoTopicCreation* policy as below:
> >
> > > {
> > > allowAutoTopicCreation: true,
> > > allowAutoTopicCreationType: "partitioned",
> > > defaultNumPartitions: 1
> > > }
> >
> >
> > This policy will make brokers automatically create the partitioned topic
> > when the topic does not exist. But we don't check if users use topic
> > names like "persistent://public/default/XXXX-partition-YYY".
> > In our logic, this topic will pass the automatically created
> > partition topic metadata check. We will make the partitioned metadata like
> > this:
> >
> > Z-NODE:
> >
> > paritioned-topics/public/defualt/persistent/XXXX-partition-YYY
> >
> > {"partitions": 1}
> >
> >
> > The things here stand at the broker's side. Everything looks like we
> > expect. Even the topic name includes "-partition-" keywords.
> >
> > And then, we continue the process. The Pulsar Client(Java) will get this
> > metadata and then try to create separate topics with `-partition-{index}`.
> > But there have small trick things,
> > The client try to invoke
> > `TopicName.get(topicName).getPartition(partitionIndex).toString();` method
> > to get paritioned topic name. however, we have `partition` keywords check
> > in the method `TopicName.get(topicName).getPartition(partitionIndex)`[1].
> > If the topic name includes `-partition-` keywords, it will return the
> > original topic name directly. So, The client will use this topic name to
> > create the consumer.
> >
> > Here we go. Let's summarize:
> >
> > 1. We make the topic `persistent://public/default/XXXX-partition-YYY` a
> > partitioned topic.
> > 2. The client tries to create the topic using
> > `persistent://public/default/XXXX-partition-YYY` not
> > `persistent://public/default/XXXX-partition-YYY-partition-0`
> >
> > In this condition, we have s problem for users. If they want to delete this
> > topic, awkward things happen.
> >
> > - If the users want to use delete-partition-topic admin API, The broker
> > will return the error "Partitioned Topic Name should not contain
> > '-partition-'".
> > - If the users want to use delete-topic admin API, The broker will delete
> > this topic, but the broker doesn't remove the partition topic metadata.
> >
> > So, maybe there have many methods to solve this problem. It looks like
> > removing the `partition` keywords check for the `delete-partition` admin
> > API. But it seems not to make sense. So, we chose to reject this topic
> > creation directly and push a PR for it[2].
> >
> > Things are not over yet. In the past few days. After discussing with
> > Penghui Li and Baodi Shi. We found another problem that will affect the
> > dead letter queue(DLQ) If we reject topic creation directly. So, I push a
> > PR[3] to revert it soon. And then draft this discussion.
> >
> > Please remember the phenomenon described above and take it into DLQ with
> > the partitioned topic.
> >
> > When we want to create a Share model subscription, for example, that topic
> > name is "persistent://public/default/test-topic". And we enabled DLQ.
> > The client will try to create a partitioned topic that looks like
> > `persistent://public/default/test-topic-partition-0`. And then, when we
> > need to use DLQ, the client will use a new producer that topic name follows
> > the DLQ default naming format:
> >
> > topicname-subscriptionname-DLQ =>
> > > persistent://public/default/test-topic-partition-0-test-sub-DLQ
> >
> >
> > With the allowAutoTopicCreation policy, we mentioned before. the topic
> > `persistent://public/default/test-topic-partition-0-test-sub-DLQ` will
> > create the partitioned topic metadata.
> >
> > Z-NODE:
> >
> >
> > paritioned-topics/public/defualt/persistent/test-topic-partition-0-test-sub-DLQ
> >
> > {"partitions": 1}
> >
> >
> > And the client will also do the same things we mentioned above.
> >
> > So, there have some problems:
> >
> > - The DLQ topic is partitioned, but the client doesn't use
> >
> > `persistent://public/default/test-topic-partition-0-test-sub-DLQ-partition-0`
> > to create this topic.
> > - If the auto-creation policy `defaultNumPartitions` > 1, and the
> > subscription type is `Exclusive`. the error will occur. Because the
> > consumer already exists. Please check this logic.[4]
> > - If the auto-creation policy `defaultNumPartitions` > 1, and the
> > subscription type is `Shared`. we will create many consumers/producers to
> > the same topic. The number is the number of partitions.[4]
> > - Because this DLQ is not partitioned topic name in the broker and it has
> > partitioned topic metadata. So, If we have some checks in the broker, we
> > will get errors. Just like the replicator.[5]
> >
> >
> > *Problem*
> > After background explanation, we've got some problems to want to discuss:
> >
> > 1. Should we reject the partitioned topic metadata creation when the topic
> > name contains the `-partition-` keywords?
> > 2. If you want to reject the partitioned topic metadata creation, how can
> > you handle DLQ?
> > 3. If we don't let the client do the `-partition-` keyword check in
> > `TopicName.get(topicName).getPartition(partitionIndex).toString();`, is the
> > DLQ of the partition what we expect?
> > 4. If we want to reject topic creation with `-partition-` keywords and make
> > DLQ a non-partitioned topic. could change the DLQ naming format from
> > `persistent://public/default/test-topic-partition-0-test-sub-DLQ` to
> > `persistent://public/default/test-topic-test-sub-DLQ-partition-0`? because
> > we will check it when creating partitioned topic metadata and then make it
> > a non-partitioned topic.[6]
> >
> >
> >
> > Best,
> > Mattison
> >
> > *Reference*
> >
> > [1]
> >
> > https://github.com/apache/pulsar/blob/7576c9303513ef8212452ff64a5a53ec7def6a5b/pulsar-common/src/main/java/org/apache/pulsar/common/naming/TopicName.java#L231-L237
> > [2] https://github.com/apache/pulsar/pull/14920
> > [3] https://github.com/apache/pulsar/pull/16066
> > [4]
> >
> > https://github.com/apache/pulsar/blob/7576c9303513ef8212452ff64a5a53ec7def6a5b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/MultiTopicsConsumerImpl.java#L1049-L1072
> > [5]
> >
> > https://github.com/apache/pulsar/blob/0239d417e054be524e1e3fda9a7937647720289e/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractReplicator.java#L247-L259
> > [6]
> >
> > https://github.com/apache/pulsar/blob/1097dbdf4853218b73dec3be5c83a82c26fe8385/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java#L2618-L2667
> >
Re: [DISCUSS] Reject partitioned topic creation when the topic name contains the `partition` keyword.
Posted by Dave Maughan <da...@streamnative.io.INVALID>.
Hi Mattison,
1. Should we reject the partitioned topic metadata creation when the topic
> name contains the `-partition-` keywords?
I'm not familiar with all the details but my naive opinion would be that we
should avoid placing constraints on users if we can. In this case that
would mean removing any technical issues that arise due to `-partition-`
being in the topic name, rather than rejecting topics with that string in
the name.
Regards,
Dave
On Thu, Jun 16, 2022 at 4:20 AM mattison chao <ma...@apache.org>
wrote:
> Hello, everyone.
>
> I want to start a discussion about *Reject partitioned topic creation when
> the topic name contains the `-partition-` keyword.*
> Please feel free to express your opinion.
>
> *Background*
>
> We first found this problem two months ago. When the user sets the
> *allowAutoTopicCreation* policy as below:
>
> > {
> > allowAutoTopicCreation: true,
> > allowAutoTopicCreationType: "partitioned",
> > defaultNumPartitions: 1
> > }
>
>
> This policy will make brokers automatically create the partitioned topic
> when the topic does not exist. But we don't check if users use topic
> names like "persistent://public/default/XXXX-partition-YYY".
> In our logic, this topic will pass the automatically created
> partition topic metadata check. We will make the partitioned metadata like
> this:
>
> Z-NODE:
>
> paritioned-topics/public/defualt/persistent/XXXX-partition-YYY
>
> {"partitions": 1}
>
>
> The things here stand at the broker's side. Everything looks like we
> expect. Even the topic name includes "-partition-" keywords.
>
> And then, we continue the process. The Pulsar Client(Java) will get this
> metadata and then try to create separate topics with `-partition-{index}`.
> But there have small trick things,
> The client try to invoke
> `TopicName.get(topicName).getPartition(partitionIndex).toString();` method
> to get paritioned topic name. however, we have `partition` keywords check
> in the method `TopicName.get(topicName).getPartition(partitionIndex)`[1].
> If the topic name includes `-partition-` keywords, it will return the
> original topic name directly. So, The client will use this topic name to
> create the consumer.
>
> Here we go. Let's summarize:
>
> 1. We make the topic `persistent://public/default/XXXX-partition-YYY` a
> partitioned topic.
> 2. The client tries to create the topic using
> `persistent://public/default/XXXX-partition-YYY` not
> `persistent://public/default/XXXX-partition-YYY-partition-0`
>
> In this condition, we have s problem for users. If they want to delete this
> topic, awkward things happen.
>
> - If the users want to use delete-partition-topic admin API, The broker
> will return the error "Partitioned Topic Name should not contain
> '-partition-'".
> - If the users want to use delete-topic admin API, The broker will delete
> this topic, but the broker doesn't remove the partition topic metadata.
>
> So, maybe there have many methods to solve this problem. It looks like
> removing the `partition` keywords check for the `delete-partition` admin
> API. But it seems not to make sense. So, we chose to reject this topic
> creation directly and push a PR for it[2].
>
> Things are not over yet. In the past few days. After discussing with
> Penghui Li and Baodi Shi. We found another problem that will affect the
> dead letter queue(DLQ) If we reject topic creation directly. So, I push a
> PR[3] to revert it soon. And then draft this discussion.
>
> Please remember the phenomenon described above and take it into DLQ with
> the partitioned topic.
>
> When we want to create a Share model subscription, for example, that topic
> name is "persistent://public/default/test-topic". And we enabled DLQ.
> The client will try to create a partitioned topic that looks like
> `persistent://public/default/test-topic-partition-0`. And then, when we
> need to use DLQ, the client will use a new producer that topic name follows
> the DLQ default naming format:
>
> topicname-subscriptionname-DLQ =>
> > persistent://public/default/test-topic-partition-0-test-sub-DLQ
>
>
> With the allowAutoTopicCreation policy, we mentioned before. the topic
> `persistent://public/default/test-topic-partition-0-test-sub-DLQ` will
> create the partitioned topic metadata.
>
> Z-NODE:
>
>
> paritioned-topics/public/defualt/persistent/test-topic-partition-0-test-sub-DLQ
>
> {"partitions": 1}
>
>
> And the client will also do the same things we mentioned above.
>
> So, there have some problems:
>
> - The DLQ topic is partitioned, but the client doesn't use
>
> `persistent://public/default/test-topic-partition-0-test-sub-DLQ-partition-0`
> to create this topic.
> - If the auto-creation policy `defaultNumPartitions` > 1, and the
> subscription type is `Exclusive`. the error will occur. Because the
> consumer already exists. Please check this logic.[4]
> - If the auto-creation policy `defaultNumPartitions` > 1, and the
> subscription type is `Shared`. we will create many consumers/producers to
> the same topic. The number is the number of partitions.[4]
> - Because this DLQ is not partitioned topic name in the broker and it has
> partitioned topic metadata. So, If we have some checks in the broker, we
> will get errors. Just like the replicator.[5]
>
>
> *Problem*
> After background explanation, we've got some problems to want to discuss:
>
> 1. Should we reject the partitioned topic metadata creation when the topic
> name contains the `-partition-` keywords?
> 2. If you want to reject the partitioned topic metadata creation, how can
> you handle DLQ?
> 3. If we don't let the client do the `-partition-` keyword check in
> `TopicName.get(topicName).getPartition(partitionIndex).toString();`, is the
> DLQ of the partition what we expect?
> 4. If we want to reject topic creation with `-partition-` keywords and make
> DLQ a non-partitioned topic. could change the DLQ naming format from
> `persistent://public/default/test-topic-partition-0-test-sub-DLQ` to
> `persistent://public/default/test-topic-test-sub-DLQ-partition-0`? because
> we will check it when creating partitioned topic metadata and then make it
> a non-partitioned topic.[6]
>
>
>
> Best,
> Mattison
>
> *Reference*
>
> [1]
>
> https://github.com/apache/pulsar/blob/7576c9303513ef8212452ff64a5a53ec7def6a5b/pulsar-common/src/main/java/org/apache/pulsar/common/naming/TopicName.java#L231-L237
> [2] https://github.com/apache/pulsar/pull/14920
> [3] https://github.com/apache/pulsar/pull/16066
> [4]
>
> https://github.com/apache/pulsar/blob/7576c9303513ef8212452ff64a5a53ec7def6a5b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/MultiTopicsConsumerImpl.java#L1049-L1072
> [5]
>
> https://github.com/apache/pulsar/blob/0239d417e054be524e1e3fda9a7937647720289e/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractReplicator.java#L247-L259
> [6]
>
> https://github.com/apache/pulsar/blob/1097dbdf4853218b73dec3be5c83a82c26fe8385/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java#L2618-L2667
>