merlimat commented on a change in pull request #9351:
URL: https://github.com/apache/pulsar/pull/9351#discussion_r571317435
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java
##########
@@ -418,7 +382,7 @@ protected Policies getNamespacePolicies(NamespaceName
namespaceName) {
try {
final String namespace = namespaceName.toString();
final String policyPath = AdminResource.path(POLICIES, namespace);
- Policies policies = policiesCache().get(policyPath)
+ Policies policies = namespaceResources().get(policyPath)
Review comment:
nit: ideally namespace resources should take a namespace name and
internally resolve the path on metadata store.
##########
File path:
pulsar-metadata/src/main/java/org/apache/pulsar/metadata/cache/impl/MetadataCacheImpl.java
##########
@@ -118,7 +120,7 @@ private MetadataCacheImpl(MetadataStore store,
MetadataSerde<T> serde) {
@Override
public CompletableFuture<Void> readModifyUpdateOrCreate(String path,
Function<Optional<T>, T> modifyFunction) {
- return objCache.get(path)
+ return executeWithRetry(() -> objCache.get(path)
Review comment:
This part is already in master, can you merge or rebase?
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/web/PulsarWebResource.java
##########
@@ -1021,4 +1026,75 @@ protected void validateBrokerName(String broker) throws
MalformedURLException {
throw new
WebApplicationException(Response.temporaryRedirect(redirect).build());
}
}
+
+ /*
+ * Get the list of namespaces (on every cluster) for a given property.
+ *
+ * @param property the property name
+ * @return the list of namespaces
+ */
+ protected List<String> getListOfNamespaces(String property) throws
Exception {
Review comment:
```suggestion
protected List<String> getListOfNamespaces(String tenant) throws
Exception {
```
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java
##########
@@ -132,20 +131,18 @@ protected void internalCreateNamespace(Policies policies)
{
try {
int maxNamespacesPerTenant =
pulsar().getConfiguration().getMaxNamespacesPerTenant();
- //no distributed locks are added here.In a concurrent scenario,
the threshold will be exceeded.
+ // no distributed locks are added here.In a concurrent scenario,
the threshold will be exceeded.
if (maxNamespacesPerTenant > 0) {
List<String> namespaces =
getListOfNamespaces(namespaceName.getTenant());
if (namespaces != null && namespaces.size() >
maxNamespacesPerTenant) {
throw new RestException(Status.PRECONDITION_FAILED,
"Exceed the maximum number of namespace in tenant
:" + namespaceName.getTenant());
}
}
- policiesCache().invalidate(path(POLICIES,
namespaceName.toString()));
-
- zkCreateOptimistic(path(POLICIES, namespaceName.toString()),
jsonMapper().writeValueAsBytes(policies));
+ namespaceResources().create(path(POLICIES,
namespaceName.toString()), policies);
log.info("[{}] Created namespace {}", clientAppId(),
namespaceName);
- } catch (KeeperException.NodeExistsException e) {
- log.warn("[{}] Failed to create namespace {} - already exists",
clientAppId(), namespaceName);
+ } catch (NotFoundException e) {
+ log.error("[{}] namespace already exists {}", clientAppId(),
namespaceName, e);
Review comment:
The exception and the log message seem to have diverged now
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
