Re: FW: Memory leak

[Michael Dick <mi...@gmail.com>]

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 dev@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 <da...@gmail.com> 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 <da...@gmail.com> 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:michael.d.dick@gmail.com]
> > > Sent: Tuesday, July 14, 2009 7:29 AM
> > > To: users@openjpa.apache.org
> > > 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 <da...@gmail.com>
> > > 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:michael.d.dick@gmail.com]
> > >> > Sent: Monday, July 13, 2009 12:49 PM
> > >> > To: users@openjpa.apache.org
> > >> > 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 <da...@gmail.com>
> > >> > 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
>