[hibernate-dev] SynchronizationRegistry: expose read (lookup) operations?

2017-10-25 Thread Sanne Grinovero 
Hi Steve,

do you think it would be sensible for me to explore introducing some
kind of synchronization lookup method on
org.hibernate.resource.transaction.spi.SynchronizationRegistry ?

Today it only exposes a `registerSynchronization` method, which we use
extensively, but then we also have quite some complexity in the Search
code caused by the fact that we can't look the synchronizations up in
a later phase.
Essentially our Synchronization is stateful and we need to update it later.

I'd love to propose a change for ORM6 so allow registering such things
under some kind of id (a string?) so that one can look them back.

current SPI:

public void registerSynchronization(Synchronization synchronization)

temptative proposal (didn't try it yet..):

   public void registerSynchronization(String id, Synchronization
synchronization);

   public void Synchronization getSynchronization(String id);


does it sound reasonable in principle?

This would imply other users should make up an id unique for their use
case. Alternatively I could live with a Class used as an id, or we
could have the new methods in addition to the existing method for
people not interested in looking things up.

thanks,
Sanne
___
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev

Re: [hibernate-dev] SynchronizationRegistry: expose read (lookup) operations?

2017-10-25 Thread Jonathan Halliday 

The javax.transaction version of that interface already functions as a 
per-transaction hashmap, with put/get operations available. If you can 
use it directly, then just pick a suitable lookup key - FQCN or even the 
sync impl .class itself, since the key is Object type. If not then at 
least retain the method signatures and just delegate them down through 
the spi.

https://docs.oracle.com/javaee/5/api/javax/transaction/TransactionSynchronizationRegistry.html

Jonathan.

On 25/10/17 14:32, Sanne Grinovero wrote:
> Hi Steve,
> 
> do you think it would be sensible for me to explore introducing some
> kind of synchronization lookup method on
> org.hibernate.resource.transaction.spi.SynchronizationRegistry ?
> 
> Today it only exposes a `registerSynchronization` method, which we use
> extensively, but then we also have quite some complexity in the Search
> code caused by the fact that we can't look the synchronizations up in
> a later phase.
> Essentially our Synchronization is stateful and we need to update it later.
> 
> I'd love to propose a change for ORM6 so allow registering such things
> under some kind of id (a string?) so that one can look them back.
> 
> current SPI:
> 
>  public void registerSynchronization(Synchronization synchronization)
> 
> temptative proposal (didn't try it yet..):
> 
> public void registerSynchronization(String id, Synchronization
> synchronization);
> 
> public void Synchronization getSynchronization(String id);
> 
> 
> does it sound reasonable in principle?
> 
> This would imply other users should make up an id unique for their use
> case. Alternatively I could live with a Class used as an id, or we
> could have the new methods in addition to the existing method for
> people not interested in looking things up.
> 
> thanks,
> Sanne
> ___
> hibernate-dev mailing list
> hibernate-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hibernate-dev
> 

-- 
Registered in England and Wales under Company Registration No. 03798903 
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
___
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev

Re: [hibernate-dev] SynchronizationRegistry: expose read (lookup) operations?

2017-10-25 Thread Radim Vansa 
I had the same issue in 2LC and we ended up passing 
CacheTransactionContext to all SPI calls (for ORM6). In case of 
Infinispan impl this will be a stateful object, probably implementing 
the Synchronization iface as well (to reduce object count), and 2LC will 
store all the beforeCompletion/afterCompletion work in there.

Currently we can workaround by registering multiple synchronizations but 
as the RPCs can be done asynchronously doing it in parallel will reduce 
the latency.

The only drawback is that JTA's suspend and resume cannot be honored 
properly but I've been told that the rest of ORM is not working 
perfectly either when you try to run two concurrent transactions using 
single Session.

Radim

On 10/25/2017 03:32 PM, Sanne Grinovero wrote:
> Hi Steve,
>
> do you think it would be sensible for me to explore introducing some
> kind of synchronization lookup method on
> org.hibernate.resource.transaction.spi.SynchronizationRegistry ?
>
> Today it only exposes a `registerSynchronization` method, which we use
> extensively, but then we also have quite some complexity in the Search
> code caused by the fact that we can't look the synchronizations up in
> a later phase.
> Essentially our Synchronization is stateful and we need to update it later.
>
> I'd love to propose a change for ORM6 so allow registering such things
> under some kind of id (a string?) so that one can look them back.
>
> current SPI:
>
>  public void registerSynchronization(Synchronization synchronization)
>
> temptative proposal (didn't try it yet..):
>
> public void registerSynchronization(String id, Synchronization
> synchronization);
>
> public void Synchronization getSynchronization(String id);
>
>
> does it sound reasonable in principle?
>
> This would imply other users should make up an id unique for their use
> case. Alternatively I could live with a Class used as an id, or we
> could have the new methods in addition to the existing method for
> people not interested in looking things up.
>
> thanks,
> Sanne
> ___
> hibernate-dev mailing list
> hibernate-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hibernate-dev


-- 
Radim Vansa 
JBoss Performance Team

___
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev

Re: [hibernate-dev] SynchronizationRegistry: expose read (lookup) operations?

2017-10-25 Thread Sanne Grinovero 
Hi Jonathan!

So I have multiple options, but fundamentally I have an ORM
SynchronizationRegistry today. We could either follow the example of
the javax.transaction API in evolving the ORM SPI, or apparently I
could explore making our Synchronization stateless and store the state
in this other map instead, or maybe I try refactor it all to stick to
the standard APIs - however I wonder if it will still work for a JDBC
transaction.

Either way I'll take the fact that the standard API exposes such a
functionality as a sign that this could be sensible to expose.

Thanks,
Sanne

On 25 October 2017 at 15:37, Jonathan Halliday
 wrote:
>
> The javax.transaction version of that interface already functions as a
> per-transaction hashmap, with put/get operations available. If you can use
> it directly, then just pick a suitable lookup key - FQCN or even the sync
> impl .class itself, since the key is Object type. If not then at least
> retain the method signatures and just delegate them down through the spi.
>
> https://docs.oracle.com/javaee/5/api/javax/transaction/TransactionSynchronizationRegistry.html
>
> Jonathan.
>
>
> On 25/10/17 14:32, Sanne Grinovero wrote:
>>
>> Hi Steve,
>>
>> do you think it would be sensible for me to explore introducing some
>> kind of synchronization lookup method on
>> org.hibernate.resource.transaction.spi.SynchronizationRegistry ?
>>
>> Today it only exposes a `registerSynchronization` method, which we use
>> extensively, but then we also have quite some complexity in the Search
>> code caused by the fact that we can't look the synchronizations up in
>> a later phase.
>> Essentially our Synchronization is stateful and we need to update it
>> later.
>>
>> I'd love to propose a change for ORM6 so allow registering such things
>> under some kind of id (a string?) so that one can look them back.
>>
>> current SPI:
>>
>>  public void registerSynchronization(Synchronization synchronization)
>>
>> temptative proposal (didn't try it yet..):
>>
>> public void registerSynchronization(String id, Synchronization
>> synchronization);
>>
>> public void Synchronization getSynchronization(String id);
>>
>>
>> does it sound reasonable in principle?
>>
>> This would imply other users should make up an id unique for their use
>> case. Alternatively I could live with a Class used as an id, or we
>> could have the new methods in addition to the existing method for
>> people not interested in looking things up.
>>
>> thanks,
>> Sanne
>> ___
>> hibernate-dev mailing list
>> hibernate-dev@lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>>
>
> --
> Registered in England and Wales under Company Registration No. 03798903
> Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
___
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev

Re: [hibernate-dev] SynchronizationRegistry: expose read (lookup) operations?

2017-10-25 Thread Steve Ebersole 
Yes, but that is only available in JTA environments.  Ours is available no
matter what kind of transaction environment we are executing in.

Sanne, how about instead a separate registration method accepting a
"registration key"?  E.g.:

public interface SynchronizationRegistry extends Serializable {
// ~~~
// current api
void registerSynchronization(Synchronization synchronization);

// ~~~
// additional api
void registerSynchronization(Object key, Synchronization
synchronization);
Synchronization findSynchronization(Object key);
}

or:

public interface KeyableSynchronization extends Synchronization {
Object getKey();
}

which can be used during #registerSynchronization(Synchronization)
processing to add to a Map as well?



On Wed, Oct 25, 2017 at 9:37 AM Jonathan Halliday <
jonathan.halli...@redhat.com> wrote:

>
> The javax.transaction version of that interface already functions as a
> per-transaction hashmap, with put/get operations available. If you can
> use it directly, then just pick a suitable lookup key - FQCN or even the
> sync impl .class itself, since the key is Object type. If not then at
> least retain the method signatures and just delegate them down through
> the spi.
>
>
> https://docs.oracle.com/javaee/5/api/javax/transaction/TransactionSynchronizationRegistry.html
>
> Jonathan.
>
> On 25/10/17 14:32, Sanne Grinovero wrote:
> > Hi Steve,
> >
> > do you think it would be sensible for me to explore introducing some
> > kind of synchronization lookup method on
> > org.hibernate.resource.transaction.spi.SynchronizationRegistry ?
> >
> > Today it only exposes a `registerSynchronization` method, which we use
> > extensively, but then we also have quite some complexity in the Search
> > code caused by the fact that we can't look the synchronizations up in
> > a later phase.
> > Essentially our Synchronization is stateful and we need to update it
> later.
> >
> > I'd love to propose a change for ORM6 so allow registering such things
> > under some kind of id (a string?) so that one can look them back.
> >
> > current SPI:
> >
> >  public void registerSynchronization(Synchronization synchronization)
> >
> > temptative proposal (didn't try it yet..):
> >
> > public void registerSynchronization(String id, Synchronization
> > synchronization);
> >
> > public void Synchronization getSynchronization(String id);
> >
> >
> > does it sound reasonable in principle?
> >
> > This would imply other users should make up an id unique for their use
> > case. Alternatively I could live with a Class used as an id, or we
> > could have the new methods in addition to the existing method for
> > people not interested in looking things up.
> >
> > thanks,
> > Sanne
> > ___
> > hibernate-dev mailing list
> > hibernate-dev@lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/hibernate-dev
> >
>
> --
> Registered in England and Wales under Company Registration No. 03798903
> Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
>
___
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev

Re: [hibernate-dev] SynchronizationRegistry: expose read (lookup) operations?

2017-10-25 Thread Sanne Grinovero 
On 25 October 2017 at 16:05, Steve Ebersole  wrote:
> Yes, but that is only available in JTA environments.  Ours is available no
> matter what kind of transaction environment we are executing in.
>
> Sanne, how about instead a separate registration method accepting a
> "registration key"?  E.g.:
>
> public interface SynchronizationRegistry extends Serializable {
> // ~~~
> // current api
> void registerSynchronization(Synchronization synchronization);
>
> // ~~~
> // additional api
> void registerSynchronization(Object key, Synchronization
> synchronization);
> Synchronization findSynchronization(Object key);
> }
>
> or:
>
> public interface KeyableSynchronization extends Synchronization {
> Object getKey();
> }
>
> which can be used during #registerSynchronization(Synchronization)
> processing to add to a Map as well?

Yes that would work for me, but thinking about the implementation it
implies you'd need to hold on to both a Set and a Map, and then we'd
be exposed to silly usage like people adding the same synchronization
twice in two different ways?

I'd rather expose a single consistent way: having to make up an id
doesn't seem too inconvenient considering it's an SPI.

Thanks,
Sanne



>
>
> On Wed, Oct 25, 2017 at 9:37 AM Jonathan Halliday
>  wrote:
>>
>>
>> The javax.transaction version of that interface already functions as a
>> per-transaction hashmap, with put/get operations available. If you can
>> use it directly, then just pick a suitable lookup key - FQCN or even the
>> sync impl .class itself, since the key is Object type. If not then at
>> least retain the method signatures and just delegate them down through
>> the spi.
>>
>>
>> https://docs.oracle.com/javaee/5/api/javax/transaction/TransactionSynchronizationRegistry.html
>>
>> Jonathan.
>>
>> On 25/10/17 14:32, Sanne Grinovero wrote:
>> > Hi Steve,
>> >
>> > do you think it would be sensible for me to explore introducing some
>> > kind of synchronization lookup method on
>> > org.hibernate.resource.transaction.spi.SynchronizationRegistry ?
>> >
>> > Today it only exposes a `registerSynchronization` method, which we use
>> > extensively, but then we also have quite some complexity in the Search
>> > code caused by the fact that we can't look the synchronizations up in
>> > a later phase.
>> > Essentially our Synchronization is stateful and we need to update it
>> > later.
>> >
>> > I'd love to propose a change for ORM6 so allow registering such things
>> > under some kind of id (a string?) so that one can look them back.
>> >
>> > current SPI:
>> >
>> >  public void registerSynchronization(Synchronization
>> > synchronization)
>> >
>> > temptative proposal (didn't try it yet..):
>> >
>> > public void registerSynchronization(String id, Synchronization
>> > synchronization);
>> >
>> > public void Synchronization getSynchronization(String id);
>> >
>> >
>> > does it sound reasonable in principle?
>> >
>> > This would imply other users should make up an id unique for their use
>> > case. Alternatively I could live with a Class used as an id, or we
>> > could have the new methods in addition to the existing method for
>> > people not interested in looking things up.
>> >
>> > thanks,
>> > Sanne
>> > ___
>> > hibernate-dev mailing list
>> > hibernate-dev@lists.jboss.org
>> > https://lists.jboss.org/mailman/listinfo/hibernate-dev
>> >
>>
>> --
>> Registered in England and Wales under Company Registration No. 03798903
>> Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
___
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev

Re: [hibernate-dev] SynchronizationRegistry: expose read (lookup) operations?

2017-10-25 Thread Steve Ebersole 
> Yes that would work for me, but thinking about the implementation it
> implies you'd need to hold on to both a Set and a Map, and then we'd
> be exposed to silly usage like people adding the same synchronization
> twice in two different ways?
>

Does it?  Nothing in the SPI requires us to store things in any specific
way.  E.g. we can keep just a Map - when we are passed
a KeyableSynchronization we'd use that key, when we are passed a
non-KeyableSynchronization Synchronization we'd generate one ourselves.

And we cant stop people from every conceivable "silly usage".  At some
point we are professional developers and should be able to do the non-silly
things ;)

And as far as your "register the thing twice" worry... rhetorically, what
stops them from calling:

reg.register( "abc", MySync.INSTANCE )
reg.register( "123", MySync.INSTANCE )

Nothing.


I'd rather expose a single consistent way: having to make up an id
> doesn't seem too inconvenient considering it's an SPI.
>

Well, again, I don't see how KeyableSynchronization is a "inconsistent"
approach.  In fact out of the 2, it is my preferance.
___
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev

Re: [hibernate-dev] SynchronizationRegistry: expose read (lookup) operations?

2017-10-25 Thread Jonathan Halliday 

sure, though I was thinking in terms of storing the stateful sync 
directly into the registry, not separating out the state first. Your api 
proposal made that storage a side effect of the registration call, but 
with the javax.transaction api it's an additional call, roughly

tsr.registerSync(statefulThing);
tsr.put(org.hibernate.specialkey, statefulThing);

less elegant, but more flexible in that you can stick things other than 
Synchronizations in there too if you ever need to.

Direct use in a JDBC transaction would need a dummy 
javax.transaction.TransactionSynchronizationRegistry API impl I guess, 
which does seem rather silly if you already got an spi to avoid that, 
but OTOH would not require interface changes.

Ultimately you need a tx context to tie it back to though, since the 
underlying map is per-tx. IIRC in the narayana impl the TSR actually 
delegates to the tx object, which is where the map storage resides. 
Adding the TSR to the JTA spec api was kind of a kludge to avoid 
interfaces changes in the days before defender methods (or whatever they 
would up being called). Most of the TSR methods logically belong to 
Transaction or TransactionManager. If you're keeping map put/get 
semantics decoupled from registerSync semantics, then there is no 
requirement for them to belong on the SynchronizationRegistry rather 
than some other api.

Jonathan.

On 25/10/17 16:02, Sanne Grinovero wrote:
> Hi Jonathan!
> 
> So I have multiple options, but fundamentally I have an ORM
> SynchronizationRegistry today. We could either follow the example of
> the javax.transaction API in evolving the ORM SPI, or apparently I
> could explore making our Synchronization stateless and store the state
> in this other map instead, or maybe I try refactor it all to stick to
> the standard APIs - however I wonder if it will still work for a JDBC
> transaction.
> 
> Either way I'll take the fact that the standard API exposes such a
> functionality as a sign that this could be sensible to expose.
> 
> Thanks,
> Sanne
> 
> On 25 October 2017 at 15:37, Jonathan Halliday
>  wrote:
>>
>> The javax.transaction version of that interface already functions as a
>> per-transaction hashmap, with put/get operations available. If you can use
>> it directly, then just pick a suitable lookup key - FQCN or even the sync
>> impl .class itself, since the key is Object type. If not then at least
>> retain the method signatures and just delegate them down through the spi.
>>
>> https://docs.oracle.com/javaee/5/api/javax/transaction/TransactionSynchronizationRegistry.html
>>
>> Jonathan.
>>
>>
>> On 25/10/17 14:32, Sanne Grinovero wrote:
>>>
>>> Hi Steve,
>>>
>>> do you think it would be sensible for me to explore introducing some
>>> kind of synchronization lookup method on
>>> org.hibernate.resource.transaction.spi.SynchronizationRegistry ?
>>>
>>> Today it only exposes a `registerSynchronization` method, which we use
>>> extensively, but then we also have quite some complexity in the Search
>>> code caused by the fact that we can't look the synchronizations up in
>>> a later phase.
>>> Essentially our Synchronization is stateful and we need to update it
>>> later.
>>>
>>> I'd love to propose a change for ORM6 so allow registering such things
>>> under some kind of id (a string?) so that one can look them back.
>>>
>>> current SPI:
>>>
>>>   public void registerSynchronization(Synchronization synchronization)
>>>
>>> temptative proposal (didn't try it yet..):
>>>
>>>  public void registerSynchronization(String id, Synchronization
>>> synchronization);
>>>
>>>  public void Synchronization getSynchronization(String id);
>>>
>>>
>>> does it sound reasonable in principle?
>>>
>>> This would imply other users should make up an id unique for their use
>>> case. Alternatively I could live with a Class used as an id, or we
>>> could have the new methods in addition to the existing method for
>>> people not interested in looking things up.
>>>
>>> thanks,
>>> Sanne
>>> ___
>>> hibernate-dev mailing list
>>> hibernate-dev@lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>>>
>>
>> --
>> Registered in England and Wales under Company Registration No. 03798903
>> Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
> 

-- 
Registered in England and Wales under Company Registration No. 03798903 
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
___
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev

Re: [hibernate-dev] SynchronizationRegistry: expose read (lookup) operations?

2017-10-25 Thread Steve Ebersole 
Jonathan, we aren't going to be exposing this or using this
via TransactionSynchronizationRegistry.  Your comment about a "dummy" in
the JDBC txn case is exactly why.  We already have such an abstraction :
SynchronizationRegistry

On Wed, Oct 25, 2017 at 10:22 AM Steve Ebersole  wrote:

> Yes that would work for me, but thinking about the implementation it
>> implies you'd need to hold on to both a Set and a Map, and then we'd
>> be exposed to silly usage like people adding the same synchronization
>> twice in two different ways?
>>
>
> Does it?  Nothing in the SPI requires us to store things in any specific
> way.  E.g. we can keep just a Map - when we are passed
> a KeyableSynchronization we'd use that key, when we are passed a
> non-KeyableSynchronization Synchronization we'd generate one ourselves.
>
> And we cant stop people from every conceivable "silly usage".  At some
> point we are professional developers and should be able to do the non-silly
> things ;)
>
> And as far as your "register the thing twice" worry... rhetorically, what
> stops them from calling:
>
> reg.register( "abc", MySync.INSTANCE )
> reg.register( "123", MySync.INSTANCE )
>
> Nothing.
>
>
> I'd rather expose a single consistent way: having to make up an id
>> doesn't seem too inconvenient considering it's an SPI.
>>
>
> Well, again, I don't see how KeyableSynchronization is a "inconsistent"
> approach.  In fact out of the 2, it is my preferance.
>
>
___
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev

Re: [hibernate-dev] SynchronizationRegistry: expose read (lookup) operations?

2017-10-25 Thread Steve Ebersole 
Also, unless I am mistaken `TransactionSynchronizationRegistry#put` works
on the principle that the "current transaction" is associated with the
current thread.  I absolutely want to stay away from that as an assumption
here.  It simply does not hold true in the JDBC txn case.

On Wed, Oct 25, 2017 at 10:24 AM Steve Ebersole  wrote:

> Jonathan, we aren't going to be exposing this or using this
> via TransactionSynchronizationRegistry.  Your comment about a "dummy" in
> the JDBC txn case is exactly why.  We already have such an abstraction :
> SynchronizationRegistry
>
> On Wed, Oct 25, 2017 at 10:22 AM Steve Ebersole 
> wrote:
>
>> Yes that would work for me, but thinking about the implementation it
>>> implies you'd need to hold on to both a Set and a Map, and then we'd
>>> be exposed to silly usage like people adding the same synchronization
>>> twice in two different ways?
>>>
>>
>> Does it?  Nothing in the SPI requires us to store things in any specific
>> way.  E.g. we can keep just a Map - when we are passed
>> a KeyableSynchronization we'd use that key, when we are passed a
>> non-KeyableSynchronization Synchronization we'd generate one ourselves.
>>
>> And we cant stop people from every conceivable "silly usage".  At some
>> point we are professional developers and should be able to do the non-silly
>> things ;)
>>
>> And as far as your "register the thing twice" worry... rhetorically, what
>> stops them from calling:
>>
>> reg.register( "abc", MySync.INSTANCE )
>> reg.register( "123", MySync.INSTANCE )
>>
>> Nothing.
>>
>>
>> I'd rather expose a single consistent way: having to make up an id
>>> doesn't seem too inconvenient considering it's an SPI.
>>>
>>
>> Well, again, I don't see how KeyableSynchronization is a "inconsistent"
>> approach.  In fact out of the 2, it is my preferance.
>>
>>
___
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev

Re: [hibernate-dev] SynchronizationRegistry: expose read (lookup) operations?

2017-10-25 Thread Jonathan Halliday 

right, a lot of JTA works on the 'tx bound to thread' approach and it's 
a right pain in async I/O thread pooled environments like vert.x  That's 
one of the reasons why sticking a get/put api on the Transaction 
instance abstraction instead may make more sense, though I'm guessing 
your SynchronizationRegistry is instance per tx rather than a singleton 
like the JTA one is, so it may not make much difference which object 
carries the functionality for your case?

Jonathan.

On 25/10/17 16:25, Steve Ebersole wrote:
> Also, unless I am mistaken `TransactionSynchronizationRegistry#put` 
> works on the principle that the "current transaction" is associated with 
> the current thread.  I absolutely want to stay away from that as an 
> assumption here.  It simply does not hold true in the JDBC txn case.
> 
> On Wed, Oct 25, 2017 at 10:24 AM Steve Ebersole  > wrote:
> 
> Jonathan, we aren't going to be exposing this or using this
> via TransactionSynchronizationRegistry.  Your comment about a
> "dummy" in the JDBC txn case is exactly why.  We already have such
> an abstraction : SynchronizationRegistry
> 
> On Wed, Oct 25, 2017 at 10:22 AM Steve Ebersole  > wrote:
> 
> Yes that would work for me, but thinking about the
> implementation it
> implies you'd need to hold on to both a Set and a Map, and
> then we'd
> be exposed to silly usage like people adding the same
> synchronization
> twice in two different ways?
> 
> 
> Does it?  Nothing in the SPI requires us to store things in any
> specific way.  E.g. we can keep just a Map - when we are passed
> a KeyableSynchronization we'd use that key, when we are passed a
> non-KeyableSynchronization Synchronization we'd generate one
> ourselves.
> 
> And we cant stop people from every conceivable "silly usage". 
> At some point we are professional developers and should be able
> to do the non-silly things ;)
> 
> And as far as your "register the thing twice" worry...
> rhetorically, what stops them from calling:
> 
> reg.register( "abc", MySync.INSTANCE )
> reg.register( "123", MySync.INSTANCE )
> 
> Nothing.
> 
> 
> I'd rather expose a single consistent way: having to make up
> an id
> doesn't seem too inconvenient considering it's an SPI.
> 
> 
> Well, again, I don't see how KeyableSynchronization is a
> "inconsistent" approach.  In fact out of the 2, it is my preferance.
> 

-- 
Registered in England and Wales under Company Registration No. 03798903 
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
___
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev

Re: [hibernate-dev] SynchronizationRegistry: expose read (lookup) operations?

2017-10-25 Thread Steve Ebersole 
The SynchronizationRegistry is kept per-LogicalConncection which might span
multiple physical transactions.

On Wed, Oct 25, 2017 at 10:37 AM Jonathan Halliday <
jonathan.halli...@redhat.com> wrote:

>
> right, a lot of JTA works on the 'tx bound to thread' approach and it's
> a right pain in async I/O thread pooled environments like vert.x  That's
> one of the reasons why sticking a get/put api on the Transaction
> instance abstraction instead may make more sense, though I'm guessing
> your SynchronizationRegistry is instance per tx rather than a singleton
> like the JTA one is, so it may not make much difference which object
> carries the functionality for your case?
>
> Jonathan.
>
> On 25/10/17 16:25, Steve Ebersole wrote:
> > Also, unless I am mistaken `TransactionSynchronizationRegistry#put`
> > works on the principle that the "current transaction" is associated with
> > the current thread.  I absolutely want to stay away from that as an
> > assumption here.  It simply does not hold true in the JDBC txn case.
> >
> > On Wed, Oct 25, 2017 at 10:24 AM Steve Ebersole  > > wrote:
> >
> > Jonathan, we aren't going to be exposing this or using this
> > via TransactionSynchronizationRegistry.  Your comment about a
> > "dummy" in the JDBC txn case is exactly why.  We already have such
> > an abstraction : SynchronizationRegistry
> >
> > On Wed, Oct 25, 2017 at 10:22 AM Steve Ebersole  > > wrote:
> >
> > Yes that would work for me, but thinking about the
> > implementation it
> > implies you'd need to hold on to both a Set and a Map, and
> > then we'd
> > be exposed to silly usage like people adding the same
> > synchronization
> > twice in two different ways?
> >
> >
> > Does it?  Nothing in the SPI requires us to store things in any
> > specific way.  E.g. we can keep just a Map - when we are passed
> > a KeyableSynchronization we'd use that key, when we are passed a
> > non-KeyableSynchronization Synchronization we'd generate one
> > ourselves.
> >
> > And we cant stop people from every conceivable "silly usage".
> > At some point we are professional developers and should be able
> > to do the non-silly things ;)
> >
> > And as far as your "register the thing twice" worry...
> > rhetorically, what stops them from calling:
> >
> > reg.register( "abc", MySync.INSTANCE )
> > reg.register( "123", MySync.INSTANCE )
> >
> > Nothing.
> >
> >
> > I'd rather expose a single consistent way: having to make up
> > an id
> > doesn't seem too inconvenient considering it's an SPI.
> >
> >
> > Well, again, I don't see how KeyableSynchronization is a
> > "inconsistent" approach.  In fact out of the 2, it is my
> preferance.
> >
>
> --
> Registered in England and Wales under Company Registration No. 03798903
> Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
>
___
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev

Re: [hibernate-dev] SynchronizationRegistry: expose read (lookup) operations?

2017-10-25 Thread Jonathan Halliday 

so you have some form of implicit cleanup that de-registers 
Synchronizations when the transaction finishes, or do they stay 
registered and re-fire for each tx on the connection?

As to the multiple registration semantics, IIRC the arjuna code allows 
the same Synchronization to be registered multiple times and in such 
case it will be called multiple times too. No reason you need to follow 
that model unless you're delegating to the tx engine though. If you go 
for 'we always generate the key for you', then calling

String id = sr.register(sync)

twice could just give you back the same id. Downside is you need to 
store that id someplace, which misses the point of allowing a fixed 
constant as the key. I'm rather assuming that's the requirement here, 
since if you're needing to keep and pass around a uniq key then you may 
as well just pass around the sync object itself? Anyhow, is a key of any 
form necessary for that use case? Could retrieval just be

sr.getSynchronizationsOfType(thing.class)

to match on class/interface, which should be all that's needed to find 
e.g. the cache/search/whatever synchronization.  Not having a key means 
saves having to define how it behaves. Lookup maybe slower if you're 
going to support polymorphism though, but the number of Synchronizations 
per tx should be small.

Jonathan.

On 25/10/17 16:39, Steve Ebersole wrote:
> The SynchronizationRegistry is kept per-LogicalConncection which might 
> span multiple physical transactions.
> 
> On Wed, Oct 25, 2017 at 10:37 AM Jonathan Halliday 
> mailto:jonathan.halli...@redhat.com>> wrote:
> 
> 
> right, a lot of JTA works on the 'tx bound to thread' approach and it's
> a right pain in async I/O thread pooled environments like vert.x  That's
> one of the reasons why sticking a get/put api on the Transaction
> instance abstraction instead may make more sense, though I'm guessing
> your SynchronizationRegistry is instance per tx rather than a singleton
> like the JTA one is, so it may not make much difference which object
> carries the functionality for your case?
> 
> Jonathan.
> 
> On 25/10/17 16:25, Steve Ebersole wrote:
>  > Also, unless I am mistaken `TransactionSynchronizationRegistry#put`
>  > works on the principle that the "current transaction" is
> associated with
>  > the current thread.  I absolutely want to stay away from that as an
>  > assumption here.  It simply does not hold true in the JDBC txn case.
>  >
>  > On Wed, Oct 25, 2017 at 10:24 AM Steve Ebersole
> mailto:st...@hibernate.org>
>  > >> wrote:
>  >
>  >     Jonathan, we aren't going to be exposing this or using this
>  >     via TransactionSynchronizationRegistry.  Your comment about a
>  >     "dummy" in the JDBC txn case is exactly why.  We already have
> such
>  >     an abstraction : SynchronizationRegistry
>  >
>  >     On Wed, Oct 25, 2017 at 10:22 AM Steve Ebersole
> mailto:st...@hibernate.org>
>  >     >> wrote:
>  >
>  >             Yes that would work for me, but thinking about the
>  >             implementation it
>  >             implies you'd need to hold on to both a Set and a
> Map, and
>  >             then we'd
>  >             be exposed to silly usage like people adding the same
>  >             synchronization
>  >             twice in two different ways?
>  >
>  >
>  >         Does it?  Nothing in the SPI requires us to store things
> in any
>  >         specific way.  E.g. we can keep just a Map - when we are
> passed
>  >         a KeyableSynchronization we'd use that key, when we are
> passed a
>  >         non-KeyableSynchronization Synchronization we'd generate one
>  >         ourselves.
>  >
>  >         And we cant stop people from every conceivable "silly usage".
>  >         At some point we are professional developers and should
> be able
>  >         to do the non-silly things ;)
>  >
>  >         And as far as your "register the thing twice" worry...
>  >         rhetorically, what stops them from calling:
>  >
>  >         reg.register( "abc", MySync.INSTANCE )
>  >         reg.register( "123", MySync.INSTANCE )
>  >
>  >         Nothing.
>  >
>  >
>  >             I'd rather expose a single consistent way: having to
> make up
>  >             an id
>  >             doesn't seem too inconvenient considering it's an SPI.
>  >
>  >
>  >         Well, again, I don't see how KeyableSynchronization is a
>  >         "inconsistent" approach.  In fact out of the 2, it is my
> preferance.
>  >
> 
> --
> Registered in England and Wales under Company Registration No. 03798903
> Directors: Michael Cunningham, Michael ("Mike

Re: [hibernate-dev] SynchronizationRegistry: expose read (lookup) operations?

2017-10-25 Thread Sanne Grinovero 
On 25 October 2017 at 16:22, Steve Ebersole  wrote:
>
>> Yes that would work for me, but thinking about the implementation it
>> implies you'd need to hold on to both a Set and a Map, and then we'd
>> be exposed to silly usage like people adding the same synchronization
>> twice in two different ways?
>
>
> Does it?  Nothing in the SPI requires us to store things in any specific
> way.  E.g. we can keep just a Map - when we are passed a
> KeyableSynchronization we'd use that key, when we are passed a
> non-KeyableSynchronization Synchronization we'd generate one ourselves.

Sure it's possible but it introduces complexity; considering this is
an SPI I'd assume we can just enforce a different contract and keep it
simple.

>
> And we cant stop people from every conceivable "silly usage".  At some point
> we are professional developers and should be able to do the non-silly things
> ;)
>
> And as far as your "register the thing twice" worry... rhetorically, what
> stops them from calling:
>
> reg.register( "abc", MySync.INSTANCE )
> reg.register( "123", MySync.INSTANCE )
>
> Nothing.

No doubt, but also if I write such code it's self-explanatory what I get.

Some less trivial example gets me in undefined places:

reg.register( "abc", MySync.INSTANCE )
Synchronization instance = reg.get("abc");
reg.register(instance);

quiz:
 - will my Syncrhonization instance get all notifications twice?

Just thinking that it would be better to avoid undefined expectations.

Are you aiming at backwards compatibility?
In that case I'm happy to explore non-breaking alternatives, I was
merely assuming that SPIs would be wildfly different anyway so no big
deal in breaking one more.

Thanks,
Sanne


>
>
>> I'd rather expose a single consistent way: having to make up an id
>> doesn't seem too inconvenient considering it's an SPI.
>
>
> Well, again, I don't see how KeyableSynchronization is a "inconsistent"
> approach.  In fact out of the 2, it is my preferance.
>
___
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev

Re: [hibernate-dev] SynchronizationRegistry: expose read (lookup) operations?

2017-10-25 Thread Sanne Grinovero 
On 25 October 2017 at 17:13, Jonathan Halliday
 wrote:
>
> so you have some form of implicit cleanup that de-registers Synchronizations
> when the transaction finishes, or do they stay registered and re-fire for
> each tx on the connection?
>
> As to the multiple registration semantics, IIRC the arjuna code allows the
> same Synchronization to be registered multiple times and in such case it
> will be called multiple times too. No reason you need to follow that model
> unless you're delegating to the tx engine though. If you go for 'we always
> generate the key for you', then calling
>
> String id = sr.register(sync)
>
> twice could just give you back the same id. Downside is you need to store
> that id someplace, which misses the point of allowing a fixed constant as
> the key. I'm rather assuming that's the requirement here, since if you're
> needing to keep and pass around a uniq key then you may as well just pass
> around the sync object itself?

I agree, that's what I'd want to avoid - at least in the Hibernate Search case.

> Anyhow, is a key of any form necessary for
> that use case? Could retrieval just be
>
> sr.getSynchronizationsOfType(thing.class)
>
> to match on class/interface, which should be all that's needed to find e.g.
> the cache/search/whatever synchronization.  Not having a key means saves
> having to define how it behaves. Lookup maybe slower if you're going to
> support polymorphism though, but the number of Synchronizations per tx
> should be small.

That would work for me, and I wouldn't need polymorphism. Exact class
will do just fine.

So this approach might allow to keep the registration API as-is, but
if we allow to register
multiple instances of the same class the retrieval would return a
random instance.

That's not a problem for my use case and could be addressed with javadoc,
probably a nice compromise.
Incidentally maps keyed by Class are very efficient so that's nice too.

Would it suffice for you too Radim? Since noone else ever asked for
such a feature I think the two of us represent the users population :)

