You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by dp...@apache.org on 2012/03/08 10:19:18 UTC

svn commit: r1298311 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/mk/store/CopyingGC.java main/java/org/apache/jackrabbit/mk/store/DefaultRevisionStore.java test/java/org/apache/jackrabbit/mk/store/CopyHeadRevisionTest.java

Author: dpfister
Date: Thu Mar  8 09:19:18 2012
New Revision: 1298311

URL: http://svn.apache.org/viewvc?rev=1298311&view=rev
Log:
Add copying GC for revisions
- ensure commits in to space are linked correctly


Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/CopyingGC.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/DefaultRevisionStore.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/mk/store/CopyHeadRevisionTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/CopyingGC.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/CopyingGC.java?rev=1298311&r1=1298310&r2=1298311&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/CopyingGC.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/CopyingGC.java Thu Mar  8 09:19:18 2012
@@ -18,7 +18,9 @@ package org.apache.jackrabbit.mk.store;
 
 import java.io.Closeable;
 import java.io.InputStream;
+import java.util.Comparator;
 import java.util.Iterator;
+import java.util.TreeSet;
 
 import org.apache.jackrabbit.mk.model.ChildNode;
 import org.apache.jackrabbit.mk.model.ChildNodeEntriesMap;
@@ -35,8 +37,8 @@ import org.apache.jackrabbit.oak.model.N
  * store to a "to" revision store. It assumes that both stores share the same blob
  * store.
  * 
