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(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev

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 <jonathan.halliday(a)redhat.com&gt; 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/TransactionSynchro... > > 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(a)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

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.halliday(a)redhat.com&gt; 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/TransactionSynchro... > > 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(a)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

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.

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 <steve(a)hibernate.org <mailto:steve@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 <steve(a)hibernate.org <mailto:steve@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.

The SynchronizationRegistry is kept per-LogicalConncection which might span multiple physical transactions. On Wed, Oct 25, 2017 at 10:37 AM Jonathan Halliday <jonathan.halliday(a)redhat.com <mailto:jonathan.halliday@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 <steve(a)hibernate.org <mailto:steve@hibernate.org> > <mailto:steve@hibernate.org <mailto:steve@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 <steve(a)hibernate.org <mailto:steve@hibernate.org> >     <mailto:steve@hibernate.org <mailto:steve@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") O'Neill, Eric Shander

On 25 October 2017 at 17:13, Jonathan Halliday <jonathan.halliday(a)redhat.com&gt; 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 >> <jonathan.halliday(a)redhat.com <mailto:jonathan.halliday@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 >> <steve(a)hibernate.org <mailto:steve@hibernate.org> >> > <mailto:steve@hibernate.org <mailto:steve@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 >> <steve(a)hibernate.org <mailto:steve@hibernate.org> >> > <mailto:steve@hibernate.org <mailto:steve@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") O'Neill, Eric Shander >> > -- > Registered in England and Wales under Company Registration No. 03798903 > Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander

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(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev