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 mr...@apache.org on 2014/12/08 14:13:22 UTC
svn commit: r1643808 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/plugins/document/
main/java/org/apache/jackrabbit/oak/plugins/document/util/
test/java/org/apache/jackrabbit/oak/plugins/document/
test/java/org/apache/ja...
Author: mreutegg
Date: Mon Dec 8 13:13:22 2014
New Revision: 1643808
URL: http://svn.apache.org/r1643808
Log:
OAK-2131: Reduce usage of _lastRev
Added:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevs.java (with props)
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevTest.java (with props)
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Branch.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java
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/SplitOperations.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgentTest.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryTest.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/UtilsTest.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Branch.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Branch.java?rev=1643808&r1=1643807&r2=1643808&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Branch.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Branch.java Mon Dec 8 13:13:22 2014
@@ -192,7 +192,8 @@ class Branch {
/**
* Gets the most recent unsaved last revision at <code>readRevision</code>
- * or earlier in this branch for the given <code>path</code>.
+ * or earlier in this branch for the given <code>path</code>. Documents with
+ * explicit updates are not tracked and this method may return {@code null}.
*
* @param path the path of a node.
* @param readRevision the read revision.
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java?rev=1643808&r1=1643807&r2=1643808&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java Mon Dec 8 13:13:22 2014
@@ -28,6 +28,7 @@ import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
+import com.google.common.base.Function;
import com.google.common.collect.Sets;
import org.apache.jackrabbit.oak.commons.json.JsopStream;
import org.apache.jackrabbit.oak.commons.json.JsopWriter;
@@ -36,6 +37,9 @@ import org.apache.jackrabbit.oak.commons
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import static com.google.common.collect.Iterables.filter;
+import static com.google.common.collect.Iterables.transform;
+import static org.apache.jackrabbit.oak.commons.PathUtils.denotesRoot;
import static org.apache.jackrabbit.oak.plugins.document.Collection.NODES;
import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.COLLISIONS;
import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.SPLIT_CANDIDATE_THRESHOLD;
@@ -256,7 +260,7 @@ public class Commit {
} else {
while (!PathUtils.isAncestor(commitRootPath, p)) {
commitRootPath = PathUtils.getParentPath(commitRootPath);
- if (PathUtils.denotesRoot(commitRootPath)) {
+ if (denotesRoot(commitRootPath)) {
break;
}
}
@@ -280,12 +284,6 @@ public class Commit {
} else {
NodeDocument.setCommitRoot(op, revision, commitRootDepth);
if (op.isNew()) {
- if (baseBranchRevision == null) {
- // for new non-branch nodes we can safely set _lastRev on
- // insert. for existing nodes the _lastRev is updated by
- // the background thread to avoid concurrent updates
- NodeDocument.setLastRev(op, revision);
- }
newNodes.add(op);
} else {
changedNodes.add(op);
@@ -297,7 +295,6 @@ public class Commit {
// it is the root of a subtree added in a commit.
// so we try to add the root like all other nodes
NodeDocument.setRevision(commitRoot, revision, commitValue);
- NodeDocument.setLastRev(commitRoot, revision);
newNodes.add(commitRoot);
}
try {
@@ -312,9 +309,6 @@ public class Commit {
// (because there might be a conflict)
NodeDocument.unsetRevision(commitRoot, revision);
}
- // setting _lastRev is only safe on insert. now the
- // background thread needs to take care of it
- NodeDocument.unsetLastRev(op, revision.getClusterId());
changedNodes.add(op);
}
newNodes.clear();
@@ -381,7 +375,7 @@ public class Commit {
private void updateParentChildStatus() {
final Set<String> processedParents = Sets.newHashSet();
for (String path : addedNodes) {
- if (PathUtils.denotesRoot(path)) {
+ if (denotesRoot(path)) {
continue;
}
@@ -407,7 +401,6 @@ public class Commit {
}
for (UpdateOp op : newDocuments) {
UpdateOp reverse = op.getReverseOperation();
- NodeDocument.unsetLastRev(reverse, revision.getClusterId());
store.findAndUpdate(NODES, reverse);
}
UpdateOp removeCollision = new UpdateOp(commitRoot.getId(), false);
@@ -542,7 +535,7 @@ public class Commit {
public void applyToCache(Revision before, boolean isBranchCommit) {
HashMap<String, ArrayList<String>> nodesWithChangedChildren = new HashMap<String, ArrayList<String>>();
for (String p : modifiedNodes) {
- if (PathUtils.denotesRoot(p)) {
+ if (denotesRoot(p)) {
continue;
}
String parent = PathUtils.getParentPath(p);
@@ -554,6 +547,7 @@ public class Commit {
list.add(p);
}
DiffCache.Entry cacheEntry = nodeStore.getDiffCache().newEntry(before, revision);
+ LastRevTracker tracker = nodeStore.createTracker(revision, isBranchCommit);
List<String> added = new ArrayList<String>();
List<String> removed = new ArrayList<String>();
List<String> changed = new ArrayList<String>();
@@ -575,10 +569,12 @@ public class Commit {
}
UpdateOp op = operations.get(path);
boolean isNew = op != null && op.isNew();
- boolean pendingLastRev = op == null
- || !NodeDocument.hasLastRev(op, revision.getClusterId());
- nodeStore.applyChanges(revision, path, isNew, pendingLastRev,
- isBranchCommit, added, removed, changed, cacheEntry);
+ if (op == null || !hasContentChanges(op) || denotesRoot(path)) {
+ // track intermediate node and root
+ tracker.track(path);
+ }
+ nodeStore.applyChanges(revision, path, isNew,
+ added, removed, changed, cacheEntry);
}
cacheEntry.done();
}
@@ -592,14 +588,14 @@ public class Commit {
}
private void markChanged(String path) {
- if (!PathUtils.denotesRoot(path) && !PathUtils.isAbsolute(path)) {
+ if (!denotesRoot(path) && !PathUtils.isAbsolute(path)) {
throw new IllegalArgumentException("path: " + path);
}
while (true) {
if (!modifiedNodes.add(path)) {
break;
}
- if (PathUtils.denotesRoot(path)) {
+ if (denotesRoot(path)) {
break;
}
path = PathUtils.getParentPath(path);
@@ -621,4 +617,16 @@ public class Commit {
NodeDocument.setDeleted(op, revision, true);
}
+ private static final Function<UpdateOp.Key, String> KEY_TO_NAME =
+ new Function<UpdateOp.Key, String>() {
+ @Override
+ public String apply(UpdateOp.Key input) {
+ return input.getName();
+ }
+ };
+
+ private static boolean hasContentChanges(UpdateOp op) {
+ return filter(transform(op.getChanges().keySet(),
+ KEY_TO_NAME), Utils.PROPERTY_OR_DELETED).iterator().hasNext();
+ }
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java?rev=1643808&r1=1643807&r2=1643808&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java Mon Dec 8 13:13:22 2014
@@ -932,33 +932,15 @@ public final class DocumentNodeStore
* @param rev the commit revision
* @param path the path
* @param isNew whether this is a new node
- * @param pendingLastRev whether the node has a pending _lastRev to write
- * @param isBranchCommit whether this is from a branch commit
* @param added the list of added child nodes
* @param removed the list of removed child nodes
* @param changed the list of changed child nodes.
*
*/
public void applyChanges(Revision rev, String path,
- boolean isNew, boolean pendingLastRev,
- boolean isBranchCommit, List<String> added,
+ boolean isNew, List<String> added,
List<String> removed, List<String> changed,
DiffCache.Entry cacheEntry) {
- LastRevTracker tracker = createTracker(rev);
- if (disableBranches) {
- if (pendingLastRev) {
- tracker.track(path);
- }
- } else {
- if (isBranchCommit) {
- Revision branchRev = rev.asBranchRevision();
- tracker = branches.getBranchCommit(branchRev);
- }
- if (isBranchCommit || pendingLastRev) {
- // write back _lastRev with background thread
- tracker.track(path);
- }
- }
if (isNew) {
DocumentNodeState.Children c = new DocumentNodeState.Children();
Set<String> set = Sets.newTreeSet();
@@ -1295,6 +1277,28 @@ public final class DocumentNodeStore
return writer.toString();
}
+ /**
+ * Creates a tracker for the given commit revision.
+ *
+ * @param r a commit revision.
+ * @param isBranchCommit whether this is a branch commit.
+ * @return a _lastRev tracker for the given commit revision.
+ */
+ LastRevTracker createTracker(final @Nonnull Revision r,
+ final boolean isBranchCommit) {
+ if (isBranchCommit && !disableBranches) {
+ Revision branchRev = r.asBranchRevision();
+ return branches.getBranchCommit(branchRev);
+ } else {
+ return new LastRevTracker() {
+ @Override
+ public void track(String path) {
+ unsavedLastRevisions.put(path, r);
+ }
+ };
+ }
+ }
+
//------------------------< Observable >------------------------------------
@Override
@@ -1602,21 +1606,6 @@ public final class DocumentNodeStore
//-----------------------------< internal >---------------------------------
- /**
- * Creates a tracker for the given commit revision.
- *
- * @param r a commit revision.
- * @return a _lastRev tracker for the given commit revision.
- */
- private LastRevTracker createTracker(final @Nonnull Revision r) {
- return new LastRevTracker() {
- @Override
- public void track(String path) {
- unsavedLastRevisions.put(path, r);
- }
- };
- }
-
private static void diffProperties(DocumentNodeState from,
DocumentNodeState to,
JsopWriter w) {
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java?rev=1643808&r1=1643807&r2=1643808&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java Mon Dec 8 13:13:22 2014
@@ -155,14 +155,10 @@ public class LastRevRecoveryAgent {
if (currentLastRev != null) {
knownLastRevs.put(doc.getPath(), currentLastRev);
}
- Revision lostLastRev = determineMissedLastRev(doc, clusterId);
+ // 1. determine last committed modification on document
+ Revision lastModifiedRev = determineLastModification(doc, clusterId);
- //1. Update lastRev for this doc
- if (lostLastRev != null) {
- unsaved.put(doc.getPath(), lostLastRev);
- }
-
- Revision lastRevForParents = lostLastRev != null ? lostLastRev : currentLastRev;
+ Revision lastRevForParents = Utils.max(lastModifiedRev, currentLastRev);
//If both currentLastRev and lostLastRev are null it means
//that no change is done by suspect cluster on this document
@@ -257,21 +253,17 @@ public class LastRevRecoveryAgent {
}
/**
- * Determines the last revision value which needs to set for given clusterId
- * on the passed document. If the last rev entries are consisted
+ * Determines the last committed modification to the given document by
+ * a {@code clusterId}.
*
- * @param doc NodeDocument where lastRev entries needs to be fixed
- * @param clusterId clusterId for which lastRev has to be checked
- * @return lastRev which needs to be updated. <tt>null</tt> if no
- * updated is required i.e. lastRev entries are valid
+ * @param doc a document.
+ * @param clusterId clusterId for which the last committed modification is
+ * looked up.
+ * @return the commit revision of the last modification by {@code clusterId}
+ * to the given document.
*/
@CheckForNull
- private Revision determineMissedLastRev(NodeDocument doc, int clusterId) {
- Revision currentLastRev = doc.getLastRev().get(clusterId);
- if (currentLastRev == null) {
- currentLastRev = new Revision(0, 0, clusterId);
- }
-
+ private Revision determineLastModification(NodeDocument doc, int clusterId) {
ClusterPredicate cp = new ClusterPredicate(clusterId);
// Merge sort the revs for which changes have been made
@@ -285,22 +277,12 @@ public class LastRevRecoveryAgent {
StableRevisionComparator.REVERSE
);
- // Look for latest valid revision > currentLastRev
- // if found then lastRev needs to be fixed
+ Revision lastModified = null;
+ // Look for latest valid revision
for (Revision rev : revs) {
- if (rev.compareRevisionTime(currentLastRev) > 0) {
- rev = doc.getCommitRevision(rev);
- if (rev != null) {
- return rev;
- }
- } else {
- // No valid revision found > currentLastRev
- // indicates that lastRev is valid for given clusterId
- // and no further checks are required
- break;
- }
+ lastModified = Utils.max(lastModified, doc.getCommitRevision(rev));
}
- return null;
+ return lastModified;
}
/**
Added: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevs.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevs.java?rev=1643808&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevs.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevs.java Mon Dec 8 13:13:22 2014
@@ -0,0 +1,77 @@
+/*
+ * 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.HashMap;
+import java.util.Map;
+
+import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+import org.apache.jackrabbit.oak.plugins.document.util.Utils;
+
+/**
+ * Helper class to track when a node was last modified.
+ */
+final class LastRevs {
+
+ private final Map<Integer, Revision> revs;
+
+ private final Revision readRevision;
+
+ private final Branch branch;
+
+ private Revision branchRev;
+
+ LastRevs(Map<Integer, Revision> revs, Revision readRevision, Branch branch) {
+ this.revs = new HashMap<Integer, Revision>(revs);
+ this.readRevision = readRevision;
+ this.branch = branch;
+ }
+
+ void update(@Nullable Revision rev) {
+ if (rev == null) {
+ return;
+ }
+ Revision r = revs.get(rev.getClusterId());
+ if (r == null || rev.compareRevisionTime(r) > 0) {
+ revs.put(rev.getClusterId(), rev);
+ }
+ }
+
+ void updateBranch(@Nullable Revision rev) {
+ if (rev == null) {
+ return;
+ }
+ rev = rev.asBranchRevision();
+ if (branch != null && branch.containsCommit(rev)
+ && readRevision.compareRevisionTime(rev) >= 0) {
+ branchRev = Utils.max(branchRev, rev);
+ }
+ }
+
+ @CheckForNull
+ Revision getBranchRevision() {
+ return branchRev;
+ }
+
+ @Nonnull
+ Map<Integer, Revision> get() {
+ return revs;
+ }
+}
Propchange: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevs.java
------------------------------------------------------------------------------
svn:eol-style = native
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=1643808&r1=1643807&r2=1643808&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 Mon Dec 8 13:13:22 2014
@@ -58,6 +58,7 @@ import static com.google.common.collect.
import static org.apache.jackrabbit.oak.plugins.document.UpdateOp.Key;
import static org.apache.jackrabbit.oak.plugins.document.UpdateOp.Operation;
import static org.apache.jackrabbit.oak.plugins.document.util.Utils.isRevisionNewer;
+import static org.apache.jackrabbit.oak.plugins.document.util.Utils.resolveCommitRevision;
/**
* A document storing data about a node.
@@ -750,7 +751,12 @@ public final class NodeDocument extends
@Nonnull Revision readRevision,
@Nullable Revision lastModified) {
Map<Revision, String> validRevisions = Maps.newHashMap();
- Revision min = getLiveRevision(nodeStore, readRevision, validRevisions);
+ Branch branch = nodeStore.getBranches().getBranch(readRevision);
+ LastRevs lastRevs = new LastRevs(getLastRev(), readRevision, branch);
+ // overlay with unsaved last modified from this instance
+ lastRevs.update(lastModified);
+
+ Revision min = getLiveRevision(nodeStore, readRevision, validRevisions, lastRevs);
if (min == null) {
// deleted
return null;
@@ -764,7 +770,7 @@ public final class NodeDocument extends
}
// first check local map, which contains most recent values
Value value = getLatestValue(nodeStore, getLocalMap(key),
- min, readRevision, validRevisions);
+ min, readRevision, validRevisions, lastRevs);
// check if there may be more recent values in a previous document
if (value != null && !getPreviousRanges().isEmpty()) {
@@ -784,7 +790,7 @@ public final class NodeDocument extends
if (value == null && !getPreviousRanges().isEmpty()) {
// check complete revision history
value = getLatestValue(nodeStore, getValueMap(key),
- min, readRevision, validRevisions);
+ min, readRevision, validRevisions, lastRevs);
}
String propertyName = Utils.unescapePropertyName(key);
String v = value != null ? value.value : null;
@@ -801,17 +807,11 @@ public final class NodeDocument extends
// _lastRev.
// when was this node last modified?
- Branch branch = nodeStore.getBranches().getBranch(readRevision);
- Map<Integer, Revision> lastRevs = Maps.newHashMap(getLastRev());
- // overlay with unsaved last modified from this instance
- if (lastModified != null) {
- lastRevs.put(nodeStore.getClusterId(), lastModified);
- }
Revision branchBase = null;
if (branch != null) {
branchBase = branch.getBase(readRevision);
}
- for (Revision r : lastRevs.values()) {
+ for (Revision r : lastRevs.get().values()) {
// ignore if newer than readRevision
if (isRevisionNewer(nodeStore, r, readRevision)) {
// the node has a _lastRev which is newer than readRevision
@@ -840,9 +840,10 @@ public final class NodeDocument extends
if (branch != null) {
// read from a branch
// -> possibly overlay with unsaved last revs from branch
- Revision r = branch.getUnsavedLastRevision(path, readRevision);
+ lastRevs.updateBranch(branch.getUnsavedLastRevision(path, readRevision));
+ Revision r = lastRevs.getBranchRevision();
if (r != null) {
- lastRevision = r.asBranchRevision();
+ lastRevision = r;
}
}
n.setLastRevision(lastRevision);
@@ -857,19 +858,21 @@ public final class NodeDocument extends
* @param maxRev the maximum revision to return
* @param validRevisions the map of revisions to commit value already
* checked against maxRev and considered valid.
+ * @param lastRevs to keep track of the last modification.
* @return the earliest revision, or null if the node is deleted at the
* given revision
*/
@CheckForNull
public Revision getLiveRevision(RevisionContext context, Revision maxRev,
- Map<Revision, String> validRevisions) {
+ Map<Revision, String> validRevisions,
+ LastRevs lastRevs) {
// check local deleted map first
Value value = getLatestValue(context, getLocalDeleted(),
- null, maxRev, validRevisions);
+ null, maxRev, validRevisions, lastRevs);
if (value == null && !getPreviousRanges().isEmpty()) {
// need to check complete map
value = getLatestValue(context, getDeleted(),
- null, maxRev, validRevisions);
+ null, maxRev, validRevisions, lastRevs);
}
return value != null && "false".equals(value.value) ? value.revision : null;
@@ -1158,16 +1161,6 @@ public final class NodeDocument extends
revision.toString());
}
- public static boolean hasLastRev(@Nonnull UpdateOp op, int clusterId) {
- return checkNotNull(op).getChanges().containsKey(
- new Key(LAST_REV, new Revision(0, 0, clusterId)));
- }
-
- public static void unsetLastRev(@Nonnull UpdateOp op, int clusterId) {
- checkNotNull(op).unsetMapEntry(LAST_REV,
- new Revision(0, 0, clusterId));
- }
-
public static void setCommitRoot(@Nonnull UpdateOp op,
@Nonnull Revision revision,
int commitRootDepth) {
@@ -1345,7 +1338,7 @@ public final class NodeDocument extends
if (context.getBranches().getBranch(readRevision) == null
&& !readRevision.isBranch()) {
// resolve commit revision
- revision = Utils.resolveCommitRevision(revision, commitValue);
+ revision = resolveCommitRevision(revision, commitValue);
// readRevision is not from a branch
// compare resolved revision as is
return !isRevisionNewer(context, revision, readRevision);
@@ -1364,7 +1357,7 @@ public final class NodeDocument extends
return false;
}
}
- return includeRevision(context, Utils.resolveCommitRevision(revision, commitValue), readRevision);
+ return includeRevision(context, resolveCommitRevision(revision, commitValue), readRevision);
}
/**
@@ -1427,6 +1420,7 @@ public final class NodeDocument extends
* @param readRevision the maximum revision
* @param validRevisions map of revision to commit value considered valid
* against the given readRevision.
+ * @param lastRevs to keep track of the most recent modification.
* @return the value, or null if not found
*/
@CheckForNull
@@ -1434,14 +1428,10 @@ public final class NodeDocument extends
@Nonnull Map<Revision, String> valueMap,
@Nullable Revision min,
@Nonnull Revision readRevision,
- @Nonnull Map<Revision, String> validRevisions) {
+ @Nonnull Map<Revision, String> validRevisions,
+ @Nonnull LastRevs lastRevs) {
for (Map.Entry<Revision, String> entry : valueMap.entrySet()) {
Revision propRev = entry.getKey();
- // ignore revisions newer than readRevision
- // -> these are not visible anyway
- if (isRevisionNewer(context, propRev, readRevision)) {
- continue;
- }
String commitValue = validRevisions.get(propRev);
if (commitValue == null) {
// resolve revision
@@ -1454,15 +1444,21 @@ public final class NodeDocument extends
continue;
}
}
- if (min != null && isRevisionNewer(context, min,
- Utils.resolveCommitRevision(propRev, commitValue))) {
+
+ Revision commitRev = resolveCommitRevision(propRev, commitValue);
+ if (Utils.isCommitted(commitValue)) {
+ lastRevs.update(commitRev);
+ } else {
+ // branch commit
+ lastRevs.updateBranch(commitRev.asBranchRevision());
+ }
+
+ if (min != null && isRevisionNewer(context, min, commitRev)) {
continue;
}
if (isValidRevision(context, propRev, commitValue, readRevision, validRevisions)) {
// TODO: need to check older revisions as well?
- return new Value(
- Utils.resolveCommitRevision(propRev, commitValue),
- entry.getValue());
+ return new Value(commitRev, entry.getValue());
}
}
return null;
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/SplitOperations.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/SplitOperations.java?rev=1643808&r1=1643807&r2=1643808&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/SplitOperations.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/SplitOperations.java Mon Dec 8 13:13:22 2014
@@ -30,12 +30,10 @@ import java.util.TreeMap;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
import org.apache.jackrabbit.oak.plugins.document.memory.MemoryDocumentStore;
import org.apache.jackrabbit.oak.plugins.document.util.Utils;
-import com.google.common.base.Predicate;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
@@ -50,11 +48,11 @@ import static org.apache.jackrabbit.oak.
import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.SPLIT_RATIO;
import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.SplitDocType;
import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.isCommitRootEntry;
-import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.isDeletedEntry;
import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.isRevisionsEntry;
import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.removePrevious;
import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.setHasBinary;
import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.setPrevious;
+import static org.apache.jackrabbit.oak.plugins.document.util.Utils.PROPERTY_OR_DELETED;
import static org.apache.jackrabbit.oak.plugins.document.util.Utils.isRevisionNewer;
/**
@@ -64,14 +62,6 @@ class SplitOperations {
private static final DocumentStore STORE = new MemoryDocumentStore();
- private static final Predicate<String> PROPERTY_OR_DELETED =
- new Predicate<String>() {
- @Override
- public boolean apply(@Nullable String input) {
- return Utils.isPropertyName(input) || isDeletedEntry(input);
- }
- };
-
private final NodeDocument doc;
private final String path;
private final String id;
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java?rev=1643808&r1=1643807&r2=1643808&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java Mon Dec 8 13:13:22 2014
@@ -32,17 +32,20 @@ import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
+import com.google.common.base.Predicate;
import com.mongodb.BasicDBObject;
import org.apache.commons.codec.binary.Hex;
import org.apache.jackrabbit.oak.commons.PathUtils;
import org.apache.jackrabbit.oak.plugins.document.Revision;
import org.apache.jackrabbit.oak.plugins.document.RevisionContext;
+import org.apache.jackrabbit.oak.plugins.document.StableRevisionComparator;
import org.bson.types.ObjectId;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import static com.google.common.base.Preconditions.checkNotNull;
+import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.isDeletedEntry;
/**
* Utility methods.
@@ -78,6 +81,16 @@ public class Utils {
private static final Charset UTF_8 = Charset.forName("UTF-8");
/**
+ * A predicate for property and _deleted names.
+ */
+ public static final Predicate<String> PROPERTY_OR_DELETED = new Predicate<String>() {
+ @Override
+ public boolean apply(@Nullable String input) {
+ return Utils.isPropertyName(input) || isDeletedEntry(input);
+ }
+ };
+
+ /**
* Make sure the name string does not contain unnecessary baggage (shared
* strings).
* <p>
@@ -462,4 +475,23 @@ public class Utils {
return context.getRevisionComparator().compare(x, previous) > 0;
}
+ /**
+ * Returns the revision with the newer timestamp or {@code null} if both
+ * revisions are {@code null}. The implementation will return the first
+ * revision if both have the same timestamp.
+ *
+ * @param a the first revision (or {@code null}).
+ * @param b the second revision (or {@code null}).
+ * @return the revision with the newer timestamp.
+ */
+ @CheckForNull
+ public static Revision max(@Nullable Revision a, @Nullable Revision b) {
+ if (a == null) {
+ return b;
+ } else if (b == null) {
+ return a;
+ }
+ return StableRevisionComparator.INSTANCE.compare(a, b) >= 0 ? a : b;
+ }
+
}
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java?rev=1643808&r1=1643807&r2=1643808&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java Mon Dec 8 13:13:22 2014
@@ -253,11 +253,9 @@ public class DocumentNodeStoreTest {
String id = Utils.getIdFromPath("/foo/node");
NodeDocument doc = docStore.find(Collection.NODES, id);
assertNotNull("document with id " + id + " does not exist", doc);
- assertTrue(!doc.getLastRev().isEmpty());
id = Utils.getIdFromPath("/bar/node");
doc = docStore.find(Collection.NODES, id);
assertNotNull("document with id " + id + " does not exist", doc);
- assertTrue(!doc.getLastRev().isEmpty());
mk.dispose();
}
@@ -395,30 +393,6 @@ public class DocumentNodeStoreTest {
nodeStore3.dispose();
}
- // OAK-1820
- @Test
- public void setLastRevOnCommitForNewNode() throws Exception {
- DocumentNodeStore ns = new DocumentMK.Builder()
- .setAsyncDelay(0).getNodeStore();
- // add a first child node. this will set the children flag on root
- // and move the commit root to the root
- NodeBuilder builder = ns.getRoot().builder();
- builder.child("foo");
- ns.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
-
- // the second time, the added node is also the commit root, this
- // is the case we are interested in
- builder = ns.getRoot().builder();
- builder.child("bar");
- ns.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
-
- NodeDocument doc = ns.getDocumentStore().find(NODES,
- Utils.getIdFromPath("/bar"));
- assertEquals(1, doc.getLastRev().size());
-
- ns.dispose();
- }
-
@Test
public void modifiedReset() throws Exception {
Clock clock = new Clock.Virtual();
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgentTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgentTest.java?rev=1643808&r1=1643807&r2=1643808&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgentTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgentTest.java Mon Dec 8 13:13:22 2014
@@ -127,8 +127,7 @@ public class LastRevRecoveryAgentTest {
b2.child("x").child("y").child("z").setProperty("foo", "bar");
ds2.merge(b2, EmptyHook.INSTANCE, CommitInfo.EMPTY);
- NodeDocument z1 = getDocument(ds1, "/x/y/z");
- Revision zlastRev2 = z1.getLastRev().get(c2Id);
+ Revision zlastRev2 = ds2.getHeadRevision();
long leaseTime = ds1.getClusterInfo().getLeaseTime();
ds1.runBackgroundOperations();
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryTest.java?rev=1643808&r1=1643807&r2=1643808&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryTest.java Mon Dec 8 13:13:22 2014
@@ -29,7 +29,6 @@ import org.junit.Before;
import org.junit.Test;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
public class LastRevRecoveryTest {
@@ -87,7 +86,9 @@ public class LastRevRecoveryTest {
NodeDocument x1 = getDocument(ds1, "/x");
Revision zlastRev2 = z1.getLastRev().get(c2Id);
- assertNotNull(zlastRev2);
+ // /x/y/z is a new node and does not have a _lastRev
+ assertNull(zlastRev2);
+ Revision head2 = ds2.getHeadRevision();
//lastRev should not be updated for C #2
assertNull(y1.getLastRev().get(c2Id));
@@ -98,9 +99,9 @@ public class LastRevRecoveryTest {
recovery.recover(Iterators.forArray(x1,z1), c2Id);
//Post recovery the lastRev should be updated for /x/y and /x
- assertEquals(zlastRev2, getDocument(ds1, "/x/y").getLastRev().get(c2Id));
- assertEquals(zlastRev2, getDocument(ds1, "/x").getLastRev().get(c2Id));
- assertEquals(zlastRev2, getDocument(ds1, "/").getLastRev().get(c2Id));
+ assertEquals(head2, getDocument(ds1, "/x/y").getLastRev().get(c2Id));
+ assertEquals(head2, getDocument(ds1, "/x").getLastRev().get(c2Id));
+ assertEquals(head2, getDocument(ds1, "/").getLastRev().get(c2Id));
}
private NodeDocument getDocument(DocumentNodeStore nodeStore, String path) {
Added: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevTest.java?rev=1643808&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevTest.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevTest.java Mon Dec 8 13:13:22 2014
@@ -0,0 +1,69 @@
+/*
+ * 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 org.apache.jackrabbit.oak.spi.commit.CommitInfo;
+import org.apache.jackrabbit.oak.spi.commit.EmptyHook;
+import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
+import org.junit.Test;
+
+import static org.apache.jackrabbit.oak.plugins.document.Collection.NODES;
+import static org.apache.jackrabbit.oak.plugins.document.util.Utils.getIdFromPath;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+/**
+ * Checks if _lastRev entries are correctly set and updated.
+ */
+public class LastRevTest {
+
+ @Test
+ public void lastRev() throws Exception {
+ DocumentNodeStore store = new DocumentMK.Builder()
+ .setAsyncDelay(0).getNodeStore();
+ DocumentStore docStore = store.getDocumentStore();
+
+ NodeBuilder root = store.getRoot().builder();
+ for (int i = 0; i < 10; i++) {
+ NodeBuilder child = root.child("child-" + i);
+ for (int j = 0; j < 10; j++) {
+ child.child("test-" + j);
+ }
+ }
+ store.merge(root, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+ store.runBackgroundOperations();
+
+ for (int i = 0; i < 10; i++) {
+ String parentPath = "/child-" + i;
+ assertLastRevSize(docStore, parentPath, 0);
+ for (int j = 0; j < 10; j++) {
+ String path = parentPath + "/test-" + j;
+ assertLastRevSize(docStore, path, 0);
+ }
+ }
+
+ store.dispose();
+ }
+
+ private static void assertLastRevSize(DocumentStore store,
+ String path, int size) {
+ NodeDocument doc = store.find(NODES, getIdFromPath(path));
+ assertNotNull(doc);
+ assertEquals("_lastRev: " + doc.getLastRev(), size, doc.getLastRev().size());
+ }
+
+}
Propchange: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevTest.java
------------------------------------------------------------------------------
svn:eol-style = native
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/UtilsTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/UtilsTest.java?rev=1643808&r1=1643807&r2=1643808&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/UtilsTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/UtilsTest.java Mon Dec 8 13:13:22 2014
@@ -24,6 +24,7 @@ import org.junit.Test;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
/**
@@ -91,4 +92,18 @@ public class UtilsTest {
time = System.currentTimeMillis() - time;
System.out.println(time);
}
+
+ @Test
+ public void max() {
+ Revision a = new Revision(42, 0, 1);
+ Revision b = new Revision(43, 0, 1);
+ assertSame(b, Utils.max(a, b));
+
+ Revision a1 = new Revision(42, 1, 1);
+ assertSame(a1, Utils.max(a, a1));
+
+ assertSame(a, Utils.max(a, null));
+ assertSame(a, Utils.max(null, a));
+ assertNull(Utils.max(null, null));
+ }
}