Well, I must confess a high degree of ignorance on EM creation/cleanup prior to this issue :)
Looking into the configuration, the application uses Spring's JpaTransactionManager for aop initiated transactions, in conjunction with Spring's SharedEntityManagerBean, which tries to join the current thread's open transaction if available. When I've got some more time I'll see if I can figure out who isn't closing their EM. On Thu, Jul 23, 2009 at 9:07 AM, David Minor <[email protected]> wrote: > > -----Original Message----- > From: Patrick Linskey [mailto:[email protected]] > Sent: Wednesday, July 22, 2009 6:00 PM > To: [email protected] > Subject: Re: FW: Memory leak > > > In my case it wasn't something I was aware of -- the application (from > > a > > vendor) implements Spring 2.0's AbstractEntityManagerFactoryBean such > > that it calls > > PersistenceProviderImpl.createEntityManagerFactory(String name, String > > resource, Map m), which seems to use the finalizing broker by default. > > Bummer. There is a Spring bean that uses the > createContainerEntityManagerFactory() signature; too bad that the > application doesn't use it. > > > I assume that since I ended up with a memory leak, Spring 2.0's > > cleanup practices are dubious, and the finalizing broker is necessary. > > FWIW, I don't know enough about the semantics of > AbstractEntityManagerFactoryBean to know whether Spring should be expected > to clean up EMFs obtained from it. > > > So am I correct in guessing that the non-finalizing broker should be > > the one in the ConcurrentHashMap (with hard references), and the > > finalizing broker should be in the weak reference set? > > I haven't read through the older parts of the thread enough to be sure, and > don't have the source code handy right now, so I don't have a good answer > for you right now. > > -Patrick > > On Wed, Jul 22, 2009 at 5:55 PM, David Minor<[email protected]> wrote: > > Patrick, many thanks for replying. > > > > In my case it wasn't something I was aware of -- the application (from > > a > > vendor) implements Spring 2.0's AbstractEntityManagerFactoryBean such > > that it calls > > PersistenceProviderImpl.createEntityManagerFactory(String name, String > > resource, Map m), which seems to use the finalizing broker by default. > > > > I assume that since I ended up with a memory leak, Spring 2.0's > > cleanup practices are dubious, and the finalizing broker is necessary. > > > > So am I correct in guessing that the non-finalizing broker should be > > the one in the ConcurrentHashMap (with hard references), and the > > finalizing broker should be in the weak reference set? > > > > > >> -----Original Message----- > >> From: Patrick Linskey [mailto:[email protected]] > >> Sent: Wednesday, July 22, 2009 4:58 PM > >> To: [email protected] > >> Subject: Re: FW: Memory leak > >> > >> Hi, > >> > >> Is there any particular reason why you need the finalizing broker? > >> One (vastly preferable) option would be to use the non-finalizing > >> broker, which, of course, means that you're promising to close your > >> brokers at the appropriate lifecycle moments. > >> > >> Certainly, the bug deserves to be addressed, as the whole point of > >> the finalizing mode is to be robust in an environment with dubious > >> cleanup practices. > >> > >> -Patrick > >> > >> On Wed, Jul 22, 2009 at 4:15 PM, Russell Collins< > >> [email protected]> wrote: > >> > Is there a configuration setting or work around for this in the > meantime? > >> > > >> > > >> > Russell Collins > >> > Sr. Software Engineer > >> > McLane Advanced Technology > >> > > >> > "Do or do not, there is no try." - Yoda > >> > > >> > > >> > -----Original Message----- > >> > From: David Minor [mailto:[email protected]] > >> > Sent: Wednesday, July 22, 2009 11:12 AM > >> > To: [email protected]; [email protected] > >> > Subject: Re: FW: Memory leak > >> > > >> > I've created http://issues.apache.org/jira/browse/OPENJPA-1193 > >> > > >> > One thought I had was that the if statement might simply be missing > >> > a ! -- since this is a concurrency improvement for non-finalizing > >> > brokers, perhaps the non-finalizing brokers were supposed to be > >> > switched to the ConcurrentHashMap, rather than the finalizing brokers. > >> > > >> > Is Patrick Linskey still involved in openjpa? I haven't seen his > >> > name for awhile. > >> > > >> > > >> >> -----Original Message----- > >> >> From: Michael Dick [mailto:[email protected]] > >> >> Sent: Wednesday, July 22, 2009 6:47 AM > >> >> To: [email protected]; [email protected] > >> >> Subject: Re: FW: Memory leak > >> >> > >> >> Hi David, > >> >> > >> >> At the moment I don't have a ton of free time to dive into the > >> >> change or the peformance bottleneck that it resolved. In the > >> >> interest of providing some relief (trunk & 1.3.x) I'd be happy to > >> >> make this change configurable (maybe something on > >> >> openjpa.conf.Compatibility) so you can get the old behavior. > >> >> > >> >> I've cross posted to d...@openjpa to see if there are any objections. > >> >> > >> >> In any event would you mind opening a JIRA issue (I'd do it but > >> >> then you have to subscribe to get notifications etc.). > >> >> > >> >> On Tue, Jul 21, 2009 at 6:37 PM, David Minor <[email protected]> > >> >> wrote: > >> >> > >> >> > Looks like this code was checked in a year ago by Patrick > >> >> > Linskey with > >> >> > >> >> > the comment "Improve concurrency by actively managing > >> >> > AbstractBrokerFactory's broker set when using non-finalizing > brokers. > >> >> > Credit goes to Arunabh Hazarika for identifying the bottleneck > >> >> > and > >> >> prototyping this solution." > >> >> > > >> >> > I'm not sure why _brokers was changed with regards to > >> >> > FinalizingBrokerImpl though. BrokerImpl's free() method was > >> >> > modified to call the factory to remove the it from _brokers. > >> >> > FinalizingBrokerImpl calls free() from its finalizer, but the > >> >> > finalizer will never be called if there is a reference in _brokers. > >> >> > > >> >> > Anyone have any ideas? > >> >> > > >> >> > On Tue, Jul 21, 2009 at 11:24 AM, David Minor > >> >> > <[email protected]> > >> >> wrote: > >> >> > > >> >> > > Hi Mike, > >> >> > > > >> >> > > I've tracked the problem down to > >> >> > > org.apache.openjpa.kernel.AbstractBrokerFactory. The heap dump > >> >> > > lists > >> >> > >> >> > > the _brokers Set as containing all the references to > >> >> > > FinalizingBrokerImpl, > >> >> > and > >> >> > > it appears the assignment of this set was changed to this: > >> >> > > > >> >> > > if (FinalizingBrokerImpl.class.isAssignableFrom( > >> >> > > bv.getTemplateBrokerType(_conf))) { > >> >> > > return MapBackedSet.decorate(new > >> >> > > ConcurrentHashMap(), > >> >> > > new Object() { }); > >> >> > > } else { > >> >> > > return new ConcurrentReferenceHashSet( > >> >> > > ConcurrentReferenceHashSet.WEAK); > >> >> > > } > >> >> > > > >> >> > > It used to be assigned to the weak reference hash set as in > >> >> > > the else > >> >> > >> >> > > statement. Forcing the second assignment fixes the problem. > >> >> > > > >> >> > > > >> >> > > > > >> >> > > > -----Original Message----- > >> >> > > > From: Michael Dick [mailto:[email protected]] > >> >> > > > Sent: Tuesday, July 14, 2009 7:29 AM > >> >> > > > To: [email protected] > >> >> > > > Subject: Re: FW: Memory leak > >> >> > > > > >> >> > > > Hi David, > >> >> > > > > >> >> > > > There have been a few changes in PersistenceProviderImpl. > >> >> > > > One was to make the non-finalizing BrokerImpl the default > >> >> > > > (must be overridden in your > >> >> > > > config) another that might be interesting was adding a pool > >> >> > > > of EntityManagerFactories. > >> >> > > > > >> >> > > > From what I've seen the EMF pool is not used by default, but > >> >> > > > it's possible that the AbstractEntityManagerFactoryBean is > >> >> > > > setting it (the property is named EntityManagerFactoryPool > >> >> > > > in case that helps). I took a quick pass at setting the > >> >> > > > pooling property and the only way I saw it take effect was > >> >> > > > to pass it in as a JVM arg (might be something in my eclipse > >> >> > > > env though - and I'm on > >> >> 2.0.0-SNAPSHOT ATM). > >> >> > > > > >> >> > > > Hope this gives you a starting point, if not keep replying > >> >> > > > and we'll > >> >> > try > >> >> > > > to help > >> >> > > > > >> >> > > > -mike > >> >> > > > > >> >> > > > On Mon, Jul 13, 2009 at 8:00 PM, David Minor > >> >> > > > <[email protected]> > >> >> > > > wrote: > >> >> > > > > >> >> > > >> Hi Mike, > >> >> > > >> > >> >> > > >> Nothing else has changed. The application extends spring > >> >> > > >> 2.0's AbstractEntityManagerFactoryBean class (apparently so > >> >> > > >> that the persistence.xml file can be named something > different). > >> >> > > >> > >> >> > > >> I notice it is checking the return type of > >> >> > > >> AbstractEntityManagerFactoryBean's getPersistenceProvider() > >> >> > > >> for an instance of openjpa's PersistenceProviderImpl, and > >> >> > > >> doing something different depending on whether it finds it > >> >> > > >> or not. Has anything changed with regards to this class? > >> >> > > >> > >> >> > > >> > -----Original Message----- > >> >> > > >> > From: Michael Dick [mailto:[email protected]] > >> >> > > >> > Sent: Monday, July 13, 2009 12:49 PM > >> >> > > >> > To: [email protected] > >> >> > > >> > Subject: Re: Memory leak > >> >> > > >> > > >> >> > > >> > Hi David, > >> >> > > >> > > >> >> > > >> > FinalizingBrokerImpl will close itself and free resources > >> >> > > >> > when it's GC'ed. > >> >> > > >> > It sounds like something else is holding on to a lot of > >> >> > > >> > references to FBImpl (I'd guess something changed > "upstream"). > >> >> > > >> > > >> >> > > >> > One cause is if the application creates a large number of > >> >> > > >> > EntityManagers and doesn't close them (or creates a large > >> >> > > >> > number of EMFactories which don't get closed since > >> >> > > >> > closing an EMF will close > >> >> > > > its EMs). > >> >> > > >> > > >> >> > > >> > Did anything else change or did you just upgrade OpenJPA > >> >> versions? > >> >> > > >> > > >> >> > > >> > -mike > >> >> > > >> > > >> >> > > >> > On Mon, Jul 13, 2009 at 11:34 AM, David Minor > >> >> > > >> > <[email protected]> > >> >> > > >> > wrote: > >> >> > > >> > > >> >> > > >> >> Upgrading openjpa from 1.0.1 to 1.2.1 seems to introduce > >> >> > > >> >> a memory leak > >> >> > > >> > > >> >> > > >> >> in our application -- leaving the server running for a > >> >> > > >> >> few days results in OOM errors (there are quartz tasks > >> >> > > >> >> making simple openjpa > >> >> > > > > >> >> > > >> >> selects during this time). A heap dump reveals > >> >> > > >> >> org.apache.openjpa.kernel.FinalizingBrokerImpl as the > >> >> > > >> >> dominant > >> >> > >> >> > > >> >> object, > >> >> > > >> > > >> >> > > >> >> according to Eclipse's memory analysis plugin. > >> >> > > >> >> > >> >> > > >> >> Does anyone have an idea of what might be causing this? > >> >> > > >> >> > >> >> > > >> >> -- > >> >> > > >> >> _____________ > >> >> > > >> >> David Minor > >> >> > > >> >> > >> >> > > >> > > >> >> > > >> > > >> >> > > >> > > >> >> > > >> > > >> >> > > >> > >> >> > > >> > >> >> > > >> > >> >> > > >> -- > >> >> > > >> _____________ > >> >> > > >> David Minor > >> >> > > >> > >> >> > > > > >> >> > > > > >> >> > > > > >> >> > > > > >> >> > > > >> >> > > > >> >> > > > >> >> > > -- > >> >> > > _____________ > >> >> > > David Minor > >> >> > > > >> >> > > > >> >> > > >> >> > > >> >> > -- > >> >> > _____________ > >> >> > David Minor > >> >> > > >> >> > >> >> > >> >> > >> >> > >> > > >> > > >> > -- > >> > _____________ > >> > David Minor > >> > > >> > >> > >> > >> -- > >> Patrick Linskey > >> 202 669 5907 > >> > >> > >> > >> > > > > > > -- > > _____________ > > David Minor > > > > > > -- > Patrick Linskey > 202 669 5907 > > > > -- _____________ David Minor
