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/07/13 06:21:27 UTC
[GitHub] [pulsar] gaozhangmin opened a new pull request, #16562: fix flaky test
gaozhangmin opened a new pull request, #16562:
URL: https://github.com/apache/pulsar/pull/16562
### Motivation
Fixes #16561
### Modifications
make sure namespacebundle is owned
### Documentation
- [x] `doc-not-needed`
--
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] gaozhangmin commented on pull request #16562: [fix][flaky-test] testSplitBundleForMultiTimes
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on PR #16562:
URL: https://github.com/apache/pulsar/pull/16562#issuecomment-1199006118
/pulsarbot run-failure-checks
--
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 commented on pull request #16562: [fix][flaky-test] testSplitBundleForMultiTimes
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on PR #16562:
URL: https://github.com/apache/pulsar/pull/16562#issuecomment-1210470129
@gaozhangmin Could you please add some detailed description about this issue and how this PR will fix the issue? It will help the reviewers to understand 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] codelipenghui commented on a diff in pull request #16562: [fix][flaky-test] testSplitBundleForMultiTimes
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on code in PR #16562:
URL: https://github.com/apache/pulsar/pull/16562#discussion_r919719004
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java:
##########
@@ -1828,10 +1830,18 @@ public void testSplitBundleForMultiTimes() throws Exception{
String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
BundlesData data = BundlesData.builder().numBundles(4).build();
admin.namespaces().createNamespace(namespace, data);
+ LookupOptions optionsHttps = LookupOptions.builder().authoritative(false).requestHttps(true).readOnly(false).build();
+ URL localWebServiceUrl = new URL(pulsar.getSafeWebServiceAddress());
for (int i = 0; i < 10; i ++) {
final BundlesData bundles = admin.namespaces().getBundles(namespace);
final String bundle = bundles.getBoundaries().get(0) + "_" + bundles.getBoundaries().get(1);
+ NamespaceBundle testBundle = new NamespaceBundle(NamespaceName.get(namespace),
+ Range.range(Long.decode(bundles.getBoundaries().get(0)), BoundType.CLOSED,
+ Long.decode(bundles.getBoundaries().get(1)), BoundType.OPEN),
+ pulsar.getNamespaceService().getNamespaceBundleFactory());
+ doReturn(Optional.of(localWebServiceUrl)).when(nsSvc).getWebServiceUrl(testBundle, optionsHttps);
Review Comment:
Can change to `doReturn(Optional.of(localWebServiceUrl)).when(nsSvc).getWebServiceUrl(testBundle, any());` since we don't care about the lookup options in this case?
--
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] nodece commented on a diff in pull request #16562: [fix][flaky-test] testSplitBundleForMultiTimes
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #16562:
URL: https://github.com/apache/pulsar/pull/16562#discussion_r919864601
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java:
##########
@@ -1828,10 +1830,18 @@ public void testSplitBundleForMultiTimes() throws Exception{
String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
BundlesData data = BundlesData.builder().numBundles(4).build();
admin.namespaces().createNamespace(namespace, data);
+ LookupOptions optionsHttps = LookupOptions.builder().authoritative(false).requestHttps(true).readOnly(false).build();
+ URL localWebServiceUrl = new URL(pulsar.getSafeWebServiceAddress());
for (int i = 0; i < 10; i ++) {
final BundlesData bundles = admin.namespaces().getBundles(namespace);
final String bundle = bundles.getBoundaries().get(0) + "_" + bundles.getBoundaries().get(1);
+ NamespaceBundle testBundle = new NamespaceBundle(NamespaceName.get(namespace),
+ Range.range(Long.decode(bundles.getBoundaries().get(0)), BoundType.CLOSED,
+ Long.decode(bundles.getBoundaries().get(1)), BoundType.OPEN),
+ pulsar.getNamespaceService().getNamespaceBundleFactory());
+ doReturn(Optional.of(localWebServiceUrl)).when(nsSvc).getWebServiceUrl(testBundle, optionsHttps);
Review Comment:
You also using `mockWebUrl()`.
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java:
##########
@@ -1828,10 +1830,18 @@ public void testSplitBundleForMultiTimes() throws Exception{
String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
BundlesData data = BundlesData.builder().numBundles(4).build();
admin.namespaces().createNamespace(namespace, data);
+ LookupOptions optionsHttps = LookupOptions.builder().authoritative(false).requestHttps(true).readOnly(false).build();
+ URL localWebServiceUrl = new URL(pulsar.getSafeWebServiceAddress());
for (int i = 0; i < 10; i ++) {
final BundlesData bundles = admin.namespaces().getBundles(namespace);
final String bundle = bundles.getBoundaries().get(0) + "_" + bundles.getBoundaries().get(1);
+ NamespaceBundle testBundle = new NamespaceBundle(NamespaceName.get(namespace),
+ Range.range(Long.decode(bundles.getBoundaries().get(0)), BoundType.CLOSED,
+ Long.decode(bundles.getBoundaries().get(1)), BoundType.OPEN),
+ pulsar.getNamespaceService().getNamespaceBundleFactory());
+ doReturn(Optional.of(localWebServiceUrl)).when(nsSvc).getWebServiceUrl(testBundle, optionsHttps);
Review Comment:
You can also using `mockWebUrl()`.
--
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] gaozhangmin commented on a diff in pull request #16562: [fix][flaky-test] testSplitBundleForMultiTimes
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on code in PR #16562:
URL: https://github.com/apache/pulsar/pull/16562#discussion_r919733068
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java:
##########
@@ -1828,10 +1830,18 @@ public void testSplitBundleForMultiTimes() throws Exception{
String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
BundlesData data = BundlesData.builder().numBundles(4).build();
admin.namespaces().createNamespace(namespace, data);
+ LookupOptions optionsHttps = LookupOptions.builder().authoritative(false).requestHttps(true).readOnly(false).build();
+ URL localWebServiceUrl = new URL(pulsar.getSafeWebServiceAddress());
for (int i = 0; i < 10; i ++) {
final BundlesData bundles = admin.namespaces().getBundles(namespace);
final String bundle = bundles.getBoundaries().get(0) + "_" + bundles.getBoundaries().get(1);
+ NamespaceBundle testBundle = new NamespaceBundle(NamespaceName.get(namespace),
+ Range.range(Long.decode(bundles.getBoundaries().get(0)), BoundType.CLOSED,
+ Long.decode(bundles.getBoundaries().get(1)), BoundType.OPEN),
+ pulsar.getNamespaceService().getNamespaceBundleFactory());
+ doReturn(Optional.of(localWebServiceUrl)).when(nsSvc).getWebServiceUrl(testBundle, optionsHttps);
Review Comment:
I tries, the test will fail
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java:
##########
@@ -1828,10 +1830,18 @@ public void testSplitBundleForMultiTimes() throws Exception{
String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
BundlesData data = BundlesData.builder().numBundles(4).build();
admin.namespaces().createNamespace(namespace, data);
+ LookupOptions optionsHttps = LookupOptions.builder().authoritative(false).requestHttps(true).readOnly(false).build();
+ URL localWebServiceUrl = new URL(pulsar.getSafeWebServiceAddress());
for (int i = 0; i < 10; i ++) {
final BundlesData bundles = admin.namespaces().getBundles(namespace);
final String bundle = bundles.getBoundaries().get(0) + "_" + bundles.getBoundaries().get(1);
+ NamespaceBundle testBundle = new NamespaceBundle(NamespaceName.get(namespace),
+ Range.range(Long.decode(bundles.getBoundaries().get(0)), BoundType.CLOSED,
+ Long.decode(bundles.getBoundaries().get(1)), BoundType.OPEN),
+ pulsar.getNamespaceService().getNamespaceBundleFactory());
+ doReturn(Optional.of(localWebServiceUrl)).when(nsSvc).getWebServiceUrl(testBundle, optionsHttps);
Review Comment:
I tries, the test will fail, @codelipenghui
--
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] gaozhangmin commented on pull request #16562: [fix][flaky-test] testSplitBundleForMultiTimes
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on PR #16562:
URL: https://github.com/apache/pulsar/pull/16562#issuecomment-1212689639
> @gaozhangmin Could you please add some detailed description about this issue and how this PR will fix the issue? It will help the reviewers to understand it.
@codelipenghui updated
--
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] gaozhangmin commented on pull request #16562: [fix][flaky-test] testSplitBundleForMultiTimes
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on PR #16562:
URL: https://github.com/apache/pulsar/pull/16562#issuecomment-1193969685
@nodece @codelipenghui PTAL again
--
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] gaozhangmin commented on pull request #16562: [fix][flaky-test] testSplitBundleForMultiTimes
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on PR #16562:
URL: https://github.com/apache/pulsar/pull/16562#issuecomment-1184050711
/pulsarbot run-failure-checks
--
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] nodece commented on a diff in pull request #16562: [fix][flaky-test] testSplitBundleForMultiTimes
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #16562:
URL: https://github.com/apache/pulsar/pull/16562#discussion_r919866205
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java:
##########
@@ -1828,10 +1830,18 @@ public void testSplitBundleForMultiTimes() throws Exception{
String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
BundlesData data = BundlesData.builder().numBundles(4).build();
admin.namespaces().createNamespace(namespace, data);
+ LookupOptions optionsHttps = LookupOptions.builder().authoritative(false).requestHttps(true).readOnly(false).build();
+ URL localWebServiceUrl = new URL(pulsar.getSafeWebServiceAddress());
for (int i = 0; i < 10; i ++) {
final BundlesData bundles = admin.namespaces().getBundles(namespace);
final String bundle = bundles.getBoundaries().get(0) + "_" + bundles.getBoundaries().get(1);
+ NamespaceBundle testBundle = new NamespaceBundle(NamespaceName.get(namespace),
+ Range.range(Long.decode(bundles.getBoundaries().get(0)), BoundType.CLOSED,
+ Long.decode(bundles.getBoundaries().get(1)), BoundType.OPEN),
+ pulsar.getNamespaceService().getNamespaceBundleFactory());
+ doReturn(Optional.of(localWebServiceUrl)).when(nsSvc).getWebServiceUrl(testBundle, optionsHttps);
Review Comment:
Do you have a way to reproduce this?
--
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- merged pull request #16562: [fix][flaky-test] testSplitBundleForMultiTimes
Posted by GitBox <gi...@apache.org>.
Technoboy- merged PR #16562:
URL: https://github.com/apache/pulsar/pull/16562
--
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