Author: chetanm
Date: Thu Sep 15 07:20:59 2016
New Revision: 1760868

URL: http://svn.apache.org/viewvc?rev=1760868&view=rev
Log:
OAK-4412 - Lucene hybrid index

Refactor the TimedRefreshPolicy to extra policy for sync index i.e. 
RefreshOnWritePolicy which does the refresh upon write itself without waiting 
for any refresh interval

Changed the NRTIndex to reuse the reader and make use of openIfChanged to 
ensure that readers are not created fresh if no change has happened. This fact 
is later used in IndexNode to avoid refreshing the IndexSearcher. This is an 
extra defence in addition to update flag in policy implementation.

Added:
    
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/IndexUpdateListener.java
      - copied, changed from r1760867, 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ReaderRefreshPolicy.java
    
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/RefreshOnReadPolicy.java
   (with props)
    
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/RefreshOnWritePolicy.java
      - copied, changed from r1760867, 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ReaderRefreshPolicy.java
    
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/RecordingRunnable.java
      - copied, changed from r1760867, 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ReaderRefreshPolicy.java
    
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/RefreshOnReadPolicyTest.java
   (with props)
    
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/RefreshOnWritePolicyTest.java
      - copied, changed from r1760867, 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ReaderRefreshPolicy.java
Modified:
    
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNode.java
    
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/NRTIndex.java
    
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/NRTIndexFactory.java
    
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ReaderRefreshPolicy.java
    
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/TimedRefreshPolicy.java
    
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/NRTIndexTest.java
    
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/TimedRefreshPolicyTest.java

Modified: 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNode.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNode.java?rev=1760868&r1=1760867&r2=1760868&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNode.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNode.java
 Thu Sep 15 07:20:59 2016
