Re: [hibernate-dev] HHH-13916 / WFLY-13259

I'd say if you clearly mark the old method deprecated (or even remove

Given that there's a workaround for the issue, I'd agree with Sanne to make this an ORM 6+ only improvement. Indeed I'm no longer in the team and hence could not be the maintainer for such a module, but I'd be happy to discuss improvements for ORM6 caching. I've not been aware of any discussions on that yet. On the Infinispan side, just a couple of weeks back Tristan was asking me about the need for a remote Infinispan cache provider for ORM. Maybe ORM6 offers a good opportunity to rework the provider and make it easy to maintain a local/remote provider. Galder On Wed, Apr 15, 2020 at 1:42 PM Sanne Grinovero <sanne(a)hibernate.org&gt; wrote: > On Wed, 15 Apr 2020 at 02:16, Gail Badner <gbadner(a)redhat.com&gt; wrote: > > > > FWIW, there's no point in fixing HHH-13916 unless we hear that > Infinispan is going to use the fix. > > I suppose we can expect that that will eventually happen - I just > don't know when and were to best make a note of such a need? I'd say > if you clearly mark the old method deprecated (or even remove it?) it > will be guaranteed we don't forget about this improvement. > > As far as I know, there isn't an Infinispan 2LC codebase for ORM6 yet, > and there will be more differences likely? > > Also keep in mind that Galder no longer works on Infinispan, he's now > focused on GraalVM and JDK engineering. I'm glad he's still around and > we might be able to bother him for suggestions and wisdom, but I'm not > sure yet who's going to be responsible to create such a new module. > Radim is on the performance team; as always, would be awesome to have > his help but I don't know of the performance team will be able to own > the module maintenance. > > Thanks, > Sanne > > > > > > Radim/Galder, WDYT? > > > > Thanks, > > Gail > > > > On Tue, Apr 14, 2020 at 3:25 PM Gail Badner <gbadner(a)redhat.com&gt; wrote: > >> > >> Hi Sanne, > >> > >> I've reworked HHH-13916 to use this alternative, and set the fix > version to 6-wishlist. > >> > >> Regards, > >> Gail > >> > >> On Tue, Apr 14, 2020 at 1:23 AM Sanne Grinovero <sanne(a)hibernate.org&gt; > wrote: > >>> > >>> Hi Gail, > >>> > >>> I would go ahead with this improvement for ORM 6 and avoid spending > >>> your precious time on a v5 improvement - especially if it's going to > >>> require coordination with both the Infinispan and WildFly teams. > >>> > >>> Thanks > >>> > >>> On Fri, 10 Apr 2020 at 00:56, Gail Badner <gbadner(a)redhat.com&gt; wrote: > >>> > > >>> > I've been looking into how to fix this issue: > >>> > > >>> > https://hibernate.atlassian.net/browse/HHH-13916 > >>> > https://issues.redhat.com/browse/WFLY-13259 > >>> > > >>> > The crux of the matter is when Hibernate calls > CacheHelper.fromSharedCache( > >>> > session, cacheKey, cachAccess ), and the entity is not found in the > cache, > >>> > Infinispan stores a PendingPut containing a > >>> > SharedSessionContractImplementor instance. > >>> > > >>> > IIUC, as an optimization, Infinispan assumes that the entity not > found in > >>> > the cache will ultimately be added to the cache after it is loaded > from the > >>> > database. If that doesn't happen, the PendingPut will expire and > will > >>> > eventually be removed. Until it expires, > >>> > the SharedSessionContractImplementor instance cannot be > garbage-collected. > >>> > > >>> > This is particularly a problem if the cache is not disabled while a > large > >>> > amount of cacheable data is being imported. This is the particular > use case > >>> > described by WFLY-13259. There is a reproducer attached that > >>> > throws OutOfMemoryError. > >>> > > >>> > The obvious workaround is to set org.hibernate.CacheMode.IGNORE (or > >>> > possibly CacheMode.PUT?) while importing data. > >>> > > >>> > I discussed this briefly with Sanne, and we agree that an > improvement would > >>> > be to not store a SharedSessionContractImplementor in a PendingPut > at all. > >>> > > >>> > There is already a way to get a UUID for the session by calling > >>> > SharedSessionContractImplementor#getSessionIdentifier(). > Unfortunately, the > >>> > implementation in AbstractSharedSessionContract indicates that > frequent > >>> > "UUID generations will cause a significant amount of contention". > >>> > > >>> > Sanne has suggested returning a "token" that is just a new Object. > I've > >>> > created a branch > >>> > <https://github.com/gbadner/hibernate-core/tree/HHH-13916_token> > [1] that > >>> > does this. > >>> > > >>> > Infinispan would need to be updated so that PendingPut#owner is set > >>> > to SharedSessionContractImplementor#getSessionToken() (instead of > >>> > the SharedSessionContractImplementor object). > >>> > > >>> > Looking at the Infinispan code, I see that code that would be > affected is > >>> > in > >>> > org.infinispan.hibernate.cache.commons.access.PutFromLoadValidator, > which > >>> > is used by infinispan-hibernate-cache-v51. > >>> > > >>> > IIUC, in order to fix this any time soon for WildFly or EAP 7.x, > [1] would > >>> > have to be backported to both Hibernate ORM 5.1 and 5.3 branches, > and the > >>> > Hibernate versions would have to be updated in Infinispan before > Infinispan > >>> > could be updated to use > SharedSessionContractImplementor#getSessionToken(). > >>> > > >>> > Galder/Radim, are there any plans for > >>> > dropping infinispan-hibernate-cache-v51? > >>> > > >>> > Are there other places where the SharedSessionContractImplementor > is stored > >>> > in the cache? > >>> > > >>> > Aside from infinispan-hibernate-cache-v51, do you see anything > about [1] > >>> > that would cause problems? > >>> > > >>> > If not, when do you think we could coordinate this change? Do we > need to > >>> > wait for Hibernate ORM 6.0? > >>> > > >>> > This is considered an improvement, so it's not urgent. It would be > nice to > >>> > fix this though. > >>> > > >>> > Galder/Radim, please provide your input so we figure out when it > can be > >>> > fixed. > >>> > > >>> > Thanks, > >>> > Gail > >>> > > >>> > [1] https://github.com/gbadner/hibernate-core/tree/HHH-13916_token > >>> > [2] > >>> > _______________________________________________ > >>> > hibernate-dev mailing list > >>> > hibernate-dev(a)lists.jboss.org > >>> > https://lists.jboss.org/mailman/listinfo/hibernate-dev > >>> > > >>> > >