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

[Gail Badner]

FWIW, there's no point in fixing HHH-13916 unless we hear that Infinispan
is going to use the fix.

Radim/Galder, WDYT?

Thanks,
Gail

On Tue, Apr 14, 2020 at 3:25 PM Gail Badner <gbadner at redhat.com> wrote:

> 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 at hibernate.org>
> 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 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
>> >
>>
>>