MVStore bugfixes / tests

h2/src/docsrc/html/changelog.html

@@ -18,13 +18,15 @@ Change Log
 <h1>Change Log</h1>
 
 <h2>Next Version (unreleased)</h2>
-<ul><li>The profiler tool can now process files with full thread dumps.
+<ul><li>Issue 545: Unnecessary duplicate code was removed. 
+</li><li>The profiler tool can now process files with full thread dumps.
 </li><li>MVStore mode: the CLOB and BLOB storage was re-implemented and is
     now much faster than with the PageStore (which is still the default storage).
 </li><li>MVStore mode: creating indexes is now much faster
     (in many cases faster than with the default PageStore).
 </li><li>Various bugs in the MVStore storage and have been fixed,
     including a bug in the R-tree implementation.
+    The database could get corrupt if there were transient IO exceptions while storing.
 </li><li>The method org.h2.expression.Function.getCost could throw a NullPointException.
 </li><li>Storing LOBs in separate files (outside of the main database file)
     is no longer supported for new databases.

h2/src/docsrc/html/roadmap.html

@@ -41,6 +41,7 @@ See also <a href="build.html#providing_patches">Providing Patches</a>.
 </li><li>Remove support for the old-style outer join syntax using "(+)" because it is buggy.
 </li><li>Change license to MPL 2.0.
 </li><li>Not allow relative database URLs like jdbc:h2:test; instead, require using jdbc:h2:./test.
+</li><li>Document that FILE_LOCK=SERIALIZED is not supported with the MVStore mode.
 </li></ul>
 
 <h2>Priority 1</h2>

h2/src/main/org/h2/mvstore/MVStore.java

@@ -660,7 +660,16 @@ public class MVStore {
         header.position(BLOCK_SIZE);
         header.put(bytes);
         header.rewind();
-        fileStore.writeFully(0, header);
+        write(0, header);
+    }
+    
+    private void write(long pos, ByteBuffer buffer) {
+        try {
+            fileStore.writeFully(pos, buffer);
+        } catch (IllegalStateException e) {
+            close();
+            throw e;
+        }
     }
 
     /**
@@ -945,8 +954,7 @@ public class MVStore {
         buff.put(new byte[BLOCK_SIZE - header.length]);
 
         buff.position(0);
-        fileStore.writeFully(filePos, buff.getBuffer());
-
+        write(filePos, buff.getBuffer());
         releaseWriteBuffer(buff);
 
         // overwrite the header if required
@@ -1214,7 +1222,7 @@ public class MVStore {
             // fill the header with zeroes
             buff.put(new byte[BLOCK_SIZE - header.length]);
             buff.position(0);
-            fileStore.writeFully(end, buff.getBuffer());
+            write(end, buff.getBuffer());
             releaseWriteBuffer(buff);
             markMetaChanged();
             meta.put("chunk." + c.id, c.asString());
@@ -1252,7 +1260,7 @@ public class MVStore {
             // fill the header with zeroes
             buff.put(new byte[BLOCK_SIZE - header.length]);
             buff.position(0);
-            fileStore.writeFully(pos, buff.getBuffer());
+            write(pos, buff.getBuffer());
             releaseWriteBuffer(buff);
             markMetaChanged();
             meta.put("chunk." + c.id, c.asString());
@@ -1772,7 +1780,7 @@ public class MVStore {
             ByteBuffer header = ByteBuffer.allocate(BLOCK_SIZE);
             header.put(bytes);
             header.rewind();
-            fileStore.writeFully(fileStore.size(), header);
+            write(fileStore.size(), header);
             readStoreHeader();
             readMeta();
         }

h2/src/main/org/h2/mvstore/cache/FilePathCache.java

@@ -99,7 +99,9 @@ public class FilePathCache extends FilePathWrapper {
                 }
             }
             dst.put(buff.array(), off, len);
-            return len;
+            ; // add test to TestFileSystem
+            // return len;
+            return len == 0 ? -1 : len;
         }
 
         private static long getCachePos(long pos) {

h2/src/main/org/h2/mvstore/db/MVTableEngine.java

@@ -306,19 +306,23 @@ public class MVTableEngine implements TableEngine {
          * @param maxCompactTime the maximum time in milliseconds to compact
          */
         public void close(long maxCompactTime) {
-            if (!store.isClosed() && store.getFileStore() != null) {
-                if (!store.getFileStore().isReadOnly()) {
-                    transactionStore.close();
-                    long start = System.currentTimeMillis();
-                    while (store.compact(90)) {
-                        long time = System.currentTimeMillis() - start;
-                        if (time > maxCompactTime) {
-                            break;
+            try {
+                if (!store.isClosed() && store.getFileStore() != null) {
+                    if (!store.getFileStore().isReadOnly()) {
+                        transactionStore.close();
+                        long start = System.currentTimeMillis();
+                        while (store.compact(90)) {
+                            long time = System.currentTimeMillis() - start;
+                            if (time > maxCompactTime) {
+                                break;
+                            }
                         }
                     }
+                    store.close();
                 }
-                store.close();
-            }
+            } catch (IllegalStateException e) {
+                throw DbException.get(ErrorCode.IO_EXCEPTION_1, e, "Closing");
+            }                
         }
 
         /**

h2/src/main/org/h2/store/FileLister.java

@@ -6,13 +6,16 @@
  */
 package org.h2.store;
 
+import java.nio.channels.FileChannel;
 import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.List;
+
 import org.h2.constant.ErrorCode;
 import org.h2.engine.Constants;
 import org.h2.message.DbException;
 import org.h2.message.TraceSystem;
+import org.h2.store.fs.FilePath;
 import org.h2.store.fs.FileUtils;
 import org.h2.util.New;
 
@@ -44,6 +47,15 @@ public class FileLister {
                     throw DbException.get(
                             ErrorCode.CANNOT_CHANGE_SETTING_WHEN_OPEN_1, message).getSQLException();
                 }
+            } else if (fileName.endsWith(Constants.SUFFIX_MV_FILE)) {
+                try {
+                    FileChannel f = FilePath.get(fileName).open("r");
+                    java.nio.channels.FileLock lock = f.tryLock(0, Long.MAX_VALUE, true);
+                    lock.release();
+                } catch (Exception e) {
+                    throw DbException.get(
+                            ErrorCode.CANNOT_CHANGE_SETTING_WHEN_OPEN_1, e, message).getSQLException();
+                }
             }
         }
     }