@@ -85,6 +85,8 @@ public class IndexNode {
 
     private boolean closed = false;
 
+    private List<LuceneIndexReader> nrtReaders;
+
     IndexNode(String name, IndexDefinition definition, List<LuceneIndexReader> 
readers, @Nullable NRTIndex nrtIndex)
             throws IOException {
         checkArgument(!readers.isEmpty());
@@ -92,7 +94,8 @@ public class IndexNode {
         this.definition = definition;
         this.readers = readers;
         this.nrtIndex = nrtIndex;
-        this.indexSearcher = new IndexSearcher(createReader());
+        this.nrtReaders = getNRTReaders();
+        this.indexSearcher = new IndexSearcher(createReader(nrtReaders));
         this.refreshPolicy = nrtIndex != null ? nrtIndex.getRefreshPolicy() : 
ReaderRefreshPolicy.NEVER;
     }
 
@@ -158,8 +161,14 @@ public class IndexNode {
     }
 
     private void refreshReaders(){
-        indexSearcher = new IndexSearcher(createReader());
-        log.debug("Refreshed reader for index [{}]", definition);
+        List<LuceneIndexReader> newNRTReaders = getNRTReaders();
+        //The list reference would differ if index got updated
+        //so if they are same no need to reinitialize the searcher
+        if (newNRTReaders != nrtReaders) {
+            nrtReaders = newNRTReaders;
+            indexSearcher = new IndexSearcher(createReader(nrtReaders));
+            log.debug("Refreshed reader for index [{}]", definition);
+        }
     }
 
     private LuceneIndexReader getDefaultReader(){
@@ -167,8 +176,7 @@ public class IndexNode {
         return readers.get(0);
     }
 
-    private IndexReader createReader() {
-        List<LuceneIndexReader> nrtReaders = getNRTReaders();
+    private IndexReader createReader(List<LuceneIndexReader> nrtReaders) {
         if (readers.size() == 1 && nrtReaders.isEmpty()){
             return readers.get(0).getReader();
         }

Copied: 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/IndexUpdateListener.java
 (from r1760867, 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ReaderRefreshPolicy.java)
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/IndexUpdateListener.java?p2=jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/IndexUpdateListener.java&p1=jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ReaderRefreshPolicy.java&r1=1760867&r2=1760868&rev=1760868&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ReaderRefreshPolicy.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/IndexUpdateListener.java
 Thu Sep 15 07:20:59 2016
@@ -19,20 +19,7 @@
 
 package org.apache.jackrabbit.oak.plugins.index.lucene.hybrid;
 
-public interface ReaderRefreshPolicy {
-    ReaderRefreshPolicy NEVER = new ReaderRefreshPolicy() {
-        @Override
-        public void refreshOnReadIfRequired(Runnable refreshCallback) {
-            //Never refresh
-        }
+public interface IndexUpdateListener extends ReaderRefreshPolicy {
 
-        @Override
-        public void refreshOnWriteIfRequired(Runnable refreshCallback) {
-            //Never refresh
-        }
-    };
-
-    void refreshOnReadIfRequired(Runnable refreshCallback);
-
-    void refreshOnWriteIfRequired(Runnable refreshCallback);
+    void updated();
 }

Modified: 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/NRTIndex.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/NRTIndex.java?rev=1760868&r1=1760867&r2=1760868&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/NRTIndex.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/NRTIndex.java
 Thu Sep 15 07:20:59 2016
@@ -28,6 +28,7 @@ import java.util.concurrent.atomic.Atomi
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import org.apache.commons.io.FileUtils;
 import org.apache.jackrabbit.oak.plugins.index.lucene.IndexCopier;
@@ -47,6 +48,7 @@ import org.apache.lucene.store.NRTCachin
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
 
 
@@ -62,16 +64,18 @@ public class NRTIndex implements Closeab
     private final IndexDefinition definition;
     private final IndexCopier indexCopier;
     private final LuceneIndexReader previousReader;
-    private final TimedRefreshPolicy refreshPolicy;
+    private final IndexUpdateListener refreshPolicy;
 
     private IndexWriter indexWriter;
     private NRTIndexWriter nrtIndexWriter;
     private File indexDir;
     private Directory directory;
+    private DirectoryReader dirReader;
     private boolean closed;
+    private List<LuceneIndexReader> readers;
 
     public NRTIndex(IndexDefinition definition, IndexCopier indexCopier,
-                    TimedRefreshPolicy refreshPolicy, @Nullable NRTIndex 
previous) {
+                    IndexUpdateListener refreshPolicy, @Nullable NRTIndex 
previous) {
         this.definition = definition;
         this.indexCopier = indexCopier;
         this.refreshPolicy = refreshPolicy;
@@ -80,7 +84,8 @@ public class NRTIndex implements Closeab
 
     @CheckForNull
     LuceneIndexReader getPrimaryReader() {
-        return createReader();
+        DirectoryReader reader = createReader();
+        return reader != null ? new NRTReader(reader) : null;
     }
 
     public LuceneIndexWriter getWriter() throws IOException {
@@ -91,18 +96,30 @@ public class NRTIndex implements Closeab
         return nrtIndexWriter;
     }
 
-    public List<LuceneIndexReader> getReaders() {
+    /**
+     * Returns the list of LuceneIndexReader. If the writer has not received
+     * any updates between 2 calls to this method then same list would be
+     * returned.
+     */
+    public synchronized List<LuceneIndexReader> getReaders() {
         checkState(!closed);
-        List<LuceneIndexReader> readers = Lists.newArrayListWithCapacity(2);
-        LuceneIndexReader latestReader = createReader();
+        DirectoryReader latestReader = createReader();
+        //reader not changed i.e. no change in index
+        //reuse old readers
+        if (latestReader == dirReader && readers != null){
+            return readers;
+        }
+        List<LuceneIndexReader> newReaders = Lists.newArrayListWithCapacity(2);
         if (latestReader != null) {
-            readers.add(latestReader);
+            newReaders.add(new NRTReader(latestReader));
         }
 
         //Old reader should be added later
         if (previousReader != null) {
-            readers.add(previousReader);
+            newReaders.add(previousReader);
         }
+        dirReader = latestReader;
+        readers = ImmutableList.copyOf(newReaders);
         return readers;
     }
 
@@ -141,18 +158,31 @@ public class NRTIndex implements Closeab
         return indexDir;
     }
 
+    /**
+     * If index was updated then a new reader would be returned otherwise
+     * existing reader would be returned
+     */
     @CheckForNull
-    private LuceneIndexReader createReader() {
+    private synchronized DirectoryReader createReader() {
         checkState(!closed);
         //Its possible that readers are obtained
         //before anything gets indexed
         if (indexWriter == null) {
             return null;
         }
+        DirectoryReader result = dirReader;
         try {
             //applyDeletes is false as layers above would take care of
             //stale result
-            return new NRTReader(DirectoryReader.open(indexWriter, false));
+            if (dirReader == null) {
+                result = DirectoryReader.open(indexWriter, false);
+            } else {
+                DirectoryReader newReader = 
DirectoryReader.openIfChanged(dirReader, indexWriter, false);
+                if (newReader != null) {
+                    result = newReader;
+                }
+            }
+            return result;
         } catch (IOException e) {
             log.warn("Error opening index [{}]", e);
         }
@@ -166,6 +196,11 @@ public class NRTIndex implements Closeab
         //TODO make these configurable
         directory = new NRTCachingDirectory(fsdir, 1, 1);
         IndexWriterConfig config = 
IndexWriterUtils.getIndexWriterConfig(definition, false);
+
+        //TODO Explore following for optimizing indexing speed
+        //config.setUseCompoundFile(false);
+        //config.setRAMBufferSizeMB(1024*1024*25);
+
         indexWriter = new IndexWriter(directory, config);
         return new NRTIndexWriter(indexWriter);
     }
@@ -179,7 +214,7 @@ public class NRTIndex implements Closeab
         private final IndexReader indexReader;
 
         public NRTReader(IndexReader indexReader) {
-            this.indexReader = indexReader;
+            this.indexReader = checkNotNull(indexReader);
         }
 
         @Override

Modified: 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/NRTIndexFactory.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/NRTIndexFactory.java?rev=1760868&r1=1760867&r2=1760868&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/NRTIndexFactory.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/NRTIndexFactory.java
 Thu Sep 15 07:20:59 2016
@@ -102,7 +102,11 @@ public class NRTIndexFactory implements
         return existing.get(existing.size() - 1);
     }
 
-    private TimedRefreshPolicy getRefreshPolicy(IndexDefinition definition) {
-        return new TimedRefreshPolicy(definition.isSyncIndexingEnabled(), 
clock, TimeUnit.SECONDS, refreshDeltaInSecs);
+    private IndexUpdateListener getRefreshPolicy(IndexDefinition definition) {
+        if (definition.isSyncIndexingEnabled()){
+            return new RefreshOnWritePolicy();
+            //return new RefreshOnReadPolicy(clock, TimeUnit.SECONDS, 
refreshDeltaInSecs);
+        }
+        return new TimedRefreshPolicy(clock, TimeUnit.SECONDS, 
refreshDeltaInSecs);
     }
 }

Modified: 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ReaderRefreshPolicy.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ReaderRefreshPolicy.java?rev=1760868&r1=1760867&r2=1760868&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ReaderRefreshPolicy.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ReaderRefreshPolicy.java
 Thu Sep 15 07:20:59 2016
@@ -32,7 +32,25 @@ public interface ReaderRefreshPolicy {
         }
     };
 
+    /**
+     * This would be invoked before any query is performed
+     * to provide a chance for IndexNode to refresh the readers
+     *
+     * <p>The index may or may not be updated when this method
+     * is invoked
+     *
+     * @param refreshCallback callback to refresh the readers
+     */
     void refreshOnReadIfRequired(Runnable refreshCallback);
 
+    /**
+     * This would invoked after some writes have been performed
+     * and as a final step refresh request is being made.
+     *
+     * <p>Any time its invoked it can be assumed that index has been
+     * updated
+     *
+     * @param refreshCallback callback to refresh the readers
+     */
     void refreshOnWriteIfRequired(Runnable refreshCallback);
 }

Added: 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/RefreshOnReadPolicy.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/RefreshOnReadPolicy.java?rev=1760868&view=auto
==============================================================================
--- 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/RefreshOnReadPolicy.java
 (added)
+++ 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/RefreshOnReadPolicy.java
 Thu Sep 15 07:20:59 2016
@@ -0,0 +1,95 @@
+/*
+ * 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.oak.plugins.index.lucene.hybrid;
+
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import org.apache.jackrabbit.oak.stats.Clock;
+
+/**
+ * This policy ensures that any writes that have been done to index are made 
visible
+ * *before* any read is performed. Its meant as an alternative to {@link 
RefreshOnWritePolicy}
+ * and for "sync" indexes. For "nrt" indexes {@link TimedRefreshPolicy} should 
be preferred
+ *
+ * <p>The readers are not refreshed immediately upon write. Instead they would 
be refreshed if
+ *
+ * <ul>
+ *     <li>Upon write if refreshDelta time has elapsed then readers would be 
refreshed</li>
+ *     <li>Upon read if index is found to be updated then again readers would 
be refreshed</li>
+ * </ul>
+ *
+ * <p>This policy can result in some contention if index is being frequently 
updated and
+ * queried.
+ *
+ * *This is an experimental policy. Currently it causes high contention*
+ */
+public class RefreshOnReadPolicy implements ReaderRefreshPolicy, 
IndexUpdateListener {
+    private final AtomicBoolean dirty = new AtomicBoolean();
+    private final Object lock = new Object();
+    private final Clock clock;
+    private final long refreshDelta;
+    private volatile long lastRefreshTime;
+
+    public RefreshOnReadPolicy(Clock clock, TimeUnit unit, long refreshDelta) {
+        this.clock = clock;
+        this.refreshDelta = unit.toMillis(refreshDelta);
+    }
+
+    @Override
+    public void refreshOnReadIfRequired(Runnable refreshCallback) {
+        if (dirty.get()){
+            refreshWithLock(refreshCallback, false);
+        }
+    }
+
+    @Override
+    public void refreshOnWriteIfRequired(Runnable refreshCallback) {
+        long currentTime = clock.getTime();
+        if (currentTime - lastRefreshTime > refreshDelta) {
+            //Do not set dirty instead directly refresh
+            refreshWithLock(refreshCallback, true);
+        } else {
+            synchronized (lock){
+                //Needs to be done in a lock otherwise
+                //refreshWithLock would override this
+                dirty.set(true);
+            }
+        }
+    }
+
+    @Override
+    public void updated() {
+        //Detect dirty based on call from refreshOnWriteIfRequired
+        //as that would *always* be called if the index has been updated
+        //And ensures that it gets calls after all changes for that index
+        //for that transaction got committed
+    }
+
+    private void refreshWithLock(Runnable refreshCallback, boolean 
forceRefresh) {
+        synchronized (lock){
+            if (dirty.get() || forceRefresh) {
+                refreshCallback.run();
+                dirty.set(false);
+                lastRefreshTime = clock.getTime();
+            }
+        }
+    }
+}

Propchange: 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/RefreshOnReadPolicy.java
------------------------------------------------------------------------------
    svn:eol-style = native

Copied: 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/RefreshOnWritePolicy.java
 (from r1760867, 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ReaderRefreshPolicy.java)
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/RefreshOnWritePolicy.java?p2=jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/RefreshOnWritePolicy.java&p1=jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ReaderRefreshPolicy.java&r1=1760867&r2=1760868&rev=1760868&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ReaderRefreshPolicy.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/RefreshOnWritePolicy.java
 Thu Sep 15 07:20:59 2016
@@ -19,20 +19,32 @@
 
 package org.apache.jackrabbit.oak.plugins.index.lucene.hybrid;
 
-public interface ReaderRefreshPolicy {
-    ReaderRefreshPolicy NEVER = new ReaderRefreshPolicy() {
-        @Override
-        public void refreshOnReadIfRequired(Runnable refreshCallback) {
-            //Never refresh
-        }
+import java.util.concurrent.atomic.AtomicBoolean;
 
-        @Override
-        public void refreshOnWriteIfRequired(Runnable refreshCallback) {
-            //Never refresh
-        }
-    };
+/**
+ * Policy which performs immediate refresh upon completion of writes
+ */
+public class RefreshOnWritePolicy implements ReaderRefreshPolicy, 
IndexUpdateListener {
+    private final AtomicBoolean dirty = new AtomicBoolean();
 
-    void refreshOnReadIfRequired(Runnable refreshCallback);
+    @Override
+    public void refreshOnReadIfRequired(Runnable refreshCallback) {
+        //As writer itself refreshes the index. No refresh done
+        //on read
+    }
+
+    @Override
+    public void refreshOnWriteIfRequired(Runnable refreshCallback) {
+        //For sync indexing mode we refresh the reader immediately
+        //on the writer thread. So that any read call later sees upto date 
index
+        if (dirty.get()) {
+            refreshCallback.run();
+            dirty.set(false);
+        }
+    }
 
-    void refreshOnWriteIfRequired(Runnable refreshCallback);
+    @Override
+    public void updated() {
+        dirty.set(true);
+    }
 }

Modified: 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/TimedRefreshPolicy.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/TimedRefreshPolicy.java?rev=1760868&r1=1760867&r2=1760868&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/TimedRefreshPolicy.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/TimedRefreshPolicy.java
 Thu Sep 15 07:20:59 2016
@@ -24,48 +24,28 @@ import java.util.concurrent.atomic.Atomi
 
 import org.apache.jackrabbit.oak.stats.Clock;
 
-public class TimedRefreshPolicy implements ReaderRefreshPolicy {
+public class TimedRefreshPolicy implements ReaderRefreshPolicy, 
IndexUpdateListener {
     private final AtomicBoolean dirty = new AtomicBoolean();
-    private final boolean syncIndexingMode;
     private final Clock clock;
     private final long refreshDelta;
     private volatile long lastRefreshTime;
 
-    public TimedRefreshPolicy(boolean syncIndexingMode, Clock clock, TimeUnit 
unit, long refreshDelta) {
-        this.syncIndexingMode = syncIndexingMode;
+    public TimedRefreshPolicy(Clock clock, TimeUnit unit, long refreshDelta) {
         this.clock = clock;
         this.refreshDelta = unit.toMillis(refreshDelta);
     }
 
     @Override
     public void refreshOnReadIfRequired(Runnable refreshCallback) {
-        if (syncIndexingMode) {
-            //As writer itself refreshes the index. No refresh done
-            //on read
-            return;
-        }
         refreshIfRequired(refreshCallback);
     }
 
     @Override
     public void refreshOnWriteIfRequired(Runnable refreshCallback) {
-        if (syncIndexingMode) {
-            //For sync indexing mode we refresh the reader immediately
-            //on the writer thread. So that any read call later sees upto date 
index
-
-            //Another possibility is to refresh the readers upon first query 
post index update
-            //but that would mean that if multiple queries get invoked 
simultaneously then
-            //others would get blocked. So here we take hit on write side. If 
that proves to
-            //be problematic query side refresh can be looked into
-            if (dirty.get()) {
-                refreshCallback.run();
-                dirty.set(false);
-            }
-        } else {
-            refreshIfRequired(refreshCallback);
-        }
+        refreshIfRequired(refreshCallback);
     }
 
+    @Override
     public void updated() {
         dirty.set(true);
     }
@@ -80,6 +60,4 @@ public class TimedRefreshPolicy implemen
             }
         }
     }
-
-
 }

Modified: 
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/NRTIndexTest.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/NRTIndexTest.java?rev=1760868&r1=1760867&r2=1760868&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/NRTIndexTest.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/NRTIndexTest.java
 Thu Sep 15 07:20:59 2016
@@ -47,6 +47,7 @@ import static org.junit.Assert.assertEqu
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
@@ -170,6 +171,26 @@ public class NRTIndexTest {
         assertNotEquals(idx1.getIndexDir(), idx2.getIndexDir());
     }
 
+    @Test
+    public void sameReaderIfNoChange() throws Exception{
+        IndexDefinition idxDefn = getSyncIndexDefinition("/foo");
+        NRTIndex idx1 = indexFactory.createIndex(idxDefn);
+        LuceneIndexWriter w1 = idx1.getWriter();
+
+        Document d1 = new Document();
+        d1.add(newPathField("/a/b"));
+        w1.updateDocument("/a/b", d1);
+
+        List<LuceneIndexReader> readers = idx1.getReaders();
+        List<LuceneIndexReader> readers2 = idx1.getReaders();
+
+        assertSame(readers, readers2);
+
+        w1.updateDocument("/a/b", d1);
+        List<LuceneIndexReader> readers3 = idx1.getReaders();
+        assertNotSame(readers2, readers3);
+    }
+
     private IndexDefinition getSyncIndexDefinition(String indexPath) {
         builder.setProperty(IndexConstants.INDEX_PATH, indexPath);
         TestUtil.enableIndexingMode(builder, IndexingMode.NRT);

Copied: 
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/RecordingRunnable.java
 (from r1760867, 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ReaderRefreshPolicy.java)
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/RecordingRunnable.java?p2=jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/RecordingRunnable.java&p1=jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ReaderRefreshPolicy.java&r1=1760867&r2=1760868&rev=1760868&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ReaderRefreshPolicy.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/RecordingRunnable.java
 Thu Sep 15 07:20:59 2016
@@ -19,20 +19,27 @@
 
 package org.apache.jackrabbit.oak.plugins.index.lucene.hybrid;
 
-public interface ReaderRefreshPolicy {
-    ReaderRefreshPolicy NEVER = new ReaderRefreshPolicy() {
-        @Override
-        public void refreshOnReadIfRequired(Runnable refreshCallback) {
-            //Never refresh
-        }
-
-        @Override
-        public void refreshOnWriteIfRequired(Runnable refreshCallback) {
-            //Never refresh
-        }
-    };
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 
-    void refreshOnReadIfRequired(Runnable refreshCallback);
+class RecordingRunnable implements Runnable {
+    private boolean invoked;
+    @Override
+    public void run() {
+        invoked = true;
+    }
 
-    void refreshOnWriteIfRequired(Runnable refreshCallback);
+    public void assertInvokedAndReset(){
+        assertTrue(invoked);
+        reset();
+    }
+
+    public void assertNotInvokedAndReset(){
+        assertFalse(invoked);
+        reset();
+    }
+
+    public void reset(){
+        invoked = false;
+    }
 }

Added: 
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/RefreshOnReadPolicyTest.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/RefreshOnReadPolicyTest.java?rev=1760868&view=auto
==============================================================================
--- 
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/RefreshOnReadPolicyTest.java
 (added)
+++ 
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/RefreshOnReadPolicyTest.java
 Thu Sep 15 07:20:59 2016
@@ -0,0 +1,83 @@
+/*
+ * 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.oak.plugins.index.lucene.hybrid;
+
+import java.util.concurrent.TimeUnit;
+
+import org.apache.jackrabbit.oak.stats.Clock;
+import org.junit.Test;
+
+public class RefreshOnReadPolicyTest {
+    private Clock clock = new Clock.Virtual();
+    private RecordingRunnable refreshCallback = new RecordingRunnable();
+    private RefreshOnReadPolicy policy = new RefreshOnReadPolicy(clock, 
TimeUnit.SECONDS, 1);
+    private long refreshDelta = TimeUnit.SECONDS.toMillis(1) + 1;
+
+    @Test
+    public void noRefreshOnReadIfNotUpdated() throws Exception{
+        policy.refreshOnReadIfRequired(refreshCallback);
+        refreshCallback.assertNotInvokedAndReset();
+    }
+
+    @Test
+    public void refreshOnFirstWrite() throws Exception{
+        clock.waitUntil(System.currentTimeMillis());
+
+        policy.refreshOnWriteIfRequired(refreshCallback);
+        refreshCallback.assertInvokedAndReset();
+    }
+
+    @Test
+    public void refreshOnReadAfterWrite() throws Exception{
+        clock.waitUntil(System.currentTimeMillis());
+
+        policy.refreshOnWriteIfRequired(refreshCallback);
+        refreshCallback.reset();
+        //Call again without change in time
+        policy.refreshOnWriteIfRequired(refreshCallback);
+
+        //This time callback should not be invoked
+        refreshCallback.assertNotInvokedAndReset();
+
+        policy.refreshOnReadIfRequired(refreshCallback);
+        //On read the callback should be invoked
+        refreshCallback.assertInvokedAndReset();
+    }
+
+    @Test
+    public void refreshOnWriteWithTimeElapsed() throws Exception{
+        clock.waitUntil(System.currentTimeMillis());
+
+        policy.refreshOnWriteIfRequired(refreshCallback);
+        refreshCallback.reset();
+
+        //Call again without change in time
+        policy.refreshOnWriteIfRequired(refreshCallback);
+
+        //This time callback should not be invoked
+        refreshCallback.assertNotInvokedAndReset();
+
+        clock.waitUntil(clock.getTime() + refreshDelta);
+
+        policy.refreshOnWriteIfRequired(refreshCallback);
+        refreshCallback.assertInvokedAndReset();
+    }
+
+}
\ No newline at end of file

Propchange: 
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/RefreshOnReadPolicyTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Copied: 
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/RefreshOnWritePolicyTest.java
 (from r1760867, 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ReaderRefreshPolicy.java)
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/RefreshOnWritePolicyTest.java?p2=jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/RefreshOnWritePolicyTest.java&p1=jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ReaderRefreshPolicy.java&r1=1760867&r2=1760868&rev=1760868&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ReaderRefreshPolicy.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/RefreshOnWritePolicyTest.java
 Thu Sep 15 07:20:59 2016
@@ -19,20 +19,30 @@
 
 package org.apache.jackrabbit.oak.plugins.index.lucene.hybrid;
 
-public interface ReaderRefreshPolicy {
-    ReaderRefreshPolicy NEVER = new ReaderRefreshPolicy() {
-        @Override
-        public void refreshOnReadIfRequired(Runnable refreshCallback) {
-            //Never refresh
-        }
-
-        @Override
-        public void refreshOnWriteIfRequired(Runnable refreshCallback) {
-            //Never refresh
-        }
-    };
+import org.junit.Test;
 
-    void refreshOnReadIfRequired(Runnable refreshCallback);
+public class RefreshOnWritePolicyTest {
+    private RecordingRunnable refreshCallback = new RecordingRunnable();
 
-    void refreshOnWriteIfRequired(Runnable refreshCallback);
-}
+    @Test
+    public void noRefreshOnRead() throws Exception{
+        RefreshOnWritePolicy policy = new RefreshOnWritePolicy();
+        policy.refreshOnReadIfRequired(refreshCallback);
+        refreshCallback.assertNotInvokedAndReset();
+
+        //Even after update it should not be refreshed
+        policy.updated();
+        policy.refreshOnReadIfRequired(refreshCallback);
+        refreshCallback.assertNotInvokedAndReset();
+    }
+
+    @Test
+    public void refreshOnWrite() throws Exception{
+        RefreshOnWritePolicy policy = new RefreshOnWritePolicy();
+
+        policy.updated();
+        policy.refreshOnWriteIfRequired(refreshCallback);
+        refreshCallback.assertInvokedAndReset();
+    }
+
+}
\ No newline at end of file

Modified: 
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/TimedRefreshPolicyTest.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/TimedRefreshPolicyTest.java?rev=1760868&r1=1760867&r2=1760868&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/TimedRefreshPolicyTest.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/TimedRefreshPolicyTest.java
 Thu Sep 15 07:20:59 2016
@@ -24,9 +24,6 @@ import java.util.concurrent.TimeUnit;
 import org.apache.jackrabbit.oak.stats.Clock;
 import org.junit.Test;
 
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-
 public class TimedRefreshPolicyTest {
     private Clock clock = new Clock.Virtual();
     private RecordingRunnable refreshCallback = new RecordingRunnable();
@@ -34,9 +31,9 @@ public class TimedRefreshPolicyTest {
     @Test
     public void dirtyAndFirstCheck() throws Exception{
         clock.waitUntil(System.currentTimeMillis());
-        TimedRefreshPolicy policy = new TimedRefreshPolicy(false, clock, 
TimeUnit.SECONDS, 1);
+        TimedRefreshPolicy policy = new TimedRefreshPolicy(clock, 
TimeUnit.SECONDS, 1);
         policy.refreshOnWriteIfRequired(refreshCallback);
-        refreshCallback.assertNotInvokedAndRest();
+        refreshCallback.assertNotInvokedAndReset();
 
         policy.updated();
 
@@ -44,47 +41,47 @@ public class TimedRefreshPolicyTest {
         refreshCallback.assertInvokedAndReset();
 
         policy.refreshOnWriteIfRequired(refreshCallback);
-        refreshCallback.assertNotInvokedAndRest();
+        refreshCallback.assertNotInvokedAndReset();
     }
 
     @Test
     public void dirtyAndNotElapsedTimed() throws Exception{
         clock.waitUntil(System.currentTimeMillis());
-        TimedRefreshPolicy policy = new TimedRefreshPolicy(false, clock, 
TimeUnit.SECONDS, 1);
+        TimedRefreshPolicy policy = new TimedRefreshPolicy(clock, 
TimeUnit.SECONDS, 1);
 
         policy.updated();
         policy.refreshOnWriteIfRequired(refreshCallback);
         refreshCallback.assertInvokedAndReset();
 
         policy.refreshOnWriteIfRequired(refreshCallback);
-        refreshCallback.assertNotInvokedAndRest();
+        refreshCallback.assertNotInvokedAndReset();
 
         policy.updated();
         //Given time has not elapsed it should still be false
         policy.refreshOnWriteIfRequired(refreshCallback);
-        refreshCallback.assertNotInvokedAndRest();
+        refreshCallback.assertNotInvokedAndReset();
     }
 
     @Test
     public void dirtyAndElapsedTime() throws Exception{
         clock.waitUntil(System.currentTimeMillis());
-        TimedRefreshPolicy policy = new TimedRefreshPolicy(false, clock, 
TimeUnit.SECONDS, 1);
+        TimedRefreshPolicy policy = new TimedRefreshPolicy(clock, 
TimeUnit.SECONDS, 1);
 
         policy.updated();
         policy.refreshOnWriteIfRequired(refreshCallback);
         refreshCallback.assertInvokedAndReset();
 
         policy.refreshOnWriteIfRequired(refreshCallback);
-        refreshCallback.assertNotInvokedAndRest();
+        refreshCallback.assertNotInvokedAndReset();
 
         policy.updated();
         //Given time has not elapsed it should still be false
         //in both reader and writer mode
         policy.refreshOnWriteIfRequired(refreshCallback);
-        refreshCallback.assertNotInvokedAndRest();
+        refreshCallback.assertNotInvokedAndReset();
 
         policy.refreshOnWriteIfRequired(refreshCallback);
-        refreshCallback.assertNotInvokedAndRest();
+        refreshCallback.assertNotInvokedAndReset();
 
         //Let the refresh delta time elapse
         long refreshDelta = TimeUnit.SECONDS.toMillis(1) + 1;
@@ -94,7 +91,7 @@ public class TimedRefreshPolicyTest {
         refreshCallback.assertInvokedAndReset();
 
         policy.refreshOnWriteIfRequired(refreshCallback);
-        refreshCallback.assertNotInvokedAndRest();
+        refreshCallback.assertNotInvokedAndReset();
 
         policy.updated();
         //Do similar check for read
@@ -104,53 +101,6 @@ public class TimedRefreshPolicyTest {
         refreshCallback.assertInvokedAndReset();
 
         policy.refreshOnReadIfRequired(refreshCallback);
-        refreshCallback.assertNotInvokedAndRest();
+        refreshCallback.assertNotInvokedAndReset();
     }
-
-    @Test
-    public void syncIndex() throws Exception{
-        clock.waitUntil(System.currentTimeMillis());
-        TimedRefreshPolicy policy = new TimedRefreshPolicy(true, clock, 
TimeUnit.SECONDS, 1);
-
-        policy.updated();
-        policy.refreshOnWriteIfRequired(refreshCallback);
-        refreshCallback.assertInvokedAndReset();
-
-        policy.updated();
-
-        //Write would not lead to reader refresh as time has yet not expired
-        policy.refreshOnWriteIfRequired(refreshCallback);
-        refreshCallback.assertInvokedAndReset();
-
-        //Already refreshed so reader should not be refreshed again
-        policy.refreshOnReadIfRequired(refreshCallback);
-        refreshCallback.assertNotInvokedAndRest();
-
-        //Checking on write again should not lead to refresh
-        policy.refreshOnWriteIfRequired(refreshCallback);
-        refreshCallback.assertNotInvokedAndRest();
-    }
-
-    private static class RecordingRunnable implements Runnable {
-        private boolean invoked;
-        @Override
-        public void run() {
-            invoked = true;
-        }
-
-        public void assertInvokedAndReset(){
-            assertTrue(invoked);
-            reset();
-        }
-
-        public void assertNotInvokedAndRest(){
-            assertFalse(invoked);
-            reset();
-        }
-
-        void reset(){
-            invoked = false;
-        }
-    }
-
 }
\ No newline at end of file



Reply via email to