You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by un...@apache.org on 2012/06/14 14:55:29 UTC
svn commit: r1350221 - in /jackrabbit/trunk/jackrabbit-core/src:
main/java/org/apache/jackrabbit/core/persistence/bundle/ConsistencyCheckerImpl.java
test/java/org/apache/jackrabbit/core/data/ConsistencyCheckerImplTest.java
Author: unico
Date: Thu Jun 14 12:55:29 2012
New Revision: 1350221
URL: http://svn.apache.org/viewvc?rev=1350221&view=rev
Log:
JCR-3267 fix bundles immediately during checkbundle in order to avoid lost update problem; also add unit test for consistency fixing
Added:
jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/ConsistencyCheckerImplTest.java
Modified:
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/ConsistencyCheckerImpl.java
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/ConsistencyCheckerImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/ConsistencyCheckerImpl.java?rev=1350221&r1=1350220&r2=1350221&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/ConsistencyCheckerImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/ConsistencyCheckerImpl.java Thu Jun 14 12:55:29 2012
@@ -76,8 +76,6 @@ public class ConsistencyCheckerImpl {
boolean fix, Set<ReportItem> reports, String lostNFoundId)
throws RepositoryException {
int count = 0;
- Collection<NodePropBundle> modifications = new ArrayList<NodePropBundle>();
- Set<NodeId> orphaned = new HashSet<NodeId>();
NodeId lostNFound = null;
if (fix && lostNFoundId != null) {
@@ -116,9 +114,7 @@ public class ConsistencyCheckerImpl {
error(id.toString(), "No bundle found for id '"
+ id + "'");
} else {
- checkBundleConsistency(id, bundle, fix,
- modifications, lostNFound, orphaned,
- reports);
+ checkBundleConsistency(id, bundle, fix, lostNFound, reports);
count++;
if (count % 1000 == 0 && listener == null) {
@@ -174,8 +170,7 @@ public class ConsistencyCheckerImpl {
+ id + "'");
}
} else {
- checkBundleConsistency(id, bundle, fix, modifications,
- lostNFound, orphaned, reports);
+ checkBundleConsistency(id, bundle, fix, lostNFound, reports);
if (recursive) {
for (NodePropBundle.ChildNodeEntry entry : bundle
@@ -197,49 +192,6 @@ public class ConsistencyCheckerImpl {
}
}
- // repair collected broken bundles
- if (fix && !modifications.isEmpty()) {
- info(null, pm + ": Fixing " + modifications.size()
- + " inconsistent bundle(s)...");
- for (NodePropBundle bundle : modifications) {
- try {
- info(bundle.getId().toString(), pm + ": Fixing bundle '"
- + bundle.getId() + "'");
- bundle.markOld(); // use UPDATE instead of INSERT
- pm.storeBundle(bundle);
- pm.evictBundle(bundle.getId());
- } catch (ItemStateException e) {
- error(bundle.getId().toString(), pm
- + ": Error storing fixed bundle: " + e);
- }
- }
- }
-
- if (fix && lostNFoundId != null && !orphaned.isEmpty()) {
- // do we have things to add to "lost+found"?
- try {
- NodePropBundle lfBundle = pm.loadBundle(lostNFound);
- if (lfBundle == null) {
- error(lostNFoundId, "specified 'lost+found' node does not exist");
- } else if (!NameConstants.NT_UNSTRUCTURED.equals(lfBundle
- .getNodeTypeName())) {
- error(lostNFoundId, "specified 'lost+found' node is not of type nt:unstructered");
- } else {
- lfBundle.markOld();
- for (NodeId orphan : orphaned) {
- String nodeName = orphan + "-"
- + System.currentTimeMillis();
- lfBundle.addChildNodeEntry(NF.create("", nodeName),
- orphan);
- }
- pm.storeBundle(lfBundle);
- pm.evictBundle(lfBundle.getId());
- }
- } catch (Exception ex) {
- error(null, "trying orphan adoption", ex);
- }
- }
-
log.info(pm + ": checked " + count + " bundles.");
return count;
@@ -255,13 +207,8 @@ public class ConsistencyCheckerImpl {
* the bundle to check
* @param fix
* if <code>true</code>, repair things that can be repaired
- * @param modifications
- * if <code>fix == true</code>, collect the repaired
- * {@linkplain NodePropBundle bundles} here
*/
- private void checkBundleConsistency(NodeId id, NodePropBundle bundle,
- boolean fix, Collection<NodePropBundle> modifications,
- NodeId lostNFoundId, Set<NodeId> orphaned, Set<ReportItem> reports) {
+ private void checkBundleConsistency(NodeId id, NodePropBundle bundle, boolean fix, NodeId lostNFoundId, Set<ReportItem> reports) {
// log.info(name + ": checking bundle '" + id + "'");
// skip all virtual nodes
@@ -354,7 +301,7 @@ public class ConsistencyCheckerImpl {
for (NodePropBundle.ChildNodeEntry entry : missingChildren) {
bundle.getChildNodeEntries().remove(entry);
}
- modifications.add(bundle);
+ fixBundle(bundle);
}
// check parent reference
@@ -374,10 +321,18 @@ public class ConsistencyCheckerImpl {
+ "'";
log.error(message);
addMessage(reports, id, message);
- orphaned.add(id);
- if (lostNFoundId != null) {
+ if (fix && lostNFoundId != null) {
+ // add a child to lost+found
+ NodePropBundle lfBundle = pm.loadBundle(lostNFoundId);
+ lfBundle.markOld();
+ String nodeName = id + "-" + System.currentTimeMillis();
+ lfBundle.addChildNodeEntry(NF.create("", nodeName), id);
+ pm.storeBundle(lfBundle);
+ pm.evictBundle(lostNFoundId);
+
+ // set lost+found parent
bundle.setParentId(lostNFoundId);
- modifications.add(bundle);
+ fixBundle(bundle);
}
}
} else {
@@ -403,18 +358,18 @@ public class ConsistencyCheckerImpl {
+ parentId + "'";
log.error(message);
addMessage(reports, id, message);
-
- int l = (int) System.currentTimeMillis();
- int r = new Random().nextInt();
- int n = l + r;
- String nodeName = Integer.toHexString(n);
- parentBundle.addChildNodeEntry(
- NF.create("{}" + nodeName), id);
- log.info("NodeState '" + id
- + "' adds itself to its parent node '"
- + parentId + "' with a new name '" + nodeName
- + "'");
- modifications.add(parentBundle);
+ if (fix) {
+ int l = (int) System.currentTimeMillis();
+ int r = new Random().nextInt();
+ int n = l + r;
+ String nodeName = Integer.toHexString(n);
+ parentBundle.addChildNodeEntry(NF.create("{}" + nodeName), id);
+ log.info("NodeState '" + id
+ + "' adds itself to its parent node '"
+ + parentId + "' with a new name '" + nodeName
+ + "'");
+ fixBundle(parentBundle);
+ }
}
} else {
return;
@@ -486,4 +441,15 @@ public class ConsistencyCheckerImpl {
listener.error(id, message);
}
}
+
+ private void fixBundle(NodePropBundle bundle) {
+ try {
+ log.info(pm + ": Fixing bundle '" + bundle.getId() + "'");
+ bundle.markOld(); // use UPDATE instead of INSERT
+ pm.storeBundle(bundle);
+ pm.evictBundle(bundle.getId());
+ } catch (ItemStateException e) {
+ log.error(pm + ": Error storing fixed bundle: " + e);
+ }
+ }
}
Added: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/ConsistencyCheckerImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/ConsistencyCheckerImplTest.java?rev=1350221&view=auto
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/ConsistencyCheckerImplTest.java (added)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/ConsistencyCheckerImplTest.java Thu Jun 14 12:55:29 2012
@@ -0,0 +1,182 @@
+/*
+ * 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.data;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+import javax.jcr.RepositoryException;
+
+import org.apache.jackrabbit.core.id.NodeId;
+import org.apache.jackrabbit.core.persistence.bundle.AbstractBundlePersistenceManager;
+import org.apache.jackrabbit.core.persistence.bundle.ConsistencyCheckerImpl;
+import org.apache.jackrabbit.core.persistence.util.BLOBStore;
+import org.apache.jackrabbit.core.persistence.util.NodePropBundle;
+import org.apache.jackrabbit.core.state.ItemStateException;
+import org.apache.jackrabbit.core.state.NoSuchItemStateException;
+import org.apache.jackrabbit.core.state.NodeReferences;
+import org.apache.jackrabbit.spi.NameFactory;
+import org.apache.jackrabbit.spi.commons.name.NameConstants;
+import org.apache.jackrabbit.spi.commons.name.NameFactoryImpl;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import junit.framework.TestCase;
+
+public class ConsistencyCheckerImplTest extends TestCase {
+
+ private static final Logger log = LoggerFactory.getLogger(ConsistencyCheckerImplTest.class);
+
+ private static final NameFactory nameFactory = NameFactoryImpl.getInstance();
+
+ public void testFixAbandonedNode() throws RepositoryException {
+ NodePropBundle bundle1 = new NodePropBundle(new NodeId(0, 0));
+ NodePropBundle bundle2 = new NodePropBundle(new NodeId(0, 1));
+
+ // node2 has a reference to node 1 as its parent, but node 1 doesn't have
+ // a corresponding child node entry
+ bundle2.setParentId(bundle1.getId());
+
+ MockPersistenceManager pm = new MockPersistenceManager(Arrays.asList(bundle1, bundle2));
+ ConsistencyCheckerImpl checker = new ConsistencyCheckerImpl(pm, null);
+
+ // run checker with fix = true
+ checker.check(null, false, true, null);
+
+ bundle1 = pm.loadBundle(bundle1.getId());
+ assertEquals(1, bundle1.getChildNodeEntries().size());
+ assertEquals(bundle2.getId(), bundle1.getChildNodeEntries().get(0).getId());
+ }
+
+ /*
+ * There was a bug where when there were multiple abandoned nodes by the same parent
+ * only one of them was fixed. Hence this separate test case for this scenario.
+ */
+ public void testFixMultipleAbandonedNodesBySameParent() throws RepositoryException {
+ NodePropBundle bundle1 = new NodePropBundle(new NodeId(0, 0));
+ NodePropBundle bundle2 = new NodePropBundle(new NodeId(0, 1));
+ NodePropBundle bundle3 = new NodePropBundle(new NodeId(1, 0));
+
+
+ // node2 and node3 have a reference to node1 as its parent, but node1 doesn't have
+ // corresponding child node entries
+ bundle2.setParentId(bundle1.getId());
+ bundle3.setParentId(bundle1.getId());
+
+ MockPersistenceManager pm = new MockPersistenceManager(Arrays.asList(bundle1, bundle2, bundle3));
+ ConsistencyCheckerImpl checker = new ConsistencyCheckerImpl(pm, null);
+
+ // run checker with fix = true
+ checker.check(null, false, true, null);
+
+ bundle1 = pm.loadBundle(bundle1.getId());
+ assertEquals(2, bundle1.getChildNodeEntries().size());
+ assertEquals(bundle2.getId(), bundle1.getChildNodeEntries().get(0).getId());
+ assertEquals(bundle3.getId(), bundle1.getChildNodeEntries().get(1).getId());
+ }
+
+ public void testAddOrphanedNodeToLostAndFound() throws RepositoryException {
+ NodePropBundle lostAndFound = new NodePropBundle(new NodeId(0, 0));
+ // lost and found must be of type nt:unstructured
+ lostAndFound.setNodeTypeName(NameConstants.NT_UNSTRUCTURED);
+
+ NodePropBundle orphaned = new NodePropBundle(new NodeId(0, 1));
+ // set non-existent parent node id
+ orphaned.setParentId(new NodeId(1, 0));
+
+ MockPersistenceManager pm = new MockPersistenceManager(Arrays.asList(lostAndFound, orphaned));
+ ConsistencyCheckerImpl checker = new ConsistencyCheckerImpl(pm, null);
+
+ // run checker with fix = true
+ checker.check(null, false, true, lostAndFound.getId().toString());
+
+ lostAndFound = pm.loadBundle(lostAndFound.getId());
+ assertEquals(1, lostAndFound.getChildNodeEntries().size());
+ assertEquals(orphaned.getId(), lostAndFound.getChildNodeEntries().get(0).getId());
+
+ orphaned = pm.loadBundle(orphaned.getId());
+ assertEquals(lostAndFound.getId(), orphaned.getParentId());
+ }
+
+ private static class MockPersistenceManager extends AbstractBundlePersistenceManager {
+
+ private Map<NodeId, NodePropBundle> bundles = new LinkedHashMap<NodeId, NodePropBundle>();
+
+ private MockPersistenceManager(List<NodePropBundle> bundles) {
+ for (NodePropBundle bundle : bundles) {
+ this.bundles.put(bundle.getId(), bundle);
+ }
+ }
+
+ public List<NodeId> getAllNodeIds(final NodeId after, final int maxCount) throws ItemStateException, RepositoryException {
+ List<NodeId> allNodeIds = new ArrayList<NodeId>();
+ boolean add = after == null;
+ for (NodeId nodeId : bundles.keySet()) {
+ if (add) {
+ allNodeIds.add(nodeId);
+ }
+ if (!add) {
+ add = nodeId.equals(after);
+ }
+ }
+ return allNodeIds;
+ }
+
+ @Override
+ protected NodePropBundle loadBundle(final NodeId id) {
+ return bundles.get(id);
+ }
+
+ @Override
+ protected void evictBundle(final NodeId id) {
+ }
+
+ @Override
+ protected void storeBundle(final NodePropBundle bundle) throws ItemStateException {
+ bundles.put(bundle.getId(), bundle);
+ }
+
+ @Override
+ protected void destroyBundle(final NodePropBundle bundle) throws ItemStateException {
+ bundles.remove(bundle.getId());
+ }
+
+ @Override
+ protected void destroy(final NodeReferences refs) throws ItemStateException {
+ }
+
+ @Override
+ protected void store(final NodeReferences refs) throws ItemStateException {
+ }
+
+ @Override
+ protected BLOBStore getBlobStore() {
+ return null;
+ }
+
+ public NodeReferences loadReferencesTo(final NodeId id) throws NoSuchItemStateException, ItemStateException {
+ return null;
+ }
+
+ public boolean existsReferencesTo(final NodeId targetId) throws ItemStateException {
+ return false;
+ }
+ }
+}
\ No newline at end of file