You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by ju...@apache.org on 2014/06/13 01:30:06 UTC

svn commit: r1602316 - in /jackrabbit/oak/branches/1.0: ./ oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/

Author: jukka
Date: Thu Jun 12 23:30:05 2014
New Revision: 1602316

URL: http://svn.apache.org/r1602316
Log:
1.0.1: Merged revision 1590697 (OAK-1775)

Modified:
    jackrabbit/oak/branches/1.0/   (props changed)
    jackrabbit/oak/branches/1.0/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNode.java
    jackrabbit/oak/branches/1.0/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexTracker.java
    jackrabbit/oak/branches/1.0/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndex.java

Propchange: jackrabbit/oak/branches/1.0/
------------------------------------------------------------------------------
  Merged /jackrabbit/oak/trunk:r1590697

Modified: jackrabbit/oak/branches/1.0/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNode.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNode.java?rev=1602316&r1=1602315&r2=1602316&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNode.java (original)
+++ jackrabbit/oak/branches/1.0/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNode.java Thu Jun 12 23:30:05 2014
@@ -24,6 +24,8 @@ import static org.apache.jackrabbit.oak.
 
 import java.io.File;
 import java.io.IOException;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import org.apache.jackrabbit.oak.spi.state.NodeState;
 import org.apache.jackrabbit.oak.spi.state.ReadOnlyBuilder;
