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 2020/11/11 16:30:20 UTC
[GitHub] [pulsar] eolivelli opened a new pull request #8526: Reduce memory leaks in pulsar-broker tests
eolivelli opened a new pull request #8526:
URL: https://github.com/apache/pulsar/pull/8526
### Motivation
The tests in pulsar-broker are full of memory leaks, especially about thread spawned by non closed instances of:
- PulsarService
- BrokerService
- ManagedLedgerFactoryImpl
- Executors in general
This is the second part of a series of fixes, with this change it looks like the suite is able to use at max 1GB of head and there is a huge reduction of the number of threads.
Unfortunately there are still leaks of Bookies and ZooKeeper server instances to be fixed
### Modifications
- fix the tests
- use "alwaysRun=true" in order to ensure that resources are called (the default is "false" and if some test is skipped resources are not released)
### Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #8526: Reduce memory leaks in pulsar-broker tests
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #8526:
URL: https://github.com/apache/pulsar/pull/8526#issuecomment-726072690
/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #8526: Reduce memory leaks in pulsar-broker tests
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #8526:
URL: https://github.com/apache/pulsar/pull/8526#issuecomment-726105137
/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #8526: Reduce memory leaks in pulsar-broker tests
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #8526:
URL: https://github.com/apache/pulsar/pull/8526#issuecomment-725945707
/pulsarbot rerun-failure-tests
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #8526: Reduce memory leaks in pulsar-broker tests
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #8526:
URL: https://github.com/apache/pulsar/pull/8526#issuecomment-725890610
Thank you guys for review, I have merged with current master in order to fix checkstyle issues.
I hope we can merge soon
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #8526: Reduce memory leaks in pulsar-broker tests
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #8526:
URL: https://github.com/apache/pulsar/pull/8526#issuecomment-725527533
@jiazhai @codelipenghui @lhotari PTAL
this is the second part,
once this part lands to master I will follow up with the third part, about shutting down every BK and ZK service
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on a change in pull request #8526: Reduce memory leaks in pulsar-broker tests
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #8526:
URL: https://github.com/apache/pulsar/pull/8526#discussion_r521928482
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiOffloadTest.java
##########
@@ -178,7 +178,7 @@ public void testOffloadPolicies() throws Exception {
assertNull(offload3);
}
- @Test(timeOut = 20000)
+ @Test
Review comment:
@rdhabalia
as @merlimat probably those timeout are not needed, also when you enable the timeout feature the system has to handle the execution in a different way and also if there is a timeout the JVM remains in an unknown status.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #8526: Reduce memory leaks in pulsar-broker tests
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #8526:
URL: https://github.com/apache/pulsar/pull/8526#issuecomment-725931264
I should also add that these kind of leaks may affect the execution of the other test cases, because the PulsarService/Broker/ManagedLedgerFactory still connect to ZK/BK and if you leave a zombie service running it may connect to the ZK ensemble of the other test cases, resulting in very unpredictable behavior.
I am new to TestNG, butI did not find a way to assert that after every test class we are not leaking any broker or pulsarclient open.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #8526: Reduce memory leaks in pulsar-broker tests
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #8526:
URL: https://github.com/apache/pulsar/pull/8526#issuecomment-725534483
@jiazhai @codelipenghui
I am fixing checkstyle issues, but it looks like that some of them were not introduced by this patch
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [pulsar] rdhabalia commented on a change in pull request #8526: Reduce memory leaks in pulsar-broker tests
Posted by GitBox <gi...@apache.org>.
rdhabalia commented on a change in pull request #8526:
URL: https://github.com/apache/pulsar/pull/8526#discussion_r521556212
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiOffloadTest.java
##########
@@ -178,7 +178,7 @@ public void testOffloadPolicies() throws Exception {
assertNull(offload3);
}
- @Test(timeOut = 20000)
+ @Test
Review comment:
is there any reason to remove timeout 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] merlimat commented on a change in pull request #8526: Reduce memory leaks in pulsar-broker tests
Posted by GitBox <gi...@apache.org>.
merlimat commented on a change in pull request #8526:
URL: https://github.com/apache/pulsar/pull/8526#discussion_r521723115
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiOffloadTest.java
##########
@@ -178,7 +178,7 @@ public void testOffloadPolicies() throws Exception {
assertNull(offload3);
}
- @Test(timeOut = 20000)
+ @Test
Review comment:
We have default timeouts applied to all the tests in any 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] jiazhai merged pull request #8526: Reduce memory leaks in pulsar-broker tests
Posted by GitBox <gi...@apache.org>.
jiazhai merged pull request #8526:
URL: https://github.com/apache/pulsar/pull/8526
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #8526: Reduce memory leaks in pulsar-broker tests
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #8526:
URL: https://github.com/apache/pulsar/pull/8526#issuecomment-726021235
/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org