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

2020-04-17 Thread Steve Ebersole
Not really sure what "better" names you have in mind, but to me only one of
these 2 is really an identifier.  This new method is more a niche value to
me.  So not sure why you want to rename the original.

On Fri, Apr 17, 2020 at 3:36 PM Sanne Grinovero  wrote:

> On Thu, 16 Apr 2020 at 01:06, Gail Badner  wrote:
> >
> > Hi Galder,
> >
> > Thanks for the response. I agree this would be a good change for H6. I'm
> happy to hear that this is on someone's radar for Infinispan.
> >
> > Sanne, regarding:
> > > 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.
> >
> > Are you suggesting that
> SharedSessionContractImplementor#getSessionIdentifier be deprecated?
> > If so, it has other uses, so it really can't be deprecated.
>
> Right, sorry let me clarify that. What I'm getting at is that clearly
> the current method is being used in different contexts; I see at leat
> two main ones:
>  - requiring a Serializable identifier (across the wire for
> clustering, and to be stored in databases) - typically an UUID.
>  - some form of "identity token" as we discussed on Zulip, which is
> far cheaper to generate but unsuitable for serialization.
>
> When we get to such realizations, I generally think it's a good idea
> to create more suitably named versions for *both* such use cases so to
> avoid the overloaded notion we currently have.
> Then, as second step deprecate (or remove) the original method.
>
> Since we're discussing a major release, I'd remove the existing
> method, or if you prefer the more conservative approach create JIRA
> for it to be removed just before Final if that's more convenient -
> for example keeping it a little longer could facilitate testing of the
> previews without already needing the custom 2LC release but
> I suspect it's not really worth it to be overzealous.
>
> Thanks,
> Sanne
>
>
> >
> > I've moved the fix version for HHH-13916 to 6.0.0.Beta1 and created a
> wip/6.0 PR: https://github.com/hibernate/hibernate-orm/pull/3341 .
> >
> > Regards,
> > Gail
> >
> >
> > On Wed, Apr 15, 2020 at 7:58 AM Galder Zamarreno 
> wrote:
> >>
> >> 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 
> wrote:
> >>>
> >>> On Wed, 15 Apr 2020 at 02:16, Gail Badner  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 
> 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 <
> sa...@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 
> 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
> >>> 

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

2020-04-17 Thread Sanne Grinovero
On Thu, 16 Apr 2020 at 01:06, Gail Badner  wrote:
>
> Hi Galder,
>
> Thanks for the response. I agree this would be a good change for H6. I'm 
> happy to hear that this is on someone's radar for Infinispan.
>
> Sanne, regarding:
> > 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.
>
> Are you suggesting that SharedSessionContractImplementor#getSessionIdentifier 
> be deprecated?
> If so, it has other uses, so it really can't be deprecated.

Right, sorry let me clarify that. What I'm getting at is that clearly
the current method is being used in different contexts; I see at leat
two main ones:
 - requiring a Serializable identifier (across the wire for
clustering, and to be stored in databases) - typically an UUID.
 - some form of "identity token" as we discussed on Zulip, which is
far cheaper to generate but unsuitable for serialization.

When we get to such realizations, I generally think it's a good idea
to create more suitably named versions for *both* such use cases so to
avoid the overloaded notion we currently have.
Then, as second step deprecate (or remove) the original method.

Since we're discussing a major release, I'd remove the existing
method, or if you prefer the more conservative approach create JIRA
for it to be removed just before Final if that's more convenient -
for example keeping it a little longer could facilitate testing of the
previews without already needing the custom 2LC release but
I suspect it's not really worth it to be overzealous.

Thanks,
Sanne


>
> I've moved the fix version for HHH-13916 to 6.0.0.Beta1 and created a wip/6.0 
> PR: https://github.com/hibernate/hibernate-orm/pull/3341 .
>
> Regards,
> Gail
>
>
> On Wed, Apr 15, 2020 at 7:58 AM Galder Zamarreno  wrote:
>>
>> 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  wrote:
>>>
>>> On Wed, 15 Apr 2020 at 02:16, Gail Badner  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  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  
>>> >> 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  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 

Re: [hibernate-dev] Remove or disable the WildFly feature packs of Hibernate ORM?

2020-04-17 Thread Sanne Grinovero
Hi Fabio,

yes good point. Infinispan has been using our feature packs as well, but
they are in a similar situation and will have to change strategy.

Even if others will need them, we don't have much choice: the existing
feature packs won't work with the newer WildFly versions. So we'll keep
going with our plan to delete these, and then if there's compelling need
for a "Galleon XP" pack to be delivered in the future... we'll seen when
the demands materialize.

Regarding older WildFly versions which we still support: obviously we're
not backporting the removal to any supported branch so older Wildfly
versions (and older Infinispan versions) will be able to continue using
them.

Thanks
Sanne

