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/10/10 12:00:42 UTC
[GitHub] [pulsar] coderzc opened a new pull request, #17887: [improve][test] Improve SimpleProducerConsumerTest to reduce the execution time
coderzc opened a new pull request, #17887:
URL: https://github.com/apache/pulsar/pull/17887
Fixed
* https://github.com/apache/pulsar/issues/17634
* https://github.com/apache/pulsar/issues/17643
### Modifications
Only execution once `setup`/`clean` and clean `tenants`/`namespaces`/`producer`/`consumer` after the end of each test execution.
Execution time after improve test:
```
Tests run: 102, Failures: 0, Errors: 0, Skipped: 1, Time elapsed: 461.255 s - in org.apache.pulsar.client.api.SimpleProducerConsumerTest
```
```
Tests run: 102, Failures: 0, Errors: 0, Skipped: 1, Time elapsed: 434.111 s - in org.apache.pulsar.broker.service.persistent.SimpleProducerConsumerTestStreamingDispatcherTest
```
### Documentation
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
- [ ] `doc-required`
(Your PR needs to update docs and you will update later)
- [x] `doc-not-needed`
(Please explain why)
- [ ] `doc`
(Your PR contains doc changes)
- [ ] `doc-complete`
(Docs have been already added)
### Matching PR in forked repository
PR in forked repository: https://github.com/coderzc/pulsar/pull/7
--
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 closed pull request #17887: [improve][test] Improve SimpleProducerConsumerTest to reduce the execution time
Posted by GitBox <gi...@apache.org>.
codelipenghui closed pull request #17887: [improve][test] Improve SimpleProducerConsumerTest to reduce the execution time
URL: https://github.com/apache/pulsar/pull/17887
--
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 #17887: [improve][test] Improve SimpleProducerConsumerTest to reduce the execution time
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #17887:
URL: https://github.com/apache/pulsar/pull/17887#discussion_r983713420
##########
pulsar-broker/src/test/java/org/apache/pulsar/client/api/SimpleProducerConsumerTest.java:
##########
@@ -125,13 +126,35 @@ public class SimpleProducerConsumerTest extends ProducerConsumerBase {
private static final int RECEIVE_TIMEOUT_SHORT_MILLIS = 200 * TIMEOUT_MULTIPLIER;
private static final int RECEIVE_TIMEOUT_MEDIUM_MILLIS = 1000 * TIMEOUT_MULTIPLIER;
- @BeforeMethod(alwaysRun = true)
+ @BeforeClass
@Override
protected void setup() throws Exception {
super.internalSetup();
super.producerBaseSetup();
}
+ @AfterMethod(alwaysRun = true)
+ public void rest() throws Exception {
+ pulsar.getConfiguration().setForceDeleteTenantAllowed(true);
+ pulsar.getConfiguration().setForceDeleteNamespaceAllowed(true);
+
+ for (String tenant : admin.tenants().getTenants()) {
+ for (String namespace : admin.namespaces().getNamespaces(tenant)) {
+ admin.namespaces().deleteNamespace(namespace, true);
Review Comment:
Suggest to use `deleteNamespaceGraceFully`, because `delete namespace` is not a stable cmd, see https://github.com/apache/pulsar/pull/17157.
Note that `deleteNamespaceGraceFully` runs slower than `delete namespace` and is unsuitable for all scenarios.
--
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 #17887: [improve][test] Improve SimpleProducerConsumerTest to reduce the execution time
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #17887:
URL: https://github.com/apache/pulsar/pull/17887#discussion_r996733611
##########
pulsar-broker/src/test/java/org/apache/pulsar/client/api/SimpleProducerConsumerTest.java:
##########
@@ -125,13 +126,35 @@ public class SimpleProducerConsumerTest extends ProducerConsumerBase {
private static final int RECEIVE_TIMEOUT_SHORT_MILLIS = 200 * TIMEOUT_MULTIPLIER;
private static final int RECEIVE_TIMEOUT_MEDIUM_MILLIS = 1000 * TIMEOUT_MULTIPLIER;
- @BeforeMethod(alwaysRun = true)
+ @BeforeClass
@Override
protected void setup() throws Exception {
super.internalSetup();
super.producerBaseSetup();
}
+ @AfterMethod(alwaysRun = true)
+ public void rest() throws Exception {
+ pulsar.getConfiguration().setForceDeleteTenantAllowed(true);
+ pulsar.getConfiguration().setForceDeleteNamespaceAllowed(true);
+
+ for (String tenant : admin.tenants().getTenants()) {
+ for (String namespace : admin.namespaces().getNamespaces(tenant)) {
+ admin.namespaces().deleteNamespace(namespace, true);
Review Comment:
@ethqunzhong I see, could you `rebase master` and see if it reproduces?
--
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] ethqunzhong commented on a diff in pull request #17887: [improve][test] Improve SimpleProducerConsumerTest to reduce the execution time
Posted by GitBox <gi...@apache.org>.
ethqunzhong commented on code in PR #17887:
URL: https://github.com/apache/pulsar/pull/17887#discussion_r995526974
##########
pulsar-broker/src/test/java/org/apache/pulsar/client/api/SimpleProducerConsumerTest.java:
##########
@@ -125,13 +126,35 @@ public class SimpleProducerConsumerTest extends ProducerConsumerBase {
private static final int RECEIVE_TIMEOUT_SHORT_MILLIS = 200 * TIMEOUT_MULTIPLIER;
private static final int RECEIVE_TIMEOUT_MEDIUM_MILLIS = 1000 * TIMEOUT_MULTIPLIER;
- @BeforeMethod(alwaysRun = true)
+ @BeforeClass
@Override
protected void setup() throws Exception {
super.internalSetup();
super.producerBaseSetup();
}
+ @AfterMethod(alwaysRun = true)
+ public void rest() throws Exception {
+ pulsar.getConfiguration().setForceDeleteTenantAllowed(true);
+ pulsar.getConfiguration().setForceDeleteNamespaceAllowed(true);
+
+ for (String tenant : admin.tenants().getTenants()) {
+ for (String namespace : admin.namespaces().getNamespaces(tenant)) {
+ admin.namespaces().deleteNamespace(namespace, true);
Review Comment:
> Suggest to use `deleteNamespaceGraceFully`, because `delete namespace` is not a stable cmd, see #17157.
>
> Note that `deleteNamespaceGraceFully` runs slower than `delete namespace` and is unsuitable for all scenarios.
in my CI test, There are two failures related to these changes:
Time elapsed failures while excute `deleteNamespaceGraceFully `
see the checks in https://github.com/apache/pulsar/actions/runs/3242148385/jobs/5326408439
could i skip this test temporary？
--
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] ethqunzhong commented on pull request #17887: [improve][test] Improve SimpleProducerConsumerTest to reduce the execution time
Posted by GitBox <gi...@apache.org>.
ethqunzhong commented on PR #17887:
URL: https://github.com/apache/pulsar/pull/17887#issuecomment-1278691423
similar test failures in the CI for this pull in the flaky test suite. see the checks
https://github.com/apache/pulsar/actions/runs/3242148385/jobs/5326408439
--
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] ethqunzhong commented on a diff in pull request #17887: [improve][test] Improve SimpleProducerConsumerTest to reduce the execution time
Posted by GitBox <gi...@apache.org>.
ethqunzhong commented on code in PR #17887:
URL: https://github.com/apache/pulsar/pull/17887#discussion_r996716705
##########
pulsar-broker/src/test/java/org/apache/pulsar/client/api/SimpleProducerConsumerTest.java:
##########
@@ -125,13 +126,35 @@ public class SimpleProducerConsumerTest extends ProducerConsumerBase {
private static final int RECEIVE_TIMEOUT_SHORT_MILLIS = 200 * TIMEOUT_MULTIPLIER;
private static final int RECEIVE_TIMEOUT_MEDIUM_MILLIS = 1000 * TIMEOUT_MULTIPLIER;
- @BeforeMethod(alwaysRun = true)
+ @BeforeClass
@Override
protected void setup() throws Exception {
super.internalSetup();
super.producerBaseSetup();
}
+ @AfterMethod(alwaysRun = true)
+ public void rest() throws Exception {
+ pulsar.getConfiguration().setForceDeleteTenantAllowed(true);
+ pulsar.getConfiguration().setForceDeleteNamespaceAllowed(true);
+
+ for (String tenant : admin.tenants().getTenants()) {
+ for (String namespace : admin.namespaces().getNamespaces(tenant)) {
+ admin.namespaces().deleteNamespace(namespace, true);
Review Comment:
@poorbarcode hi
my CI check always fails in Pulsar CI Flaky / Flaky tests suite . seem releted to run `deleteNamespaceGraceFully`
cloud you please help me confirm the reason? thx
https://github.com/apache/pulsar/actions/runs/3242148385/jobs/5359655987
--
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] ethqunzhong commented on a diff in pull request #17887: [improve][test] Improve SimpleProducerConsumerTest to reduce the execution time
Posted by GitBox <gi...@apache.org>.
ethqunzhong commented on code in PR #17887:
URL: https://github.com/apache/pulsar/pull/17887#discussion_r996769102
##########
pulsar-broker/src/test/java/org/apache/pulsar/client/api/SimpleProducerConsumerTest.java:
##########
@@ -125,13 +126,35 @@ public class SimpleProducerConsumerTest extends ProducerConsumerBase {
private static final int RECEIVE_TIMEOUT_SHORT_MILLIS = 200 * TIMEOUT_MULTIPLIER;
private static final int RECEIVE_TIMEOUT_MEDIUM_MILLIS = 1000 * TIMEOUT_MULTIPLIER;
- @BeforeMethod(alwaysRun = true)
+ @BeforeClass
@Override
protected void setup() throws Exception {
super.internalSetup();
super.producerBaseSetup();
}
+ @AfterMethod(alwaysRun = true)
+ public void rest() throws Exception {
+ pulsar.getConfiguration().setForceDeleteTenantAllowed(true);
+ pulsar.getConfiguration().setForceDeleteNamespaceAllowed(true);
+
+ for (String tenant : admin.tenants().getTenants()) {
+ for (String namespace : admin.namespaces().getNamespaces(tenant)) {
+ admin.namespaces().deleteNamespace(namespace, true);
Review Comment:
>
@poorbarcode thx your reply.
--
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 #17887: [improve][test] Improve SimpleProducerConsumerTest to reduce the execution time
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #17887:
URL: https://github.com/apache/pulsar/pull/17887#discussion_r996735951
##########
pulsar-broker/src/test/java/org/apache/pulsar/client/api/SimpleProducerConsumerTest.java:
##########
@@ -125,13 +126,35 @@ public class SimpleProducerConsumerTest extends ProducerConsumerBase {
private static final int RECEIVE_TIMEOUT_SHORT_MILLIS = 200 * TIMEOUT_MULTIPLIER;
private static final int RECEIVE_TIMEOUT_MEDIUM_MILLIS = 1000 * TIMEOUT_MULTIPLIER;
- @BeforeMethod(alwaysRun = true)
+ @BeforeClass
@Override
protected void setup() throws Exception {
super.internalSetup();
super.producerBaseSetup();
}
+ @AfterMethod(alwaysRun = true)
+ public void rest() throws Exception {
+ pulsar.getConfiguration().setForceDeleteTenantAllowed(true);
+ pulsar.getConfiguration().setForceDeleteNamespaceAllowed(true);
+
+ for (String tenant : admin.tenants().getTenants()) {
+ for (String namespace : admin.namespaces().getNamespaces(tenant)) {
+ admin.namespaces().deleteNamespace(namespace, true);
Review Comment:
Hi @ethqunzhong If you said PR #17803, then the error does not affect the merge
--
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] coderzc commented on pull request #17887: [improve][test] Improve SimpleProducerConsumerTest to reduce the execution time
Posted by GitBox <gi...@apache.org>.
coderzc commented on PR #17887:
URL: https://github.com/apache/pulsar/pull/17887#issuecomment-1272199125
@codelipenghui PTAL.
--
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 #17887: [improve][test] Improve SimpleProducerConsumerTest to reduce the execution time
Posted by GitBox <gi...@apache.org>.
codelipenghui merged PR #17887:
URL: https://github.com/apache/pulsar/pull/17887
--
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] coderzc commented on a diff in pull request #17887: [improve][test] Improve SimpleProducerConsumerTest to reduce the execution time
Posted by GitBox <gi...@apache.org>.
coderzc commented on code in PR #17887:
URL: https://github.com/apache/pulsar/pull/17887#discussion_r984163323
##########
pulsar-broker/src/test/java/org/apache/pulsar/client/api/SimpleProducerConsumerTest.java:
##########
@@ -125,13 +126,35 @@ public class SimpleProducerConsumerTest extends ProducerConsumerBase {
private static final int RECEIVE_TIMEOUT_SHORT_MILLIS = 200 * TIMEOUT_MULTIPLIER;
private static final int RECEIVE_TIMEOUT_MEDIUM_MILLIS = 1000 * TIMEOUT_MULTIPLIER;
- @BeforeMethod(alwaysRun = true)
+ @BeforeClass
@Override
protected void setup() throws Exception {
super.internalSetup();
super.producerBaseSetup();
}
+ @AfterMethod(alwaysRun = true)
+ public void rest() throws Exception {
+ pulsar.getConfiguration().setForceDeleteTenantAllowed(true);
+ pulsar.getConfiguration().setForceDeleteNamespaceAllowed(true);
+
+ for (String tenant : admin.tenants().getTenants()) {
+ for (String namespace : admin.namespaces().getNamespaces(tenant)) {
+ admin.namespaces().deleteNamespace(namespace, true);
Review Comment:
Ok, 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] nicoloboschi commented on pull request #17887: [improve][test] Improve SimpleProducerConsumerTest to reduce the execution time
Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on PR #17887:
URL: https://github.com/apache/pulsar/pull/17887#issuecomment-1278623641
@coderzc @codelipenghui there were failures in the CI for this pull in the flaky test suite. See the checks https://github.com/apache/pulsar/actions/runs/3219150517
There are two failures related to these changes:
- SimpleProducerConsumerTest.rest
Cannot invoke "org.apache.pulsar.broker.PulsarService.getConfiguration()" because "this.pulsar" is null
- SimpleProducerConsumerTest.testConcurrentConsumerReceiveWhileReconnect
Cannot invoke "org.apache.pulsar.client.api.PulsarClient.newConsumer()" because "this.pulsarClient" is null
Could you please address them?
--
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