-
1. Re: Memory on test suite
clebert.suconic Aug 14, 2009 11:12 AM (in response to timfox)I added -verbose:gc for that...
I could get a much cleaner run by removing the Pinger attributes on Pinger.close:public void close() { if (future != null) { future.cancel(false); future = null; } closed = true; channel0.setHandler(null); extraHandler = null; connectionFailedAction = null; channel0 = null; }
extraHandler will have a reference to the PostOffice->MessagingService somehow (I don't remember the exacts now.. but I could get more detail if needed).
I can't commit anything now as I have a bunch of debug/garbage on my work copy now. -
2. Re: Memory on test suite
timfox Aug 14, 2009 11:19 AM (in response to timfox)Pinger does not exist any more after my latest commit
-
3. Re: Memory on test suite
clebert.suconic Aug 15, 2009 12:37 PM (in response to timfox)At the end of the tests, there is about 900 PostOffice instances hanging around.
I will run some tests later this weekend trying to identify the places that are probably holding the instances.
It would be better for this diagnose if the Pingrunnable was shutdown right after the close call... It would be better to identify when the leak happened opposed to a regular delayed shutdown.
On the InVM transport, the PintRunnable will indirectly hold an instance to the Server. (somewhere through InVMConnection).
I will post more details later this weekend. -
4. Re: Memory on test suite
timfox Aug 15, 2009 12:51 PM (in response to timfox)I've fixed the current gc issues - so now if ClientSession or JBossConnection instances are kept open they will not cause connections to leak.
This may have been causing the leak you are seeing.
Just running tests now, will commit soon. -
5. Re: Memory on test suite
timfox Aug 15, 2009 12:57 PM (in response to timfox)One other thing I noticed: Now if you don't close your jms connections or core client sessions you'll see a warning in the logs when it's closed at gc.
You'll notice that *lots* of tests in the test suite are throwing warnings - in other words we have many poorly written tests that don't close connections properly!
And yes you're right. If you don't close a connection, with the invm transport, it will keep a reference right through to the server and back which will mean whole servers won't be garbage collected.
This should be fixed now (when I commit in the next few hours) -
6. Re: Memory on test suite
timfox Aug 15, 2009 3:20 PM (in response to timfox)I've also added some diagnostics to aid in diagnosing where sessions were created that subsequently weren't closed.
When gc closes a connection you will see something like this in the logs:Finalizer] 20:14:43,244 WARNING [org.jboss.messaging.core.client.impl.DelegatingSession] I'm closing a ClientSession you left open. Please make sure you close all ClientSessions explicitly before letting them go out of scope! [Finalizer] 20:14:43,244 WARNING [org.jboss.messaging.core.client.impl.DelegatingSession] The session you didn't close was created here: java.lang.Exception at org.jboss.messaging.core.client.impl.DelegatingSession.<init>(DelegatingSession.java:83) at org.jboss.messaging.core.client.impl.ConnectionManagerImpl.createSession(ConnectionManagerImpl.java:375) at org.jboss.messaging.core.client.impl.ClientSessionFactoryImpl.createSessionInternal(ClientSessionFactoryImpl.java:968) at org.jboss.messaging.core.client.impl.ClientSessionFactoryImpl.createSession(ClientSessionFactoryImpl.java:758) at org.jboss.messaging.tests.integration.client.CommitRollbackTest.testReceiveWithCommit(CommitRollbackTest.java:62) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:597) at junit.framework.TestCase.runTest(TestCase.java:164) at junit.framework.TestCase.runBare(TestCase.java:130) at junit.framework.TestResult$1.protect(TestResult.java:106) at junit.framework.TestResult.runProtected(TestResult.java:124) at junit.framework.TestResult.run(TestResult.java:109) at junit.framework.TestCase.run(TestCase.java:120) at junit.framework.TestSuite.runTest(TestSuite.java:230) at junit.framework.TestSuite.run(TestSuite.java:225) at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.run(JUnitTestRunner.java:421) at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.launch(JUnitTestRunner.java:912) at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.main(JUnitTestRunner.java:743)
The stack trace will tell you where in the code the session was created, that later wasn't closed.
So we just need to run the test suite and look through the logs for these stack traces and fix the tests that don't close the sessions. -
7. Re: Memory on test suite
timfox Aug 16, 2009 8:43 AM (in response to timfox)I fixed a lot of these tests, but there are still quite a few more.
Most were client integration tests, and management integration tests.
Andy, Jeff, Clebert - can you run the test suite then go through the logs looking for the "i'm closing" logs and fixing tests appropriately? -
8. Re: Memory on test suite
clebert.suconic Aug 16, 2009 1:48 PM (in response to timfox)I believe the bridge is leaking connections from what i saw on the dumps.
I will work on those tomorrow as well. -
9. Re: Memory on test suite
timfox Aug 16, 2009 2:22 PM (in response to timfox)If the bridge is leaking connections you'll see it in the logs too.
-
10. Re: Memory on test suite
clebert.suconic Aug 17, 2009 1:08 AM (in response to timfox)There is something else leaking on the Bridge.
I will be able to trace it tomorrow morning first thing.
I will keep another testsuite running. The number of instances of PostOffice are not as big as they were before, but they are still leaking when the BridgeTests or ClusteringTests are running. -
11. Re: Memory on test suite
clebert.suconic Aug 17, 2009 1:13 AM (in response to timfox)Actually it doesn't seem as bad. The nubmers just came down on the logs as I'm running it.
[junit] |Class | Instances| Bytes| [junit] |org.jboss.messaging.core.postoffice.impl.PostOfficeImpl | 22| 3344| [junit] |org.jboss.messaging.core.postoffice.impl.PostOfficeImpl | 27| 4104| [junit] |org.jboss.messaging.core.postoffice.impl.PostOfficeImpl | 27| 4104| [junit] |org.jboss.messaging.core.postoffice.impl.PostOfficeImpl | 29| 4408| [junit] |org.jboss.messaging.core.postoffice.impl.PostOfficeImpl$Reaper | 29| 928| [junit] org.jboss.messaging.core.postoffice.impl.PostOfficeImpl$Reaper.run(PostOfficeImpl.java:944) [junit] org.jboss.messaging.core.postoffice.impl.PostOfficeImpl$Reaper.run(PostOfficeImpl.java:944) [junit] org.jboss.messaging.core.postoffice.impl.PostOfficeImpl$Reaper.run(PostOfficeImpl.java:944) [junit] org.jboss.messaging.core.postoffice.impl.PostOfficeImpl$Reaper.run(PostOfficeImpl.java:944) [junit] org.jboss.messaging.core.postoffice.impl.PostOfficeImpl$Reaper.run(PostOfficeImpl.java:944) [junit] |org.jboss.messaging.core.postoffice.impl.PostOfficeImpl | 2| 304|
I would prefer the instances getting GCed right after the test was finished.
So, I will check this tomorrow to make sure there are no problems around it.. but just in case.
The testsuite with the profiling takes longer (about 2X because of the dump at the end of every test). So.. I will post more results tomorrow morning about this. -
12. Re: Memory on test suite
timfox Aug 17, 2009 3:08 AM (in response to timfox)Guys - let's not do any more profiling for now.
The first thing to do is look through the logs for the log lines that I mentioned earlier - there are quite a few of these still being logged (including clustering tests).
You can get a copy of the logs from the last run on Hudson- this will save some time as you don't have to run the test suite locally (90 minutes).
Once all those leakages from the logs have been fixed, *then* do some profiling if you like, but let's clear the low hanging fruit first. -
13. Re: Memory on test suite
jmesnil Aug 17, 2009 9:06 AM (in response to timfox)based on hudson build #53 http://hudson.qa.jboss.com/hudson/view/JBM%202/job/JBM2-tests-clone/53, there are still a few "you left open session" warnings:
RedeliveryConsumerTest.testRedeliveryMessageStrict
- test expects a client crash, it is normal the session is not closed
SessionCloseOnGCTest
- tests do not close session on purpose
SessionTest.testFailureListener
- test simulates a connection failure
=> I've added a session.close at the end of test to clean up
ClusterWithBackupFailoverTest.testFailAllNodes
FailoverNoSessionsFailoverTest.testNoFailoverAndCreateNewSession
FailoverPreAcknowledgeTest.testPreAcknowledgeFailoverTest
FailoverScheduledMessageTest.testScheduled
FailureListenerOnFailoverTest.testFailureListenersNotCalledOnReconnection
- all these tests failed before the sessions are closed but the calls are there
SecurityTest
=> I created https://jira.jboss.org/jira/browse/JBMESSAGING-1714 for this and fixed it. Commit should be taken into account in build #55 -
14. Re: Memory on test suite
timfox Aug 17, 2009 9:14 AM (in response to timfox)"jmesnil" wrote:
ClusterWithBackupFailoverTest.testFailAllNodes
FailoverNoSessionsFailoverTest.testNoFailoverAndCreateNewSession
FailoverPreAcknowledgeTest.testPreAcknowledgeFailoverTest
FailoverScheduledMessageTest.testScheduled
FailureListenerOnFailoverTest.testFailureListenersNotCalledOnReconnection
- all these tests failed before the sessions are closed but the calls are there
Even if a test fails, it shouldn't leave any connections/sessions open.
These should be closed in finally blocks or as appropriate.
The only tests where we should receive warnings are the GC tests where we are specifically testing for this functionality.