Thanks,
Sanne


>
> Jonathan.
>
> On 25/10/17 16:39, Steve Ebersole wrote:
>>
>> The SynchronizationRegistry is kept per-LogicalConncection which might
>> span multiple physical transactions.
>>
>> On Wed, Oct 25, 2017 at 10:37 AM Jonathan Halliday
>> mailto:jonathan.halli...@redhat.com>> wrote:
>>
>>
>> right, a lot of JTA works on the 'tx bound to thread' approach and
>> it's
>> a right pain in async I/O thread pooled environments like vert.x
>> That's
>> one of the reasons why sticking a get/put api on the Transaction
>> instance abstraction instead may make more sense, though I'm guessing
>> your SynchronizationRegistry is instance per tx rather than a
>> singleton
>> like the JTA one is, so it may not make much difference which object
>> carries the functionality for your case?
>>
>> Jonathan.
>>
>> On 25/10/17 16:25, Steve Ebersole wrote:
>>  > Also, unless I am mistaken `TransactionSynchronizationRegistry#put`
>>  > works on the principle that the "current transaction" is
>> associated with
>>  > the current thread.  I absolutely want to stay away from that as an
>>  > assumption here.  It simply does not hold true in the JDBC txn
>> case.
>>  >
>>  > On Wed, Oct 25, 2017 at 10:24 AM Steve Ebersole
>> mailto:st...@hibernate.org>
>>  > >> wrote:
>>  >
>>  > Jonathan, we aren't going to be exposing this or using this
>>  > via TransactionSynchronizationRegistry.  Your comment about a
>>  > "dummy" in the JDBC txn case is exactly why.  We already have
>> such
>>  > an abstraction : SynchronizationRegistry
>>  >
>>  > On Wed, Oct 25, 2017 at 10:22 AM Steve Ebersole
>> mailto:st...@hibernate.org>
>>  > >>
>> wrote:
>>  >
>>  > Yes that would work for me, but thinking about the
>>  > implementation it
>>  > implies you'd need to hold on to both a Set and a
>> Map, and
>>  > then we'd
>>  > be exposed to silly usage like people adding the same
>>  > synchronization
>>  > twice in two different ways?
>>  >
>>  >
>>  > Does it?  Nothing in the SPI requires us to store things
>> in any
>>  > specific way.  E.g. we can keep just a Map - when we are
>> passed
>>  > a KeyableSynchronization we'd use that key, when we are
>> passed a
>>  > non-KeyableSynchronization Synchronization we'd generate
>> one
>>  > ourselves.
>>  >
>>  > And we cant stop people from every conceivable "silly
>> usage".
>>  > At some point we are professional developers and should
>> be able
>>  > to do the non-sil