On Fri, 17 Apr 2020 at 05:55, Fabio Massimo Ercoli 
wrote:

> Hello,
>
> I take advantage of the topic to say that there is an `integrationtests`
> module in Infinispan too using feature packs, which are usings in turn the
> feature packs of the Hibernate Search 5.
> I have to ask the ISPN guys on Zulip about that, maybe they have already
> planned to remove such modules and maybe we can get rid of them with the
> Search 6 integration.
>
> Thanks,
> Fabio
>
> On Fri, Apr 17, 2020 at 12:26 AM Sanne Grinovero 
> wrote:
>
>> We have a PR for this now, if someone would like to have a look:
>>  - https://github.com/hibernate/hibernate-orm/pull/3347
>>
>> One thing to note, is that it also removes all Arquillian based tests:
>> we had some which were testing more than just the basics of our
>> feature pack; although most had been disabled already for other
>> reasons.  Sadly even the few useful ones will need to be removed as
>> well.
>>
>> After this, we no longer have Arquillian among the test dependencies
>> either... back to basics.
>>
>> Thanks,
>> Sanne
>>
>> On Thu, 16 Apr 2020 at 16:18, Yoann Rodiere  wrote:
>> >
>> > > Any specific reason you say that Yoann?
>> >
>> > No, I was just thinking that moving to Jarakarta JPA and Jakarta CDI
>> had a high chance of breaking OSGi tests, unless the Jakarta artifacts
>> properly support OSGi.
>> >
>> > But it was just a hunch; if it works, I don't have anything against
>> keeping it.
>> >
>> >
>> > Yoann Rodière
>> > Hibernate Team
>> > yo...@hibernate.org
>> >
>> >
>> > On Thu, 16 Apr 2020 at 16:12, Steve Ebersole 
>> wrote:
>> >>
>> >> I do not plan on dropping hibernate-osgi.  Its there; it works.  Any
>> >> specific reason you say that Yoann?
>> >>
>> >> Brett is the only one to do anything with that "tutorial".  Though as
>> time
>> >> went on its focus was more a way to find problems when the paxexam
>> tests
>> >> failed - they almost always give useless feedback.  If you think
>> people are
>> >> using it as a tutorial then I agree we should remove it or update it.
>> >> Depending whether it still has benefit for troubleshooting I'd actually
>> >> just clarify its intent somehow rather removing it.  Brett?
>> >>
>> >>
>> >>
>> >> On Thu, Apr 16, 2020 at 8:42 AM Sanne Grinovero 
>> wrote:
>> >>
>> >> > Agreed, I'll remove it.  Thanks Steve and Yoann!
>> >> >
>> >> > I'll remove them from 5.5 soon, you can then merge the removal in 6.0
>> >> > or do it differently if that's easier.
>> >> >
>> >> > This is the JIRA:
>> >> >  - https://hibernate.atlassian.net/browse/HHH-13952
>> >> >
>> >> > Regarding OSGi (which Yoann raised): I agree we shouldn't spend
>> >> > significant amounts of time on it, but as long as tests still work
>> and
>> >> > it doesn't get in the way I'll leave them be.
>> >> > I have a related JIRA for OSGi as well: the tutorial is using very
>> old
>> >> > (unmaintained?) dependency versions, so it will either need to be
>> >> > removed (people can always read the older tutorial), or it should be
>> >> > updated (and tested).
>> >> > I might give it a shot to update it, but I'm afraid it will need a
>> >> > full set of Jakarta EE 9 dependencies available as well.
>> >> >  - https://hibernate.atlassian.net/browse/HHH-13951
>> >> >
>> >> > Yoann, great idea about the platform agnostic testrunner. We should
>> >> > keep that in mind, hopefully we'll be able to make it happen
>> >> > eventually.
>> >> >
>> >> > Thanks,
>> >> > Sanne
>> >> >
>> >> >
>> >> > On Thu, 16 Apr 2020 at 13:46, Steve Ebersole 
>> wrote:
>> >> > >
>> >> > > Considering the changes in WildFly, I also think we should just
>> drop
>> >> > hibernate-orm-modules.
>> >> > >
>> >> > > For 6.0 I will definitely do this.  I also think removing it is
>> good for
>> >> > 5.5.
>> >> > >
>> >> > > Not sure it is even worth the hassle for earlier releases however.
>> >> > >
>> >> > >
>> >> > > On Wed, Apr 15, 2020 at 5:03 PM Sanne Grinovero <
>> sa...@hibernate.org>
>> >> > wrote:
>> >> > >>
>> >> > >> As we're working to upgrade to Jakarta EE 9, our feature packs for
>> >> > >> Wildfly are not going to be functional for a while at least.
>> >> > >>
>> >> > >> Not only do we have to upgrade to JPA 3, but we also need to
>> upgrade
>> >> > >> our integrations with a different Validator, a