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 2022/05/10 11:37:02 UTC
[GitHub] [pulsar] mattisonchao opened a new pull request, #15527: [improve][broker] Make some methods of `ClusterBase` pure async.
mattisonchao opened a new pull request, #15527:
URL: https://github.com/apache/pulsar/pull/15527
### Motivation
See PIP #14365 and change tracker #15043.
### Modifications
- Make `ClusterBase` `getBrokerWithNamespaceIsolationPolicy ` / `setNamespaceIsolationPolicy` to pure async.
### Verifying this change
- [x] Make sure that the change passes the CI checks.
`AdminTest#clusters` already cover this change.
### Documentation
- [x] `no-need-doc`
(Please explain why)
--
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] Technoboy- commented on a diff in pull request #15527: [improve][broker] Make some methods of `ClusterBase` pure async.
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on code in PR #15527:
URL: https://github.com/apache/pulsar/pull/15527#discussion_r873339460
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/ClustersBase.java:
##########
@@ -522,8 +523,9 @@ private BrokerNamespaceIsolationData internalGetBrokerNsIsolationData(
NamespaceIsolationPolicyImpl nsPolicyImpl = new NamespaceIsolationPolicyImpl(policyData);
if (nsPolicyImpl.isPrimaryBroker(broker) || nsPolicyImpl.isSecondaryBroker(broker)) {
namespaceRegexes.addAll(policyData.getNamespaces());
- brokerIsolationData.primary(nsPolicyImpl.isPrimaryBroker(broker));
- brokerIsolationData.policyName(name);
+ if (nsPolicyImpl.isPrimaryBroker(broker)) {
+ brokerIsolationData.primary(true);
+ }
Review Comment:
No need to change.
--
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] liudezhi2098 commented on a diff in pull request #15527: [improve][broker] Make some methods of `ClusterBase` pure async.
Posted by GitBox <gi...@apache.org>.
liudezhi2098 commented on code in PR #15527:
URL: https://github.com/apache/pulsar/pull/15527#discussion_r869769521
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/ClustersBase.java:
##########
@@ -64,12 +65,13 @@
import org.apache.pulsar.common.policies.impl.NamespaceIsolationPolicies;
import org.apache.pulsar.common.policies.impl.NamespaceIsolationPolicyImpl;
import org.apache.pulsar.common.util.FutureUtil;
-import org.apache.pulsar.common.util.ObjectMapperFactory;
import org.apache.pulsar.metadata.api.MetadataStoreException;
import org.apache.pulsar.metadata.api.MetadataStoreException.NotFoundException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import static javax.ws.rs.core.Response.Status.PRECONDITION_FAILED;
Review Comment:
check checkstyle
--
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] mattisonchao commented on a diff in pull request #15527: [improve][broker] Make some methods of `ClusterBase` pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on code in PR #15527:
URL: https://github.com/apache/pulsar/pull/15527#discussion_r873242932
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/ClustersBase.java:
##########
@@ -64,12 +65,13 @@
import org.apache.pulsar.common.policies.impl.NamespaceIsolationPolicies;
import org.apache.pulsar.common.policies.impl.NamespaceIsolationPolicyImpl;
import org.apache.pulsar.common.util.FutureUtil;
-import org.apache.pulsar.common.util.ObjectMapperFactory;
import org.apache.pulsar.metadata.api.MetadataStoreException;
import org.apache.pulsar.metadata.api.MetadataStoreException.NotFoundException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import static javax.ws.rs.core.Response.Status.PRECONDITION_FAILED;
Review Comment:
fixed, 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] mattisonchao commented on a diff in pull request #15527: [improve][broker] Make some methods of `ClusterBase` pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on code in PR #15527:
URL: https://github.com/apache/pulsar/pull/15527#discussion_r873358557
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/ClustersBase.java:
##########
@@ -522,8 +523,9 @@ private BrokerNamespaceIsolationData internalGetBrokerNsIsolationData(
NamespaceIsolationPolicyImpl nsPolicyImpl = new NamespaceIsolationPolicyImpl(policyData);
if (nsPolicyImpl.isPrimaryBroker(broker) || nsPolicyImpl.isSecondaryBroker(broker)) {
namespaceRegexes.addAll(policyData.getNamespaces());
- brokerIsolationData.primary(nsPolicyImpl.isPrimaryBroker(broker));
- brokerIsolationData.policyName(name);
+ if (nsPolicyImpl.isPrimaryBroker(broker)) {
+ brokerIsolationData.primary(true);
+ }
Review Comment:
Yes, It's an issue caused by rebase operation, I will fix it.
--
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] mattisonchao commented on a diff in pull request #15527: [improve][broker] Make some methods of `ClusterBase` pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on code in PR #15527:
URL: https://github.com/apache/pulsar/pull/15527#discussion_r873358912
##########
pulsar-common/src/main/java/org/apache/pulsar/common/util/FutureUtil.java:
##########
@@ -47,6 +50,18 @@ public static CompletableFuture<Void> waitForAll(Collection<? extends Completabl
return CompletableFuture.allOf(futures.toArray(new CompletableFuture[0]));
}
+ public static <T> CompletableFuture<List<T>> waitForAll(Stream<CompletableFuture<List<T>>> futures) {
+ return futures.reduce(CompletableFuture.completedFuture(new ArrayList<>()),
+ (pre, curr) -> pre.thenCompose(preV -> curr.thenApply(currV -> {
+ preV.addAll(currV);
+ return preV;
+ })));
+ }
+
+ public static <T> CompletableFuture<List<T>> waitForAll(List<CompletableFuture<List<T>>> futures) {
+ return waitForAll(futures.stream());
Review Comment:
Yes, I will fix it and mark it resolved.
--
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] Technoboy- commented on a diff in pull request #15527: [improve][broker] Make some methods of `ClusterBase` pure async.
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on code in PR #15527:
URL: https://github.com/apache/pulsar/pull/15527#discussion_r873342728
##########
pulsar-common/src/main/java/org/apache/pulsar/common/util/FutureUtil.java:
##########
@@ -47,6 +50,18 @@ public static CompletableFuture<Void> waitForAll(Collection<? extends Completabl
return CompletableFuture.allOf(futures.toArray(new CompletableFuture[0]));
}
+ public static <T> CompletableFuture<List<T>> waitForAll(Stream<CompletableFuture<List<T>>> futures) {
+ return futures.reduce(CompletableFuture.completedFuture(new ArrayList<>()),
+ (pre, curr) -> pre.thenCompose(preV -> curr.thenApply(currV -> {
+ preV.addAll(currV);
+ return preV;
+ })));
+ }
+
+ public static <T> CompletableFuture<List<T>> waitForAll(List<CompletableFuture<List<T>>> futures) {
+ return waitForAll(futures.stream());
Review Comment:
And here.
--
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] mattisonchao commented on a diff in pull request #15527: [improve][broker] Make some methods of `ClusterBase` pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on code in PR #15527:
URL: https://github.com/apache/pulsar/pull/15527#discussion_r873358557
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/ClustersBase.java:
##########
@@ -522,8 +523,9 @@ private BrokerNamespaceIsolationData internalGetBrokerNsIsolationData(
NamespaceIsolationPolicyImpl nsPolicyImpl = new NamespaceIsolationPolicyImpl(policyData);
if (nsPolicyImpl.isPrimaryBroker(broker) || nsPolicyImpl.isSecondaryBroker(broker)) {
namespaceRegexes.addAll(policyData.getNamespaces());
- brokerIsolationData.primary(nsPolicyImpl.isPrimaryBroker(broker));
- brokerIsolationData.policyName(name);
+ if (nsPolicyImpl.isPrimaryBroker(broker)) {
+ brokerIsolationData.primary(true);
+ }
Review Comment:
Yes, It's an issue caused by rebase operation, I will fix it and mark it resolved.
--
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] codelipenghui merged pull request #15527: [improve][broker] Make some methods of `ClusterBase` pure async.
Posted by GitBox <gi...@apache.org>.
codelipenghui merged PR #15527:
URL: https://github.com/apache/pulsar/pull/15527
--
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] mattisonchao commented on a diff in pull request #15527: [improve][broker] Make some methods of `ClusterBase` pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on code in PR #15527:
URL: https://github.com/apache/pulsar/pull/15527#discussion_r873242932
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/ClustersBase.java:
##########
@@ -64,12 +65,13 @@
import org.apache.pulsar.common.policies.impl.NamespaceIsolationPolicies;
import org.apache.pulsar.common.policies.impl.NamespaceIsolationPolicyImpl;
import org.apache.pulsar.common.util.FutureUtil;
-import org.apache.pulsar.common.util.ObjectMapperFactory;
import org.apache.pulsar.metadata.api.MetadataStoreException;
import org.apache.pulsar.metadata.api.MetadataStoreException.NotFoundException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import static javax.ws.rs.core.Response.Status.PRECONDITION_FAILED;
Review Comment:
fixed, thanks. and I will mark this comment as resolved.
--
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] mattisonchao commented on a diff in pull request #15527: [improve][broker] Make some methods of `ClusterBase` pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on code in PR #15527:
URL: https://github.com/apache/pulsar/pull/15527#discussion_r873242932
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/ClustersBase.java:
##########
@@ -64,12 +65,13 @@
import org.apache.pulsar.common.policies.impl.NamespaceIsolationPolicies;
import org.apache.pulsar.common.policies.impl.NamespaceIsolationPolicyImpl;
import org.apache.pulsar.common.util.FutureUtil;
-import org.apache.pulsar.common.util.ObjectMapperFactory;
import org.apache.pulsar.metadata.api.MetadataStoreException;
import org.apache.pulsar.metadata.api.MetadataStoreException.NotFoundException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import static javax.ws.rs.core.Response.Status.PRECONDITION_FAILED;
Review Comment:
fixed, thanks. and I will mark this comment is resolved.
--
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