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 <
>>> ashwin.ja...@gmail.com> 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 <
>>> ashwin.ja...@gmail.com> 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 h2-database+unsubscr...@googlegroups.com.
>> To post to this group, send email to h2-database@googlegroups.com.
>> 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 h2-database+unsubscr...@googlegroups.com.
To post to this group, send email to h2-database@googlegroups.com.
Visit this group at http://groups.google.com/group/h2-database.
For more options, visit https://groups.google.com/groups/opt_out.
