Rafał,
I have a launchbar <http://www.obdev.at/products/launchbar/index.html> script for your name (out of respect to you, of course). ;-) I'd rather have an explicit, well-explained, and reliable API that doesn't work the way we would like (yet) instead of an API trying to be something that it isn't and, therefore, behave unexpectedly & unpredictabely. For example, "Write code that saves changes, but assume changes to objects retrieved from the API *may* be persisted before you call save(); this will be fixed in the next major revision of the API." Cheers, -Burke On Fri, Mar 9, 2012 at 2:09 PM, Rafal Korytkowski <[email protected]> wrote: > Ben, I don't see a problem in clearing the session in our > Context.closeSession(). It'll work in Mark's case, but it won't fix > all the problems. If Hibernates chooses to flush the session before > you call Context.clearSesssion() and Context.closeSession(), the > changes will hit the db and when you call Context.openSession() and > try to retrieve the data you'll see them as if they were persisted > anyway as long as the transaction wasn't rolled back. > > Burke, where do you find "ł" on your keyboard ;-) Hibernate chooses to > flush at very specific points. That is whenever you try to get > something from the db and it's not available in the session (cache) or > it's in the session, but marked as modified. It tries not to be eager > with flushes and does it only in cases when you access the same table > as you modified, but as reported here you cannot rely on that: > https://hibernate.onjira.com/browse/HHH-2399 > > In TRUNK-3069 I made flushes happen whenever you called save..., > void..., etc. and only then. At least it's when you would normally > expect them to happen. The drawback is still that all objects are > flushed not only the one you intend. > > I don't like it anymore for the reason that it won't work with the MDS > module that needs exclusive control over the Hibernate flushes and it > won't be easy to make it work with that approach, unless I'll open a > back gate to disable Context.flushSession() ;-) > > I also agree with Mark that we shouldn't be fighting with > Hibernate/JPA most of the time, but accept its pros and cons and maybe > consider detaching objects from Hibernate in API 2.0. > > -Rafal > > 2012/3/9 Mark Goodrich <[email protected]>: > > It just seems like the way our API is written and is used, the > assumption is that changes made to a domain object are only persisted if a > saveObject() service method is called. But by default Hibernate persists > changes to any attached objects automatically, and we are struggling > without complete success to stop "premature" flushes, which make the prior > assumption false. I haven't read the API 2.0 notes, but my suggestion > would be in 2.0 we give up on that assumption instead of trying to enforce > it. > > > > Mark > > ________________________________________ > > From: [email protected] [[email protected]] On Behalf Of Burke Mamlin [ > [email protected]] > > Sent: Friday, March 09, 2012 12:54 PM > > To: [email protected] > > Subject: Re: [OPENMRS-DEV] Hibernate flush mode > > > > Rafał, > > > > If calls the API are made within the "// don't call a save patient here" > excerpt in Mark's example, isn't it likely that the change will be > persisted if Hibernate chooses to flush at any point? > > > > It seems that both save() methods and Context.closeSession() should > perform a flush – i.e., ensure Hibernate has a chance to save any unsaved > changes. Then we can at least provide a guarantee that a save() method or > closeSession() will avoid persisting changes. > > > > -Burke > > > > On Fri, Mar 9, 2012 at 12:23 PM, Rafal Korytkowski <[email protected] > <mailto:[email protected]>> wrote: > > The problem in Mark's test is that when you call > > Context.closeSession() within a transaction (our unit tests are > > transactional by default) it only dettaches the session from the > > transaction manager and doesn't close or even clear the session. It's > > stored and attached again when you call Context.openSession(). If you > > call Context.clearSession() before Context.closeSession() then it'll > > work as you expect. > > > > Roger, Ben explained the problem with dettaching objects from > > Hibernate. Our domain objects are interconnected and lazily > > initialized and we would have to initialize them prior to dettaching > > from Hibernate. Sometimes a graph of objects that needs to be > > initialized is quite vast and it could kill performance. It would > > require some redesigning of our API and our business model classes, > > which is quite a big change. > > > > -Rafal > > > > On 9 March 2012 17:50, Friedman, Roger (CDC/CGH/DGHA) (CTR) > > <[email protected]<mailto:[email protected]>> wrote: > >> Rafa -- > >> Thanks for the spadework. > >> ConceptDAO contains a wealth of not so great ideas. I think > working on REST has made us more aware of subclasses and helper classes and > how to represent them. > >> Is it an option to have only the DAO layer deal with > Hibernate-connected objects, to have it serve disconnected objects on read > and reconnect them on write? Wouldn't that mean our services and the API > would no longer be engaged with the session and its cache? > >> Saludos, Roger > >> > >> -----Original Message----- > >> From: [email protected]<mailto:[email protected]> [mailto:[email protected] > <mailto:[email protected]>] On Behalf Of Rafal Korytkowski > >> Sent: Friday, March 09, 2012 9:54 AM > >> To: [email protected]<mailto: > [email protected]> > >> Subject: [OPENMRS-DEV] Hibernate flush mode > >> > >> Hey, > >> > >> I have been experimenting with Hibernate flush modes recently. The > motivation was that we experienced premature flushes triggered by Hibernate > while retrieving data from the DB, which made us temporarily switch from > FlushMode.AUTO to FlushMode.COMMIT or MANUAL to execute some parts of code > like validation. > >> > >> The initial attempt for a more general solution was TRUNK-3069, which > switched from the AUTO to COMMIT mode for all transactions. The flush was > triggered by us around any method annotated with > @Transactional(readOnly=false), but not around > @Transactional(readOnly=true). It seemed like a good approach at first, but > then I discovered TRUNK-3103. The problem could be eventually resolved by > annotating with @Transactional dao methods, which we are considering. > >> > >> Anyway TRUNK-3069 disables much of Hibernate functionality to handle > flushes for us and now I think it is not how we should approach the problem. > >> > >> I believe we need to stay with the AUTO flush mode and tune it only > when it is needed. Unfortunately, we cannot change the flush mode in any > other place than the DAO layer where we have access to hibernate's session, > whereas most of the time we actually need to control it in the service > layer. > >> > >> So far whenever we needed to execute a service method in the manual > flush mode our approach was to go down to the DAO layer, which resulted in > such strange constructions as in getDefaultConceptMapType [0], where we put > in the DAO layer code that really belonged to the service layer. > >> > >> We have a few possibilities to deal with that. > >> > >> 1. We continue to handle the flush issue in DAOs the way it was before. > >> 2. We have something like CustomSessionFlushTask [1]. > >> 3. We have Context.getFlushMode() and Context.setFlushMode(flushMode). > >> We need our own FlushMode enum so that we don't introduce a dependency > on Hibernate in the service layer. > >> 4. We have @ManualFlush annotation to annotate service methods that we > want to explicitly execute in the manual flush mode. It's a more elegant > variation of 2., but slightly less useful since it requires to create a > dedicated method. For instance we have the getConcept method and if we want > it to be executed in one place only in the manual flush mode we need to > create a second method getConceptInManualFlush for the purpose of > annotating it with @ManualFlush. > >> > >> I am really curious what do you think or if there is anyone who has > more experience with that. > >> > >> [0] - > https://source.openmrs.org/browse/~br=1.9.x/OpenMRS/branches/1.9.x/api/src/main/java/org/openmrs/api/db/hibernate/HibernateConceptDAO.java?r=26243 > >> [1] - > https://source.openmrs.org/browse/~br=trunk/Modules/metadatasharing/trunk/src/org/openmrs/module/metadatasharing/api/db/hibernate/CustomSessionFlushTask.java?r=26268 > >> > >> -Rafal > >> > > > _________________________________________ 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]
