jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mdue...@apache.org
Subject svn commit: r1517480 - in /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel: KernelNodeStore.java KernelNodeStoreBranch.java
Date Mon, 26 Aug 2013 09:48:29 GMT
Author: mduerig
Date: Mon Aug 26 09:48:28 2013
New Revision: 1517480

URL: http://svn.apache.org/r1517480
Log:
OAK-975: Refactor KernelNodeStoreBranch
Refactor KNSB to use state pattern instead of ad-hoc case discrimination

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeStore.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeStoreBranch.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeStore.java?rev=1517480&r1=1517479&r2=1517480&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeStore.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeStore.java
Mon Aug 26 09:48:28 2013
@@ -146,7 +146,7 @@ public class KernelNodeStore extends Abs
 
     @Override
     public NodeStoreBranch branch() {
-        return new KernelNodeStoreBranch(this, getRoot(), mergeLock);
+        return new KernelNodeStoreBranch(this, mergeLock, getRoot());
     }
 
     /**
@@ -184,12 +184,7 @@ public class KernelNodeStore extends Abs
 
     //-----------------------------------------------------------< internal >---
 
-    @Nonnull
-    MicroKernel getKernel() {
-        return kernel;
-    }
-
-    KernelNodeState getRootState(String revision) {
+    private KernelNodeState getRootState(String revision) {
         try {
             return cache.get(revision + "/");
         } catch (ExecutionException e) {
@@ -197,12 +192,20 @@ public class KernelNodeStore extends Abs
         }
     }
 
-    NodeState commit(String jsop, String baseRevision) {
-        return getRootState(kernel.commit("", jsop, baseRevision, null));
+    KernelNodeState commit(String jsop, KernelNodeState base) {
+        return getRootState(kernel.commit("", jsop, base.getRevision(), null));
+    }
+
+    KernelNodeState branch(KernelNodeState base) {
+        return getRootState(kernel.branch(base.getRevision()));
+    }
+
+    KernelNodeState rebase(KernelNodeState branchHead, KernelNodeState base) {
+        return getRootState(kernel.rebase(branchHead.getRevision(), base.getRevision()));
     }
 
-    NodeState merge(String headRevision) {
-        return getRootState(kernel.merge(headRevision, null));
+    NodeState merge(KernelNodeState branchHead) {
+        return getRootState(kernel.merge(branchHead.getRevision(), null));
     }
 
 }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeStoreBranch.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeStoreBranch.java?rev=1517480&r1=1517479&r2=1517480&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeStoreBranch.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeStoreBranch.java
Mon Aug 26 09:48:28 2013
@@ -16,15 +16,15 @@
  */
 package org.apache.jackrabbit.oak.kernel;
 
-import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
-import static com.google.common.base.Preconditions.checkState;
 import static org.apache.jackrabbit.oak.commons.PathUtils.elements;
 import static org.apache.jackrabbit.oak.commons.PathUtils.getName;
 import static org.apache.jackrabbit.oak.commons.PathUtils.getParentPath;
 
 import java.util.concurrent.locks.Lock;
 
+import javax.annotation.Nonnull;
+
 import org.apache.jackrabbit.mk.api.MicroKernel;
 import org.apache.jackrabbit.mk.api.MicroKernelException;
 import org.apache.jackrabbit.oak.api.CommitFailedException;