- * TODO: Ensure the integrity of the parental relationship when copying revisions
- *       in a GC cycle (because there may be missing intermediate commits).
+ * In the current design, a revision is reachable, if it is either the head revision
+ * or requested during the GC cycle.
  */
 public class CopyingGC implements RevisionStore, Closeable {
     
@@ -54,6 +56,23 @@ public class CopyingGC implements Revisi
      * Flag indicating whether a GC cycle is running.
      */
     private volatile boolean running;
+    
+    /**
+     * First commit id of "to" store.
+     */
+    private String firstCommitId;
+    
+    /**
+     * Map of commits that have been accessed while a GC cycle is running; these
+     * need to be "re-linked" with a preceding, possibly not adjacent parent
+     * commit before saving them back to the "to" revision store.
+     */
+    private final TreeSet<MutableCommit> commits = new TreeSet<MutableCommit>(
+            new Comparator<MutableCommit>() {
+                public int compare(MutableCommit o1, MutableCommit o2) {
+                    return o1.getId().compareTo(o2.getId());
+                }
+            });
 
     /**
      * Create a new instance of this class.
@@ -72,17 +91,35 @@ public class CopyingGC implements Revisi
      * @throws Exception if an error occurs
      */
     public void start() throws Exception {
-        running = true;
+        commits.clear();
+        firstCommitId = rsTo.getHeadCommitId();
         
-        copy(rsFrom.getHeadCommit());
+        // Copy the head commit
+        MutableCommit commitTo = copy(rsFrom.getHeadCommit());
+        commitTo.setParentId(rsTo.getHeadCommitId());
+        String revId = rsTo.putCommit(commitTo);
+        rsTo.setHeadCommitId(revId);
+
+        // Add this as sentinel
+        commits.add(commitTo);
+
+        running = true;
     }
     
     /**
      * Stop GC cycle.
      */
-    public void stop() {
+    public void stop() throws Exception {
         running = false;
         
+        if (commits.size() > 1) {
+            String parentId = firstCommitId;
+            for (MutableCommit commit : commits) {
+                commit.setParentId(parentId);
+                rsTo.putCommit(commit);
+                parentId = commit.getId();
+            }
+        }
         // TODO: swap rsFrom/rsTo and reset them
         rsFrom = rsTo;
         rsTo = null;
@@ -100,18 +137,15 @@ public class CopyingGC implements Revisi
     /**
      * Copy a commit and all the nodes belonging to it, starting at the root node.
      * 
-     * @param commit commit to be copied
+     * @param commit commit to copy
+     * @return commit in the "to" store, not yet persisted
      * @throws Exception if an error occurs
      */
-    public void copy(StoredCommit commit) throws Exception {
+    private MutableCommit copy(StoredCommit commit) throws Exception {
         StoredNode nodeFrom = rsFrom.getNode(commit.getRootNodeId());
         copy(nodeFrom);
         
-        MutableCommit commitTo = new MutableCommit(commit);
-        commitTo.setParentId(rsTo.getHeadCommitId());
-        
-        String revId = rsTo.putCommit(commitTo);
-        rsTo.setHeadCommitId(revId);
+        return new MutableCommit(commit);
     }
     
     /**
@@ -162,13 +196,7 @@ public class CopyingGC implements Revisi
                 // ignore, better add a has() method
             }
         }
-        StoredCommit commit = rsFrom.getCommit(id);
-        if (!running) {
-            return commit;
-        }
-        // synchronously copy the commit and the nodes it references to the target store
-        copy(commit);
-        return rsTo.getCommit(id);
+        return rsFrom.getCommit(id);
     }
 
     public ChildNodeEntriesMap getCNEMap(Id id) throws NotFoundException,
@@ -194,7 +222,12 @@ public class CopyingGC implements Revisi
                 // ignore, better add a has() method
             }
         }
-        return rsFrom.getRootNode(commitId);
+        // Copy this commit
+        StoredCommit commit = rsFrom.getCommit(commitId);
+        if (running) {
+            commits.add(copy(commit));
+        }
+        return rsFrom.getNode(commit.getRootNodeId());
     }
 
     public StoredCommit getHeadCommit() throws Exception {

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/DefaultRevisionStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/DefaultRevisionStore.java?rev=1298311&r1=1298310&r2=1298311&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/DefaultRevisionStore.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/DefaultRevisionStore.java Thu Mar  8 09:19:18 2012
@@ -138,25 +138,6 @@ public class DefaultRevisionStore implem
         return result;
     }
 
-    /**
-     * Convert a fixed-size byte array of size 8 into a long.
-     * 
-     * @param value byte array
-     * @return long
-     */
-    private static long bytesToLong(byte[] value) {
-        long result = 0;
-        
-        if (value.length != 8) {
-            throw new IllegalArgumentException("Value must be a byte array of size 8");
-        }
-        for (int i = 0; i < value.length; i++) {
-            result |= (value[i] & 0xff);
-            result <<= 8;
-        }
-        return result;
-    }
-    
     //--------------------------------------------------------< RevisionStore >
 
     public Id putNode(MutableNode node) throws Exception {
@@ -216,7 +197,6 @@ public class DefaultRevisionStore implem
             id = StringUtils.convertBytesToHex(rawId);
         } else {
             rawId = StringUtils.convertHexToBytes(id);
-            headCounter = bytesToLong(rawId);
         }
         pm.writeCommit(rawId, commit);
 
@@ -237,6 +217,11 @@ public class DefaultRevisionStore implem
         try {
             pm.writeHead(commitId);
             headId = commitId;
+            
+            long headCounter = Long.parseLong(headId, 16);
+            if (headCounter > this.headCounter) {
+                this.headCounter = headCounter;
+            }
         } finally {
             headLock.writeLock().unlock();
         }

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/mk/store/CopyHeadRevisionTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/mk/store/CopyHeadRevisionTest.java?rev=1298311&r1=1298310&r2=1298311&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/mk/store/CopyHeadRevisionTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/mk/store/CopyHeadRevisionTest.java Thu Mar  8 09:19:18 2012
@@ -23,8 +23,12 @@ import java.io.File;
 
 import org.apache.jackrabbit.mk.MicroKernelImpl;
 import org.apache.jackrabbit.mk.Repository;
+import org.apache.jackrabbit.mk.api.MicroKernel;
 import org.apache.jackrabbit.mk.api.MicroKernelException;
 import org.apache.jackrabbit.mk.fs.FileUtils;
+import org.apache.jackrabbit.mk.json.fast.Jsop;
+import org.apache.jackrabbit.mk.json.fast.JsopArray;
+import org.apache.jackrabbit.mk.json.fast.JsopObject;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -49,7 +53,7 @@ public class CopyHeadRevisionTest {
     
     @Test
     public void testCopyHeadRevisionToNewStore() throws Exception {
-        String[] revs = new String[3];
+        String[] revs = new String[5];
         
         DefaultRevisionStore rsFrom = new DefaultRevisionStore();
         rsFrom.initialize(new File("target/mk1"));
@@ -59,20 +63,23 @@ public class CopyHeadRevisionTest {
 
         CopyingGC gc = new CopyingGC(rsFrom, rsTo);
         
-        MicroKernelImpl mk = new MicroKernelImpl(new Repository(gc));
-        revs[0] = mk.commit("/",  "+\"a\" : { \"c\":{}, \"d\":{} }", mk.getHeadRevision(), null);
-        revs[1] = mk.commit("/",  "+\"b\" : {}", mk.getHeadRevision(), null);
+        MicroKernel mk = new MicroKernelImpl(new Repository(gc));
+        revs[0] = mk.commit("/", "+\"a\" : { \"c\":{}, \"d\":{} }", mk.getHeadRevision(), null);
+        revs[1] = mk.commit("/", "+\"b\" : {}", mk.getHeadRevision(), null);
+        revs[2] = mk.commit("/b", "+\"e\" : {}", mk.getHeadRevision(), null);
+        revs[3] = mk.commit("/a/c", "+\"f\" : {}", mk.getHeadRevision(), null);
 
         // Simulate a GC cycle start
         gc.start();
 
-        revs[2] = mk.commit("/b", "+\"e\" : {}", mk.getHeadRevision(), null);
+        revs[4] = mk.commit("/b/e", "+\"g\" : {}", mk.getHeadRevision(), null);
+        mk.getJournal(revs[2], revs[2], "");
         
         // Simulate a GC cycle stop
         gc.stop();
         
         // Assert head revision is contained after GC
-        assertEquals(mk.getHeadRevision(), revs[2]);
+        assertEquals(mk.getHeadRevision(), revs[revs.length - 1]);
         
         // Assert unused revision was GCed
         try {
@@ -81,5 +88,14 @@ public class CopyHeadRevisionTest {
         } catch (MicroKernelException e) {
             // ignore
         }
+        
+        // Verify journal integrity: referenced revision must still be available and linked in chain
+        JsopArray a = (JsopArray) Jsop.parse(mk.getRevisions(0, Integer.MAX_VALUE));
+        for (int i = 0; i < a.size(); i++) {
+            if (((JsopObject) a.get(i)).get("id").equals(revs[2])) {
+                return; 
+            }
+        }
+        fail("Revision not appearing in list of revisions: "+ revs[2]);
     }
 }