Merge pull request #2248 from adamretter/feature/corruption-tests-and-fixes

Fix several Journalling and Recovery issues for XML and Binary documents

Author: dizzzz

build/scripts/junit.xml

@@ -321,6 +321,8 @@
             <formatter type="plain"/>
             <formatter type="xml"/>
             <test name="org.exist.storage.AllStorageTests" todir="${junit.reports.dat}"/>
+            <test name="org.exist.storage.RecoverBinaryTest" todir="${junit.reports.dat}"/>
+            <test name="org.exist.storage.RecoverXmlTest" todir="${junit.reports.dat}"/>
         </junit>
     </target>
 

extensions/indexes/lucene/test/src/org/exist/indexing/lucene/ConcurrencyTest.java

@@ -52,6 +52,8 @@
 
 public class ConcurrencyTest {
 
+    private static final long TIMEOUT_TERMINATION = 1000 * 60 * 3; // 3 minutes (in milliseconds)
+
     @ClassRule
     public static final ExistXmldbEmbeddedServer existEmbeddedServer = new ExistXmldbEmbeddedServer();
 
@@ -79,7 +81,7 @@ public void store() {
                 try {
                     storeRemoveDocs(name);
                 } catch(final XMLDBException | IOException e) {
-                    e.printStackTrace();;
+                    e.printStackTrace();
                     fail(e.getMessage());
                 }
             };
@@ -89,7 +91,7 @@ public void store() {
         executor.shutdown();
 		boolean terminated = false;
 		try {
-			terminated = executor.awaitTermination(60 * 60, TimeUnit.SECONDS);
+			terminated = executor.awaitTermination(TIMEOUT_TERMINATION, TimeUnit.MILLISECONDS);
 		} catch (final InterruptedException e) {
 		    //Nothing to do
 		}
@@ -116,7 +118,7 @@ public void update() {
         executor.shutdown();
 		boolean terminated = false;
 		try {
-			terminated = executor.awaitTermination(60 * 60, TimeUnit.SECONDS);
+			terminated = executor.awaitTermination(TIMEOUT_TERMINATION, TimeUnit.MILLISECONDS);
 		} catch (final InterruptedException e) {
 		    //Nothing to do
 		}

src/org/exist/collections/MutableCollection.java

@@ -1651,13 +1651,24 @@ public BinaryDocument addBinaryResource(final Txn transaction, final DBBroker br
             }
 
             if (oldDoc != null) {
-                LOG.debug("removing old document " + oldDoc.getFileURI());
+                if (LOG.isDebugEnabled()) {
+                    LOG.debug("removing old document db entry" + oldDoc.getFileURI());
+                }
+
                 if (!broker.preserveOnCopy(preserve)) {
                     updateModificationTime(blob);
                 }
-                broker.removeResource(transaction, oldDoc);
+
+                broker.removeResourceMetadata(transaction, oldDoc);
+
+                broker.getIndexController().setDocument(oldDoc, StreamListener.ReindexMode.REMOVE_BINARY);
+                broker.getIndexController().flush();
+
+                // NOTE(AR): the actual binary file on disk will be removed/overwritten in broker#storeBinaryResource below
+                //broker.removeResource(transaction, oldDoc);
             }
 
+            // store the binary content (create/replace)
             broker.storeBinaryResource(transaction, blob, is);
             addDocument(transaction, broker, blob, oldDoc);
 

src/org/exist/management/impl/Database.java

@@ -57,6 +57,16 @@ public String getInstanceId() {
         return pool.getId();
     }
 
+    @Override
+    public String getStatus() {
+        return pool.getStatus();
+    }
+
+    @Override
+    public void shutdown() {
+        pool.shutdown();
+    }
+
     @Override
     public int getMaxBrokers() {
         return pool.getMax();

src/org/exist/management/impl/DatabaseMXBean.java

@@ -24,7 +24,11 @@
 * $Id$
 */
 public interface DatabaseMXBean extends PerInstanceMBean {
-    
+
+    String getStatus();
+
+    void shutdown();
+
     int getMaxBrokers();
 
     int getAvailableBrokers();
@@ -44,4 +48,4 @@ public interface DatabaseMXBean extends PerInstanceMBean {
     public long getUptime();
 
     public String getExistHome();
-}
+}

src/org/exist/storage/AbstractBinaryLoggable.java

@@ -0,0 +1,114 @@
+/*
+ * eXist Open Source Native XML Database
+ * Copyright (C) 2001-2018 The eXist Project
+ * http://exist-db.org
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+package org.exist.storage;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.exist.storage.journal.AbstractLoggable;
+
+import javax.annotation.Nullable;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+/**
+ * @author Adam Retter <[email protected]>
+ */
+public abstract class AbstractBinaryLoggable extends AbstractLoggable {
+    private static final Logger LOG = LogManager.getLogger(AbstractBinaryLoggable.class);
+
+    public AbstractBinaryLoggable(final byte type, final long transactionId) {
+        super(type, transactionId);
+    }
+
+    /**
+     * Get's the absolute path as a byte array.
+     *
+     * @param path the path to encode.
+     *
+     * @return the absolute path, UTF-8 encoded as bytes
+     */
+    @Nullable protected static byte[] getPathData(@Nullable final Path path) {
+        if (path == null) {
+            return null;
+        }
+        return path.toAbsolutePath().toString().getBytes(UTF_8);
+    }
+
+    /**
+     * Get's the path from a byte array encoded by {@link #getPathData(Path)}.
+     *
+     * @param pathData the path data to decode.
+     *
+     * @return the path
+     */
+    @Nullable protected static Path getPath(@Nullable final byte[] pathData) {
+        if (pathData == null) {
+            return null;
+        }
+        return Paths.get(new String(pathData, UTF_8));
+    }
+
+    /**
+     * Converts the first two bytes of an integer into
+     * an unsigned short and stores the result into a short.
+     *
+     * @param i the integer
+     *
+     * @return the unsigned short stored in a short.
+     */
+    protected static short asUnsignedShort(final int i) {
+        return (short)(i & 0xFFFF);
+    }
+
+    /**
+     * Converts an unsigned short stored in a short back
+     * into an integer. Inverse of {@link #asUnsignedShort(int)}.
+     *
+     * @param s the unsigned short as a short.
+     *
+     * @return the integer.
+     */
+    protected static int asSignedInt(final short s) {
+        return s & 0xFFFF;
+    }
+
+    /**
+     * Check that the length of a path does not need more storage than we have available (i.e. 2 bytes).
+     *
+     * @param loggableName The name of the loggable (for formatting error messages).
+     * @param pathName The name of the path (for formatting error messages).
+     * @param path The path to check the length of.
+     */
+    protected static void checkPathLen(final String loggableName, final String pathName, @Nullable final byte path[]) {
+        if (path == null) {
+            return;
+        }
+
+        final int len = path.length;
+        if (len <= 0) {
+            LOG.error(loggableName + ": " + pathName + " path has a zero length");
+        } else if(len > 0xFFFF) {
+            LOG.error(loggableName + ": " + pathName + " path needs more than 65,535 bytes. Path will be truncated: " + new String(path, UTF_8));
+        }
+    }
+}

src/org/exist/storage/BrokerPool.java

@@ -174,6 +174,10 @@ private enum Event {
             .build()
     );
 
+    public String getStatus() {
+        return status.getCurrentState().name();
+    }
+
     /**
      * The number of brokers for the database instance
      */
@@ -362,7 +366,6 @@ private enum Event {
 
     private StartupTriggersManager startupTriggersManager;
 
-
     /**
      * Creates and configures the database instance.
      *

src/org/exist/storage/BrokerPools.java

@@ -166,7 +166,10 @@ public static void configure(final String instanceName, final int minBrokers, fi
                 return;
             }
 
-            LOG.debug("Configuring database instance '" + instanceName + "'...");
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("Configuring database instance '" + instanceName + "'...");
+            }
+
             try {
                 //Create the instance
                 final BrokerPool instance = new BrokerPool(instanceName, minBrokers, maxBrokers, config, statusObserver);