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

[Sanne Grinovero]

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 at redhat.com> 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 at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>