jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From un...@apache.org
Subject svn commit: r1444518 - in /jackrabbit/branches/2.6/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:17:17 GMT
Author: unico
Date: Sun Feb 10 09:17:16 2013
New Revision: 1444518

URL: http://svn.apache.org/r1444518
Log:
JCR-3517 backport: double check capability for eliminating false positives due to concurrent
updates

Modified:
    jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/ConsistencyCheck.java
    jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/ConsistencyCheckError.java
    jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/SearchIndex.java
    jackrabbit/branches/2.6/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/query/lucene/SearchIndexConsistencyCheckTest.java

Modified: jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/ConsistencyCheck.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/ConsistencyCheck.java?rev=1444518&r1=1444517&r2=1444518&view=diff
==============================================================================
--- jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/ConsistencyCheck.java
(original)
+++ jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/ConsistencyCheck.java
Sun Feb 10 09:17:16 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/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/ConsistencyCheckError.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/ConsistencyCheckError.java?rev=1444518&r1=1444517&r2=1444518&view=diff
==============================================================================
--- jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/ConsistencyCheckError.java
(original)
+++ jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/ConsistencyCheckError.java
Sun Feb 10 09:17:16 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/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/SearchIndex.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/SearchIndex.java?rev=1444518&r1=1444517&r2=1444518&view=diff
==============================================================================
--- jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/SearchIndex.java
(original)
+++ jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/SearchIndex.java
Sun Feb 10 09:17:16 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/branches/2.6/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/query/lucene/SearchIndexConsistencyCheckTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.6/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/query/lucene/SearchIndexConsistencyCheckTest.java?rev=1444518&r1=1444517&r2=1444518&view=diff
==============================================================================
--- jackrabbit/branches/2.6/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/query/lucene/SearchIndexConsistencyCheckTest.java
(original)
+++ jackrabbit/branches/2.6/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/query/lucene/SearchIndexConsistencyCheckTest.java
Sun Feb 10 09:17:16 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();



Mime
View raw message