Re: [hibernate-dev] SynchronizationRegistry: expose read (lookup) operations?

2017-10-26 Thread Radim Vansa 
On 10/26/2017 01:23 AM, Sanne Grinovero wrote:
> On 25 October 2017 at 17:13, Jonathan Halliday
>  wrote:
>> so you have some form of implicit cleanup that de-registers Synchronizations
>> when the transaction finishes, or do they stay registered and re-fire for
>> each tx on the connection?
>>
>> As to the multiple registration semantics, IIRC the arjuna code allows the
>> same Synchronization to be registered multiple times and in such case it
>> will be called multiple times too. No reason you need to follow that model
>> unless you're delegating to the tx engine though. If you go for 'we always
>> generate the key for you', then calling
>>
>> String id = sr.register(sync)
>>
>> twice could just give you back the same id. Downside is you need to store
>> that id someplace, which misses the point of allowing a fixed constant as
>> the key. I'm rather assuming that's the requirement here, since if you're
>> needing to keep and pass around a uniq key then you may as well just pass
>> around the sync object itself?
> I agree, that's what I'd want to avoid - at least in the Hibernate Search 
> case.
>
>> Anyhow, is a key of any form necessary for
>> that use case? Could retrieval just be
>>
>> sr.getSynchronizationsOfType(thing.class)
>>
>> to match on class/interface, which should be all that's needed to find e.g.
>> the cache/search/whatever synchronization.  Not having a key means saves
>> having to define how it behaves. Lookup maybe slower if you're going to
>> support polymorphism though, but the number of Synchronizations per tx
>> should be small.
> That would work for me, and I wouldn't need polymorphism. Exact class
> will do just fine.
>
> So this approach might allow to keep the registration API as-is, but
> if we allow to register
> multiple instances of the same class the retrieval would return a
> random instance.
>
> That's not a problem for my use case and could be addressed with javadoc,
> probably a nice compromise.
> Incidentally maps keyed by Class are very efficient so that's nice too.
>
> Would it suffice for you too Radim? Since noone else ever asked for
> such a feature I think the two of us represent the users population :)

