You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by st...@apache.org on 2006/11/03 17:00:57 UTC
svn commit: r470869 - in /jackrabbit/trunk/jackrabbit/src:
main/java/org/apache/jackrabbit/core/state/
test/java/org/apache/jackrabbit/core/
Author: stefan
Date: Fri Nov 3 08:00:56 2006
New Revision: 470869
URL: http://svn.apache.org/viewvc?view=rev&rev=470869
Log:
JJCR-584: Improve handling of concurrent node modifications
Added:
jackrabbit/trunk/jackrabbit/src/main/java/org/apache/jackrabbit/core/state/NodeStateMerger.java (with props)
jackrabbit/trunk/jackrabbit/src/test/java/org/apache/jackrabbit/core/ConcurrentNodeModificationTest.java (with props)
Modified:
jackrabbit/trunk/jackrabbit/src/main/java/org/apache/jackrabbit/core/state/ChangeLog.java
jackrabbit/trunk/jackrabbit/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java
jackrabbit/trunk/jackrabbit/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java
jackrabbit/trunk/jackrabbit/src/test/java/org/apache/jackrabbit/core/TestAll.java
Modified: jackrabbit/trunk/jackrabbit/src/main/java/org/apache/jackrabbit/core/state/ChangeLog.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit/src/main/java/org/apache/jackrabbit/core/state/ChangeLog.java?view=diff&rev=470869&r1=470868&r2=470869
==============================================================================
--- jackrabbit/trunk/jackrabbit/src/main/java/org/apache/jackrabbit/core/state/ChangeLog.java (original)
+++ jackrabbit/trunk/jackrabbit/src/main/java/org/apache/jackrabbit/core/state/ChangeLog.java Fri Nov 3 08:00:56 2006
@@ -131,6 +131,17 @@
}
/**
+ * Return a flag indicating whether a given item state is marked as
+ * deleted in this log.
+ *
+ * @return <code>true</code> if item state is marked as deleted in this
+ * log; <code>false</code> otherwise
+ */
+ public boolean deleted(ItemId id) {
+ return deletedStates.containsKey(id);
+ }
+
+ /**
* Return a node references object given its id. Returns
* <code>null</code> if the node reference is not in the modified
* section.
Added: jackrabbit/trunk/jackrabbit/src/main/java/org/apache/jackrabbit/core/state/NodeStateMerger.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit/src/main/java/org/apache/jackrabbit/core/state/NodeStateMerger.java?view=auto&rev=470869
==============================================================================
--- jackrabbit/trunk/jackrabbit/src/main/java/org/apache/jackrabbit/core/state/NodeStateMerger.java (added)
+++ jackrabbit/trunk/jackrabbit/src/main/java/org/apache/jackrabbit/core/state/NodeStateMerger.java Fri Nov 3 08:00:56 2006
@@ -0,0 +1,211 @@
+/*
+ * 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.core.state;
+
+import org.apache.jackrabbit.core.PropertyId;
+import org.apache.jackrabbit.core.ItemId;
+import org.apache.jackrabbit.name.QName;
+
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.HashSet;
+
+/**
+ * Internal utility class used for merging concurrent changes that occured
+ * on different <code>NodeState</code> instances representing the same node.
+ * <p/>
+ * See http://issues.apache.org/jira/browse/JCR-584.
+ */
+class NodeStateMerger {
+
+ /**
+ * Tries to silently merge the given <code>state</code> with its
+ * externally (e.g. through another session) modified overlayed state
+ * in order to avoid an <code>InvalidItemStateException</code>.
+ * <p/>
+ * See http://issues.apache.org/jira/browse/JCR-584.
+ *
+ * @param state node state whose modified overlayed state should be
+ * merged
+ * @param context used for analyzing the context of the modifications
+ * @return true if the changes could be successfully merged into the
+ * given node state; false otherwise
+ */
+ static boolean merge(NodeState state, MergeContext context) {
+
+ NodeState overlayedState = (NodeState) state.getOverlayedState();
+ if (overlayedState == null
+ || state.getModCount() == overlayedState.getModCount()) {
+ return false;
+ }
+
+ /**
+ * some examples for trivial non-conflicting changes:
+ * - s1 added child node a, s2 removes child node b
+ * - s1 adds child node a, s2 adds child node b
+ * - s1 adds child node a, s2 adds mixin type
+ *
+ * conflicting changes causing staleness:
+ * - s1 added non-sns child node or property a,
+ * s2 added non-sns child node or property a => name clash
+ * - either session reordered child nodes
+ * (some combinations could possibly be merged)
+ * - either session moved node
+ */
+
+ // compare current transient state with externally modified
+ // overlayed state and determine what has been changed by whom
+
+ // child node entries order
+ if (!state.getReorderedChildNodeEntries().isEmpty()) {
+ // for now we don't even try to merge the result of
+ // a reorder operation
+ return false;
+ }
+
+ // mixin types
+ if (!state.getMixinTypeNames().equals(overlayedState.getMixinTypeNames())) {
+ // the mixins have been modified but by just looking at the diff we
+ // can't determine where the change happened since the diffs of either
+ // removing a mixin from the overlayed or adding a mixin to the
+ // transient state would look identical...
+ return false;
+ }
+
+ // parent id
+ if (state.getParentId() != null
+ && !state.getParentId().equals(overlayedState.getParentId())) {
+ return false;
+ }
+
+ // child node entries
+ if (!state.getChildNodeEntries().equals(
+ overlayedState.getChildNodeEntries())) {
+ ArrayList added = new ArrayList();
+ ArrayList removed = new ArrayList();
+
+ for (Iterator iter = state.getAddedChildNodeEntries().iterator();
+ iter.hasNext();) {
+ NodeState.ChildNodeEntry cne =
+ (NodeState.ChildNodeEntry) iter.next();
+
+ if (context.isAdded(cne.getId())) {
+ // a new child node entry has been added to this state;
+ // check for name collisions with other state
+ if (overlayedState.hasPropertyName(cne.getName())
+ && !context.isDeleted(new PropertyId(state.getNodeId(), cne.getName()))) {
+ // conflicting names
+ return false;
+ }
+ if (overlayedState.hasChildNodeEntry(cne.getName())) {
+ // conflicting names
+ if (cne.getIndex() < 2) {
+ // todo check if same-name siblings are allowed
+ return false;
+ }
+ // assume same-name siblings are allowed since index is >= 2
+ }
+
+ added.add(cne);
+ }
+ }
+
+ for (Iterator iter = state.getRemovedChildNodeEntries().iterator();
+ iter.hasNext();) {
+ NodeState.ChildNodeEntry cne =
+ (NodeState.ChildNodeEntry) iter.next();
+ if (context.isDeleted(cne.getId())) {
+ // a child node entry has been removed from this node state
+ removed.add(cne);
+ }
+ }
+
+ // copy child node antries from other state and
+ // re-apply changes made on this state
+ state.setChildNodeEntries(overlayedState.getChildNodeEntries());
+ for (Iterator iter = added.iterator(); iter.hasNext();) {
+ NodeState.ChildNodeEntry cne =
+ (NodeState.ChildNodeEntry) iter.next();
+ state.addChildNodeEntry(cne.getName(), cne.getId());
+ }
+ for (Iterator iter = removed.iterator(); iter.hasNext();) {
+ NodeState.ChildNodeEntry cne =
+ (NodeState.ChildNodeEntry) iter.next();
+ state.removeChildNodeEntry(cne.getId());
+ }
+ }
+
+ // property names
+ if (!state.getPropertyNames().equals(
+ overlayedState.getPropertyNames())) {
+ HashSet added = new HashSet();
+ HashSet removed = new HashSet();
+
+ for (Iterator iter = state.getAddedPropertyNames().iterator();
+ iter.hasNext();) {
+ QName name = (QName) iter.next();
+ PropertyId propId =
+ new PropertyId(state.getNodeId(), name);
+ if (context.isAdded(propId)) {
+ // a new property name has been added to this state;
+ // check for name collisions
+ if (overlayedState.hasPropertyName(name)
+ || overlayedState.hasChildNodeEntry(name)) {
+ // conflicting names
+ return false;
+ }
+
+ added.add(name);
+ }
+ }
+ for (Iterator iter = state.getRemovedPropertyNames().iterator();
+ iter.hasNext();) {
+ QName name = (QName) iter.next();
+ PropertyId propId =
+ new PropertyId(state.getNodeId(), name);
+ if (context.isDeleted(propId)) {
+ // a property name has been removed from this state
+ removed.add(name);
+ }
+ }
+
+ // copy property names from other and
+ // re-apply changes made on this state
+ state.setPropertyNames(overlayedState.getPropertyNames());
+ for (Iterator iter = added.iterator(); iter.hasNext();) {
+ QName name = (QName) iter.next();
+ state.addPropertyName(name);
+ }
+ for (Iterator iter = removed.iterator(); iter.hasNext();) {
+ QName name = (QName) iter.next();
+ state.removePropertyName(name);
+ }
+ }
+
+ // finally sync modification count
+ state.setModCount(overlayedState.getModCount());
+
+ return true;
+ }
+
+ //-----------------------------------------------------< inner interfaces >
+
+ static interface MergeContext {
+ boolean isAdded(ItemId id);
+ boolean isDeleted(ItemId id);
+ }
+}
Propchange: jackrabbit/trunk/jackrabbit/src/main/java/org/apache/jackrabbit/core/state/NodeStateMerger.java
------------------------------------------------------------------------------
svn:eol-style = native
Propchange: jackrabbit/trunk/jackrabbit/src/main/java/org/apache/jackrabbit/core/state/NodeStateMerger.java
------------------------------------------------------------------------------
svn:keywords = author date id revision url
Modified: jackrabbit/trunk/jackrabbit/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java?view=diff&rev=470869&r1=470868&r2=470869
==============================================================================
--- jackrabbit/trunk/jackrabbit/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java (original)
+++ jackrabbit/trunk/jackrabbit/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java Fri Nov 3 08:00:56 2006
@@ -112,6 +112,7 @@
}
//-------------------------------------------------------------< Dumpable >
+
/**
* {@inheritDoc}
*/
@@ -135,6 +136,7 @@
}
//-----------------------------------------------------< ItemStateManager >
+
/**
* {@inheritDoc}
*/
@@ -204,6 +206,7 @@
}
//--------------------------------------------< UpdatableItemStateManager >
+
/**
* {@inheritDoc}
*/
@@ -516,6 +519,7 @@
}
//------< methods for creating & discarding transient ItemState instances >
+
/**
* @param id
* @param nodeTypeName
@@ -708,6 +712,7 @@
/**
* Add an <code>ItemStateListener</code>
+ *
* @param listener the new listener to be informed on modifications
*/
public void addListener(ItemStateListener listener) {
@@ -716,6 +721,7 @@
/**
* Remove an <code>ItemStateListener</code>
+ *
* @param listener an existing listener
*/
public void removeListener(ItemStateListener listener) {
@@ -772,6 +778,25 @@
// local state was modified
ItemState transientState = transientStore.get(modified.getId());
if (transientState != null) {
+ if (transientState.isNode() && !transientState.isStale()) {
+ // try to silently merge non-conflicting changes (JCR-584)
+ NodeStateMerger.MergeContext context =
+ new NodeStateMerger.MergeContext() {
+ public boolean isAdded(ItemId id) {
+ ItemState is = transientStore.get(id);
+ return is != null
+ && is.getStatus() == ItemState.STATUS_NEW;
+ }
+
+ public boolean isDeleted(ItemId id) {
+ return atticStore.contains(id);
+ }
+ };
+ if (NodeStateMerger.merge((NodeState) transientState, context)) {
+ // merge succeeded
+ return;
+ }
+ }
transientState.setStatus(ItemState.STATUS_STALE_MODIFIED);
visibleState = transientState;
}
Modified: jackrabbit/trunk/jackrabbit/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java?view=diff&rev=470869&r1=470868&r2=470869
==============================================================================
--- jackrabbit/trunk/jackrabbit/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java (original)
+++ jackrabbit/trunk/jackrabbit/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java Fri Nov 3 08:00:56 2006
@@ -543,9 +543,33 @@
ItemState state = (ItemState) iter.next();
state.connect(getItemState(state.getId()));
if (state.isStale()) {
- String msg = state.getId() + " has been modified externally";
- log.debug(msg);
- throw new StaleItemStateException(msg);
+ boolean merged = false;
+ if (state.isNode()) {
+ NodeStateMerger.MergeContext context =
+ new NodeStateMerger.MergeContext() {
+ public boolean isAdded(ItemId id) {
+ try {
+ ItemState is = local.get(id);
+ return is != null
+ && is.getStatus() == ItemState.STATUS_NEW;
+ } catch (NoSuchItemStateException e) {
+ return false;
+ }
+ }
+
+ public boolean isDeleted(ItemId id) {
+ return local.deleted(id);
+ }
+ };
+
+ merged = NodeStateMerger.merge((NodeState) state, context);
+ }
+ if (!merged) {
+ String msg = state.getId() + " has been modified externally";
+ log.debug(msg);
+ throw new StaleItemStateException(msg);
+ }
+ // merge succeeded, fall through
}
// update modification count (will be persisted as well)
Added: jackrabbit/trunk/jackrabbit/src/test/java/org/apache/jackrabbit/core/ConcurrentNodeModificationTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit/src/test/java/org/apache/jackrabbit/core/ConcurrentNodeModificationTest.java?view=auto&rev=470869
==============================================================================
--- jackrabbit/trunk/jackrabbit/src/test/java/org/apache/jackrabbit/core/ConcurrentNodeModificationTest.java (added)
+++ jackrabbit/trunk/jackrabbit/src/test/java/org/apache/jackrabbit/core/ConcurrentNodeModificationTest.java Fri Nov 3 08:00:56 2006
@@ -0,0 +1,140 @@
+/*
+ * 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.core;
+
+import org.apache.jackrabbit.test.AbstractJCRTest;
+
+import javax.jcr.Node;
+import javax.jcr.Session;
+import javax.jcr.Value;
+import java.util.Random;
+import java.util.ArrayList;
+import java.util.Iterator;
+
+/**
+ * Performs a test with n sessions concurrently performing non-conflicting
+ * modifications on the <i>same</i> node.
+ * <p/>
+ * See http://issues.apache.org/jira/browse/JCR-584.
+ */
+public class ConcurrentNodeModificationTest extends AbstractJCRTest {
+
+ private static final int NUM_ITERATIONS = 1;
+ private static final int NUM_SESSIONS = 10;
+ private static final int NUM_NODES = 100;
+
+ final ArrayList exceptions = new ArrayList();
+
+ /**
+ * Runs the test.
+ */
+ public void testConcurrentNodeModificationSessions() throws Exception {
+ int n = NUM_ITERATIONS;
+ while (n-- > 0) {
+ Thread[] threads = new Thread[NUM_SESSIONS];
+ for (int i = 0; i < threads.length; i++) {
+ // create new session
+ Session session = helper.getSuperuserSession();
+ TestSession ts = new TestSession("s" + i, session);
+ Thread t = new Thread(ts);
+ t.setName((NUM_ITERATIONS - n) + "-s" + i);
+ t.start();
+ log.println("Thread#" + i + " started");
+ threads[i] = t;
+ Thread.sleep(100);
+ }
+ for (int i = 0; i < threads.length; i++) {
+ threads[i].join();
+ }
+ }
+
+ if (!exceptions.isEmpty()) {
+ Exception e = null;
+ for (Iterator it = exceptions.iterator(); it.hasNext();) {
+ e = (Exception) it.next();
+ e.printStackTrace(log);
+ }
+ throw e;
+ //fail();
+ }
+ }
+
+ //--------------------------------------------------------< inner classes >
+ class TestSession implements Runnable {
+
+ Session session;
+ String identity;
+ Random r;
+
+ TestSession(String identity, Session s) {
+ session = s;
+ this.identity = identity;
+ r = new Random();
+ }
+
+ private void randomSleep() {
+ long l = r.nextInt(900) + 200;
+ try {
+ Thread.sleep(l);
+ } catch (InterruptedException ie) {
+ }
+ }
+
+ public void run() {
+
+ log.println("started.");
+ String state = "";
+ try {
+ Node n = session.getRootNode().getNode(testPath);
+
+ String propName = "testprop" + Math.random();
+
+ state = "setting property " + propName;
+ n.setProperty(propName, "Hello World!");
+ session.save();
+ randomSleep();
+
+ state = "removing property " + propName;
+ n.setProperty(propName, (Value) null);
+ session.save();
+ randomSleep();
+
+ for (int i = 0; i < NUM_NODES; i++) {
+ state = "adding subnode " + i;
+ //Node n1 = n.addNode("x" + i, "nt:unstructured");
+ Node n1 = n.addNode("x" + identity + i, "nt:unstructured");
+ state = "adding property to subnode " + i;
+ n1.setProperty("testprop", "xxx");
+ if (i % 10 == 0) {
+ state = "saving pending subnodes";
+ session.save();
+ }
+ randomSleep();
+ }
+ session.save();
+ } catch (Exception e) {
+ log.println("Exception while " + state + ": " + e.getMessage());
+ //e.printStackTrace();
+ exceptions.add(e);
+ } finally {
+ session.logout();
+ }
+
+ log.println("ended.");
+ }
+ }
+}
Propchange: jackrabbit/trunk/jackrabbit/src/test/java/org/apache/jackrabbit/core/ConcurrentNodeModificationTest.java
------------------------------------------------------------------------------
svn:eol-style = native
Propchange: jackrabbit/trunk/jackrabbit/src/test/java/org/apache/jackrabbit/core/ConcurrentNodeModificationTest.java
------------------------------------------------------------------------------
svn:keywords = author date id revision url
Modified: jackrabbit/trunk/jackrabbit/src/test/java/org/apache/jackrabbit/core/TestAll.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit/src/test/java/org/apache/jackrabbit/core/TestAll.java?view=diff&rev=470869&r1=470868&r2=470869
==============================================================================
--- jackrabbit/trunk/jackrabbit/src/test/java/org/apache/jackrabbit/core/TestAll.java (original)
+++ jackrabbit/trunk/jackrabbit/src/test/java/org/apache/jackrabbit/core/TestAll.java Fri Nov 3 08:00:56 2006
@@ -37,6 +37,7 @@
//suite.addTestSuite(ConcurrencyTest.class);
//suite.addTestSuite(ConcurrentSaveTest.class);
+ //suite.addTestSuite(ConcurrentNodeModificationTest.class);
//suite.addTestSuite(LockTest.class);
suite.addTestSuite(NamespaceRegistryImplTest.class);
suite.addTestSuite(TransientRepositoryTest.class);