RobertIndie commented on code in PR #22182: URL: https://github.com/apache/pulsar/pull/22182#discussion_r1533359134
########## pip/pip-344.md: ########## @@ -0,0 +1,115 @@ +# PIP-344: Correct the behavior of the public API pulsarClient.getPartitionsForTopic(topicName) + +# Background knowledge + +**Topic auto-creation** +- The partitioned topic auto-creation is dependent on `pulsarClient.getPartitionsForTopic` + - It triggers partitioned metadata creation by `pulsarClient.getPartitionsForTopic` + - And triggers the topic partition creation by producers' registration and consumers' registration. +- When calling `pulsarClient.getPartitionsForTopic(topicName)`, Pulsar will automatically create the partitioned topic metadata if it does not exist, either using `HttpLookupService` or `BinaryProtoLookupService`. + +**Now `pulsarClient.getPartitionsForTopic`'s behavior** +| case | broker allow `auto-create` | param allow <br> `create if not exists` | non-partitioned topic | partitioned topic | current behavior | +| --- | --- | --- | --- | --- | --- | +| 1 | `true/false` | `true/false` | `exists: true` | | REST API: `partitions: 0`<br> Binary API: `partitions: 0` | +| 2 | `true/false` | `true/false` | | `exists: true` <br> `partitions: 3` | REST API: `partitions: 3`<br> Binary API: `partitions: 3` | +| 3 | `true` | `true` | | | REST API: <br> - `create new: true` <br> - `partitions: 3` <br> Binary API: <br> - `create new: true` <br> - `partitions: 3` <br> | +| 4 | `true` | `false` | | | REST API: <br> - `create new: false` <br> - `partitions: 0` <br> Binary API: <br> not support <br> | +| 5 | `false` | `true` | | | REST API: <br> - `create new: false` <br> - `partitions: 0` <br> Binary API: <br> - `create new: false` <br> - `partitions: 0` <br> | + +- Broker allows `auto-create`: see also the config `allowAutoTopicCreation` in `broker.conf`. +- Param allow <br> `create if not exists` + - Regarding the HTTP API `PersistentTopics.getPartitionedMetadata`, it is an optional param which named `checkAllowAutoCreation,` and the default value is `false`. + - Regarding the `pulsar-admin` API, it depends on the HTTP API `PersistentTopics.getPartitionedMetadata`, and it always sets the param `checkAllowAutoCreation` to `false` and can not be set manually. + - Regarding the client API `HttpLookupService.getPartitionedTopicMetadata`, it depends on the HTTP API `PersistentTopics.getPartitionedMetadata`, and it always sets the param `checkAllowAutoCreation` to `true` and can not be set manually. + - Regarding the client API `BinaryProtoLookupService.getPartitionedTopicMetadata`, it always tries to create partitioned metadata. +- `REST API & HTTP API`: Since there are only two implementations of the 4 ways to get partitioned metadata, we call HTTP API `PersistentTopics.getPartitionedMetadata`, `pulsar-admin`, and `HttpLookupService.getPartitionedTopicMetadata` HTTP API, and call `BinaryProtoLookupService.getPartitionedTopicMetadata` Binary API. + +# Motivation + +The param `create if not exists` of the Binary API is always `true.` + +- For case 4 of `pulsarClient.getPartitionsForTopic`'s behavior, it always tries to create the partitioned metadata, but the API name is `getxxx`. +- For case 5 of `pulsarClient.getPartitionsForTopic`'s behavior, it returns a `0` partitioned metadata, but the topic does not exist. For the correct behavior of this case, we had discussed [here](https://github.com/apache/pulsar/issues/8813) before. +- BTW, [flink-connector-pulsar](https://github.com/apache/flink-connector-pulsar/blob/main/flink-connector-pulsar/src/main/java/org/apache/flink/connector/pulsar/sink/writer/topic/ProducerRegister.java#L221-L227) is using this API to create partitioned topic metadata. + +# Goals + +- Regarding the case 4: Add a new API `PulsarClient.getPartitionsForTopic(String, boolean)` to support the feature that just get partitioned topic metadata and do not try to create one. See detail below. +- Regarding the case 5: Instead of returning a `0` partitioned metadata, respond to a not found error when calling `pulsarClient.getPartitionsForTopic(String)` if the topic does not exist. + +# Detailed Design + +## Public-facing Changes + +When you call the public API `pulsarClient.getPartitionsForTopic`, pulsar will not create the partitioned metadata anymore. + +### Public API +**LookupService.java** +```java + +- CompletableFuture<PartitionedTopicMetadata> getPartitionedTopicMetadata(TopicName topicName); + ++ // A new API that contains an additional param "createIfAutoCreationEnabled." ++ CompletableFuture<PartitionedTopicMetadata> getPartitionedTopicMetadata(TopicName topicName, boolean createIfAutoCreationEnabled); Review Comment: I think we should document clearly which exceptions will be thrown exactly. The exception should also be part of the interface changes, so I think we need to explain it clearly in the proposal. IIUC, this method may throw two types of exceptions: not found and not supported. But which specific exception class is used? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
