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

Hi Sanne, I've reworked HHH-13916 <https://hibernate.atlassian.net/browse/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 > > > >