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

[Sanne Grinovero]

On Wed, 15 Apr 2020 at 02:16, Gail Badner <gbadner at redhat.com> 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 at redhat.com> 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 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
>>> >
>>>