@@ -39,73 +39,56 @@ import org.apache.jackrabbit.oak.spi.sta
 /**
  * {@code NodeStoreBranch} based on {@link MicroKernel} branching and merging.
  * This implementation keeps changes in memory up to a certain limit and writes
- * them back when the to the Microkernel branch when the limit is exceeded.
+ * them back to the Microkernel branch when the limit is exceeded.
  */
 class KernelNodeStoreBranch extends AbstractNodeStoreBranch {
 
     /** The underlying store to which this branch belongs */
     private final KernelNodeStore store;
 
-    /**
-     * Lock for coordinating concurrent merge operations
-     */
+    /** Lock for coordinating concurrent merge operations */
     private final Lock mergeLock;
 
-    /** Root state of the base revision of this branch */
-    private KernelNodeState base;
+    /**
+     * State of the this branch. Either {@link Unmodified}, {@link InMemory}, {@link Persisted}
+     * or {@link Merged}.
+     * @see BranchState
+     */
+    private BranchState branchState;
 
-    /** Root state of the transient head revision on top of persisted branch, null if merged.
*/
-    private NodeState head;
+    public KernelNodeStoreBranch(KernelNodeStore kernelNodeStore, Lock mergeLock,
+            KernelNodeState base) {
 
-    /** Head revision of persisted branch, null if not yet branched*/
-    private String headRevision;
+        this.store = checkNotNull(kernelNodeStore);
+        this.mergeLock = checkNotNull(mergeLock);
+        branchState = new Unmodified(checkNotNull(base));
+    }
 
-    /** Number of updates to this branch via {@link #setRoot(NodeState)} */
-    private int updates = 0;
-
-    KernelNodeStoreBranch(KernelNodeStore store, KernelNodeState root, Lock mergeLock) {
-        this.store = store;
-        this.base = root;
-        this.head = root;
-        this.mergeLock = mergeLock;
+    @Override
+    public String toString() {
+        return branchState.toString();
     }
 
+    @Nonnull
     @Override
     public NodeState getBase() {
-        return base;
+        return branchState.getBase();
     }
 
+    @Nonnull
     @Override
     public NodeState getHead() {
-        checkNotMerged();
-        return head;
+        return branchState.getHead();
     }
 
     @Override
     public void setRoot(NodeState newRoot) {
-        checkNotMerged();
-        if (!head.equals(newRoot)) {
-            NodeState oldHead = head;
-            head = newRoot;
-            if (++updates > 1) {
-                // persist unless this is the first update
-                boolean success = false;
-                try {
-                    persistTransientHead();
-                    success = true;
-                } finally {
-                    if (!success) {
-                        head = oldHead;
-                    }
-                }
-            }
-        }
+        branchState.setRoot(checkNotNull(newRoot));
     }
 
     @Override
     public boolean move(String source, String target) {
-        checkNotMerged();
-        if (PathUtils.isAncestor(source, target)) {
+        if (PathUtils.isAncestor(checkNotNull(source), checkNotNull(target))) {
             return false;
         } else if (source.equals(target)) {
             return true;
@@ -124,19 +107,17 @@ class KernelNodeStoreBranch extends Abst
             // destination exists already
             return false;
         }
-
-        commit(">\"" + source + "\":\"" + target + '"');
+        branchState.persist().commit(">\"" + source + "\":\"" + target + '"');
         return true;
     }
 
     @Override
     public boolean copy(String source, String target) {
-        checkNotMerged();
-        if (!getNode(source).exists()) {
+        if (!getNode(checkNotNull(source)).exists()) {
             // source does not exist
             return false;
         }
-        NodeState destParent = getNode(getParentPath(target));
+        NodeState destParent = getNode(getParentPath(checkNotNull(target)));
         if (!destParent.exists()) {
             // parent of destination does not exist
             return false;
@@ -145,88 +126,22 @@ class KernelNodeStoreBranch extends Abst
             // destination exists already
             return false;
         }
-
-        commit("*\"" + source + "\":\"" + target + '"');
+        branchState.persist().commit("*\"" + source + "\":\"" + target + '"');
         return true;
     }
 
+    @Nonnull
     @Override
-    public NodeState merge(CommitHook hook, PostCommitHook committed) throws CommitFailedException
{
-        checkNotMerged();
-        NodeState oldRoot = head;
-        mergeLock.lock();
-        try {
-            rebase();
-            NodeState toCommit = checkNotNull(hook).processCommit(base, head);
-            head = toCommit;
-            if (head.equals(base)) {
-                // Nothing was written to this branch: return base state
-                head = null;  // Mark as merged
-                committed.contentChanged(base, base);
-                return base;
-            } else {
-                NodeState newRoot;
-                JsopDiff diff = new JsopDiff(store);
-                if (headRevision == null) {
-                    // no branch created yet, commit directly
-                    head.compareAgainstBaseState(base, diff);
-                    newRoot = store.commit(diff.toString(), base.getRevision());
-                } else {
-                    // commit into branch and merge
-                    head.compareAgainstBaseState(store.getRootState(headRevision), diff);
-                    String jsop = diff.toString();
-                    if (!jsop.isEmpty()) {
-                        headRevision = store.getKernel().commit(
-                                "", jsop, headRevision, null);
-                    }
-                    newRoot = store.merge(headRevision);
-                    headRevision = null;
-                }
-                head = null;  // Mark as merged
-                committed.contentChanged(base, newRoot);
-                return newRoot;
-            }
-        } catch (MicroKernelException e) {
-            head = oldRoot;
-            throw new CommitFailedException(
-                    "Kernel", 1,
-                    "Failed to merge changes to the underlying MicroKernel", e);
-        } finally {
-            mergeLock.unlock();
-        }
+    public NodeState merge(@Nonnull CommitHook hook, PostCommitHook committed) throws CommitFailedException
{
+        return branchState.merge(checkNotNull(hook), checkNotNull(committed));
     }
 
     @Override
     public void rebase() {
-        KernelNodeState root = store.getRoot();
-        if (head.equals(root)) {
-            // Nothing was written to this branch: set new base revision
-            head = root;
-            base = root;
-        } else if (headRevision == null) {
-            // Nothing written to persistent branch yet
-            // perform rebase in memory
-            NodeBuilder builder = root.builder();
-            getHead().compareAgainstBaseState(getBase(), new ConflictAnnotatingRebaseDiff(builder));
-            head = builder.getNodeState();
-            base = root;
-        } else {
-            // perform rebase in kernel
-            persistTransientHead();
-            headRevision = store.getKernel().rebase(headRevision, root.getRevision());
-            head = store.getRootState(headRevision);
-            base = root;
-        }
-    }
-
-    //------------------------------------------------------------< private >---
-
-    private void checkNotMerged() {
-        checkState(head != null, "Branch has already been merged");
+        branchState.rebase();
     }
 
     private NodeState getNode(String path) {
-        checkArgument(path.startsWith("/"));
         NodeState node = getHead();
         for (String name : elements(path)) {
             node = node.getChildNode(name);
@@ -234,60 +149,289 @@ class KernelNodeStoreBranch extends Abst
         return node;
     }
 
-    private void commit(String jsop) {
-        MicroKernel kernel = store.getKernel();
-        if (headRevision == null) {
-            // create the branch if this is the first commit
-            headRevision = kernel.branch(base.getRevision());
-        }
-
-        // persist transient changes first
-        persistTransientHead();
-
-        headRevision = kernel.commit("", jsop, headRevision, null);
-        head = store.getRootState(headRevision);
-    }
-
-    private void persistTransientHead() {
-        NodeState oldHead = head;
-        String oldHeadRevision = headRevision;
-        boolean success = false;
-        try {
-            MicroKernel kernel = store.getKernel();
-            JsopDiff diff = new JsopDiff(store);
-            if (headRevision == null) {
-                // no persistent branch yet
-                if (head.equals(base)) {
-                    // nothing to persist
-                    success = true;
-                    return;
-                } else {
-                    // create branch
-                    headRevision = kernel.branch(base.getRevision());
-                    head.compareAgainstBaseState(base, diff);
-                }
+    /**
+     * Sub classes of this class represent a state a branch can be in. See the individual
+     * sub classes for permissible state transitions.
+     */
+    private abstract class BranchState {
+        /** Root state of the base revision of this branch */
+        protected KernelNodeState base;
+
+        protected BranchState(KernelNodeState base) {
+            this.base = base;
+        }
+
+        /**
+         * Persist this branch to an underlying branch in the {@code MicroKernel}.
+         */
+        Persisted persist() {
+            branchState = new Persisted(base, getHead());
+            return (Persisted) branchState;
+        }
+
+        KernelNodeState getBase(){
+            return base;
+        }
+
+        @Nonnull
+        abstract NodeState getHead();
+
+        abstract void setRoot(NodeState root);
+
+        abstract void rebase();
+
+        @Nonnull
+        abstract NodeState merge(@Nonnull CommitHook hook, PostCommitHook committed) throws
CommitFailedException;
+    }
+
+    /**
+     * Instances of this class represent a branch whose base and head are the same.
+     * <p>
+     * Transitions to:
+     * <ul>
+     *     <li>{@link InMemory} on {@link #setRoot(NodeState)} if the new root differs
+     *         from the current base</li>.
+     *     <li>{@link Merged} on {@link #merge(CommitHook, PostCommitHook)}</li>
+     * </ul>
+     */
+    private class Unmodified extends BranchState {
+        Unmodified(KernelNodeState base) {
+            super(base);
+        }
+
+        @Override
+        public String toString() {
+            return "Unmodified[" + base + ']';
+        }
+
+        @Override
+        NodeState getHead() {
+            return base;
+        }
+
+        @Override
+        void setRoot(NodeState root) {
+            if (!base.equals(root)) {
+                branchState = new InMemory(base, root);
+            }
+        }
+
+        @Override
+        void rebase() {
+            base = store.getRoot();
+        }
+
+        @Override
+        NodeState merge(CommitHook hook, PostCommitHook committed) throws CommitFailedException
{
+            committed.contentChanged(base, base);
+            branchState = new Merged(base);
+            return base;
+        }
+    }
+
+    /**
+     * Instances of this class represent a branch whose base and head differ.
+     * All changes are kept in memory.
+     * <p>
+     * Transitions to:
+     * <ul>
+     *     <li>{@link Unmodified} on {@link #setRoot(NodeState)} if the new root is
the same
+     *         as the base of this branch or
+     *     <li>{@link Persisted} otherwise.
+     *     <li>{@link Merged} on {@link #merge(CommitHook, PostCommitHook)}</li>
+     * </ul>
+     */
+    private class InMemory extends BranchState {
+        /** Root state of the transient head. */
+        private NodeState head;
+
+        @Override
+        public String toString() {
+            return "InMemory[" + base + ", " + head + ']';
+        }
+
+        InMemory(KernelNodeState base, NodeState head) {
+            super(base);
+            this.head = head;
+        }
+
+        @Override
+        NodeState getHead() {
+            return head;
+        }
+
+        @Override
+        void setRoot(NodeState root) {
+            if (base.equals(root)) {
+                branchState = new Unmodified(base);
+            } else if (!head.equals(root)) {
+                head = root;
+                persist();
+            }
+        }
+
+        @Override
+        void rebase() {
+            KernelNodeState root = store.getRoot();
+            NodeBuilder builder = root.builder();
+            head.compareAgainstBaseState(base, new ConflictAnnotatingRebaseDiff(builder));
+            head = builder.getNodeState();
+            base = root;
+        }
+
+        @Override
+        NodeState merge(CommitHook hook, PostCommitHook committed) throws CommitFailedException
{
+            mergeLock.lock();
+            try {
+                rebase();
+                NodeState toCommit = checkNotNull(hook).processCommit(base, head);
+                JsopDiff diff = new JsopDiff(store);
+                toCommit.compareAgainstBaseState(base, diff);
+                NodeState newHead = store.commit(diff.toString(), base);
+                committed.contentChanged(base, newHead);
+                branchState = new Merged(base);
+                return newHead;
+            } catch (MicroKernelException e) {
+                throw new CommitFailedException(
+                        "Kernel", 1,
+                        "Failed to merge changes to the underlying MicroKernel", e);
+            } finally {
+                mergeLock.unlock();
+            }
+        }
+    }
+
+    /**
+     * Instances of this class represent a branch whose base and head differ.
+     * All changes are persisted to an underlying branch in the {@code MicroKernel}.
+     * <p>
+     * Transitions to:
+     * <ul>
+     *     <li>{@link Unmodified} on {@link #setRoot(NodeState)} if the new root is
the same
+     *         as the base of this branch.
+     *     <li>{@link Merged} on {@link #merge(CommitHook, PostCommitHook)}</li>
+     * </ul>
+     */
+    private class Persisted extends BranchState {
+        /** Root state of the transient head, top of persisted branch. */
+        private KernelNodeState head;
+
+        @Override
+        public String toString() {
+            return "Persisted[" + base + ", " + head + ']';
+        }
+
+        Persisted(KernelNodeState base, NodeState head) {
+            super(base);
+            this.head = store.branch(base);
+            persistTransientHead(head);
+        }
+
+        void commit(String jsop) {
+            if (!jsop.isEmpty()) {
+                head = store.commit(jsop, head);
+            }
+        }
+
+        @Override
+        NodeState getHead() {
+            return head;
+        }
+
+        @Override
+        void setRoot(NodeState root) {
+            if (base.equals(root)) {
+                branchState = new Unmodified(base);
+            } else if (!head.equals(root)) {
+                persistTransientHead(root);
+            }
+        }
+
+        @Override
+        void rebase() {
+            KernelNodeState root = store.getRoot();
+            if (head.equals(root)) {
+                // Nothing was written to this branch: set new base revision
+                head = root;
+                base = root;
             } else {
-                // compare against head of branch
-                NodeState branchHead = store.getRootState(headRevision);
-                if (head.equals(branchHead)) {
-                    // nothing to persist
-                    success = true;
-                    return;
+                // perform rebase in kernel
+                head = store.rebase(head, root);
+                base = root;
+            }
+        }
+
+        @Override
+        NodeState merge(CommitHook hook, PostCommitHook committed) throws CommitFailedException
{
+            mergeLock.lock();
+            try {
+                rebase();
+                NodeState toCommit = checkNotNull(hook).processCommit(base, head);
+                if (toCommit.equals(base)) {
+                    committed.contentChanged(base, base);
+                    branchState = new Merged(base);
+                    return base;
                 } else {
-                    head.compareAgainstBaseState(branchHead, diff);
+                    JsopDiff diff = new JsopDiff(store);
+                    toCommit.compareAgainstBaseState(head, diff);
+                    commit(diff.toString());
+                    NodeState newRoot = store.merge(head);
+                    committed.contentChanged(base, newRoot);
+                    branchState = new Merged(base);
+                    return newRoot;
                 }
+            } catch (MicroKernelException e) {
+                throw new CommitFailedException(
+                        "Kernel", 1,
+                        "Failed to merge changes to the underlying MicroKernel", e);
+            } finally {
+                mergeLock.unlock();
             }
-            // if we get here we have something to persist
-            // and a branch exists
-            headRevision = kernel.commit("", diff.toString(), headRevision, null);
-            head = store.getRootState(headRevision);
-            success = true;
-        } finally {
-            // revert to old state if unsuccessful
-            if (!success) {
-                head = oldHead;
-                headRevision = oldHeadRevision;
+        }
+
+        private void persistTransientHead(NodeState newHead) {
+            if (!newHead.equals(head)) {
+                JsopDiff diff = new JsopDiff(store);
+                newHead.compareAgainstBaseState(head, diff);
+                head = store.commit(diff.toString(), head);
             }
         }
     }
+
+    /**
+     * Instances of this class represent a branch that has already been merged.
+     * All methods throw an {@code IllegalStateException}.
+     * <p>
+     * Transitions to: none.
+     */
+    private class Merged extends BranchState {
+        protected Merged(KernelNodeState base) {
+            super(base);
+        }
+
+        @Override
+        public String toString() {
+            return "Merged[" + base + ']';
+        }
+
+        @Override
+        NodeState getHead() {
+            throw new IllegalStateException("Branch has already been merged");
+        }
+
+        @Override
+        void setRoot(NodeState root) {
+            throw new IllegalStateException("Branch has already been merged");
+        }
+
+        @Override
+        void rebase() {
+            throw new IllegalStateException("Branch has already been merged");
+        }
+
+        @Override
+        NodeState merge(CommitHook hook, PostCommitHook committed) throws CommitFailedException
{
+            throw new IllegalStateException("Branch has already been merged");
+        }
+    }
 }



Mime
View raw message