Return-Path: X-Original-To: apmail-jackrabbit-commits-archive@www.apache.org Delivered-To: apmail-jackrabbit-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 4FFF8E3A1 for ; Sun, 10 Feb 2013 09:10:48 +0000 (UTC) Received: (qmail 47255 invoked by uid 500); 10 Feb 2013 09:10:47 -0000 Delivered-To: apmail-jackrabbit-commits-archive@jackrabbit.apache.org Received: (qmail 47117 invoked by uid 500); 10 Feb 2013 09:10:46 -0000 Mailing-List: contact commits-help@jackrabbit.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@jackrabbit.apache.org Delivered-To: mailing list commits@jackrabbit.apache.org Received: (qmail 46875 invoked by uid 99); 10 Feb 2013 09:10:45 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 10 Feb 2013 09:10:45 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 10 Feb 2013 09:10:43 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id 0CBAC23888CD; Sun, 10 Feb 2013 09:10:25 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: 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/ Date: Sun, 10 Feb 2013 09:10:24 -0000 To: commits@jackrabbit.apache.org From: unico@apache.org X-Mailer: svnmailer-1.0.8-patched Message-Id: <20130210091025.0CBAC23888CD@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org 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 excludedIds) { this.index = index; this.handler = handler; - final HierarchyManager hierarchyManager = handler.getContext().getHierarchyManager(); excludedPaths = new HashSet(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 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 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 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 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 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 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 true if the error was verified to still exist, else false. + * @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 getNodeDocuments(NodeId id) throws RepositoryException, IOException { + final List docIds = new ArrayList(1); + final List docs = new ArrayList(); + 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 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 remove = Collections.singletonList(fooId).iterator(); + Iterator add = Collections.emptyList().iterator(); + + searchIndex.updateNodes(remove, add); + + ConsistencyCheck consistencyCheck = searchIndex.runConsistencyCheck(); + List 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.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 remove = Collections.emptyList().iterator(); + Iterator add = Collections.singletonList(nodeState).iterator(); + + searchIndex.updateNodes(remove, add); + + ConsistencyCheck consistencyCheck = searchIndex.runConsistencyCheck(); + List 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.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 remove = Collections.singletonList(fooId).iterator(); + Iterator add = Collections.emptyList().iterator(); + + searchIndex.updateNodes(remove, add); + + ConsistencyCheck consistencyCheck = searchIndex.runConsistencyCheck(); + List errors = consistencyCheck.getErrors(); + + assertEquals("Expected 2 index consistency errors", 2, errors.size()); + + remove = Collections.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 errors = consistencyCheck.getErrors(); + assertTrue(errors.isEmpty()); + } + } finally { + keepRunning = false; + } + + } + private boolean searchIndexContainsNode(SearchIndex searchIndex, NodeId nodeId) throws IOException { final List docs = new ArrayList(1); final IndexReader reader = searchIndex.getIndexReader();