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/04/04 19:04:26 UTC
[GitHub] [pulsar] poorbarcode opened a new pull request, #15026: [Test]branch-2.9: fixed eror test case at AdminApiTest2
poorbarcode opened a new pull request, #15026:
URL: https://github.com/apache/pulsar/pull/15026
### Contribution Checklist
Fixes #14863
### Motivation
see #14863
### Modifications
- `Namespace.Base.internalGetAntiAffinityNamespaces` fixed NullPointerException
- `NamespacesBase.internalDeleteNamespaceForcefully` Forcibly delete the order of `topics` and `bundles`, becase after deleted bundles, the topic will be fenced, then the operation that delete topic will failure
- fixed `AdminApi2Test` problem test case
- brokerNamespaceIsolationPoliciesUpdateOnTime: change immutable list to ArrayList
- testDistinguishTopicTypeWhenForceDeleteNamespace: fixed outdated test case. `NamespaceService.getFullListOfTopics` at last version not supports dirty data any more.
- testMaxTopicsPerNamespace: fixed outdated test case. 1. Adding asynchronous tests; 2. At last version not allow create system topic by hand-operation
### Verifying this change
- [x] Make sure that the change passes the CI checks.
### Does this pull request potentially affect one of the following parts:
*If `yes` was chosen, please highlight the changes*
- Dependencies (does it add or upgrade a dependency): (no)
- The public API: (no)
- The schema: (no)
- The default values of configurations: (no)
- The wire protocol: (no)
- The rest endpoints: (no)
- The admin cli options: (no)
- Anything that affects deployment: (don't know)
### Documentation
Need to update docs?
- [ ] `doc-required`
- [x] `no-need-doc`
(Please explain why)
- [ ] `doc`
(If this PR contains doc changes)
--
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] poorbarcode commented on pull request #15026: [test] [admin] branch-2.9: fixed eror test case at AdminApiTest2
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #15026:
URL: https://github.com/apache/pulsar/pull/15026#issuecomment-1089910893
> it is better to cherry-pick and do not let branches diverge.
Good idea. Should I split it to two submit ?
--
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] eolivelli commented on pull request #15026: [test] [admin] branch-2.9: fixed eror test case at AdminApiTest2
Posted by GitBox <gi...@apache.org>.
eolivelli commented on PR #15026:
URL: https://github.com/apache/pulsar/pull/15026#issuecomment-1089911464
I am sorry I cannot understand.
This patch it targeting branch-2.9, it cannot be a patch ported "from" branch-2.9.
if the problem is also on master branch please open a PR against master branch
--
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] eolivelli closed pull request #15026: [test] [admin] branch-2.9: fixed eror test case at AdminApiTest2
Posted by GitBox <gi...@apache.org>.
eolivelli closed pull request #15026: [test] [admin] branch-2.9: fixed eror test case at AdminApiTest2
URL: https://github.com/apache/pulsar/pull/15026
--
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] poorbarcode commented on a diff in pull request #15026: [Test]branch-2.9: fixed eror test case at AdminApiTest2
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #15026:
URL: https://github.com/apache/pulsar/pull/15026#discussion_r842055756
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiTest2.java:
##########
@@ -1027,9 +1028,11 @@ public void brokerNamespaceIsolationPoliciesUpdateOnTime() throws Exception {
parameters1.put("min_limit", "1");
parameters1.put("usage_threshold", "100");
+ ArrayList<String> primary = new ArrayList<>(2);
+ primary.add(brokerName + ".*");
NamespaceIsolationData nsPolicyData1 = NamespaceIsolationData.builder()
.namespaces(Collections.singletonList(ns1Name))
- .primary(Collections.singletonList(brokerName + ".*"))
Review Comment:
this collection will execute `clear` & `add`, so it should not be immutable
--
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] poorbarcode commented on pull request #15026: [test] [admin] branch-2.9: fixed eror test case at AdminApiTest2
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #15026:
URL: https://github.com/apache/pulsar/pull/15026#issuecomment-1089920257
> This patch it targeting branch-2.9, it cannot be a patch ported "from" branch-2.9.
I am sorry I cannot understand. Could you elaborate on that
--
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] poorbarcode commented on a diff in pull request #15026: [Test]branch-2.9: fixed eror test case at AdminApiTest2
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #15026:
URL: https://github.com/apache/pulsar/pull/15026#discussion_r842056381
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiTest2.java:
##########
@@ -1613,40 +1616,70 @@ public void testForceDeleteNamespace() throws Exception {
@Test
public void testDistinguishTopicTypeWhenForceDeleteNamespace() throws Exception {
conf.setForceDeleteNamespaceAllowed(true);
- final String ns = "prop-xyz/distinguish-topic-type-ns";
- final String exNs = "prop-xyz/ex-distinguish-topic-type-ns";
- admin.namespaces().createNamespace(ns, 2);
- admin.namespaces().createNamespace(exNs, 2);
-
- final String p1 = "persistent://" + ns + "/p1";
- final String p5 = "persistent://" + ns + "/p5";
- final String np = "persistent://" + ns + "/np";
-
+ // create namespace.
+ final String namespace01 = "prop-xyz/distinguish-topic-type-ns";
+ final String namespace02 = "prop-xyz/ex-distinguish-topic-type-ns";
+ admin.namespaces().createNamespace(namespace01, 2);
+ admin.namespaces().createNamespace(namespace02, 2);
+
+ // add partitioned & no-partitioned topics at namespace01.
+ final String p1 = "persistent://" + namespace01 + "/p1";
+ final String p5 = "persistent://" + namespace01 + "/p5";
+ final String np = "persistent://" + namespace01 + "/np";
admin.topics().createPartitionedTopic(p1, 1);
admin.topics().createPartitionedTopic(p5, 5);
admin.topics().createNonPartitionedTopic(np);
- final String exNp = "persistent://" + exNs + "/np";
- admin.topics().createNonPartitionedTopic(exNp);
- // insert an invalid topic name
+ // add a topic at namespace02.
+ final String topic01AtNamespace01 = "persistent://" + namespace02 + "/np";
+ admin.topics().createNonPartitionedTopic(topic01AtNamespace01);
+ // insert an invalid topic name at namespace02
pulsar.getLocalMetadataStore().put(
- "/managed-ledgers/" + exNs + "/persistent/", "".getBytes(), Optional.empty()).join();
+ "/managed-ledgers/" + namespace02 + "/persistent/", "".getBytes(), Optional.empty())
+ .join();
- List<String> topics = pulsar.getNamespaceService().getFullListOfTopics(NamespaceName.get(ns)).get();
- List<String> exTopics = pulsar.getNamespaceService().getFullListOfTopics(NamespaceName.get(exNs)).get();
+ // namespace02: ensure fail due to invalid topic when execute "list topic"
+ try {
Review Comment:
this api at not supports dirty data any more.
--
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] poorbarcode commented on a diff in pull request #15026: [Test]branch-2.9: fixed eror test case at AdminApiTest2
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #15026:
URL: https://github.com/apache/pulsar/pull/15026#discussion_r842054582
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -458,8 +461,12 @@ protected void internalDeleteNamespaceForcefully(AsyncResponse asyncResponse, bo
for (NamespaceBundle bundle : bundles.getBundles()) {
// check if the bundle is owned by any broker, if not then we do not need to delete the bundle
if (pulsar().getNamespaceService().getOwner(bundle).isPresent()) {
- futures.add(pulsar().getAdminClient().namespaces()
- .deleteNamespaceBundleAsync(namespaceName.toString(), bundle.getBundleRange(), true));
+ CompletableFuture<Void> futureForDeleteBundle = FutureUtil.waitForAll(futuresForDeleteTopics)
Review Comment:
Forcibly delete the order of topics and bundles, becase after deleted bundles, the topic will be fenced, then the operation that delete topic will failure
--
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] poorbarcode commented on a diff in pull request #15026: [Test]branch-2.9: fixed eror test case at AdminApiTest2
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #15026:
URL: https://github.com/apache/pulsar/pull/15026#discussion_r842057051
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiTest2.java:
##########
@@ -1745,39 +1780,6 @@ public void testMaxTopicsPerNamespace() throws Exception {
admin.topics().createNonPartitionedTopic(topic + i + i);
}
- // check first create normal topic, then system topics, unlimited even setMaxTopicsPerNamespace
Review Comment:
At last version not allow create system topic by hand-operation
--
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] poorbarcode commented on pull request #15026: [test] [admin] branch-2.9: fixed eror test case at AdminApiTest2
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #15026:
URL: https://github.com/apache/pulsar/pull/15026#issuecomment-1089920615
> if the problem is also on master branch please open a PR against master branch
I agress
--
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] eolivelli commented on pull request #15026: [test] [admin] branch-2.9: fixed eror test case at AdminApiTest2
Posted by GitBox <gi...@apache.org>.
eolivelli commented on PR #15026:
URL: https://github.com/apache/pulsar/pull/15026#issuecomment-1089922126
> I am sorry I cannot understand. Could you elaborate on that
see here:
btw please open the PR against master branch, we will cherry-pick to active branches.
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] poorbarcode commented on pull request #15026: [test] [admin] branch-2.9: fixed eror test case at AdminApiTest2
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #15026:
URL: https://github.com/apache/pulsar/pull/15026#issuecomment-1089924357
> > I am sorry I cannot understand. Could you elaborate on that
>
> see here: 
>
> btw please open the PR against master branch, we will cherry-pick to active branches. thanks.
Got it, 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] poorbarcode commented on pull request #15026: [test] [admin] branch-2.9: fixed eror test case at AdminApiTest2
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #15026:
URL: https://github.com/apache/pulsar/pull/15026#issuecomment-1089910041
> we are not fixing a test, but I see changes in pulsar-broker. Is this a patch ported from master branch ? if not...do we have a similar patch in master branch ? it is better to cherry-pick and do not let branches diverge.
from branch-2.9
--
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