You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by ch...@apache.org on 2014/07/23 14:21:55 UTC
svn commit: r1612825 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/plugins/document/
test/java/org/apache/jackrabbit/oak/plugins/document/
Author: chetanm
Date: Wed Jul 23 12:21:54 2014
New Revision: 1612825
URL: http://svn.apache.org/r1612825
Log:
OAK-1926 - UnmergedBranch state growing with empty BranchCommit leading to performance degradation
Unmerged persistent branch revs would be purged upon start for current cluster
Added:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/UnmergedBranchTest.java (with props)
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UnmergedBranches.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java?rev=1612825&r1=1612824&r2=1612825&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java Wed Jul 23 12:21:54 2014
@@ -556,30 +556,40 @@ public final class NodeDocument extends
}
/**
- * Gets a sorted map of uncommitted revisions of this document with the
+ * Purge the uncommitted revisions of this document with the
* local cluster node id as returned by the {@link RevisionContext}. These
* are the {@link #REVISIONS} entries where {@link Utils#isCommitted(String)}
* returns false.
*
+ * <p>
+ * <bold>Note</bold> - This method should only be invoked upon startup
+ * as then only we can safely assume that these revisions would not be
+ * committed
+ * </p>
+ *
* @param context the revision context.
- * @return the uncommitted revisions of this document.
+ * @return count of the revision entries purged
*/
- public SortedMap<Revision, Revision> getUncommittedRevisions(RevisionContext context) {
+ public int purgeUncommittedRevisions(RevisionContext context) {
// only look at revisions in this document.
// uncommitted revisions are not split off
Map<Revision, String> valueMap = getLocalRevisions();
- SortedMap<Revision, Revision> revisions =
- new TreeMap<Revision, Revision>(context.getRevisionComparator());
+ UpdateOp op = new UpdateOp(getId(), false);
+ int purgeCount = 0;
for (Map.Entry<Revision, String> commit : valueMap.entrySet()) {
if (!Utils.isCommitted(commit.getValue())) {
Revision r = commit.getKey();
if (r.getClusterId() == context.getClusterId()) {
- Revision b = Revision.fromString(commit.getValue());
- revisions.put(r, b);
+ purgeCount++;
+ op.removeMapEntry(REVISIONS, r);
}
}
}
- return revisions;
+
+ if (op.hasChanges()) {
+ store.findAndUpdate(Collection.NODES, op);
+ }
+ return purgeCount;
}
/**
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UnmergedBranches.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UnmergedBranches.java?rev=1612825&r1=1612824&r2=1612825&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UnmergedBranches.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UnmergedBranches.java Wed Jul 23 12:21:54 2014
@@ -21,7 +21,6 @@ import static com.google.common.base.Pre
import java.util.Comparator;
import java.util.List;
-import java.util.SortedMap;
import java.util.SortedSet;
import java.util.TreeSet;
import java.util.concurrent.CopyOnWriteArrayList;
@@ -31,6 +30,8 @@ import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import org.apache.jackrabbit.oak.plugins.document.util.Utils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* <code>UnmergedBranches</code> contains all un-merged branches of a DocumentMK
@@ -38,6 +39,8 @@ import org.apache.jackrabbit.oak.plugins
*/
class UnmergedBranches {
+ private final Logger log = LoggerFactory.getLogger(getClass());
+
/**
* Map of branches with the head of the branch as key.
*/
@@ -72,17 +75,9 @@ class UnmergedBranches {
if (doc == null) {
return;
}
- SortedMap<Revision, Revision> revisions = doc.getUncommittedRevisions(context);
- while (!revisions.isEmpty()) {
- SortedSet<Revision> commits = new TreeSet<Revision>(comparator);
- Revision head = revisions.lastKey();
- commits.add(head);
- Revision base = revisions.remove(head).asTrunkRevision();
- while (revisions.containsKey(base)) {
- commits.add(base);
- base = revisions.remove(base).asTrunkRevision();
- }
- branches.add(new Branch(commits, base));
+ int purgeCount = doc.purgeUncommittedRevisions(context);
+ if (purgeCount > 0) {
+ log.info("Purged [{}] uncommitted branch revision entries", purgeCount);
}
}
Added: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/UnmergedBranchTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/UnmergedBranchTest.java?rev=1612825&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/UnmergedBranchTest.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/UnmergedBranchTest.java Wed Jul 23 12:21:54 2014
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.jackrabbit.oak.plugins.document;
+
+import java.util.Map;
+import java.util.SortedMap;
+import java.util.TreeMap;
+
+import org.apache.jackrabbit.oak.plugins.document.memory.MemoryDocumentStore;
+import org.apache.jackrabbit.oak.plugins.document.util.Utils;
+import org.junit.Test;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class UnmergedBranchTest {
+
+ @Test
+ public void purgeUnmergedBranch() throws Exception {
+ DocumentStore testStore = new MemoryDocumentStore();
+ DocumentMK mk1 = create(testStore, 1);
+ DocumentMK mk2 = create(testStore, 2);
+
+ //1. Create branch commits on both cluster nodes
+ String rev1 = mk1.commit("", "+\"/child1\":{}", null, "");
+ String branchRev1 = mk1.branch(rev1);
+ String brev1 = mk1.commit("/child1", "^\"foo\":1", branchRev1, "");
+
+ String rev2 = mk2.commit("", "+\"/child2\":{}", null, "");
+ String branchRev2 = mk2.branch(rev2);
+ String brev2 = mk2.commit("/child2", "^\"foo\":1", branchRev2, "");
+
+ Map<Revision, Revision> revs1 = getUncommittedRevisions(mk1);
+ Map<Revision, Revision> revs2 = getUncommittedRevisions(mk2);
+
+ //2. Assert that branch rev are uncommited
+ assertTrue(revs1.containsKey(Revision.fromString(brev1).asTrunkRevision()));
+ assertTrue(revs2.containsKey(Revision.fromString(brev2).asTrunkRevision()));
+
+ //3. Restart cluster 1 so that purge happens but only for cluster 1
+ mk1.dispose();
+ mk1 = create(testStore, 1);
+ revs1 = getUncommittedRevisions(mk1);
+ revs2 = getUncommittedRevisions(mk2);
+
+ //4. Assert that post restart unmerged branch rev for c1 are purged
+ assertFalse(revs1.containsKey(Revision.fromString(brev1).asTrunkRevision()));
+ assertTrue(revs2.containsKey(Revision.fromString(brev2).asTrunkRevision()));
+
+ }
+
+ public SortedMap<Revision, Revision> getUncommittedRevisions(DocumentMK mk) {
+ // only look at revisions in this document.
+ // uncommitted revisions are not split off
+ NodeDocument doc = getRootDoc(mk);
+ Map<Revision, String> valueMap = doc.getLocalMap(NodeDocument.REVISIONS);
+ SortedMap<Revision, Revision> revisions =
+ new TreeMap<Revision, Revision>(mk.getNodeStore().getRevisionComparator());
+ for (Map.Entry<Revision, String> commit : valueMap.entrySet()) {
+ if (!Utils.isCommitted(commit.getValue())) {
+ Revision r = commit.getKey();
+ if (r.getClusterId() == mk.getNodeStore().getClusterId()) {
+ Revision b = Revision.fromString(commit.getValue());
+ revisions.put(r, b);
+ }
+ }
+ }
+ return revisions;
+ }
+
+ private NodeDocument getRootDoc(DocumentMK mk){
+ return mk.getNodeStore().getDocumentStore().find(Collection.NODES, Utils.getIdFromPath("/"));
+ }
+
+ private static DocumentMK create(DocumentStore ds, int clusterId){
+ return new DocumentMK.Builder().setAsyncDelay(0)
+ .setDocumentStore(ds).setClusterId(clusterId).open();
+ }
+}
Propchange: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/UnmergedBranchTest.java
------------------------------------------------------------------------------
svn:eol-style = native