Re: Savepoint
[email protected] Fri, 07 Aug 2009 12:28:55 +1000
| Newsgroups | gmane.comp.java.orm.simpleorm |
|---|---|
| Message-ID | <[email protected]> |
Hello Franck, OK, I now see what you mean about clone() and serializable. But I do not see why clone is inherently less inefficient than what you have done. The only extra objects that need to be SRecrordInstances themselves. There is much more work in copying the array lists, so the overhead is negligible. Certainly a properly implemented clone approach makes much fewer changes to our data structures. And I am pretty sure that save points and clone should not both be supported. But what if the SRecordInstances have business logic. You set a save point, updates some fields (and thus business rules), and then do a rollback. What happens? Should rolling back to a save point fire business rules? (Imagine, for example, a UI driven by onValidateRecord, say.) Hard questions. My current feeling is that we should implement clone efficiently (without serialization) and then leave it to the user. Maybe can only clone if not attached. This adds minimal complexity to the main line code, and gives you what you want. The problem is that Java's clone is a mess. (If we have a clone() method, should we implement Clonable?) But it does allow the user to extend clone() for SRecordInstance and thus control the process. I have not worked with clone before. But I think that if SDataSet.clone and more importantly SRecordInstance.clone call super.clone, then you will have a shallow copy. You then just need to explicitly clone the mutable objects, such as Arrays. Try not to clone immutable objects -- String does not implement Clone. Maybe just never clone the values within the values array at all. A user might want to implement Employee.clone, say, but not for your case. If we take the clone approach, then all references to Savepoints are removed from the API. It is just a technique to clone before attaching and running. All changes to the data structures also go. As a third alternative maybe not optimize clone at all. Just stick to the serialization approach. Have we done any measurements to see if it is actually inefficient? One problem with introducing clones is that if the user was doing clever things with business rules then they would have to deal with the clone case separately from the serialize case. Really bad Java design that the two are not unified in any way. But that is the way it is. (The main inefficiency of Clone would be re-creation of the String objects whose references could be otherwise shared.) Can you see any problem with the clone approach? It seems cleaner to me. Regards, Anthony At 07:07 PM 5/08/2009, you wrote: > > >Le mercredi 05 août 2009 à 14:05 +1000, <mailto:berglas%40SpreadsheetDetective.com>[email protected] >a écrit : > >> >> Flush failing is not the same as a rollback. > >Right, but most of the time, when flush fails, I will rollback, as I >don't have enougth information to recover from the situation. >> But I do see your point. Continuing on would be very hard, some dirty >> bits cleared, others not etc. >Yep. >> >> One of the common cases would be a broken optimistic lock. > >Yes, this is a typical case. But also any broken constraint or >SValidation failing. >When the user submits some data, there are many reasons why the data >might not fit into the database (although, of course, we make our best >to assist the user and help him/her avoid submitting stupid things). > >> I suppose we could purge the broken record and re read it, hopefully >> keeping other records intact. >True in the case of optimistic lock, but the broken record might also be >a new record, in which case we cannot re-read it... >> >> In removeRecord >> if ( ! hasSavepoint()) rinst.destroy(); >> is a bit dubious. destroy() really takes on two meanings. > >Previous code was 'rinst.destroy()'. In my try to keep a possibility >restore the dataSet, I don't destroy the record, as a reference is kept >to it in spRecords and spDirtyRecords. >So removeRecord means 'remove it from the dataSet in its current state, >and cleanup if possible, but cleanup is not the main goal of >removeRecord(), it is the goal of SDataSet.destroy()'. > >> And what do we do about references to removed Record? >> >Well, anyway SRecordInstance.destroy() does not do anything about >references. So I think my modification has no impact on this matter... > >But the point remain valid, what happens to references to a >removedRecord? They probably should be set to null, which is not done >now. Maybe the first thing to do would be create a testcase... >I'll try to do this. > >> Would it be better to just clone the SRecordInstances and indeed the >> whole SDataSet? Certainly fewer changes to the data structures. More >> flexible, a little less controllable. What was your reasoning? >> >This is was is done in SDataSetclone(). I think it works, but is costly. >My goal was the reduce the amount of data the is copied, ie only make a >copy of the data in SDataSet, and a copy of the data in _dirty_ records >only. There are situation where the number of dirty records in a dataSet >is much lower than the total number of records... > >> And can we think of an elegant way to handle broken optimistic locks? >> (Often it will just need to be thrown back to the user.) >> >Before sorm3 and datSets, all these problems were easier to handle: >simply detach the records, send them back to UI, re-attach them later. >But the point is we had no guarantee of thing being coherent!! The >SDataSet introduces a level of coherence, where all records live >together, or die together... Hence the problem I try to solve... > >> (I'm pretty sure that all this goes beyond Hibernate when sessions are >> saved.) >> >No idea what Hibernate does... I've not looked at it for years, since I >know Simpleorm :) . I know I should :( > >> Anthony >> >> At 05:18 AM 5/08/2009, you wrote: >> > >> > >> >Hello Anthony, >> > >> >it is indeed something we need... >> > >> >The use case is the following: >> > >> >1) the user creates a dataSet and some records in the UI layer. >> > >> >2) the dataSet is sent to the data access layer, where a session is >> >created and the dataSet is bound to it. >> > >> >3) the flush fails for whatever reason, throwing an exception. >> Session >> >rollbacks. Thanks to savepoint, the dataset is NOT destroyed, it is >> >restored in the state it was when it came from the UI layer. >> > >> >4) error are mapped to the fields, and the dataSet is sent back to >> the >> >user to allow corrections to be made >> > >> >Right now (without savepoint), the dataSet is destroyed, and the user >> >input is lost, with a sorry error message... >> > >> >Well, the truth is we could hold the user input in another data >> >structure in the UI, but our application is built around dataSet. So >> the >> >UI is build from the dataSet, either when reading from the database, >> or >> >when creating data... >> > >> >We avoid the "sorry your input is lost" scenario by testing as much >> >failure case as we can before sending the dataSet to the data access >> >layer: we manually test the foreign keys for each record, we manually >> >test the unique constraints, etc... this is costly and not very >> >efficient. And sometimes it fails. >> > >> >So having the ability to "rollback" a dataSet instead of destroying >> it >> >will really change our life... >> > >> >Whether my code does the job remains to be seen :) >> > >> >Does it make sense to you ? >> > >> >Regards, >> >Franck >> > >> >Le mardi 04 août 2009 à 20:04 +1000, <mailto:berglas% >> 40SpreadsheetDetective.com><mailto:berglas%40SpreadsheetDetective.com>[email protected] a >> >écrit : >> >> >> >> Hello Franck, >> >> >> >> Yipes. Is this something that you really needed? >> >> >> >> I'm guessing as a way to recover from broken optimistic locking. >> But >> >> that would not work -- there would be no way to correct the >> situation. >> >> >> >> This sort of thing can lead to many issues, and needs thought. What >> is >> >> your use case? >> >> >> >> We should probably take care that if a flush() fails, then >> unflushed >> >> records remain in a state that they can be flushed again. I have >> not >> >> looked, but should probably be the case already. >> >> >> >> It should cost the same as clone(), as that it what it does. And >> maybe >> >> clone would be more useful? >> >> >> >> I see you have a test case, have not had a chance to look at it >> >> carefully. >> >> >> >> But what is your use case? >> >> >> >> Let us put Map to bed with a test case, and then have a review of >> >> aggregate, before adding too many more features. >> >> >> >> Regards, >> >> >> >> Anthony >> >> >> >> At 02:23 AM 3/08/2009, you wrote: >> >> > >> >> > >> >> >Hi Anthony, >> >> > >> >> >I couldn't help but commit a new feature whose intent is to allow >> to >> >> >recover a dataset after a rollback, without the cost of the >> clone() >> >> >solution. >> >> > >> >> >If not used, the feature has no impact on the way simpleorm works, >> so >> >> I >> >> >thought it was quite harmful... >> >> > >> >> >I called it savepoint, and the usage is: >> >> > >> >> >ses.begin(ds.savepoint()); >> >> > >> >> >then, if 'ds.hasSavepoint()', you can: >> >> > >> >> >ses.rollbackAndDetachDataset(); >> >> > >> >> >The tradeoff is that ds.savepoint() is quite costly, especially >> for >> >> big >> >> >dataSets... (but should be less costly than clone()). >> >> > >> >> >Of course, comments are welcome ! >> >> > >> >> >Franck >> >> > >> >> > >> >> >> >> Spreadsheet Detective, >> >> Southern Cross Software Queensland Pty Limited >> >> 54 Gerler Street >> >> Bardon, Queensland 4065, Australia. >> >> >> >> Email: <mailto:berglas% >> 40spreadsheetdetective.com><mailto:berglas%40spreadsheetdetective.com>[email protected] >> >> www.SpreadsheetDetective.com >> >> Ph: +61 427 830248 (Australian Eastern Standard Time) >> >> >> >> "If the model seems correct only because the numbers look right, >> >> then why build the model in the first place?" >> >> >> >> >> >> >> >> >> > >> > >> >> Spreadsheet Detective, >> Southern Cross Software Queensland Pty Limited >> 54 Gerler Street >> Bardon, Queensland 4065, Australia. >> >> Email: <mailto:berglas%40spreadsheetdetective.com>[email protected] >> www.SpreadsheetDetective.com >> Ph: +61 427 830248 (Australian Eastern Standard Time) >> >> "If the model seems correct only because the numbers look right, >> then why build the model in the first place?" >> >> >> >> >> > > Dr Anthony Berglas, [email protected] Mobile: +61 4 4838 8874 Just because it is possible to push twigs along the ground with ones nose does not necessarily mean that is the best way to collect firewood. ------------------------------------ Yahoo! Groups Links <*> To visit your group on the web, go to: http://groups.yahoo.com/group/SimpleORM/ <*> Your email settings: Individual Email | Traditional <*> To change settings online go to: http://groups.yahoo.com/group/SimpleORM/join (Yahoo! ID required) <*> To change settings via email: mailto:[email protected] mailto:[email protected] <*> To unsubscribe from this group, send an email to: [email protected] <*> Your use of Yahoo! Groups is subject to: http://docs.yahoo.com/info/terms/