Author: chetanm
Date: Thu Oct  5 04:20:36 2017
New Revision: 1811155

URL: http://svn.apache.org/viewvc?rev=1811155&view=rev
Log:
OAK-6777 - IndexReader closed exception in previous reader

Actual fix

Modified:
    
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/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ReaderRefCountIT.java

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=1811155&r1=1811154&r2=1811155&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 Oct  5 04:20:36 2017
@@ -84,7 +84,9 @@ public class NRTIndex implements Closeab
     private File indexDir;
     private Directory directory;
     private DirectoryReader dirReader;
+    private DirectoryReader dirReaderUsedForPrevious;
     private boolean closed;
+    private boolean previousModeEnabled;
     private List<LuceneIndexReader> readers;
     private final List<IndexReader> openedReaders;
     private final boolean assertAllReadersClosed;
@@ -108,12 +110,19 @@ public class NRTIndex implements Closeab
         this.openTime = statisticsProvider.getTimer(metricName("OPEN_TIME"), 
StatsOptions.METRICS_ONLY).time();
     }
 
+    /**
+     * Note that this method is called from a different NRTIndex instance 
getReaders
+     * call. So "dirReader" instance changed here is different
+     */
     @CheckForNull
     private LuceneIndexReader getPrimaryReader() {
-        DirectoryReader latestReader = createReader();
-        if (latestReader != dirReader) {
-            decrementReaderUseCount(dirReader);
-            dirReader = latestReader;
+        DirectoryReader latestReader = createReader(dirReaderUsedForPrevious);
+        while (latestReader != null && !latestReader.tryIncRef()) {
+            latestReader = createReader(dirReaderUsedForPrevious);
+        }
+        if (latestReader != dirReaderUsedForPrevious) {
+            decrementReaderUseCount(dirReaderUsedForPrevious);
+            dirReaderUsedForPrevious = latestReader;
         }
         return latestReader != null ? new NRTReader(latestReader, directory) : 
null;
     }
@@ -133,12 +142,14 @@ public class NRTIndex implements Closeab
      */
     public synchronized List<LuceneIndexReader> getReaders() {
         checkState(!closed);
-        DirectoryReader latestReader = createReader();
+        checkState(!previousModeEnabled);
+        DirectoryReader latestReader = createReader(dirReader);
         //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) {
             newReaders.add(new NRTReader(latestReader, directory));
@@ -150,7 +161,7 @@ public class NRTIndex implements Closeab
             newReaders.add(previousReader);
         }
 
-        decrementReaderUseCount(dirReader);
+        decrementReaderUseCount(readers);
 
         dirReader = latestReader;
         readers = ImmutableList.copyOf(newReaders);
@@ -161,6 +172,20 @@ public class NRTIndex implements Closeab
         return refreshPolicy;
     }
 
+    /**
+     * Disconnects the previous reader used by this NRTIndex. Note that this 
would be
+     * different from 'dirReaderUsedForPrevious' which is meant to be used by 
newer NRTIndex
+     * which refers to this NRTIndex as previous
+     */
+    public void disconnectPrevious(){
+        decrementReaderUseCount(readers);
+        readers = Collections.emptyList();
+
+        //From now onwards no caller should be invoked getReaders
+        //only call for getPrimaryReader would be allowed
+        previousModeEnabled = true;
+    }
+
     public void close() throws IOException {
         if (closed) {
             return;
@@ -168,9 +193,9 @@ public class NRTIndex implements Closeab
 
         log.debug("[{}] Closing NRTIndex [{}]", definition.getIndexPath(), 
getName());
 
-        if (dirReader != null){
-            dirReader.close();
-        }
+        decrementReaderUseCount(dirReaderUsedForPrevious);
+        //'readers' already has dirReader so no need to close it explicitly
+        decrementReaderUseCount(readers);
 
         assertAllReadersAreClosed();
 
@@ -223,6 +248,14 @@ public class NRTIndex implements Closeab
         }
     }
 
+    private void decrementReaderUseCount(@Nullable List<LuceneIndexReader> 
readers) {
+        if (readers != null) {
+            for (LuceneIndexReader r : readers) {
+                decrementReaderUseCount(r.getReader());
+            }
+        }
+    }
+
     private void decrementReaderUseCount(IndexReader reader) {
         try {
             if (reader != null) {
@@ -239,7 +272,7 @@ public class NRTIndex implements Closeab
      * existing reader would be returned
      */
     @CheckForNull
-    private synchronized DirectoryReader createReader() {
+    private synchronized DirectoryReader createReader(DirectoryReader 
dirReader) {
         checkState(!closed);
         //Its possible that readers are obtained
         //before anything gets indexed
@@ -251,7 +284,7 @@ public class NRTIndex implements Closeab
             TimerStats.Context ctx = refreshTimer.time();
             //applyDeletes is false as layers above would take care of
             //stale result
-            if (dirReader == null) {
+            if (dirReader == null || dirReader.getRefCount() == 0) {
                 result = DirectoryReader.open(indexWriter, false);
             } else {
                 DirectoryReader newReader = 
DirectoryReader.openIfChanged(dirReader, indexWriter, false);

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=1811155&r1=1811154&r2=1811155&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 Oct  5 04:20:36 2017
@@ -89,8 +89,13 @@ public class NRTIndexFactory implements
 
     @Override
     public void close() throws IOException {
-        for (NRTIndex index : indexes.values()){
-            index.close();
+        for (String indexPath : indexes.keySet()) {
+            //Close backwards i.e. newest NRTIndex first and then older
+            //as newer refers to previous NRTIndex readers
+            List<NRTIndex> nrtIndexes = indexes.get(indexPath);
+            for (int i = nrtIndexes.size() -1 ; i >= 0 ; i--) {
+                nrtIndexes.get(i).close();
+            }
         }
         indexes.clear();
     }
@@ -117,6 +122,10 @@ public class NRTIndexFactory implements
             return;
         }
         NRTIndex oldest = existing.remove(0);
+
+        //Disconnect the 'oldest' from NRTIndex which refers to that
+        //i.e. the next entry in existing
+        existing.get(0).disconnectPrevious();
         try {
             oldest.close();
         } catch (IOException e) {

Modified: 
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ReaderRefCountIT.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ReaderRefCountIT.java?rev=1811155&r1=1811154&r2=1811155&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ReaderRefCountIT.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ReaderRefCountIT.java
 Thu Oct  5 04:20:36 2017
@@ -97,7 +97,6 @@ public class ReaderRefCountIT {
      * This causes the IndexNodeManager to switch to newer indexes
      * and hence lead to creation and closing of older NRTIndexes
      */
-    @Ignore("OAK-6777")
     @Test
     public void indexTrackerUpdatesAndNRT() throws Exception{
         IndexDefinitionBuilder idx = new IndexDefinitionBuilder();


Reply via email to