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/05/20 15:16:37 UTC
svn commit: r1596233 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/plugins/document/
main/java/org/apache/jackrabbit/oak/plugins/document/mongo/
test/java/org/apache/jackrabbit/oak/plugins/document/
Author: mreutegg
Date: Tue May 20 13:16:36 2014
New Revision: 1596233
URL: http://svn.apache.org/r1596233
Log:
OAK-1822: NodeDocument _modified may go back in time
Revert changes because $max is only available in MongoDB 2.6
Removed:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreIT.java
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/UpdateOp.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UpdateUtils.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDocumentStore.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentStoreTest.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.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=1596233&r1=1596232&r2=1596233&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 Tue May 20 13:16:36 2014
@@ -1205,7 +1205,7 @@ public final class NodeDocument extends
public static void setModified(@Nonnull UpdateOp op,
@Nonnull Revision revision) {
- checkNotNull(op).max(MODIFIED_IN_SECS, getModifiedInSecs(checkNotNull(revision).getTimestamp()));
+ checkNotNull(op).set(MODIFIED_IN_SECS, getModifiedInSecs(checkNotNull(revision).getTimestamp()));
}
public static void setRevision(@Nonnull UpdateOp op,
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UpdateOp.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UpdateOp.java?rev=1596233&r1=1596232&r2=1596233&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UpdateOp.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UpdateOp.java Tue May 20 13:16:36 2014
@@ -131,7 +131,9 @@ public final class UpdateOp {
* @param value the value
*/
void setMapEntry(@Nonnull String property, @Nonnull Revision revision, Object value) {
- Operation op = new Operation(Operation.Type.SET_MAP_ENTRY, value);
+ Operation op = new Operation();
+ op.type = Operation.Type.SET_MAP_ENTRY;
+ op.value = value;
changes.put(new Key(property, checkNotNull(revision)), op);
}
@@ -143,7 +145,8 @@ public final class UpdateOp {
* @param revision the revision
*/
public void removeMapEntry(@Nonnull String property, @Nonnull Revision revision) {
- Operation op = new Operation(Operation.Type.REMOVE_MAP_ENTRY, null);
+ Operation op = new Operation();
+ op.type = Operation.Type.REMOVE_MAP_ENTRY;
changes.put(new Key(property, checkNotNull(revision)), op);
}
@@ -154,23 +157,9 @@ public final class UpdateOp {
* @param value the value
*/
void set(String property, Object value) {
- Operation op = new Operation(Operation.Type.SET, value);
- changes.put(new Key(property, null), op);
- }
-
- /**
- * Set the property to the given value if the new value is higher than the
- * existing value. The property is also set to the given value if the
- * property does not yet exist.
- * <p>
- * The result of a max operation with different types of values is
- * undefined.
- *
- * @param property the name of the property to set.
- * @param value the new value for the property.
- */
- <T> void max(String property, Comparable<T> value) {
- Operation op = new Operation(Operation.Type.MAX, value);
+ Operation op = new Operation();
+ op.type = Operation.Type.SET;
+ op.value = value;
changes.put(new Key(property, null), op);
}
@@ -198,7 +187,9 @@ public final class UpdateOp {
if (isNew) {
throw new IllegalStateException("Cannot use containsMapEntry() on new document");
}
- Operation op = new Operation(Operation.Type.CONTAINS_MAP_ENTRY, exists);
+ Operation op = new Operation();
+ op.type = Operation.Type.CONTAINS_MAP_ENTRY;
+ op.value = exists;
changes.put(new Key(property, checkNotNull(revision)), op);
}
@@ -209,7 +200,9 @@ public final class UpdateOp {
* @param value the increment
*/
public void increment(@Nonnull String property, long value) {
- Operation op = new Operation(Operation.Type.INCREMENT, value);
+ Operation op = new Operation();
+ op.type = Operation.Type.INCREMENT;
+ op.value = value;
changes.put(new Key(property, null), op);
}
@@ -246,14 +239,6 @@ public final class UpdateOp {
SET,
/**
- * Set the value if the new value is higher than the existing value.
- * The new value is also considered higher, when there is no
- * existing value.
- * The sub-key is not used.
- */
- MAX,
-
- /**
* Increment the Long value with the provided Long value.
* The sub-key is not used.
*/
@@ -282,17 +267,12 @@ public final class UpdateOp {
/**
* The operation type.
*/
- public final Type type;
+ public Type type;
/**
* The value, if any.
*/
- public final Object value;
-
- Operation(Type type, Object value) {
- this.type = checkNotNull(type);
- this.value = value;
- }
+ public Object value;
@Override
public String toString() {
@@ -303,16 +283,18 @@ public final class UpdateOp {
Operation reverse = null;
switch (type) {
case INCREMENT:
- reverse = new Operation(Type.INCREMENT, -(Long) value);
+ reverse = new Operation();
+ reverse.type = Type.INCREMENT;
+ reverse.value = -(Long) value;
break;
case SET:
- case MAX:
case REMOVE_MAP_ENTRY:
case CONTAINS_MAP_ENTRY:
// nothing to do
break;
case SET_MAP_ENTRY:
- reverse = new Operation(Type.REMOVE_MAP_ENTRY, null);
+ reverse = new Operation();
+ reverse.type = Type.REMOVE_MAP_ENTRY;
break;
}
return reverse;
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UpdateUtils.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UpdateUtils.java?rev=1596233&r1=1596232&r2=1596233&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UpdateUtils.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UpdateUtils.java Tue May 20 13:16:36 2014
@@ -44,9 +44,7 @@ public class UpdateUtils {
* @param comparator
* the revision comparator.
*/
- public static void applyChanges(@Nonnull Document doc,
- @Nonnull UpdateOp update,
- @Nonnull Comparator<Revision> comparator) {
+ public static void applyChanges(@Nonnull Document doc, @Nonnull UpdateOp update, @Nonnull Comparator<Revision> comparator) {
for (Entry<Key, Operation> e : checkNotNull(update).getChanges().entrySet()) {
Key k = e.getKey();
Operation op = e.getValue();
@@ -55,15 +53,6 @@ public class UpdateUtils {
doc.put(k.toString(), op.value);
break;
}
- case MAX: {
- Comparable newValue = (Comparable) op.value;
- Object old = doc.get(k.toString());
- //noinspection unchecked
- if (old == null || newValue.compareTo(old) > 0) {
- doc.put(k.toString(), op.value);
- }
- break;
- }
case INCREMENT: {
Object old = doc.get(k.toString());
Long x = (Long) op.value;
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDocumentStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDocumentStore.java?rev=1596233&r1=1596232&r2=1596233&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDocumentStore.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDocumentStore.java Tue May 20 13:16:36 2014
@@ -570,7 +570,6 @@ public class MongoDocumentStore implemen
Operation op = entry.getValue();
switch (op.type) {
case SET:
- case MAX:
case INCREMENT: {
inserts[i].put(k.toString(), op.value);
break;
@@ -966,7 +965,6 @@ public class MongoDocumentStore implemen
@Nonnull
private static DBObject createUpdate(UpdateOp updateOp) {
BasicDBObject setUpdates = new BasicDBObject();
- BasicDBObject maxUpdates = new BasicDBObject();
BasicDBObject incUpdates = new BasicDBObject();
BasicDBObject unsetUpdates = new BasicDBObject();
@@ -982,19 +980,18 @@ public class MongoDocumentStore implemen
}
Operation op = entry.getValue();
switch (op.type) {
- case SET:
- case SET_MAP_ENTRY: {
+ case SET: {
setUpdates.append(k.toString(), op.value);
break;
}
- case MAX: {
- maxUpdates.append(k.toString(), op.value);
- break;
- }
case INCREMENT: {
incUpdates.append(k.toString(), op.value);
break;
}
+ case SET_MAP_ENTRY: {
+ setUpdates.append(k.toString(), op.value);
+ break;
+ }
case REMOVE_MAP_ENTRY: {
unsetUpdates.append(k.toString(), "1");
break;
@@ -1006,9 +1003,6 @@ public class MongoDocumentStore implemen
if (!setUpdates.isEmpty()) {
update.append("$set", setUpdates);
}
- if (!maxUpdates.isEmpty()) {
- update.append("$max", maxUpdates);
- }
if (!incUpdates.isEmpty()) {
update.append("$inc", incUpdates);
}
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentStoreTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentStoreTest.java?rev=1596233&r1=1596232&r2=1596233&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentStoreTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentStoreTest.java Tue May 20 13:16:36 2014
@@ -34,19 +34,17 @@ public abstract class AbstractDocumentSt
protected String dsname;
protected DocumentStore ds;
- protected DocumentStoreFixture dsf;
protected List<String> removeMe = new ArrayList<String>();
static final Logger LOG = LoggerFactory.getLogger(AbstractDocumentStoreTest.class);
public AbstractDocumentStoreTest(DocumentStoreFixture dsf) {
- this.dsf = dsf;
this.ds = dsf.createDocumentStore();
this.dsname = dsf.getName();
}
@After
- public void cleanUp() throws Exception {
+ public void cleanUp() {
if (!removeMe.isEmpty()) {
long start = System.nanoTime();
try {
@@ -67,7 +65,6 @@ public abstract class AbstractDocumentSt
LOG.info(removeMe.size() + " documents removed in " + elapsed + "ms (" + rate + "/ms)");
}
}
- dsf.dispose();
}
@Parameterized.Parameters
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=1596233&r1=1596232&r2=1596233&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 Tue May 20 13:16:36 2014
@@ -51,6 +51,7 @@ import org.apache.jackrabbit.oak.spi.sta
import org.apache.jackrabbit.oak.spi.state.NodeState;
import org.apache.jackrabbit.oak.stats.Clock;
import org.junit.After;
+import org.junit.Ignore;
import org.junit.Test;
import com.google.common.collect.Iterables;
@@ -418,6 +419,7 @@ public class DocumentNodeStoreTest {
ns.dispose();
}
+ @Ignore("OAK-1822")
@Test
public void modifiedReset() throws Exception {
Clock clock = new Clock.Virtual();
@@ -467,6 +469,66 @@ public class DocumentNodeStoreTest {
ns2.dispose();
}
+ @Ignore("OAK-1822")
+ @Test
+ public void modifiedResetWithDiff() throws Exception {
+ Clock clock = new Clock.Virtual();
+ clock.waitUntil(System.currentTimeMillis());
+ Revision.setClock(clock);
+ MemoryDocumentStore docStore = new MemoryDocumentStore();
+ DocumentNodeStore ns1 = new DocumentMK.Builder()
+ .setDocumentStore(docStore).setClusterId(1)
+ .setAsyncDelay(0).clock(clock)
+ // use a no-op diff cache to simulate a cache miss
+ // when the diff is made later in the test
+ .setDiffCache(AmnesiaDiffCache.INSTANCE)
+ .getNodeStore();
+ NodeBuilder builder1 = ns1.getRoot().builder();
+ builder1.child("node");
+ for (int i = 0; i < DocumentMK.MANY_CHILDREN_THRESHOLD; i++) {
+ builder1.child("node-" + i);
+ }
+ ns1.merge(builder1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+ // make sure commit is visible to other node store instance
+ ns1.runBackgroundOperations();
+
+ DocumentNodeStore ns2 = new DocumentMK.Builder()
+ .setDocumentStore(docStore).setClusterId(2)
+ .setAsyncDelay(0).clock(clock).getNodeStore();
+
+ NodeBuilder builder2 = ns2.getRoot().builder();
+ builder2.child("node").child("child-a");
+ ns2.merge(builder2, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+
+ // wait at least _modified resolution. in reality the wait may
+ // not be necessary. e.g. when the clock passes the resolution boundary
+ // exactly at this time
+ clock.waitUntil(System.currentTimeMillis() +
+ SECONDS.toMillis(MODIFIED_IN_SECS_RESOLUTION + 1));
+
+ builder1 = ns1.getRoot().builder();
+ builder1.child("node").child("child-b");
+ ns1.merge(builder1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+ // remember root for diff
+ DocumentNodeState root1 = ns1.getRoot();
+
+ builder1 = root1.builder();
+ builder1.child("node").child("child-c");
+ ns1.merge(builder1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+ // remember root for diff
+ DocumentNodeState root2 = ns1.getRoot();
+
+ ns1.runBackgroundOperations();
+ ns2.runBackgroundOperations();
+
+ String diff = ns1.diffChildren(root2, root1);
+ // must report /node as changed
+ assertEquals("^\"node\":{}", diff);
+
+ ns1.dispose();
+ ns2.dispose();
+ }
+
private static class TestHook extends EditorHook {
TestHook(final String prefix) {