Looks good with some minor comments: I think close() should do a commit on any uncommited data by default... unless the session has already been rolled back.
And close() should probably also return the last committed version for logging/diagnostic purposes. So if the session rolled back everything before calling close, it just return the last committed version of the store. On Sunday, October 27, 2013 11:12:30 PM UTC-7, Thomas Mueller wrote: > > Hi, > > What about: > > - commit(): also stores the changed (except for in-memory stores) > - rollbackTo(): keep > - store(), incrementVersion(), getCommittedVersion(): remove > - close(): just close the file without storing (same as closeImmediately > is doing now) > - closeImmediately(): remove > - hasUnsavedChanges(): rename to hasUncommittedChanges() > > That means for persistent stores, commit and store is the same. > > In addition to that, I think I will add an autoCommit feature, probably > even enabled by default, because it looks like it's quite hard for an > application to decide when to best call commit. But in this case, close() > should also store the uncommitted changes. > > What do you think? > > Regards, > Thomas > > > > On Sunday, October 27, 2013, Ashwin Jayaprakash wrote: > >> I suppose you just need commit and commitAndClose. Otherwise you just end >> up answering questions about various combinations on the mailing list :) >> >> >> On Saturday, October 26, 2013 12:11:50 AM UTC-7, Thomas Mueller wrote: >>> >>> Hi, >>> >>> > I have 1 suggestion/doubt - why is incrementVersion() even exposed? >>> Is it meant to be a checkpoint for uncommitted transactions? It's confusing. >>> >>> Yes, it is a checkpoint for uncommitted transactions. But you are right, >>> it is confusing, and it is not strictly needed, as one could use commit and >>> then rollback to the previous version if needed. >>> >>> I think I will remove this feature, and probably also remove or change >>> some related features that are questionable. The whole concept of >>> "uncommitted but saved changes" for example, I'm not quite sure it that >>> still makes sense. Also, I wonder if MVStore.close() should store anything, >>> or whether it should be replaced with closeImmediately(). If those features >>> are needed by an application, it would still be possible to implement them >>> on top of the MVStore. As far as I see, I don't need them for the database >>> engine itself. >>> >>> Regards, >>> Thomas >>> >>> >>> On Fri, Oct 25, 2013 at 7:08 PM, Ashwin Jayaprakash < >>> [email protected]> wrote: >>> >>> I have 1 suggestion/doubt - why is incrementVersion() even exposed? Is >>> it meant to be a checkpoint for uncommitted transactions? It's confusing. >>> >>> Why would I not use commit() instead to bump up the version? Because it >>> has to flush the store which might be slower than just increment? >>> >>> Can multiple threads access store concurrently? Can multiple threads >>> access diff versions of a map concurrently? >>> >>> Thanks. >>> >>> >>> >>> >>> On Friday, October 25, 2013 4:51:19 AM UTC-7, Thomas Mueller wrote: >>> >>> Hi, >>> >>> Thanks a lot for reporting! This is a very good test case. I can >>> reproduce the problem now, I will try to fix it in the next days. The >>> problem isn't really the number of entries in the map, the problem is that >>> the background thread stores the transient (uncommitted) changes to avoid >>> out of memory, and closing will try to revert that (and this is what fails >>> for some reason). >>> >>> Regards, >>> Thomas >>> >>> >>> On Fri, Oct 25, 2013 at 7:33 AM, Ashwin Jayaprakash < >>> [email protected]> wrote: >>> >>> I was trying to run this simple MVStore test, essentially trying to see >>> if the multiple read-only versions worked. >>> >>> I noticed that if the commit() statements are not called after an >>> incrementVersion(), then close() throws an unknown version exception. See >>> highlighted sections below in the code. What am I doing wrong? >>> >>> *Exception: >>> * >>> Exception in thread "main" java.lang.**IllegalArgumentExcep**tion: >>> Unknown version 2 [1.3.174/0] >>> at org.h2.mvstore.DataUtils.**newIl**legalArgumentException(** >>> DataUti**ls.java:672) >>> at org.h2.mvstore.DataUtils.**check**Argument(DataUtils.java:**659) >>> at org.h2.mvstore.MVStore.**rollbac**kTo(MVStore.java:1749) >>> at org.h2.mvstore.MVStore.close(**M**VStore.java:678) >>> at test.MvTest1.main(MvTest1.**java**:54) >>> >>> *Code: >>> * >>> public class MvTest1 { >>> public static void main(String[] args) { >>> MVStore s = new MVStore.Builder(). >>> backgroundExceptionHandler( >>> new UncaughtExceptionHandler() { >>> @Override >>> public void uncaughtException(Thread thread, >>> Throwable throwable) { >>> **** throwable.printStackTrace(); >>> } >>> }). >>> cacheSize(10). >>> compressData(). >>> encryptionKey("007".**toCharArra**y()). >>> fileStore(new OffHeapStore()). >>> pageSplitSize(6 * 1024). >>> readOnly(). >>> writeBufferSize(8). >>> writeDelay(100). >>> open(); >>> >>> >>> >>> -- >> You received this message because you are subscribed to the Google Groups >> "H2 Database" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to [email protected]. >> To post to this group, send email to [email protected]. >> Visit this group at http://groups.google.com/group/h2-database. >> For more options, visit https://groups.google.com/groups/opt_out. >> > -- You received this message because you are subscribed to the Google Groups "H2 Database" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/h2-database. For more options, visit https://groups.google.com/groups/opt_out.
