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/06/29 12:41:48 UTC
[GitHub] [pulsar] suiyuzeng opened a new pull request #11148: Increase the rate of topic creation
suiyuzeng opened a new pull request #11148:
URL: https://github.com/apache/pulsar/pull/11148
### Motivation
The count of topic created in 1 second is less than 200, when the topic count of the namespace is about 200,000.
### Modifications
The reason is that the time cost of topic exist checking is too long when the topic count is large.
In org.apache.pulsar.broker.admin.AdminResource#checkTopicExistsAsync, all the topics would be checked one by one. As the count of topic grows, the time cost will become longer.
Can we use org.apache.pulsar.broker.namespace.NamespaceService#checkTopicExists to check if the topic is exits?
--
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 #11148: Increase the rate of topic creation
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on pull request #11148:
URL: https://github.com/apache/pulsar/pull/11148#issuecomment-893251233
> > @suiyuzeng Hello, please go on
>
> Sorry, a little busy recently~
> Find another problem, such as the test case "testCreatePartitionedTopicHavingNonPartitionTopicWithPartitionSuffix". Some non partition topic has the partition suffix. In this case, we can not use the cache to check anymore. There is some checking about this issue in the current code, to ensure that we can not create no no-parititon topic whit -partition-xxx suffix. How about add a config? If a cluster has the no-partition whit the suffix, use the current way to check duplication. If no no-paritition topic whit the suffix, use the cache to check.
@suiyuzeng Nice catch!
1. I am cutting 2.8.1, would you have time to address @315157973 's comments?
2. for the restriction for reserved key-words for topic name, such as -partition-xx suffix for non-partitioned topic name, we have started a discussion for this issue: https://lists.apache.org/list.html?dev@pulsar.apache.org
--
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] suiyuzeng commented on pull request #11148: Increase the rate of topic creation
Posted by GitBox <gi...@apache.org>.
suiyuzeng commented on pull request #11148:
URL: https://github.com/apache/pulsar/pull/11148#issuecomment-882474234
--
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] 315157973 commented on pull request #11148: Increase the rate of topic creation
Posted by GitBox <gi...@apache.org>.
315157973 commented on pull request #11148:
URL: https://github.com/apache/pulsar/pull/11148#issuecomment-956101691
move to 2.8.3
--
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] suiyuzeng commented on pull request #11148: Increase the rate of topic creation
Posted by GitBox <gi...@apache.org>.
suiyuzeng commented on pull request #11148:
URL: https://github.com/apache/pulsar/pull/11148#issuecomment-883006464
@315157973 @eolivelli thanks，i will Optimize the code and add some test about this function.
--
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] 315157973 commented on pull request #11148: Increase the rate of topic creation
Posted by GitBox <gi...@apache.org>.
315157973 commented on pull request #11148:
URL: https://github.com/apache/pulsar/pull/11148#issuecomment-888048922
@suiyuzeng Hello, please go on
--
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] 315157973 commented on a change in pull request #11148: Increase the rate of topic creation
Posted by GitBox <gi...@apache.org>.
315157973 commented on a change in pull request #11148:
URL: https://github.com/apache/pulsar/pull/11148#discussion_r672227978
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java
##########
@@ -742,19 +742,28 @@ protected void internalCreatePartitionedTopic(AsyncResponse asyncResponse, int n
* @param topicName given topic name
*/
protected CompletableFuture<Boolean> checkTopicExistsAsync(TopicName topicName) {
- return pulsar().getNamespaceService().getListOfTopics(topicName.getNamespaceObject(),
- CommandGetTopicsOfNamespace.Mode.ALL)
- .thenCompose(topics -> {
- boolean exists = false;
- for (String topic : topics) {
- if (topicName.getPartitionedTopicName().equals(
- TopicName.get(topic).getPartitionedTopicName())) {
- exists = true;
- break;
+ if (topicName.isPersistent()) {
+ String path = ZkAdminPaths.partitionedTopicPath(topicName);
+ return pulsar().getPulsarResources().getNamespaceResources().getPartitionedTopicResources().
+ existsAsync(path).thenCombine(pulsar().getLocalZkCacheService().
+ managedLedgerExists(topicName.getPersistenceNamingEncoding()),
+ (isPartitionedExists, isNonPartitionedExists) -> isPartitionedExists || isNonPartitionedExists);
+
+ } else {
+ return pulsar().getNamespaceService().getListOfTopics(topicName.getNamespaceObject(),
+ CommandGetTopicsOfNamespace.Mode.NON_PERSISTENT)
Review comment:
We have multiLayerTopicsMap in BrokerService, we don't need to traverse all topics under the namespace, and then traverse again to determine whether topic exists.
We only need to traverse the bundles, and then directly use map.contains , usually the number of bundles is not too much
--
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] 315157973 commented on a change in pull request #11148: Increase the rate of topic creation
Posted by GitBox <gi...@apache.org>.
315157973 commented on a change in pull request #11148:
URL: https://github.com/apache/pulsar/pull/11148#discussion_r672227978
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java
##########
@@ -742,19 +742,28 @@ protected void internalCreatePartitionedTopic(AsyncResponse asyncResponse, int n
* @param topicName given topic name
*/
protected CompletableFuture<Boolean> checkTopicExistsAsync(TopicName topicName) {
- return pulsar().getNamespaceService().getListOfTopics(topicName.getNamespaceObject(),
- CommandGetTopicsOfNamespace.Mode.ALL)
- .thenCompose(topics -> {
- boolean exists = false;
- for (String topic : topics) {
- if (topicName.getPartitionedTopicName().equals(
- TopicName.get(topic).getPartitionedTopicName())) {
- exists = true;
- break;
+ if (topicName.isPersistent()) {
+ String path = ZkAdminPaths.partitionedTopicPath(topicName);
+ return pulsar().getPulsarResources().getNamespaceResources().getPartitionedTopicResources().
+ existsAsync(path).thenCombine(pulsar().getLocalZkCacheService().
+ managedLedgerExists(topicName.getPersistenceNamingEncoding()),
+ (isPartitionedExists, isNonPartitionedExists) -> isPartitionedExists || isNonPartitionedExists);
+
+ } else {
+ return pulsar().getNamespaceService().getListOfTopics(topicName.getNamespaceObject(),
+ CommandGetTopicsOfNamespace.Mode.NON_PERSISTENT)
Review comment:
We have multiLayerTopicsMap in BrokerService, we don't need to traverse all NON_PERSISTENT, and then traverse again to determine whether it exists.
We only need to traverse the bundles, and then directly map.contains can do it, usually the number of bundles is not too much
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java
##########
@@ -742,19 +742,28 @@ protected void internalCreatePartitionedTopic(AsyncResponse asyncResponse, int n
* @param topicName given topic name
*/
protected CompletableFuture<Boolean> checkTopicExistsAsync(TopicName topicName) {
- return pulsar().getNamespaceService().getListOfTopics(topicName.getNamespaceObject(),
- CommandGetTopicsOfNamespace.Mode.ALL)
- .thenCompose(topics -> {
- boolean exists = false;
- for (String topic : topics) {
- if (topicName.getPartitionedTopicName().equals(
- TopicName.get(topic).getPartitionedTopicName())) {
- exists = true;
- break;
+ if (topicName.isPersistent()) {
+ String path = ZkAdminPaths.partitionedTopicPath(topicName);
+ return pulsar().getPulsarResources().getNamespaceResources().getPartitionedTopicResources().
+ existsAsync(path).thenCombine(pulsar().getLocalZkCacheService().
+ managedLedgerExists(topicName.getPersistenceNamingEncoding()),
+ (isPartitionedExists, isNonPartitionedExists) -> isPartitionedExists || isNonPartitionedExists);
+
+ } else {
+ return pulsar().getNamespaceService().getListOfTopics(topicName.getNamespaceObject(),
+ CommandGetTopicsOfNamespace.Mode.NON_PERSISTENT)
Review comment:
We have multiLayerTopicsMap in BrokerService, we don't need to traverse all topics under the namespace, and then traverse again to determine whether topic exists.
We only need to traverse the bundles, and then directly use map.contains , usually the number of bundles is not too much
--
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 #11148: Increase the rate of topic creation
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on pull request #11148:
URL: https://github.com/apache/pulsar/pull/11148#issuecomment-894187834
move to 2.8.2
--
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] suiyuzeng commented on pull request #11148: Increase the rate of topic creation
Posted by GitBox <gi...@apache.org>.
suiyuzeng commented on pull request #11148:
URL: https://github.com/apache/pulsar/pull/11148#issuecomment-882474234
--
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] suiyuzeng commented on pull request #11148: Increase the rate of topic creation
Posted by GitBox <gi...@apache.org>.
suiyuzeng commented on pull request #11148:
URL: https://github.com/apache/pulsar/pull/11148#issuecomment-871335968
> Nice catch! But I think this will break the current behavior. Currently, we do not allow the duplicate topic name with the persistent or non-persistent domain, with this change it will only check the persistent topic duplication.
>
> I think we should add the non-persistent topic duplication check back.
Got it. I will update later.
--
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] 315157973 commented on a change in pull request #11148: Increase the rate of topic creation
Posted by GitBox <gi...@apache.org>.
315157973 commented on a change in pull request #11148:
URL: https://github.com/apache/pulsar/pull/11148#discussion_r672227978
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java
##########
@@ -742,19 +742,28 @@ protected void internalCreatePartitionedTopic(AsyncResponse asyncResponse, int n
* @param topicName given topic name
*/
protected CompletableFuture<Boolean> checkTopicExistsAsync(TopicName topicName) {
- return pulsar().getNamespaceService().getListOfTopics(topicName.getNamespaceObject(),
- CommandGetTopicsOfNamespace.Mode.ALL)
- .thenCompose(topics -> {
- boolean exists = false;
- for (String topic : topics) {
- if (topicName.getPartitionedTopicName().equals(
- TopicName.get(topic).getPartitionedTopicName())) {
- exists = true;
- break;
+ if (topicName.isPersistent()) {
+ String path = ZkAdminPaths.partitionedTopicPath(topicName);
+ return pulsar().getPulsarResources().getNamespaceResources().getPartitionedTopicResources().
+ existsAsync(path).thenCombine(pulsar().getLocalZkCacheService().
+ managedLedgerExists(topicName.getPersistenceNamingEncoding()),
+ (isPartitionedExists, isNonPartitionedExists) -> isPartitionedExists || isNonPartitionedExists);
+
+ } else {
+ return pulsar().getNamespaceService().getListOfTopics(topicName.getNamespaceObject(),
+ CommandGetTopicsOfNamespace.Mode.NON_PERSISTENT)
Review comment:
We have multiLayerTopicsMap in BrokerService, we don't need to traverse all NON_PERSISTENT, and then traverse again to determine whether it exists.
We only need to traverse the bundles, and then directly map.contains can do it, usually the number of bundles is not too much
--
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] github-actions[bot] commented on pull request #11148: Increase the rate of topic creation
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11148:
URL: https://github.com/apache/pulsar/pull/11148#issuecomment-956101957
@suiyuzeng: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] michaeljmarshall commented on pull request #11148: Increase the rate of topic creation
Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on pull request #11148:
URL: https://github.com/apache/pulsar/pull/11148#issuecomment-1036506047
Removing the `release/2.8.3` label as this change will target master and probably won't be cherry picked back to stable branches because it is an optimization.
--
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] suiyuzeng commented on pull request #11148: Increase the rate of topic creation
Posted by GitBox <gi...@apache.org>.
suiyuzeng commented on pull request #11148:
URL: https://github.com/apache/pulsar/pull/11148#issuecomment-891843921
> @suiyuzeng Hello, please go on
Sorry, a little busy recently~
Find another problem, such as the test case "testCreatePartitionedTopicHavingNonPartitionTopicWithPartitionSuffix". Some non partition topic has the partition suffix. In this case, we can not use the cache to check anymore. There is some checking about this issue in the current code, to ensure that we can not create no no-parititon topic whit -partition-xxx suffix. How about add a config? If a cluster has the no-partition whit the suffix, use the current way to check duplication. If no no-paritition topic whit the suffix, use the cache to check.
--
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] 315157973 commented on a change in pull request #11148: Increase the rate of topic creation
Posted by GitBox <gi...@apache.org>.
315157973 commented on a change in pull request #11148:
URL: https://github.com/apache/pulsar/pull/11148#discussion_r672227978
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java
##########
@@ -742,19 +742,28 @@ protected void internalCreatePartitionedTopic(AsyncResponse asyncResponse, int n
* @param topicName given topic name
*/
protected CompletableFuture<Boolean> checkTopicExistsAsync(TopicName topicName) {
- return pulsar().getNamespaceService().getListOfTopics(topicName.getNamespaceObject(),
- CommandGetTopicsOfNamespace.Mode.ALL)
- .thenCompose(topics -> {
- boolean exists = false;
- for (String topic : topics) {
- if (topicName.getPartitionedTopicName().equals(
- TopicName.get(topic).getPartitionedTopicName())) {
- exists = true;
- break;
+ if (topicName.isPersistent()) {
+ String path = ZkAdminPaths.partitionedTopicPath(topicName);
+ return pulsar().getPulsarResources().getNamespaceResources().getPartitionedTopicResources().
+ existsAsync(path).thenCombine(pulsar().getLocalZkCacheService().
+ managedLedgerExists(topicName.getPersistenceNamingEncoding()),
+ (isPartitionedExists, isNonPartitionedExists) -> isPartitionedExists || isNonPartitionedExists);
+
+ } else {
+ return pulsar().getNamespaceService().getListOfTopics(topicName.getNamespaceObject(),
+ CommandGetTopicsOfNamespace.Mode.NON_PERSISTENT)
Review comment:
We have multiLayerTopicsMap in BrokerService, we don't need to traverse all NON_PERSISTENT, and then traverse again to determine whether it exists.
We only need to traverse the bundles, and then directly map.contains can do it, usually the number of bundles is not too much
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java
##########
@@ -742,19 +742,28 @@ protected void internalCreatePartitionedTopic(AsyncResponse asyncResponse, int n
* @param topicName given topic name
*/
protected CompletableFuture<Boolean> checkTopicExistsAsync(TopicName topicName) {
- return pulsar().getNamespaceService().getListOfTopics(topicName.getNamespaceObject(),
- CommandGetTopicsOfNamespace.Mode.ALL)
- .thenCompose(topics -> {
- boolean exists = false;
- for (String topic : topics) {
- if (topicName.getPartitionedTopicName().equals(
- TopicName.get(topic).getPartitionedTopicName())) {
- exists = true;
- break;
+ if (topicName.isPersistent()) {
+ String path = ZkAdminPaths.partitionedTopicPath(topicName);
+ return pulsar().getPulsarResources().getNamespaceResources().getPartitionedTopicResources().
+ existsAsync(path).thenCombine(pulsar().getLocalZkCacheService().
+ managedLedgerExists(topicName.getPersistenceNamingEncoding()),
+ (isPartitionedExists, isNonPartitionedExists) -> isPartitionedExists || isNonPartitionedExists);
+
+ } else {
+ return pulsar().getNamespaceService().getListOfTopics(topicName.getNamespaceObject(),
+ CommandGetTopicsOfNamespace.Mode.NON_PERSISTENT)
Review comment:
We have multiLayerTopicsMap in BrokerService, we don't need to traverse all topics under the namespace, and then traverse again to determine whether topic exists.
We only need to traverse the bundles, and then directly use map.contains , usually the number of bundles is not too much
--
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 #11148: Increase the rate of topic creation
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on pull request #11148:
URL: https://github.com/apache/pulsar/pull/11148#issuecomment-893251233
> > @suiyuzeng Hello, please go on
>
> Sorry, a little busy recently~
> Find another problem, such as the test case "testCreatePartitionedTopicHavingNonPartitionTopicWithPartitionSuffix". Some non partition topic has the partition suffix. In this case, we can not use the cache to check anymore. There is some checking about this issue in the current code, to ensure that we can not create no no-parititon topic whit -partition-xxx suffix. How about add a config? If a cluster has the no-partition whit the suffix, use the current way to check duplication. If no no-paritition topic whit the suffix, use the cache to check.
@suiyuzeng Nice catch!
1. I am cutting 2.8.1, would you have time to address @315157973 's comments?
2. for the restriction for reserved key-words for topic name, such as -partition-xx suffix for non-partitioned topic name, we have started a discussion for this issue: https://lists.apache.org/list.html?dev@pulsar.apache.org
--
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] suiyuzeng commented on pull request #11148: Increase the rate of topic creation
Posted by GitBox <gi...@apache.org>.
suiyuzeng commented on pull request #11148:
URL: https://github.com/apache/pulsar/pull/11148#issuecomment-882474234
@codelipenghui hi, can you please take a look?
--
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