PIP 110: Support Topic metadata - PART-1 create topic with properties #12818

Technoboy- · 2021-11-15T16:27:06Z

Motivation

The original discussion mail :
https://lists.apache.org/thread/m9dkhq1fs6stsdwh78h84fsl5hs5v67f

Introduce the ability to store metadata about topics.

This would be very useful as with metadata you could add labels and other
pieces of information that would allow defining the purpose of a topic,
custom application-level properties.
This feature will allow application-level diagnostic tools and maintenance
tools to not need external databases to store such metadata.

Imagine that we could add a simple key value map (String keys and String
values) to the topic.
These metadata could be set during topic creation and also updated.

Modification

Add two new rest API reduce compatibility issue :
@path("/{property}/{cluster}/{namespace}/{topic}/partitions/properties")
@path("/{tenant}/{cluster}/{namespace}/{topic}/properties")
Add below new methods in Topics.java :
createPartitionedTopic(String topic, int numPartitions, Map<String, String> properties);
createPartitionedTopicAsync(String topic, int numPartitions, Map<String, String> properties);
createNonPartitionedTopic(String topic, Map<String, String> properties);
createNonPartitionedTopicAsync(String topic, Map<String, String> properties);

Documentation

[ x ] no-need-doc

pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/ManagedLedgerConfig.java

pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/TopicsImpl.java

codelipenghui

Do we have a way to get the properties? Looks like the PR just supports set properties, but is not able to get the properties.

pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/PersistentTopics.java

Technoboy- · 2022-01-18T12:38:28Z

Do we have a way to get the properties? Looks like the PR just supports set properties, but is not able to get the properties.

These new properties are put in PartitionedTopicMetadata. So we can get it from getPartitionedMetadata

codelipenghui · 2022-01-19T07:10:35Z

pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java

