-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Add more config for auto-topic-creation #4963
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a08e3c9
1dc9a9e
82dd840
55d4d69
42d6ea5
f05cdeb
476a403
300c359
0fe2d3a
39f9ee1
f08c9a7
356e99b
f47d5a9
41fa7ca
87dd938
a38efb3
30913de
62f2313
65978c6
84bd59b
ee69a44
0408e34
fd5da63
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,8 +18,10 @@ | |
| */ | ||
| package org.apache.pulsar.broker.admin; | ||
|
|
||
| import com.fasterxml.jackson.core.JsonProcessingException; | ||
| import static com.google.common.base.Preconditions.checkArgument; | ||
| import static org.apache.pulsar.broker.cache.ConfigurationCacheService.POLICIES; | ||
| import org.apache.pulsar.common.api.proto.PulsarApi; | ||
| import static org.apache.pulsar.common.util.Codec.decode; | ||
|
|
||
| import java.net.MalformedURLException; | ||
|
|
@@ -81,6 +83,7 @@ | |
| public abstract class AdminResource extends PulsarWebResource { | ||
| private static final Logger log = LoggerFactory.getLogger(AdminResource.class); | ||
| private static final String POLICIES_READONLY_FLAG_PATH = "/admin/flags/policies-readonly"; | ||
| private static final int PARTITIONED_TOPIC_WAIT_SYNC_TIME_MS = 1000; | ||
| public static final String PARTITIONED_TOPIC_PATH_ZNODE = "partitioned-topics"; | ||
|
|
||
| protected ZooKeeper globalZk() { | ||
|
|
@@ -500,7 +503,7 @@ protected ZooKeeperChildrenCache failureDomainListCache() { | |
| } | ||
|
|
||
| protected PartitionedTopicMetadata getPartitionedTopicMetadata(TopicName topicName, | ||
| boolean authoritative) { | ||
| boolean authoritative, boolean checkAllowAutoCreation) { | ||
| validateClusterOwnership(topicName.getCluster()); | ||
| // validates global-namespace contains local/peer cluster: if peer/local cluster present then lookup can | ||
| // serve/redirect request else fail partitioned-metadata-request so, client fails while creating | ||
|
|
@@ -519,7 +522,12 @@ protected PartitionedTopicMetadata getPartitionedTopicMetadata(TopicName topicNa | |
| } | ||
|
|
||
| String path = path(PARTITIONED_TOPIC_PATH_ZNODE, namespaceName.toString(), domain(), topicName.getEncodedLocalName()); | ||
| PartitionedTopicMetadata partitionMetadata = fetchPartitionedTopicMetadata(pulsar(), path); | ||
| PartitionedTopicMetadata partitionMetadata; | ||
| if (checkAllowAutoCreation) { | ||
| partitionMetadata = fetchPartitionedTopicMetadataCheckAllowAutoCreation(pulsar(), path, topicName); | ||
| } else { | ||
| partitionMetadata = fetchPartitionedTopicMetadata(pulsar(), path); | ||
| } | ||
|
|
||
| if (log.isDebugEnabled()) { | ||
| log.debug("[{}] Total number of partitions for topic {} is {}", clientAppId(), topicName, | ||
|
|
@@ -566,6 +574,96 @@ public PartitionedTopicMetadata deserialize(String key, byte[] content) throws E | |
| return metadataFuture; | ||
| } | ||
|
|
||
| protected static PartitionedTopicMetadata fetchPartitionedTopicMetadataCheckAllowAutoCreation( | ||
| PulsarService pulsar, String path, TopicName topicName) { | ||
| try { | ||
| return fetchPartitionedTopicMetadataCheckAllowAutoCreationAsync(pulsar, path, topicName) | ||
| .get(); | ||
| } catch (Exception e) { | ||
| if (e.getCause() instanceof RestException) { | ||
| throw (RestException) e; | ||
| } | ||
| throw new RestException(e); | ||
| } | ||
| } | ||
|
|
||
| protected static CompletableFuture<PartitionedTopicMetadata> fetchPartitionedTopicMetadataCheckAllowAutoCreationAsync( | ||
| PulsarService pulsar, String path, TopicName topicName) { | ||
| CompletableFuture<PartitionedTopicMetadata> metadataFuture = new CompletableFuture<>(); | ||
| try { | ||
| boolean allowAutoTopicCreation = pulsar.getConfiguration().isAllowAutoTopicCreation(); | ||
| String topicType = pulsar.getConfiguration().getAllowAutoTopicCreationType(); | ||
| boolean topicExist; | ||
| try { | ||
| topicExist = pulsar.getNamespaceService() | ||
| .getListOfTopics(topicName.getNamespaceObject(), PulsarApi.CommandGetTopicsOfNamespace.Mode.ALL) | ||
| .contains(topicName.toString()); | ||
| } catch (Exception e) { | ||
| log.warn("Unexpected error while getting list of topics. topic={}. Error: {}", | ||
| topicName, e.getMessage(), e); | ||
| throw new RestException(e); | ||
| } | ||
| fetchPartitionedTopicMetadataAsync(pulsar, path).whenCompleteAsync((metadata, ex) -> { | ||
| if (ex != null) { | ||
| metadataFuture.completeExceptionally(ex); | ||
| // If topic is already exist, creating partitioned topic is not allowed. | ||
| } else if (metadata.partitions == 0 && !topicExist && allowAutoTopicCreation && | ||
| TopicType.PARTITIONED.toString().equals(topicType)) { | ||
| createDefaultPartitionedTopicAsync(pulsar, path).whenComplete((defaultMetadata, e) -> { | ||
| if (e == null) { | ||
| metadataFuture.complete(defaultMetadata); | ||
|
codelipenghui marked this conversation as resolved.
Outdated
|
||
| } else if (e instanceof KeeperException) { | ||
| try { | ||
| Thread.sleep(PARTITIONED_TOPIC_WAIT_SYNC_TIME_MS); | ||
| if (!pulsar.getGlobalZkCache().exists(path)){ | ||
| metadataFuture.completeExceptionally(e); | ||
| return; | ||
| } | ||
| } catch (InterruptedException | KeeperException exc) { | ||
| metadataFuture.completeExceptionally(exc); | ||
| return; | ||
| } | ||
| fetchPartitionedTopicMetadataAsync(pulsar, path).whenComplete((metadata2, ex2) -> { | ||
| if (ex2 != null) { | ||
| metadataFuture.completeExceptionally(ex2); | ||
| } else { | ||
| metadataFuture.complete(metadata2); | ||
| } | ||
| }); | ||
| } else { | ||
| metadataFuture.completeExceptionally(e); | ||
| } | ||
| }); | ||
| } else { | ||
| metadataFuture.complete(metadata); | ||
| } | ||
| }); | ||
| } catch (Exception e) { | ||
| metadataFuture.completeExceptionally(e); | ||
| } | ||
| return metadataFuture; | ||
| } | ||
|
|
||
| protected static CompletableFuture<PartitionedTopicMetadata> createDefaultPartitionedTopicAsync( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this method is no longer an asynchronous method, it's better to rename it to createDefaultPartitionedTopic() |
||
| PulsarService pulsar, String path) { | ||
| int defaultNumPartitions = pulsar.getConfiguration().getDefaultNumPartitions(); | ||
| checkArgument(defaultNumPartitions > 0, "Default number of partitions should be more than 0"); | ||
| PartitionedTopicMetadata configMetadata = new PartitionedTopicMetadata(defaultNumPartitions); | ||
| CompletableFuture<PartitionedTopicMetadata> partitionedTopicFuture = new CompletableFuture<>(); | ||
| try { | ||
| byte[] content = jsonMapper().writeValueAsBytes(configMetadata); | ||
| ZkUtils.createFullPathOptimistic(pulsar.getGlobalZkCache().getZooKeeper(), path, content, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you can use ZkUtils.asyncCreateFullPathOptimistic to replace ZkUtils.createFullPathOptimistic, we should avoid call sync method in the callback of async method.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, I am not very sure how to use it. ZkUtils.asyncCreateFullPathOptimistic(pulsar.getGlobalZkCache().getZooKeeper(), path, content,
ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT, (rc, path1, ctx, name) -> {
if (rc == KeeperException.Code.OK.intValue()) {
partitionedTopicFuture.complete(configMetadata);
} else {
partitionedTopicFuture.completeExceptionally(
KeeperException.create(KeeperException.Code.get(rc)));
}
}, null);instead of ZkUtils.createFullPathOptimistic(pulsar.getGlobalZkCache().getZooKeeper(), path, content,
ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
partitionedTopicFuture.complete(configMetadata);
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should be okay |
||
| ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); | ||
| // we wait for the data to be synced in all quorums and the observers | ||
| Thread.sleep(PARTITIONED_TOPIC_WAIT_SYNC_TIME_MS); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And if use ZkUtils.asyncCreateFullPathOptimistic, shall we need to call Thread.sleep here?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see that every time we use
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @codelipenghui , right, we use zkcache for this zk path, so it need some wait time. |
||
| partitionedTopicFuture.complete(configMetadata); | ||
| } catch (JsonProcessingException | KeeperException | InterruptedException e) { | ||
| log.error("Failed to create default partitioned topic.", e); | ||
| partitionedTopicFuture.completeExceptionally(e); | ||
| } | ||
| return partitionedTopicFuture; | ||
| } | ||
|
|
||
| protected void validateClusterExists(String cluster) { | ||
| try { | ||
| if (!clustersCache().get(path("clusters", cluster)).isPresent()) { | ||
|
|
@@ -627,4 +725,18 @@ protected List<String> getPartitionedTopicList(TopicDomain topicDomain) { | |
| partitionedTopics.sort(null); | ||
| return partitionedTopics; | ||
| } | ||
|
|
||
| enum TopicType { | ||
| PARTITIONED("partitioned"), | ||
| NON_PARTITIONED("non-partitioned"); | ||
| private String type; | ||
|
|
||
| TopicType(String type) { | ||
| this.type = type; | ||
| } | ||
|
|
||
| public String toString() { | ||
| return type; | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's necessary to call Thread.sleep(), i think more suitable to put it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems be a higher level above the zk path update. And maybe bring in no-need sleep().