h2/src/test/org/h2/test/TestAll.java

@@ -729,7 +729,6 @@ kill -9 `jps -l | grep "org.h2.test." | cut -d " " -f 1`
         new TestBtreeIndex().runTest(this);
         new TestDiskFull().runTest(this);
         new TestCrashAPI().runTest(this);
-
         new TestFuzzOptimizations().runTest(this);
         new TestLimit().runTest(this);
         new TestRandomSQL().runTest(this);

h2/src/test/org/h2/test/db/TestIndex.java

@@ -95,7 +95,7 @@ public class TestIndex extends TestBase {
     }
 
     private void testIndexTypes() throws SQLException {
-        Connection conn = getConnection("index;MV_STORE=false");
+        Connection conn = getConnection("index");
         stat = conn.createStatement();
         for (String type : new String[] { "unique", "hash", "unique hash" }) {
             stat.execute("create table test(id int)");

h2/src/test/org/h2/test/db/TestSpatial.java

@@ -54,17 +54,10 @@ public class TestSpatial extends TestBase {
             return;
         }
         if (DataType.GEOMETRY_CLASS != null) {
-
             deleteDb("spatial");
             url = "spatial";
             testSpatial();
             deleteDb("spatial");
-
-            if (!config.mvcc) {
-                url = "spatial;MV_STORE=false";
-                testSpatial();
-                deleteDb("spatial");
-            }
         }
     }
 

h2/src/test/org/h2/test/synth/TestDiskFull.java

@@ -55,6 +55,7 @@ public class TestDiskFull extends TestBase {
         String url = "jdbc:h2:unstable:memFS:diskFull" + x +
             ";FILE_LOCK=NO;TRACE_LEVEL_FILE=0;WRITE_DELAY=10;" +
             "LOCK_TIMEOUT=100;CACHE_SIZE=4096";
+        url = getURL(url, true);
         Connection conn = null;
         Statement stat = null;
         boolean opened = false;
@@ -90,7 +91,8 @@ public class TestDiskFull extends TestBase {
                     conn.close();
                 } catch (SQLException e2) {
                     if (e2.getErrorCode() != ErrorCode.IO_EXCEPTION_1
-                            && e2.getErrorCode() != ErrorCode.IO_EXCEPTION_2) {
+                            && e2.getErrorCode() != ErrorCode.IO_EXCEPTION_2
+                            && e2.getErrorCode() != ErrorCode.DATABASE_IS_CLOSED) {
                         throw e2;
                     }
                 }

h2/src/test/org/h2/test/unit/TestAutoReconnect.java

@@ -7,7 +7,6 @@
 package org.h2.test.unit;
 
 import java.sql.Connection;
-import java.sql.DriverManager;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
@@ -44,7 +43,7 @@ public class TestAutoReconnect extends TestBase {
                 connServer.close();
             }
             org.h2.Driver.load();
-            connServer = DriverManager.getConnection(url);
+            connServer = getConnection(url);
         } else {
             server.stop();
             server.start();
@@ -95,12 +94,12 @@ public class TestAutoReconnect extends TestBase {
         }
 
         // test the database event listener
-        conn = DriverManager.getConnection(url + ";DATABASE_EVENT_LISTENER='" + MyDatabaseEventListener.class.getName() + "'");
+        conn = getConnection(url + ";DATABASE_EVENT_LISTENER='" + MyDatabaseEventListener.class.getName() + "'");
         conn.close();
 
         Statement stat;
 
-        conn = DriverManager.getConnection(url);
+        conn = getConnection(url);
         restart();
         stat = conn.createStatement();
         restart();

h2/src/test/org/h2/test/unit/TestExit.java

@@ -35,10 +35,11 @@ public class TestExit extends TestBase {
             return;
         }
         deleteDb("exit");
+        String url = getURL(OPEN_WITH_CLOSE_ON_EXIT);
         String selfDestruct = SelfDestructor.getPropertyString(60);
         String[] procDef = { "java", selfDestruct,
                 "-cp", getClassPath(),
-                getClass().getName(), "" + OPEN_WITH_CLOSE_ON_EXIT };
+                getClass().getName(), url };
         Process proc = Runtime.getRuntime().exec(procDef);
         while (true) {
             int ch = proc.getErrorStream().read();
@@ -59,9 +60,10 @@ public class TestExit extends TestBase {
         if (!getClosedFile().exists()) {
             fail("did not close database");
         }
+        url = getURL(OPEN_WITHOUT_CLOSE_ON_EXIT);
         procDef = new String[] { "java",
                 "-cp", getClassPath(), getClass().getName(),
-                "" + OPEN_WITHOUT_CLOSE_ON_EXIT };
+                url };
         proc = Runtime.getRuntime().exec(procDef);
         proc.waitFor();
         Thread.sleep(100);
@@ -70,6 +72,23 @@ public class TestExit extends TestBase {
         }
         deleteDb("exit");
     }
+    
+    private String getURL(int action) {
+        String url = "";
+        switch (action) {
+        case OPEN_WITH_CLOSE_ON_EXIT:
+            url = "jdbc:h2:" + getBaseDir() + "/exit;database_event_listener='" + MyDatabaseEventListener.class.getName()
+                    + "';db_close_on_exit=true";
+            break;
+        case OPEN_WITHOUT_CLOSE_ON_EXIT:
+            url = "jdbc:h2:" + getBaseDir() + "/exit;database_event_listener='" + MyDatabaseEventListener.class.getName()
+                    + "';db_close_on_exit=false";
+            break;
+        default:
+        }
+        url = getURL(url, true);
+        return url;
+    }
 
     /**
      * This method is called when executing this application from the command
@@ -82,25 +101,12 @@ public class TestExit extends TestBase {
         if (args.length == 0) {
             System.exit(1);
         }
-        int action = Integer.parseInt(args[0]);
-        TestExit app = new TestExit();
-        app.execute(action);
+        String url = args[0];
+        TestExit.execute(url);
     }
 
-    private void execute(int action) throws SQLException {
+    private static void execute(String url) throws SQLException {
         org.h2.Driver.load();
-        String url = "";
-        switch (action) {
-        case OPEN_WITH_CLOSE_ON_EXIT:
-            url = "jdbc:h2:" + getBaseDir() + "/exit;database_event_listener='" + MyDatabaseEventListener.class.getName()
-                    + "';db_close_on_exit=true";
-            break;
-        case OPEN_WITHOUT_CLOSE_ON_EXIT:
-            url = "jdbc:h2:" + getBaseDir() + "/exit;database_event_listener='" + MyDatabaseEventListener.class.getName()
-                    + "';db_close_on_exit=false";
-            break;
-        default:
-        }
         conn = open(url);
         Connection conn2 = open(url);
         conn2.close();

h2/src/test/org/h2/test/unit/TestFileLockProcess.java

@@ -71,7 +71,8 @@ public class TestFileLockProcess extends TestBase {
     }
 
     private void test(int count, String url) throws Exception {
-        Connection conn = DriverManager.getConnection(url);
+        url = getURL(url, true);
+        Connection conn = getConnection(url);
         String selfDestruct = SelfDestructor.getPropertyString(60);
         String[] procDef = { "java", selfDestruct,
                 "-cp", getClassPath(),

h2/src/test/org/h2/test/unit/TestFileSystem.java

@@ -16,7 +16,6 @@ import java.nio.channels.FileChannel;
 import java.nio.channels.FileChannel.MapMode;
 import java.nio.channels.FileLock;
 import java.sql.Connection;
-import java.sql.DriverManager;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Statement;
@@ -251,7 +250,7 @@ public class TestFileSystem extends TestBase {
         FileUtils.deleteRecursive(dir, false);
         Connection conn;
         Statement stat;
-        conn = DriverManager.getConnection("jdbc:h2:split:18:"+dir+"/test");
+        conn = getConnection("jdbc:h2:split:18:"+dir+"/test");
         stat = conn.createStatement();
         stat.execute(
                 "create table test(id int primary key, name varchar) " +
@@ -260,7 +259,7 @@ public class TestFileSystem extends TestBase {
         conn.close();
         Backup.execute(dir + "/test.zip", dir, "", true);
         DeleteDbFiles.execute("split:" + dir, "test", true);
-        conn = DriverManager.getConnection(
+        conn = getConnection(
                 "jdbc:h2:split:zip:"+dir+"/test.zip!/test");
         conn.createStatement().execute("select * from test where id=1");
         conn.close();
@@ -271,12 +270,12 @@ public class TestFileSystem extends TestBase {
         org.h2.Driver.load();
         deleteDb("fsMem");
         String url = "jdbc:h2:" + getBaseDir() + "/fsMem";
-        Connection conn = DriverManager.getConnection(url, "sa", "sa");
+        Connection conn = getConnection(url, "sa", "sa");
         conn.createStatement().execute("CREATE TABLE TEST AS SELECT * FROM DUAL");
         conn.createStatement().execute("BACKUP TO '" + getBaseDir() + "/fsMem.zip'");
         conn.close();
         org.h2.tools.Restore.main("-file", getBaseDir() + "/fsMem.zip", "-dir", "memFS:");
-        conn = DriverManager.getConnection("jdbc:h2:memFS:fsMem", "sa", "sa");
+        conn = getConnection("jdbc:h2:memFS:fsMem", "sa", "sa");
         ResultSet rs = conn.createStatement().executeQuery("SELECT * FROM TEST");
         rs.close();
         conn.close();
@@ -293,7 +292,7 @@ public class TestFileSystem extends TestBase {
         }
         org.h2.Driver.load();
         String url = "jdbc:h2:" + getBaseDir() + "/fsJar";
-        Connection conn = DriverManager.getConnection(url, "sa", "sa");
+        Connection conn = getConnection(url, "sa", "sa");
         Statement stat = conn.createStatement();
         stat.execute("create table test(id int primary key, name varchar, b blob, c clob)");
         stat.execute("insert into test values(1, 'Hello', SECURE_RAND(2000), space(2000))");
@@ -303,7 +302,7 @@ public class TestFileSystem extends TestBase {
         byte[] b1 = rs.getBytes(3);
         String s1 = rs.getString(4);
         conn.close();
-        conn = DriverManager.getConnection(url, "sa", "sa");
+        conn = getConnection(url, "sa", "sa");
         stat = conn.createStatement();
         stat.execute("backup to '" + getBaseDir() + "/fsJar.zip'");
         conn.close();
@@ -326,7 +325,7 @@ public class TestFileSystem extends TestBase {
             testReadOnly(f);
         }
         String urlJar = "jdbc:h2:zip:" + getBaseDir() + "/fsJar.zip!/fsJar";
-        conn = DriverManager.getConnection(urlJar, "sa", "sa");
+        conn = getConnection(urlJar, "sa", "sa");
         stat = conn.createStatement();
         rs = stat.executeQuery("select * from test");
         rs.next();

h2/src/test/org/h2/test/unit/TestOldVersion.java

@@ -41,6 +41,9 @@ public class TestOldVersion extends TestBase {
 
     @Override
     public void test() throws Exception {
+        if (config.mvStore) {
+            return;
+        }
         cl = getClassLoader("file:ext/h2-1.2.127.jar");
         driver = getDriver(cl);
         if (driver == null) {

h2/src/test/org/h2/test/unit/TestPageStore.java

@@ -70,7 +70,6 @@ public class TestPageStore extends TestBase {
         testLargeInserts();
         testLargeDatabaseFastOpen();
         testUniqueIndexReopen();
-        testExistingOld();
         testLargeRows();
         testRecoverDropIndex();
         testDropPk();
@@ -526,26 +525,6 @@ public class TestPageStore extends TestBase {
         conn.close();
     }
 
-    private void testExistingOld() throws SQLException {
-        if (config.memory) {
-            return;
-        }
-        Connection conn;
-        deleteDb("pageStoreExistingOld");
-        String url;
-        url = "jdbc:h2:" + getBaseDir() + "/pageStoreExistingOld";
-        conn = DriverManager.getConnection(url);
-        conn.createStatement().execute("create table test(id int) as select 1");
-        conn.close();
-        conn = DriverManager.getConnection(url);
-        conn.createStatement().execute("select * from test");
-        conn.close();
-        // the database is automatically converted
-        conn = DriverManager.getConnection(url);
-        assertResult("1", conn.createStatement(), "select * from test");
-        conn.close();
-    }
-
     private void testLargeRows() throws Exception {
         if (config.memory) {
             return;

h2/src/test/org/h2/test/unit/TestRecovery.java

@@ -96,9 +96,12 @@ public class TestRecovery extends TestBase {
     }
 
     private void testRedoTransactions() throws Exception {
+        if (config.mvStore) {
+            // not needed for MV_STORE=TRUE
+            return;
+        }
         DeleteDbFiles.execute(getBaseDir(), "recovery", true);
-        // not needed for MV_STORE=TRUE
-        Connection conn = getConnection("recovery;MV_STORE=false");
+        Connection conn = getConnection("recovery");
         Statement stat = conn.createStatement();
         stat.execute("set write_delay 0");
         stat.execute("create table test(id int primary key, name varchar)");
@@ -136,9 +139,12 @@ public class TestRecovery extends TestBase {
     }
 
     private void testCorrupt() throws Exception {
+        if (config.mvStore) {
+            // not needed for MV_STORE=TRUE
+            return;
+        }
         DeleteDbFiles.execute(getBaseDir(), "recovery", true);
-        // not needed for MV_STORE=TRUE
-        Connection conn = getConnection("recovery;MV_STORE=false");
+        Connection conn = getConnection("recovery");
         Statement stat = conn.createStatement();
         stat.execute("create table test(id int, name varchar) as select 1, 'Hello World1'");
         conn.close();
@@ -163,9 +169,12 @@ public class TestRecovery extends TestBase {
     }
 
     private void testWithTransactionLog() throws SQLException {
+        if (config.mvStore) {
+            // not needed for MV_STORE=TRUE
+            return;
+        }
         DeleteDbFiles.execute(getBaseDir(), "recovery", true);
-        // not needed for MV_STORE=TRUE
-        Connection conn = getConnection("recovery;MV_STORE=false");
+        Connection conn = getConnection("recovery");
         Statement stat = conn.createStatement();
         stat.execute("create table truncate(id int primary key) as select x from system_range(1, 1000)");
         stat.execute("create table test(id int primary key, data int, text varchar)");
@@ -198,10 +207,10 @@ public class TestRecovery extends TestBase {
             // expected
         }
         Recover.main("-dir", getBaseDir(), "-db", "recovery");
-        conn = getConnection("recovery;MV_STORE=false");
+        conn = getConnection("recovery");
         conn.close();
         Recover.main("-dir", getBaseDir(), "-db", "recovery", "-removePassword");
-        conn = getConnection("recovery;MV_STORE=false", getUser(), "");
+        conn = getConnection("recovery", getUser(), "");
         conn.close();
         DeleteDbFiles.execute(getBaseDir(), "recovery", true);
     }