MVStore incrementVersion and commit throws "IllegalArgumentException: Unknown version"

103 views
Skip to first unread message

Ashwin Jayaprakash

unread,
Oct 25, 2013, 1:33:43 AM10/25/13
to h2-da...@googlegroups.com
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.IllegalArgumentException: Unknown version 2 [1.3.174/0]
    at org.h2.mvstore.DataUtils.newIllegalArgumentException(DataUtils.java:672)
    at org.h2.mvstore.DataUtils.checkArgument(DataUtils.java:659)
    at org.h2.mvstore.MVStore.rollbackTo(MVStore.java:1749)
    at org.h2.mvstore.MVStore.close(MVStore.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".toCharArray()).
                fileStore(new OffHeapStore()).
                pageSplitSize(6 * 1024).
                readOnly().
                writeBufferSize(8).
                writeDelay(100).
                open();

        //Put some items.
        MVMap<String, String> names = s.openMap("names");
        for (int i = 0; i < 10000; i++) {
            names.put(System.nanoTime() + "_" + i, Long.toString(names.sizeAsLong()));
        }
        System.out.println("Map count: " + names.size() + " version: " + names.getVersion());

        //Take a snapshot.
        MVMap<String, String> namesOld = names.openVersion(names.getVersion());
        //Bump up the version.
        s.incrementVersion();
        // Either both this commit and the one below MUST be called OR NEITHER!
        // s.commit();


        //Put some more items.
        for (int i = 20000; i < 25000; i++) {
            names.put(System.nanoTime() + "_" + i, Long.toString(names.sizeAsLong()));
        }
        System.out.println("Map count: " + names.size() + " version: " + names.getVersion());
        System.out
                .println("Old map count: " + namesOld.size() + " version: " + namesOld.getVersion());

        //s.commit();
        s.close();
    }
}

Regards,
Ashwin.

Thomas Mueller

unread,
Oct 25, 2013, 7:51:19 AM10/25/13
to H2 Google Group
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


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

Ashwin Jayaprakash

unread,
Oct 25, 2013, 1:08:47 PM10/25/13
to h2-da...@googlegroups.com
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.

Thomas Mueller

unread,
Oct 26, 2013, 3:11:50 AM10/26/13
to H2 Google Group
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

Ashwin Jayaprakash

unread,
Oct 26, 2013, 11:31:07 PM10/26/13
to h2-da...@googlegroups.com
I suppose you just need commit and commitAndClose. Otherwise you just end up answering questions about various combinations on the mailing list :)

Thomas Mueller

unread,
Oct 28, 2013, 2:12:30 AM10/28/13
to h2-da...@googlegroups.com
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


--

Ashwin Jayaprakash

unread,
Oct 28, 2013, 10:46:05 PM10/28/13
to h2-da...@googlegroups.com
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.
To unsubscribe from this group and stop receiving emails from it, send an email to h2-database+unsubscribe@googlegroups.com.
Reply all
Reply to author
Forward
0 new messages