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 > _________________________________________ 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]