@@ -74,7 +76,7 @@ class IndexNode {
 
     private final IndexSearcher searcher;
 
-    private int refcount = 0;
+    private final ReadWriteLock lock = new ReentrantReadWriteLock();
 
     private boolean closed = false;
 
@@ -95,27 +97,33 @@ class IndexNode {
         return definition;
     }
 
-    synchronized IndexSearcher acquireSearcher() {
-        checkState(!closed);
-        refcount++;
+    IndexSearcher getSearcher() {
         return searcher;
     }
 
-    synchronized void releaseSearcher() throws IOException {
-        refcount--;
-        if (closed && refcount == 0) {
-            reallyClose();
+    boolean acquire() {
+        lock.readLock().lock();
+        if (closed) {
+            lock.readLock().unlock();
+            return false;
+        } else {
+            return true;
         }
     }
 
-    synchronized void close() throws IOException {
-        closed = true;
-        if (refcount == 0) {
-            reallyClose();
-        }
+    void release() {
+        lock.readLock().unlock();
     }
 
-    private void reallyClose() throws IOException {
+    void close() throws IOException {
+        lock.writeLock().lock();
+        try {
+            checkState(!closed);
+            closed = true;
+        } finally {
+            lock.writeLock().unlock();
+        }
+
         try {
             reader.close();
         } finally {

Modified: jackrabbit/oak/branches/1.0/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexTracker.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexTracker.java?rev=1602316&r1=1602315&r2=1602316&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexTracker.java (original)
+++ jackrabbit/oak/branches/1.0/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexTracker.java Thu Jun 12 23:30:05 2014
@@ -16,9 +16,16 @@
  */
 package org.apache.jackrabbit.oak.plugins.index.lucene;
 
+import static com.google.common.base.Preconditions.checkState;
+import static com.google.common.base.Predicates.in;
+import static com.google.common.base.Predicates.not;
+import static com.google.common.base.Predicates.notNull;
 import static com.google.common.collect.Lists.newArrayList;
 import static com.google.common.collect.Lists.newArrayListWithCapacity;
+import static com.google.common.collect.Maps.filterKeys;
+import static com.google.common.collect.Maps.filterValues;
 import static com.google.common.collect.Maps.newHashMap;
+import static java.util.Collections.emptyMap;
 import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NAME;
 import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.TYPE_PROPERTY_NAME;
 import static org.apache.jackrabbit.oak.plugins.index.lucene.LuceneIndexConstants.TYPE_LUCENE;
@@ -39,6 +46,7 @@ import org.apache.jackrabbit.oak.spi.sta
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 
 class IndexTracker {
@@ -49,9 +57,12 @@ class IndexTracker {
 
     private NodeState root = EMPTY_NODE;
 
-    private final Map<String, IndexNode> indices = newHashMap();
+    private volatile Map<String, IndexNode> indices = emptyMap();
 
     synchronized void close() {
+        Map<String, IndexNode> indices = this.indices;
+        this.indices = emptyMap();
+
         for (Map.Entry<String, IndexNode> entry : indices.entrySet()) {
             try {
                 entry.getValue().close();
@@ -59,44 +70,71 @@ class IndexTracker {
                 log.error("Failed to close the Lucene index at " + entry.getKey(), e);
             }
         }
-        indices.clear();
     }
 
     synchronized void update(NodeState root) {
-        List<Editor> editors = newArrayListWithCapacity(indices.size());
-        for (final String path : indices.keySet()) {
+        Map<String, IndexNode> original = indices;
+        final Map<String, IndexNode> updates = newHashMap();
+
+        List<Editor> editors = newArrayListWithCapacity(original.size());
+        for (Map.Entry<String, IndexNode> entry : original.entrySet()) {
+            final String path = entry.getKey();
+            final String name = entry.getValue().getName();
+
             List<String> elements = newArrayList();
             Iterables.addAll(elements, PathUtils.elements(path));
             elements.add(INDEX_DEFINITIONS_NAME);
-            elements.add(indices.get(path).getName());
+            elements.add(name);
             editors.add(new SubtreeEditor(new DefaultEditor() {
                 @Override
                 public void leave(NodeState before, NodeState after) {
-                    IndexNode index = indices.remove(path);
                     try {
-                        index.close();
-                    } catch (IOException e) {
-                        log.error("Failed to close Lucene index at " + path, e);
-                    }
-
-                    try {
-                        index = IndexNode.open(index.getName(), after);
-                        if (index != null) {
-                            indices.put(path, index);
-                        }
+                        // TODO: Use DirectoryReader.openIfChanged()
+                        IndexNode index = IndexNode.open(name, after);
+                        updates.put(path, index); // index can be null
                     } catch (IOException e) {
                         log.error("Failed to open Lucene index at " + path, e);
                     }
                 }
             }, elements.toArray(new String[elements.size()])));
         }
+
         EditorDiff.process(CompositeEditor.compose(editors), this.root, root);
         this.root = root;
+
+        if (!updates.isEmpty()) {
+            indices = ImmutableMap.<String, IndexNode>builder()
+                    .putAll(filterKeys(original, not(in(updates.keySet()))))
+                    .putAll(filterValues(updates, notNull()))
+                    .build();
+
+            for (String path : updates.keySet()) {
+                IndexNode index = original.get(path);
+                try {
+                    index.close();
+                } catch (IOException e) {
+                    log.error("Failed to close Lucene index at " + path, e);
+                }
+            }
+        }
+    }
+
+    IndexNode acquireIndexNode(String path) {
+        IndexNode index = indices.get(path);
+        if (index != null && index.acquire()) {
+            return index;
+        } else {
+            return findIndexNode(path);
+        }
     }
 
-    synchronized IndexNode getIndexNode(String path) {
+    private synchronized IndexNode findIndexNode(String path) {
+        // Retry the lookup from acquireIndexNode now that we're
+        // synchronized. The acquire() call is guaranteed to succeed
+        // since the close() method is also synchronized.
         IndexNode index = indices.get(path);
         if (index != null) {
+            checkState(index.acquire());
             return index;
         }
 
@@ -112,7 +150,11 @@ class IndexTracker {
                 if (TYPE_LUCENE.equals(node.getString(TYPE_PROPERTY_NAME))) {
                     index = IndexNode.open(child.getName(), node);
                     if (index != null) {
-                        indices.put(path, index);
+                        checkState(index.acquire());
+                        indices = ImmutableMap.<String, IndexNode>builder()
+                                .putAll(indices)
+                                .put(path, index)
+                                .build();
                         return index;
                     }
                 }

Modified: jackrabbit/oak/branches/1.0/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndex.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndex.java?rev=1602316&r1=1602315&r2=1602316&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndex.java (original)
+++ jackrabbit/oak/branches/1.0/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndex.java Thu Jun 12 23:30:05 2014
@@ -177,35 +177,39 @@ public class LuceneIndex implements Full
 
     @Override
     public double getCost(Filter filter, NodeState root) {
-        if (tracker.getIndexNode("/") == null) {
-            // unusable index
+        IndexNode index = tracker.acquireIndexNode("/");
+        if (index == null) { // unusable index
             return Double.POSITIVE_INFINITY;
         }
-        FullTextExpression ft = filter.getFullTextConstraint();
-        if (ft == null) {
-            // no full-text condition: don't use this index,
-            // as there might be a better one
-            return Double.POSITIVE_INFINITY;
+        try {
+            FullTextExpression ft = filter.getFullTextConstraint();
+            if (ft == null) {
+                // no full-text condition: don't use this index,
+                // as there might be a better one
+                return Double.POSITIVE_INFINITY;
+            }
+            Set<String> relPaths = getRelativePaths(ft);
+            if (relPaths.size() > 1) {
+                LOG.warn("More than one relative parent for query " + filter.getQueryStatement());
+                // there are multiple "parents", as in
+                // "contains(a/x, 'hello') and contains(b/x, 'world')"
+                return new MultiLuceneIndex(filter, root, relPaths).getCost();
+            }
+            String parent = relPaths.iterator().next();
+            if (parent.isEmpty()) {
+                // no relative properties
+                return 10;
+            }
+            // all relative properties have the same "parent", as in
+            // "contains(a/x, 'hello') and contains(a/y, 'world')" or
+            // "contains(a/x, 'hello') or contains(a/*, 'world')"
+            // TODO: proper cost calculation
+            // we assume this will cause more read operations,
+            // as we need to read the node and then the parent
+            return 15;
+        } finally {
+            index.release();
         }
-        Set<String> relPaths = getRelativePaths(ft);
-        if (relPaths.size() > 1) {
-            LOG.warn("More than one relative parent for query " + filter.getQueryStatement());
-            // there are multiple "parents", as in
-            // "contains(a/x, 'hello') and contains(b/x, 'world')"
-            return new MultiLuceneIndex(filter, root, relPaths).getCost();
-        }
-        String parent = relPaths.iterator().next();
-        if (parent.isEmpty()) {
-            // no relative properties
-            return 10;
-        }
-        // all relative properties have the same "parent", as in
-        // "contains(a/x, 'hello') and contains(a/y, 'world')" or
-        // "contains(a/x, 'hello') or contains(a/*, 'world')"
-        // TODO: proper cost calculation
-        // we assume this will cause more read operations,
-        // as we need to read the node and then the parent
-        return 15;
     }
 
     /**
@@ -255,35 +259,36 @@ public class LuceneIndex implements Full
 
     @Override
     public String getPlan(Filter filter, NodeState root) {
-        IndexNode index = tracker.getIndexNode("/");
+        IndexNode index = tracker.acquireIndexNode("/");
         checkState(index != null, "The Lucene index is not available");
-
-        FullTextExpression ft = filter.getFullTextConstraint();
-        Set<String> relPaths = getRelativePaths(ft);
-        if (relPaths.size() > 1) {
-            return new MultiLuceneIndex(filter, root, relPaths).getPlan();
-        }
-        String parent = relPaths.size() == 0 ? "" : relPaths.iterator().next();
-        // we only restrict non-full-text conditions if there is
-        // no relative property in the full-text constraint
-        boolean nonFullTextConstraints = parent.isEmpty();
-        String plan = getQuery(filter, null, nonFullTextConstraints, analyzer, index.getDefinition()) + " ft:(" + ft + ")";
-        if (!parent.isEmpty()) {
-            plan += " parent:" + parent;
+        try {
+            FullTextExpression ft = filter.getFullTextConstraint();
+            Set<String> relPaths = getRelativePaths(ft);
+            if (relPaths.size() > 1) {
+                return new MultiLuceneIndex(filter, root, relPaths).getPlan();
+            }
+            String parent = relPaths.size() == 0 ? "" : relPaths.iterator().next();
+            // we only restrict non-full-text conditions if there is
+            // no relative property in the full-text constraint
+            boolean nonFullTextConstraints = parent.isEmpty();
+            String plan = getQuery(filter, null, nonFullTextConstraints, analyzer, index.getDefinition()) + " ft:(" + ft + ")";
+            if (!parent.isEmpty()) {
+                plan += " parent:" + parent;
+            }
+            return plan;
+        } finally {
+            index.release();
         }
-        return plan;
     }
 
     @Override
     public Cursor query(final Filter filter, final NodeState root) {
-        final IndexNode index = tracker.getIndexNode("/");
-        checkState(index != null, "The Lucene index is not available");
-
         FullTextExpression ft = filter.getFullTextConstraint();
         Set<String> relPaths = getRelativePaths(ft);
         if (relPaths.size() > 1) {
             return new MultiLuceneIndex(filter, root, relPaths).query();
         }
+
         final String parent = relPaths.size() == 0 ? "" : relPaths.iterator().next();
         // we only restrict non-full-text conditions if there is
         // no relative property in the full-text constraint
@@ -338,14 +343,14 @@ public class LuceneIndex implements Full
              * @return true if any document is loaded
              */
             private boolean loadDocs() {
-                IndexNode indexNode = null;
-                IndexSearcher searcher = null;
                 ScoreDoc lastDocToRecord = null;
+
+                IndexNode indexNode = tracker.acquireIndexNode("/");
+                checkState(indexNode != null);
                 try {
-                    indexNode = acquire();
-                    searcher = indexNode.acquireSearcher();
+                    IndexSearcher searcher = indexNode.getSearcher();
                     Query query = getQuery(filter, searcher.getIndexReader(),
-                            nonFullTextConstraints, analyzer, index.getDefinition());
+                            nonFullTextConstraints, analyzer, indexNode.getDefinition());
                     TopDocs docs;
                     if (lastDoc != null) {
                         docs = searcher.searchAfter(lastDoc, query, LUCENE_QUERY_BATCH_SIZE);
@@ -363,27 +368,14 @@ public class LuceneIndex implements Full
                 } catch (IOException e) {
                     LOG.warn("query via {} failed.", LuceneIndex.this, e);
                 } finally {
-                    release(indexNode, searcher);
+                    indexNode.release();
                 }
+
                 if (lastDocToRecord != null) {
                     this.lastDoc = lastDocToRecord;
                 }
-                return !queue.isEmpty();
-            }
 
-            private IndexNode acquire() {
-                return tracker.getIndexNode("/");
-            }
-
-            private void release(IndexNode indexNode, IndexSearcher searcher){
-                try {
-                    if(searcher != null){
-                        indexNode.releaseSearcher();
-                    }
-                } catch (IOException e) {
-                    LOG.warn("Error occurred while releasing/closing the " +
-                            "IndexSearcher", e);
-                }
+                return !queue.isEmpty();
             }
         };
         return new LucenePathCursor(itr, settings);