Propagation.REQUIRES_NEW may be problematic due to locking. If we go down the path of having Propagation.SUPPORTS or even not declaring @Transactional at all on read only methods, it'll be even easier to support your use case Burke. You'll just start a transaction if you need it. It'll mean that reads will be non-transactional by default which should give us improved performance as well. There's no reason why simple reads should be transactional anyway.
Ben, I haven't tested it yet on our configuration of spring/hibernate, but I'll do it soon and let you know. -Rafal On 27 February 2012 23:07, Burke Mamlin <[email protected]> wrote: > Yeah. That's what I was hoping for – i.e., that we figure out the best > practice (at least based on what we know now) and promote/document it so > people don't have to figure this out on their own. > > -Burke > > > On Mon, Feb 27, 2012 at 4:53 PM, Darius Jazayeri > <[email protected]>wrote: > >> You can annotate a method with @Transactional(propagation=REQUIRES_NEW). >> I don't know whether this is supported on all databases, or whether there >> are any other pitfalls in practice. >> >> -Darius >> >> >> On Mon, Feb 27, 2012 at 8:45 AM, Burke Mamlin <[email protected]>wrote: >> >>> Can we devise a convention/recipe for cleanly creating a separate >>> writable transaction within a read-only transaction? If someone >>> wants/needs to create some side effects inside a read-only transaction, it >>> would be nice to have a common convention for how to implement such a side >>> effect (e.g., either appropriate annotations and/or a utility method). >>> >>> For example, a service provides a transactionless or read-only >>> transaction around a method and some process introduced by a module during >>> that method wants to write something to the database on the side. Is there >>> a way for the module to create it's own separate transaction to perform the >>> side effect without forcing the service method to create a transaction? >>> >>> -Burke >>> >>> >>> On Mon, Feb 27, 2012 at 9:59 AM, Ben Wolfe <[email protected]> wrote: >>> >>>> @Transactional(readOnly=true, propagation=Propagation.SUPPORTS) looks >>>> like the best solution. >>>> >>>> Dissecting that suggestion according to the article: >>>> >>>> *@Transactional* so that hibernate is happen and has its endpoints >>>> *readOnly=true* so that a database write doesn't occur >>>> *Propogation.SUPPORTS* so that the transaction is never really started >>>> unless asked for >>>> >>>> Did you confirm this still works with hibernate? (validating my first >>>> assumption above about @Transactional?) >>>> >>>> All read/write methods will need >>>> @Transaction(propogration=Propogration.REQUIRED) >>>> >>>> Ben >>>> >>>> >>>> On Sun, Feb 26, 2012 at 7:22 AM, Rafal Korytkowski >>>> <[email protected]>wrote: >>>> >>>>> I'd vote to put @Transactional(readOnly=true, >>>>> propagation=Propagation.SUPPORTS) at the class level of service layer >>>>> impls >>>>> and fine tune methods that aren't read only. >>>>> >>>>> Propagation.SUPPORTS allows to eliminate the transaction overhead that >>>>> Darius is hoping to achieve with the DAO example in a more general and >>>>> nicer way. More info here [0]. >>>>> >>>>> There's not much difference between having transactional DAOs and >>>>> Services. The latter is more widely used for the reason I gave, but it's a >>>>> matter of taste. >>>>> >>>>> -Rafal >>>>> >>>>> [0] - http://www.ibm.com/developerworks/java/library/j-ts1/index.html >>>>> >>>>> >>>>> On 24 February 2012 22:00, Darius Jazayeri <[email protected]>wrote: >>>>> >>>>>> Spring tries to use the existing transaction unless you say >>>>>> @Transactional(propagation=Propagation.REQUIRES_NEW). >>>>>> >>>>>> I would *love* to get rid of the @Transactional annotations on our >>>>>> services, and just put annotations on the methods that actually require >>>>>> them. >>>>>> >>>>>> -Darius >>>>>> >>>>>> >>>>>> On Fri, Feb 24, 2012 at 12:32 PM, Ben Wolfe <[email protected]> wrote: >>>>>> >>>>>>> FYI, here is the relevant ticket: >>>>>>> https://tickets.openmrs.org/browse/TRUNK-208 >>>>>>> >>>>>>> I could live with it on the DAO if we also examine all serviceimpls >>>>>>> for multiple calls to the DAO layer and add the annotation to the impl >>>>>>> if >>>>>>> it does more than one. >>>>>>> >>>>>>> Can we do away with the generic @Transactional on the class and >>>>>>> force devs to add @Transactional on methods that need it? This should >>>>>>> remove the need for @Transactional(readOnly=true) and limit the number >>>>>>> of >>>>>>> times its forgotten. Alternatively, we make the class level annotation >>>>>>> readOnly=true. >>>>>>> >>>>>>> Are we also sure that if a transaction is started at the service >>>>>>> layer that the DAO layer is using that same one instead of starting a >>>>>>> new >>>>>>> one? >>>>>>> >>>>>>> Ben >>>>>>> >>>>>>> >>>>>>> On Fri, Feb 24, 2012 at 3:13 PM, Darius Jazayeri < >>>>>>> [email protected]> wrote: >>>>>>> >>>>>>>> The reason I'd rather have them in the HibernateDAO is so we can >>>>>>>> write a service method like this, without paying the transactional >>>>>>>> penalty: >>>>>>>> >>>>>>>> public MySettings getSettings() { >>>>>>>> if (cached == null) { >>>>>>>> cached = dao.getSettings(); // this method has an >>>>>>>> @Transactional annotation >>>>>>>> } >>>>>>>> return cached; >>>>>>>> } >>>>>>>> >>>>>>>> Basically I'm saying that the transactional annotation should be >>>>>>>> put in the concrete DAO only where we actually hit a transactional >>>>>>>> datastore. >>>>>>>> >>>>>>>> If we're putting the annotations in the ServiceImpl I'd need to do >>>>>>>> something like this: >>>>>>>> >>>>>>>> public MySettings getSettings() { >>>>>>>> if (cached == null) { >>>>>>>> cached = getSettingsFromDao(); >>>>>>>> } >>>>>>>> return cached; >>>>>>>> } >>>>>>>> >>>>>>>> @Transactional(readOnly=true) >>>>>>>> private MySettings getSettingsFromDao() { >>>>>>>> return dao.getSettings(); >>>>>>>> } >>>>>>>> >>>>>>>> It's not the end of the world to have to do that, but is there any >>>>>>>> theoretical justification or best-practice recommendation that says we >>>>>>>> should put annotations in the service layer when we believe that we're >>>>>>>> calling a transactional DAO, rather than in the DAO layer when we know >>>>>>>> for >>>>>>>> sure we're hitting the datastore? >>>>>>>> >>>>>>>> -Darius >>>>>>>> >>>>>>>> PS- Definitely if a ServiceImpl or a Controller method is going to >>>>>>>> make multiple Service or DAO calls, it should explicitly mark the >>>>>>>> transaction itself. But that's independent. >>>>>>>> >>>>>>>> >>>>>>>> On Fri, Feb 24, 2012 at 10:08 AM, Ben Wolfe <[email protected]>wrote: >>>>>>>> >>>>>>>>> Yes, lets move these to the ServiceImpl. Do we have a ticket >>>>>>>>> somewhere to move the rest of our transaction annotations from >>>>>>>>> interface to >>>>>>>>> impl? >>>>>>>>> >>>>>>>>> Ben >>>>>>>>> >>>>>>>>> >>>>>>>>> On Fri, Feb 24, 2012 at 9:55 AM, Wyclif Luyima <[email protected] >>>>>>>>> > wrote: >>>>>>>>> >>>>>>>>>> @Rafal, i agree with what you are saying, so we should move them >>>>>>>>>> from HibernateVisitDao to VisitServiceImpl? >>>>>>>>>> >>>>>>>>>> Wyclif >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Fri, Feb 24, 2012 at 5:15 AM, Rafal Korytkowski < >>>>>>>>>> [email protected]> wrote: >>>>>>>>>> >>>>>>>>>>> I've found a way to have the advice around @Transactional so we >>>>>>>>>>> don't need to change anything in Visits. >>>>>>>>>>> >>>>>>>>>>> Anyway, I don't think we should move @Transactional to the DAO >>>>>>>>>>> layer. In general it's the service layer that defines units of work >>>>>>>>>>> that >>>>>>>>>>> should be transactional. If you have multiple calls to DAO in a >>>>>>>>>>> service >>>>>>>>>>> method, you will want to have @Transactional in the service method >>>>>>>>>>> anyway. >>>>>>>>>>> >>>>>>>>>>> -Rafal >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On 24 February 2012 03:11, Darius Jazayeri >>>>>>>>>>> <[email protected]>wrote: >>>>>>>>>>> >>>>>>>>>>>> Moving this to the dev list. >>>>>>>>>>>> >>>>>>>>>>>> Wyclif, I assume that you mean from HibernateVisitDAO to >>>>>>>>>>>> VisitServiceImpl... >>>>>>>>>>>> >>>>>>>>>>>> So, in the long run, we need to move the transactional >>>>>>>>>>>> annotations off of the service interface. I'd argue that they >>>>>>>>>>>> should go to >>>>>>>>>>>> the concrete DAO, and only on the methods that actually hit the >>>>>>>>>>>> database. >>>>>>>>>>>> The alternative would be to move them to the ServiceImpl. >>>>>>>>>>>> >>>>>>>>>>>> That said, I'm fine if we want to move them from >>>>>>>>>>>> HibernateVisitDAO up to the VisitService *interface* for >>>>>>>>>>>> consistency, so that we can refactor *all* our services later. >>>>>>>>>>>> But what's the argument for moving it from the HibernateDAO to the >>>>>>>>>>>> ServiceImpl? >>>>>>>>>>>> >>>>>>>>>>>> Ideally Rafal will quickly figure out how to move the recent >>>>>>>>>>>> advice code he wrote so it's triggered by any @Transactional method >>>>>>>>>>>> anywhere, and this will be moot. >>>>>>>>>>>> >>>>>>>>>>>> -Darius >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> On Thu, Feb 23, 2012 at 5:58 PM, Wyclif Luyima < >>>>>>>>>>>> [email protected]> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> Hi guys, >>>>>>>>>>>>> >>>>>>>>>>>>> @Daniel, we need to move the @Transanctional annotations from >>>>>>>>>>>>> HibernateVisitDao to ConceptServiceImpl before cutting the 1.9 RC >>>>>>>>>>>>> >>>>>>>>>>>>> Wyclif >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> ------------------------------ >>>>>>>>>>>> Click here to >>>>>>>>>>>> unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from >>>>>>>>>>>> OpenMRS Developers' mailing list >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> ------------------------------ >>>>>>>>>>> Click here to >>>>>>>>>>> unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from >>>>>>>>>>> OpenMRS Developers' mailing list >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> ------------------------------ >>>>>>>>>> Click here to >>>>>>>>>> unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from >>>>>>>>>> OpenMRS Developers' mailing list >>>>>>>>>> >>>>>>>>> >>>>>>>>> ------------------------------ >>>>>>>>> Click here to >>>>>>>>> unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from >>>>>>>>> OpenMRS Developers' mailing list >>>>>>>>> >>>>>>>> >>>>>>>> ------------------------------ >>>>>>>> Click here to >>>>>>>> unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from >>>>>>>> OpenMRS Developers' mailing list >>>>>>>> >>>>>>> >>>>>>> ------------------------------ >>>>>>> Click here to >>>>>>> unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from >>>>>>> OpenMRS Developers' mailing list >>>>>>> >>>>>> >>>>>> ------------------------------ >>>>>> Click here to >>>>>> unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from >>>>>> OpenMRS Developers' mailing list >>>>>> >>>>> >>>>> ------------------------------ >>>>> Click here to >>>>> unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from >>>>> OpenMRS Developers' mailing list >>>>> >>>> >>>> ------------------------------ >>>> Click here to >>>> unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from >>>> OpenMRS Developers' mailing list >>>> >>> >>> ------------------------------ >>> Click here to >>> unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from >>> OpenMRS Developers' mailing list >>> >> >> ------------------------------ >> Click here to >> unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from >> OpenMRS Developers' mailing list >> > > ------------------------------ > Click here to > unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from > OpenMRS Developers' mailing list > _________________________________________ To unsubscribe from OpenMRS Developers' mailing list, send an e-mail to [email protected] with "SIGNOFF openmrs-devel-l" in the body (not the subject) of your e-mail. [mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l]
