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 2013/02/10 10:10:24 UTC
svn commit: r1444515 - in /jackrabbit/trunk/jackrabbit-core/src:
main/java/org/apache/jackrabbit/core/query/lucene/
test/java/org/apache/jackrabbit/core/query/lucene/
Author: unico
Date: Sun Feb 10 09:10:24 2013
New Revision: 1444515
URL: http://svn.apache.org/r1444515
Log:
JCR-3517 double check capability for eliminating false positives due to concurrent updates
Modified:
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/ConsistencyCheck.java
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/ConsistencyCheckError.java
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/SearchIndex.java
jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/query/lucene/SearchIndexConsistencyCheckTest.java
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/ConsistencyCheck.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/ConsistencyCheck.java?rev=1444515&r1=1444514&r2=1444515&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/ConsistencyCheck.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/ConsistencyCheck.java Sun Feb 10 09:10:24 2013
@@ -17,6 +17,8 @@
package org.apache.jackrabbit.core.query.lucene;
import org.apache.jackrabbit.core.HierarchyManager;
+import org.apache.jackrabbit.core.cluster.ClusterException;
+import org.apache.jackrabbit.core.cluster.ClusterNode;
import org.apache.jackrabbit.core.persistence.IterablePersistenceManager;
import org.apache.jackrabbit.core.persistence.PersistenceManager;
import org.apache.jackrabbit.core.state.ItemState;
@@ -108,7 +110,6 @@ public class ConsistencyCheck {
private ConsistencyCheck(MultiIndex index, SearchIndex handler, Set<NodeId> excludedIds) {
this.index = index;
this.handler = handler;
-
final HierarchyManager hierarchyManager = handler.getContext().getHierarchyManager();
excludedPaths = new HashSet<Path>(excludedIds.size());
for (NodeId excludedId : excludedIds) {
@@ -207,6 +208,34 @@ public class ConsistencyCheck {
}
}
+ public void doubleCheckErrors() {
+ if (!errors.isEmpty()) {
+ log.info("Double checking errors");
+ final ClusterNode clusterNode = handler.getContext().getClusterNode();
+ if (clusterNode != null) {
+ try {
+ clusterNode.sync();
+ } catch (ClusterException e) {
+ log.error("Could not sync cluster node for double checking errors");
+ }
+ }
+ final Iterator<ConsistencyCheckError> iterator = errors.iterator();
+ while (iterator.hasNext()) {
+ try {
+ final ConsistencyCheckError error = iterator.next();
+ if (!error.doubleCheck(handler, stateMgr)) {
+ log.info("False positive: " + error.toString());
+ iterator.remove();
+ }
+ } catch (RepositoryException e) {
+ log.error("Failed to double check consistency error", e);
+ } catch (IOException e) {
+ log.error("Failed to double check consistency error", e);
+ }
+ }
+ }
+ }
+
private void loadNodes() {
log.info("Loading nodes");
try {
@@ -285,12 +314,10 @@ public class ConsistencyCheck {
Document d = reader.document(i, FieldSelectors.UUID_AND_PARENT);
NodeId id = new NodeId(d.get(FieldNames.UUID));
String parent = d.get(FieldNames.PARENT);
- NodeId parentId;
- if (parent != null && !parent.isEmpty()) {
- parentId = new NodeId(parent);
- } else {
+ if (parent == null || parent.isEmpty()) {
continue;
}
+ final NodeId parentId = new NodeId(parent);
boolean parentExists = nodeIds.containsKey(parentId);
boolean parentIndexed = parentExists && nodeIds.get(parentId);
@@ -462,6 +489,23 @@ public class ConsistencyCheck {
}
}
}
+
+ @Override
+ boolean doubleCheck(SearchIndex handler, ItemStateManager stateManager)
+ throws RepositoryException, IOException {
+ final List<Document> documents = handler.getNodeDocuments(id);
+ for (Document document : documents) {
+ final String parent = document.get(FieldNames.PARENT);
+ if (parent != null && !parent.isEmpty()) {
+ final NodeId parentId = new NodeId(parent);
+ if (handler.getNodeDocuments(parentId).isEmpty()) {
+ return true;
+ }
+ }
+ }
+ return false;
+
+ }
}
/**
@@ -469,8 +513,11 @@ public class ConsistencyCheck {
*/
private static class UnknownParent extends ConsistencyCheckError {
+ private NodeId parentId;
+
private UnknownParent(NodeId id, NodeId parentId) {
super("Node " + id + " has unknown parent: " + parentId, id);
+ this.parentId = parentId;
}
/**
@@ -487,6 +534,22 @@ public class ConsistencyCheck {
public void repair() throws IOException {
log.warn("Unknown parent for " + id + " cannot be repaired");
}
+
+ @Override
+ boolean doubleCheck(SearchIndex handler, ItemStateManager stateManager)
+ throws IOException, RepositoryException {
+ final List<Document> documents = handler.getNodeDocuments(id);
+ for (Document document : documents) {
+ final String parent = document.get(FieldNames.PARENT);
+ if (parent != null && !parent.isEmpty()) {
+ final NodeId parentId = new NodeId(parent);
+ if (parentId.equals(this.parentId) && !stateManager.hasItemState(parentId)) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
}
/**
@@ -494,8 +557,11 @@ public class ConsistencyCheck {
*/
private class WrongParent extends ConsistencyCheckError {
+ private NodeId indexedParentId;
+
private WrongParent(NodeId id, NodeId indexedParentId, NodeId actualParentId) {
super("Node " + id + " has wrong parent: " + indexedParentId + ", should be : " + actualParentId, id);
+ this.indexedParentId = indexedParentId;
}
@Override
@@ -524,6 +590,22 @@ public class ConsistencyCheck {
}
}
+ @Override
+ boolean doubleCheck(final SearchIndex handler, final ItemStateManager stateManager)
+ throws RepositoryException, IOException {
+ final List<Document> documents = handler.getNodeDocuments(id);
+ for (Document document : documents) {
+ final String parent = document.get(FieldNames.PARENT);
+ if (parent != null && !parent.isEmpty()) {
+ final NodeId parentId = new NodeId(parent);
+ if (parentId.equals(indexedParentId) && !stateManager.hasItemState(parentId)) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
}
/**
@@ -566,6 +648,12 @@ public class ConsistencyCheck {
throw new IOException(e.toString());
}
}
+
+ @Override
+ boolean doubleCheck(SearchIndex handler, ItemStateManager stateManager)
+ throws RepositoryException, IOException {
+ return handler.getNodeDocuments(id).size() > 1;
+ }
}
/**
@@ -593,6 +681,18 @@ public class ConsistencyCheck {
log.info("Removing deleted node from index: " + id);
index.removeDocument(id);
}
+
+ @Override
+ boolean doubleCheck(SearchIndex handler, ItemStateManager stateManager)
+ throws RepositoryException, IOException {
+ final List<Document> documents = handler.getNodeDocuments(id);
+ if (!documents.isEmpty()) {
+ if (!stateManager.hasItemState(id)) {
+ return true;
+ }
+ }
+ return false;
+ }
}
private class NodeAdded extends ConsistencyCheckError {
@@ -623,5 +723,17 @@ public class ConsistencyCheck {
}
}
+ @Override
+ boolean doubleCheck(SearchIndex handler, ItemStateManager stateManager)
+ throws RepositoryException, IOException {
+ final List<Document> documents = handler.getNodeDocuments(id);
+ if (documents.isEmpty()) {
+ if (stateManager.hasItemState(id)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
}
}
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/ConsistencyCheckError.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/ConsistencyCheckError.java?rev=1444515&r1=1444514&r2=1444515&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/ConsistencyCheckError.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/ConsistencyCheckError.java Sun Feb 10 09:10:24 2013
@@ -17,9 +17,12 @@
package org.apache.jackrabbit.core.query.lucene;
import org.apache.jackrabbit.core.id.NodeId;
+import org.apache.jackrabbit.core.state.ItemStateManager;
import java.io.IOException;
+import javax.jcr.RepositoryException;
+
/**
* Common base class for errors detected during the consistency check.
*/
@@ -59,4 +62,14 @@ public abstract class ConsistencyCheckEr
* @throws IOException if an error occurs while repairing.
*/
abstract void repair() throws IOException;
+
+ /**
+ * Double check the error. Used to rule out false positives in live environments.
+ *
+ * @return <code>true</code> if the error was verified to still exist, else <code>false</code>.
+ * @throws RepositoryException
+ * @throws IOException
+ */
+ abstract boolean doubleCheck(SearchIndex handler, ItemStateManager stateManager)
+ throws RepositoryException, IOException;
}
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/SearchIndex.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/SearchIndex.java?rev=1444515&r1=1444514&r2=1444515&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/SearchIndex.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/SearchIndex.java Sun Feb 10 09:10:24 2013
@@ -758,6 +758,32 @@ public class SearchIndex extends Abstrac
return ids;
}
+ List<Document> getNodeDocuments(NodeId id) throws RepositoryException, IOException {
+ final List<Integer> docIds = new ArrayList<Integer>(1);
+ final List<Document> docs = new ArrayList<Document>();
+ final IndexReader reader = getIndexReader();
+ try {
+ IndexSearcher searcher = new IndexSearcher(reader);
+ try {
+ Query q = new TermQuery(new Term(FieldNames.UUID, id.toString()));
+ searcher.search(q, new AbstractHitCollector() {
+ @Override
+ protected void collect(final int doc, final float score) {
+ docIds.add(doc);
+ }
+ });
+ for (Integer docId : docIds) {
+ docs.add(reader.document(docId, FieldSelectors.UUID_AND_PARENT));
+ }
+ } finally {
+ searcher.close();
+ }
+ } finally {
+ Util.closeOrRelease(reader);
+ }
+ return docs;
+ }
+
/**
* This method returns the QueryNodeFactory used to parse Queries. This method
* may be overridden to provide a customized QueryNodeFactory
Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/query/lucene/SearchIndexConsistencyCheckTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/query/lucene/SearchIndexConsistencyCheckTest.java?rev=1444515&r1=1444514&r2=1444515&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/query/lucene/SearchIndexConsistencyCheckTest.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/query/lucene/SearchIndexConsistencyCheckTest.java Sun Feb 10 09:10:24 2013
@@ -24,6 +24,7 @@ import java.util.Iterator;
import java.util.List;
import javax.jcr.Node;
+import javax.jcr.RepositoryException;
import javax.jcr.Session;
import org.apache.jackrabbit.core.SearchManager;
@@ -40,6 +41,8 @@ import org.apache.lucene.search.TermQuer
public class SearchIndexConsistencyCheckTest extends AbstractJCRTest {
+ private boolean keepRunning;
+
@Override
protected void setUp() throws Exception {
super.setUp();
@@ -62,7 +65,7 @@ public class SearchIndexConsistencyCheck
ConsistencyCheck consistencyCheck = searchIndex.runConsistencyCheck();
List<ConsistencyCheckError> errors = consistencyCheck.getErrors();
- assertEquals("Expected 1 index consistencey error", 1, errors.size());
+ assertEquals("Expected 1 index consistency error", 1, errors.size());
ConsistencyCheckError error = errors.iterator().next();
assertEquals("Different node was reported to be missing", error.id, fooId);
@@ -74,6 +77,36 @@ public class SearchIndexConsistencyCheck
assertTrue("Consistency check still reports errors", searchIndex.runConsistencyCheck().getErrors().isEmpty());
}
+ public void testMissingNodeDoubleCheck() throws Exception {
+ Session s = getHelper().getSuperuserSession();
+ SearchManager searchManager = TestHelper.getSearchManager(s);
+ SearchIndex searchIndex = (SearchIndex) searchManager.getQueryHandler();
+
+ Node foo = testRootNode.addNode("foo");
+ testRootNode.getSession().save();
+ NodeId fooId = new NodeId(foo.getIdentifier());
+
+ Iterator<NodeId> remove = Collections.singletonList(fooId).iterator();
+ Iterator<NodeState> add = Collections.<NodeState>emptyList().iterator();
+
+ searchIndex.updateNodes(remove, add);
+
+ ConsistencyCheck consistencyCheck = searchIndex.runConsistencyCheck();
+ List<ConsistencyCheckError> errors = consistencyCheck.getErrors();
+ assertEquals("Expected 1 index consistency error", 1, errors.size());
+
+ // now add foo to the index again so that double check finds a false positive
+ remove = Collections.<NodeId>emptyList().iterator();
+ add = Collections.singletonList(new NodeState(fooId, null, null, 1, false)).iterator();
+
+ searchIndex.updateNodes(remove, add);
+
+ consistencyCheck.doubleCheckErrors();
+
+ assertTrue("Consistency double check of missing node failed", consistencyCheck.getErrors().isEmpty());
+
+ }
+
public void testIndexContainsUnknownNode() throws Exception {
Session s = getHelper().getSuperuserSession();
@@ -102,6 +135,35 @@ public class SearchIndexConsistencyCheck
assertTrue("Consistency check still reports errors", searchIndex.runConsistencyCheck().getErrors().isEmpty());
}
+ public void testUnknownNodeDoubleCheck() throws Exception {
+
+ Session s = getHelper().getSuperuserSession();
+ SearchManager searchManager = TestHelper.getSearchManager(s);
+ SearchIndex searchIndex = (SearchIndex) searchManager.getQueryHandler();
+
+ NodeId nodeId = new NodeId(0, 0);
+ NodeState nodeState = new NodeState(nodeId, null, null, 1, false);
+
+ Iterator<NodeId> remove = Collections.<NodeId>emptyList().iterator();
+ Iterator<NodeState> add = Collections.singletonList(nodeState).iterator();
+
+ searchIndex.updateNodes(remove, add);
+
+ ConsistencyCheck consistencyCheck = searchIndex.runConsistencyCheck();
+ List<ConsistencyCheckError> errors = consistencyCheck.getErrors();
+ assertEquals("Expected 1 index consistency error", 1, errors.size());
+
+ // now remove the unknown node from the index again so that double check finds a false positive
+ remove = Collections.singletonList(nodeId).iterator();
+ add = Collections.<NodeState>emptyList().iterator();
+
+ searchIndex.updateNodes(remove, add);
+
+ consistencyCheck.doubleCheckErrors();
+
+ assertTrue("Consistency double check of deleted node failed", consistencyCheck.getErrors().isEmpty());
+ }
+
public void testIndexMissesAncestor() throws Exception {
Session s = getHelper().getSuperuserSession();
SearchManager searchManager = TestHelper.getSearchManager(s);
@@ -133,6 +195,37 @@ public class SearchIndexConsistencyCheck
assertTrue("Consistency check still reports errors", searchIndex.runConsistencyCheck().getErrors().isEmpty());
}
+ public void testMissingAncestorDoubleCheck() throws Exception {
+
+ Session s = getHelper().getSuperuserSession();
+ SearchManager searchManager = TestHelper.getSearchManager(s);
+ SearchIndex searchIndex = (SearchIndex) searchManager.getQueryHandler();
+
+ Node foo = testRootNode.addNode("foo");
+ foo.addNode("bar");
+ testRootNode.getSession().save();
+ NodeId fooId = new NodeId(foo.getIdentifier());
+
+ Iterator<NodeId> remove = Collections.singletonList(fooId).iterator();
+ Iterator<NodeState> add = Collections.<NodeState>emptyList().iterator();
+
+ searchIndex.updateNodes(remove, add);
+
+ ConsistencyCheck consistencyCheck = searchIndex.runConsistencyCheck();
+ List<ConsistencyCheckError> errors = consistencyCheck.getErrors();
+
+ assertEquals("Expected 2 index consistency errors", 2, errors.size());
+
+ remove = Collections.<NodeId>emptyList().iterator();
+ add = Collections.singletonList(new NodeState(fooId, null, null, 1, true)).iterator();
+
+ searchIndex.updateNodes(remove, add);
+
+ consistencyCheck.doubleCheckErrors();
+
+ assertTrue("Consistency double check of missing ancestor failed", consistencyCheck.getErrors().isEmpty());
+ }
+
public void testIndexContainsMultipleEntries() throws Exception {
Session s = getHelper().getSuperuserSession();
SearchManager searchManager = TestHelper.getSearchManager(s);
@@ -167,6 +260,47 @@ public class SearchIndexConsistencyCheck
assertTrue("Consistency check still reports errors", searchIndex.runConsistencyCheck().getErrors().isEmpty());
}
+ /**
+ * Stress test on the double check mechanism
+ */
+ public void testDoubleCheckStressTest() throws Exception {
+ Thread t = new Thread(new Runnable() {
+ private Session s = getHelper().getReadWriteSession();
+ @Override
+ public void run() {
+ while (keepRunning) {
+ try {
+ Node foo = s.getRootNode().getNode(testPath).addNode("foo");
+ s.save();
+ foo.remove();
+ s.save();
+ } catch (RepositoryException e) {
+ System.out.println(e);
+ }
+ }
+ }
+ });
+
+ Session s = getHelper().getSuperuserSession();
+ SearchManager searchManager = TestHelper.getSearchManager(s);
+ SearchIndex searchIndex = (SearchIndex) searchManager.getQueryHandler();
+
+ keepRunning = true;
+ try {
+ t.start();
+ Thread.sleep(100);
+ for (int i = 100; i > 0; i--) {
+ final ConsistencyCheck consistencyCheck = searchIndex.runConsistencyCheck();
+ consistencyCheck.doubleCheckErrors();
+ final List<ConsistencyCheckError> errors = consistencyCheck.getErrors();
+ assertTrue(errors.isEmpty());
+ }
+ } finally {
+ keepRunning = false;
+ }
+
+ }
+
private boolean searchIndexContainsNode(SearchIndex searchIndex, NodeId nodeId) throws IOException {
final List<Integer> docs = new ArrayList<Integer>(1);
final IndexReader reader = searchIndex.getIndexReader();