My case was already solved by the CacheTransactionContext, and it seems 
more convenient than doing any lookups. But for general use I find 
by-class lookup more appealing than registering with a custom key. The 
registrar can always use some private class; it's not like unrelated 
components would do lookup of others' synchronizations.

Radim

>
> Thanks,
> Sanne
>
>
>> Jonathan.
>>
>> On 25/10/17 16:39, Steve Ebersole wrote:
>>> The SynchronizationRegistry is kept per-LogicalConncection which might
>>> span multiple physical transactions.
>>>
>>> On Wed, Oct 25, 2017 at 10:37 AM Jonathan Halliday
>>> mailto:jonathan.halli...@redhat.com>> wrote:
>>>
>>>
>>>  right, a lot of JTA works on the 'tx bound to thread' approach and
>>> it's
>>>  a right pain in async I/O thread pooled environments like vert.x
>>> That's
>>>  one of the reasons why sticking a get/put api on the Transaction
>>>  instance abstraction instead may make more sense, though I'm guessing
>>>  your SynchronizationRegistry is instance per tx rather than a
>>> singleton
>>>  like the JTA one is, so it may not make much difference which object
>>>  carries the functionality for your case?
>>>
>>>  Jonathan.
>>>
>>>  On 25/10/17 16:25, Steve Ebersole wrote:
>>>   > Also, unless I am mistaken `TransactionSynchronizationRegistry#put`
>>>   > works on the principle that the "current transaction" is
>>>  associated with
>>>   > the current thread.  I absolutely want to stay away from that as an
>>>   > assumption here.  It simply does not hold true in the JDBC txn
>>> case.
>>>   >
>>>   > On Wed, Oct 25, 2017 at 10:24 AM Steve Ebersole
>>>  mailto:st...@hibernate.org>
>>>   > >> wrote:
>>>   >
>>>   > Jonathan, we aren't going to be exposing this or using this
>>>   > via TransactionSynchronizationRegistry.  Your comment about a
>>>   > "dummy" in the JDBC txn case is exactly why.  We already have
>>>  such
>>>   > an abstraction : SynchronizationRegistry
>>>   >
>>>   > On Wed, Oct 25, 2017 at 10:22 AM Steve Ebersole
>>>  mailto:st...@hibernate.org>
>>>   > >>
>>> wrote:
>>>   >
>>>   > Yes that would work for me, but thinking about the
>>>   > implementation it
>>>   > implies you'd need to hold on to both a Set and a
>>>  Map, and
>>>   > then we'd
>>>   > be exposed to silly usage like people adding the same
>>>   > synchronization
>>>   > twice in two different ways?
>>>   >
>>>   >
>>>   > Does it?  Nothing in the S