-            @ApiParam(value = "The number of partitions for the topic",
-                    required = true, type = "int", defaultValue = "0")
-                    int numPartitions,
+            @ApiParam(value = "The metadata for the topic",


Looks like a breaking change?

For there exists one param in this method before, we can't add a new param to this method because the body value only supports one param.

@codelipenghui It looks like not. I ran the following command in my local env:

curl -L http://localhost:8080/admin/v2/persistent/public/default/xxx/partitions -X PUT -H 'Content-Type: application/json' -d 3

We can see the partitions number was recognized correctly.

I also tried -d '{"partitions":"3","properties":{"k1":"v1","k2":"v2"}}' and it works well.

Interesting...

I have tried curl -L http://localhost:8080/admin/v2/persistent/public/default/xxx/partitions -X PUT -H 'Content-Type: application/json' -d '{"partitions":"3","properties":{"k1":"v1","k2":"v2"}}'

Got 400 at the broker side:

2022-01-19T18:41:56,584+0800 [pulsar-web-59-12] INFO org.eclipse.jetty.server.RequestLog - 127.0.0.1 - - [19/Jan/2022:18:41:56 +0800] "PUT /admin/v2/persistent/public/default/xxx/partitions HTTP/1.1" 400 212 "-" "curl/7.77.0" 2

Got exception at the client-side

Cannot deserialize value of type `int` from Object value (token `JsonToken.START_OBJECT`) at [Source: (org.glassfish.jersey.message.internal.ReaderInterceptorExecutor$UnCloseableInputStream); line: 1, column: 1]%

If we can't avoid breaking compatibility, we can add a new endpoint in v3 REST APIs

Did you compile the latest code? Here are the test input from my local env:

$ curl -L http://localhost:8080/admin/v2/persistent/public/default/xxx/partitions -X PUT -H 'Content-Type: application/json' -d '{"partitions":"3","properties":{"k1":"v1","k2":"v2"}}' $ curl -L http://localhost:8080/admin/v2/persistent/public/default/xxx/partitions {"partitions":3,"properties":{"k1":"v1","k2":"v2"}} $ curl -L http://localhost:8080/admin/v2/persistent/public/default/xxx/partitions -X DELETE $ curl -L http://localhost:8080/admin/v2/persistent/public/default/xxx/partitions -X PUT -H 'Content-Type: application/json' -d '{"partitions":"3","properties":{"k3":"v3"}}' $ curl -L http://localhost:8080/admin/v2/persistent/public/default/xxx/partitions {"partitions":3,"properties":{"k3":"v3"}}

If you used the original code, since the body must be an integer, the body {"partitions":"3","properties":{"k1":"v1","k2":"v2"}} cannot be recognized.

$ curl -L http://localhost:8080/admin/v2/persistent/public/default/xxx/partitions -X PUT -H 'Content-Type: application/json' -d '3' $ curl -L http://localhost:8080/admin/v2/persistent/public/default/xxx/partitions {"partitions":3} $ curl -L http://localhost:8080/admin/v2/persistent/public/default/xxx/partitions -X PUT -H 'Content-Type: application/json' -d '{"partitions":"3"}' Cannot deserialize value of type `int` from Object value (token `JsonToken.START_OBJECT`) at [Source: (org.glassfish.jersey.message.internal.ReaderInterceptorExecutor$UnCloseableInputStream); line: 1, column: 1]

But I don't think it breaks the backward compatibility. This PR makes the REST API available for more input.

If you used the original code, since the body must be an integer, the body {"partitions":"3","properties":{"k1":"v1","k2":"v2"}} cannot be recognized.

Yes, this is my test case, use the new Pulsar Admin version to point to the old broker.

Got it. I think there is no good way for this compatibility. We can only guarantee New Broker is compatible with Old Client

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java

BewareMyPower · 2022-01-19T08:41:43Z

BTW, could you check #12629 (comment)? I have the same concern that for a partitioned topic, what's the behavior when an individual partition's properties was set?

For example, I ran following commands:

curl -L http://localhost:8080/admin/v2/persistent/public/default/xxx/partitions -X PUT -H 'Content-Type: application/json' -d '{"partitions":"3","properties":{"k1":"v1","k2":"v2"}}'
curl -L http://localhost:8080/admin/v2/persistent/public/default/xxx-partition-0 -X DELETE
curl -L http://localhost:8080/admin/v2/persistent/public/default/xxx-partition-0 -X PUT -H 'Content-Type: application/json' -d '{"k3":"v3"}'

Create a partitioned topic xxx with 3 partitions and properties k1 -> v1, k2 -> v2.
Delete the 1st partition (xxx-partition-0).
Create the 1st partition with properties k3 -> v3.

The 3rd step works well but I think it should be prevented. /cc @eolivelli Is it what you concerned?

Technoboy- · 2022-01-19T08:52:08Z

BTW, could you check #12629 (comment)? I have the same concern that for a partitioned topic, what's the behavior when an individual partition's properties was set?

For example, I ran following commands:
curl -L http://localhost:8080/admin/v2/persistent/public/default/xxx/partitions -X PUT -H 'Content-Type: application/json' -d '{"partitions":"3","properties":{"k1":"v1","k2":"v2"}}'
curl -L http://localhost:8080/admin/v2/persistent/public/default/xxx-partition-0 -X DELETE
curl -L http://localhost:8080/admin/v2/persistent/public/default/xxx-partition-0 -X PUT -H 'Content-Type: application/json' -d '{"k3":"v3"}'
Create a partitioned topic xxx with 3 partitions and properties k1 -> v1, k2 -> v2.

Delete the 1st partition (xxx-partition-0).

Create the 1st partition with properties k3 -> v3.

The 3rd step works well but I think it should be prevented. /cc @eolivelli Is it what you concerned?

Ok。

eolivelli · 2022-01-21T07:52:39Z

pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v3/PersistentTopics.java

+@Path("/persistent")
+@Produces(MediaType.APPLICATION_JSON)
+@Api(value = "/persistent", description = "Persistent topic admin apis", tags = "persistent topic")
+public class PersistentTopics extends PersistentTopicsBase {


Why do we need this new package?

You can see the discussion in #12818 (comment).

Because when new PulsarAdmin sends the request to old broker, the old broker cannot recognize the JSON body like {"partitions":3}. /cc @codelipenghui

Because we have changed the method from createPartitionedTopic(partitions) to createPartitionedTopic(partitionedTopicMetadata)

makes sense to me.
so when you use the "new" pulsar-admin (2.10) with an old broker
how can we deal with compatibility ?

Because we have changed the method from createPartitionedTopic(partitions) to createPartitionedTopic(partitionedTopicMetadata)

It should be possible to handle that in a backwards compatible way without breaking the API. It's better to revert the addition of a new API version and instead revisit the solution in a proper way.

my understanding in this patch was that we are using the v3 endpoint only in case of passing the "properties" and to use v2 in the other cases

it looks like it is kind of a simple but in the logic to choose the endpoint

eolivelli

Please answer to my latest questions/requests.
Basically I would like to add come comments in PulsarAdmin "topicPath" method in order to say why and when we are using V3 API.

Apart from that I am +1

pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/TopicsImpl.java

### Motivation As #12818 has supported creating topics with metadata, this patch is adding a `get` API to support getting topic properties.

As #12818 has supported creating topics with metadata, this patch is adding a `get` API to support getting topic properties. (cherry picked from commit 1ebe4ee)

As apache#12818 has supported creating topics with metadata, this patch is adding a `get` API to support getting topic properties. (cherry picked from commit 1ebe4ee) (cherry picked from commit 6648f99)

syhily · 2023-02-27T20:40:13Z

Hi there. Since the metadata can be created on topic level. Any plan for adding metadata update logic for topic?

Technoboy- · 2023-05-22T22:49:49Z

Hi there. Since the metadata can be created on topic level. Any plan for adding metadata update logic for topic?

#17238

### Motivation To keep consistent with the [Java client](apache/pulsar#12818). ### Modifications - Add admin topic api CreateWithProperties - Add admin topic api GetProperties

PIP 110: Support Topic metadata (apache#12629).

304d15a

github-actions bot assigned Technoboy- Nov 15, 2021

github-actions bot added the doc-not-needed Your PR changes do not impact docs label Nov 15, 2021

fix checkstyle

d2234c4

Jason918 reviewed Nov 16, 2021

View reviewed changes

pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java Outdated Show resolved Hide resolved

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/ManagedLedgerConfig.java Outdated Show resolved Hide resolved

Technoboy- added 2 commits November 16, 2021 11:49

fix path

58eecf1

remove required=true

18efd4d

yuruguo reviewed Nov 16, 2021

View reviewed changes

pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/TopicsImpl.java Outdated Show resolved Hide resolved

Technoboy- added 15 commits November 16, 2021 14:33

change the field name from topicMetadata to properties

978f04f

change the field name from topicMetadata to properties

c62643f

fix test.

42b2f42

fix test.

fa420e5

fix test.

96fc5dc

fix test.

3972fb5

Merge branch 'master' into topic-metadata-1

8bbc2a0

fix compile failure

524911d

fix checkstyle

13e5a28

fix test.

5cdef7f

fix test.

8f4b015

fix checkstyle.

d435669

Merge branch 'master' into topic-metadata-1

ff8347f

fix checkstyle

b17c7de

Merge branch 'master' into topic-metadata-1

b3373bf

codelipenghui added this to the 2.10.0 milestone Jan 18, 2022

codelipenghui reviewed Jan 18, 2022

View reviewed changes

pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/PersistentTopics.java Outdated Show resolved Hide resolved

Remove createPartitionedTopic from v1.

ab8adba

Technoboy- changed the title ~~PIP 110: Support Topic metadata~~ PIP 110: Support Topic metadata - PART-1 create topic with properties Jan 18, 2022

Technoboy- added 2 commits January 18, 2022 21:55

remove createNonPartitionedTopic in v1

2131936

updates

629517c

codelipenghui approved these changes Jan 18, 2022

View reviewed changes

Technoboy- added 2 commits January 19, 2022 13:09

updates

a4121fe

updates

244e71e

codelipenghui reviewed Jan 19, 2022

View reviewed changes

gaoran10 reviewed Jan 19, 2022

View reviewed changes

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java Outdated Show resolved Hide resolved

Technoboy- added 2 commits January 21, 2022 15:23

updates

ed7f06e

fix checkstyle

8a009e1

eolivelli requested changes Jan 21, 2022

View reviewed changes

updates

20ff27f

codelipenghui approved these changes Jan 24, 2022

View reviewed changes

eolivelli approved these changes Jan 24, 2022

View reviewed changes

pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/TopicsImpl.java Show resolved Hide resolved

add comment for topicPath

b22e555

codelipenghui merged commit 10036d5 into apache:master Jan 25, 2022

nicoloboschi mentioned this pull request Feb 3, 2022

Can't create topics after #12818 using Pulsar master client and 2.9.x (or prior) broker (v3/persistent NOT FOUND) #14104

Closed

sijie mentioned this pull request Feb 3, 2022

ISSUE-14104: Can't create topics after #12818 using Pulsar master client and 2.9.x (or prior) broker (v3/persistent NOT FOUND) streamnative/pulsar-archived#3686

Closed

lhotari mentioned this pull request Feb 4, 2022

Remove Persistent Topics v3 API - use custom media type instead #14117

Merged

Technoboy- mentioned this pull request Jun 6, 2022

[feature][admin] Support to get topic properties. #15944

Merged

1 task

codelipenghui pushed a commit that referenced this pull request Jun 8, 2022

[feature][admin] Support to get topic properties. (#15944)

1ebe4ee

### Motivation As #12818 has supported creating topics with metadata, this patch is adding a `get` API to support getting topic properties.

Technoboy- deleted the topic-metadata-1 branch August 10, 2022 05:51

heesung-sn mentioned this pull request May 19, 2023

[fix][broker] managedLedger.getConfig().getProperties().putAll(properties) NPE #20361

Merged

15 tasks

crossoverJie mentioned this pull request Jun 4, 2024

[Improve] Add admin topic api CreateWithProperties apache/pulsar-client-go#1226

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PIP 110: Support Topic metadata - PART-1 create topic with properties #12818

PIP 110: Support Topic metadata - PART-1 create topic with properties #12818

Technoboy- commented Nov 15, 2021 •

edited

Loading

codelipenghui left a comment •

edited

Loading

Technoboy- commented Jan 18, 2022

codelipenghui Jan 19, 2022

Technoboy- Jan 19, 2022

BewareMyPower Jan 19, 2022

BewareMyPower Jan 19, 2022

codelipenghui Jan 19, 2022

codelipenghui Jan 19, 2022

codelipenghui Jan 19, 2022

BewareMyPower Jan 19, 2022 •

edited

Loading

codelipenghui Jan 21, 2022

BewareMyPower Jan 21, 2022

BewareMyPower commented Jan 19, 2022

Technoboy- commented Jan 19, 2022

eolivelli Jan 21, 2022

BewareMyPower Jan 21, 2022

Technoboy- Jan 21, 2022

eolivelli Jan 24, 2022

lhotari Feb 3, 2022

eolivelli Feb 3, 2022

eolivelli left a comment

syhily commented Feb 27, 2023

Technoboy- commented May 22, 2023

PIP 110: Support Topic metadata - PART-1 create topic with properties #12818

PIP 110: Support Topic metadata - PART-1 create topic with properties #12818

Conversation

Technoboy- commented Nov 15, 2021 • edited Loading

Motivation

Modification

Documentation

codelipenghui left a comment • edited Loading

Choose a reason for hiding this comment

Technoboy- commented Jan 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BewareMyPower Jan 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BewareMyPower commented Jan 19, 2022

Technoboy- commented Jan 19, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eolivelli left a comment

Choose a reason for hiding this comment

syhily commented Feb 27, 2023

Technoboy- commented May 22, 2023

Technoboy- commented Nov 15, 2021 •

edited

Loading

codelipenghui left a comment •

edited

Loading

BewareMyPower Jan 19, 2022 •

edited

Loading