Author: chetanm
Date: Thu Aug  3 06:27:41 2017
New Revision: 1803953

URL: http://svn.apache.org/viewvc?rev=1803953&view=rev
Log:
OAK-6500 - NRTIndex leaks file handles due to unclosed IndexReader

Add support for asserting that all reader instances are closed.

This validation currently disabled in NRTIndexTest as it causes failure

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/NRTIndexFactoryTest.java
    
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/NRTIndexTest.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=1803953&r1=1803952&r2=1803953&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 Aug  3 06:27:41 2017
@@ -22,6 +22,8 @@ package org.apache.jackrabbit.oak.plugin
 import java.io.Closeable;
 import java.io.File;
 import java.io.IOException;
+import java.util.Collections;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicInteger;
 
@@ -48,8 +50,6 @@ import org.apache.lucene.index.IndexWrit
 import org.apache.lucene.index.IndexableField;
 import org.apache.lucene.search.suggest.analyzing.AnalyzingInfixSuggester;
 import org.apache.lucene.store.Directory;
-import org.apache.lucene.store.FSDirectory;
-import org.apache.lucene.store.NRTCachingDirectory;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -86,16 +86,22 @@ public class NRTIndex implements Closeab
     private DirectoryReader dirReader;
     private boolean closed;
     private List<LuceneIndexReader> readers;
+    private final List<IndexReader> openedReaders;
+    private final boolean assertAllReadersClosed;
+
 
     public NRTIndex(IndexDefinition definition, IndexCopier indexCopier,
                     IndexUpdateListener refreshPolicy, @Nullable NRTIndex 
previous,
-                    StatisticsProvider statisticsProvider, NRTDirectoryFactory 
directoryFactory) {
+                    StatisticsProvider statisticsProvider, NRTDirectoryFactory 
directoryFactory,
+                    boolean assertAllReadersClosed) {
         this.definition = definition;
         this.indexCopier = indexCopier;
         this.refreshPolicy = refreshPolicy;
         this.previous = previous;
         this.statisticsProvider = statisticsProvider;
         this.directoryFactory = directoryFactory;
+        this.assertAllReadersClosed = assertAllReadersClosed;
+        this.openedReaders = assertAllReadersClosed ? new LinkedList<>() : 
Collections.emptyList();
 
         this.refreshTimer = 
statisticsProvider.getTimer(metricName("REFRESH_TIME"), 
StatsOptions.METRICS_ONLY);
         this.sizeHisto = statisticsProvider.getHistogram(metricName("SIZE"), 
StatsOptions.METRICS_ONLY);
@@ -148,10 +154,15 @@ public class NRTIndex implements Closeab
         return refreshPolicy;
     }
 
-    public synchronized void close() throws IOException {
+    public void close() throws IOException {
         if (closed) {
             return;
         }
+
+        log.debug("[{}] Closing NRTIndex [{}]", definition.getIndexPath(), 
getName());
+
+        assertAllReadersAreClosed();
+
         if (indexWriter != null) {
             //TODO Close call can possibly be speeded up by
             //avoiding merge and dropping stuff in memory. To be explored
@@ -180,7 +191,7 @@ public class NRTIndex implements Closeab
 
     @Override
     public String toString() {
-        return definition.getIndexPath();
+        return String.format("%s (%s)", definition.getIndexPath(), getName());
     }
 
     //For test
@@ -188,6 +199,19 @@ public class NRTIndex implements Closeab
         return indexDir;
     }
 
+    private String getName(){
+        return indexDir != null ? indexDir.getName() : "UNKNOWN";
+    }
+
+    private void assertAllReadersAreClosed() {
+        for (IndexReader r : openedReaders){
+            if (r.getRefCount() != 0){
+                String msg = String.format("Unclosed reader found with 
refCount %d for index %s", r.getRefCount(), toString());
+                throw new IllegalStateException(msg);
+            }
+        }
+    }
+
     /**
      * If index was updated then a new reader would be returned otherwise
      * existing reader would be returned
@@ -214,6 +238,11 @@ public class NRTIndex implements Closeab
                 }
             }
             ctx.stop();
+
+            if (assertAllReadersClosed && result != null && result != 
dirReader) {
+                openedReaders.add(result);
+            }
+
             return result;
         } catch (IOException e) {
             log.warn("Error opening index [{}]", e);
@@ -232,6 +261,7 @@ public class NRTIndex implements Closeab
         //config.setRAMBufferSizeMB(1024*1024*25);
 
         indexWriter = new IndexWriter(directory, config);
+        log.debug("[{}] Created NRTIndex [{}]", definition.getIndexPath(), 
getName());
         return new NRTIndexWriter(indexWriter);
     }
 

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=1803953&r1=1803952&r2=1803953&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 Aug  3 06:27:41 2017
@@ -57,6 +57,7 @@ public class NRTIndexFactory implements
     private final long refreshDeltaInSecs;
     private final StatisticsProvider statisticsProvider;
     private NRTDirectoryFactory directoryFactory = 
DefaultNRTDirFactory.INSTANCE;
+    private boolean assertAllResourcesClosed = 
Boolean.getBoolean("oak.lucene.assertAllResourcesClosed");
 
     public NRTIndexFactory(IndexCopier indexCopier, StatisticsProvider 
statisticsProvider) {
         this(indexCopier, Clock.SIMPLE, REFRESH_DELTA_IN_SECS, 
statisticsProvider);
@@ -80,7 +81,7 @@ public class NRTIndexFactory implements
         }
         String indexPath = definition.getIndexPath();
         NRTIndex current = new NRTIndex(definition, indexCopier, 
getRefreshPolicy(definition),
-                getPrevious(indexPath), statisticsProvider, directoryFactory);
+                getPrevious(indexPath), statisticsProvider, directoryFactory, 
assertAllResourcesClosed);
         indexes.put(indexPath, current);
         closeLast(indexPath);
         return current;
@@ -102,6 +103,14 @@ public class NRTIndexFactory implements
         this.directoryFactory = directoryFactory;
     }
 
+    /**
+     * Test mode upon which enables assertions to confirm that all readers are 
closed
+     * by the time NRTIndex is closed
+     */
+    public void setAssertAllResourcesClosed(boolean assertAllResourcesClosed) {
+        this.assertAllResourcesClosed = assertAllResourcesClosed;
+    }
+
     private void closeLast(String indexPath) {
         List<NRTIndex> existing = indexes.get(indexPath);
         if (existing.size() <= MAX_INDEX_COUNT){

Modified: 
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/NRTIndexFactoryTest.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/NRTIndexFactoryTest.java?rev=1803953&r1=1803952&r2=1803953&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/NRTIndexFactoryTest.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/NRTIndexFactoryTest.java
 Thu Aug  3 06:27:41 2017
@@ -56,6 +56,7 @@ public class NRTIndexFactoryTest {
     public void setUp() throws IOException {
         indexCopier = new IndexCopier(sameThreadExecutor(), 
temporaryFolder.getRoot());
         indexFactory = new NRTIndexFactory(indexCopier, 
StatisticsProvider.NOOP);
+        indexFactory.setAssertAllResourcesClosed(true);
     }
 
     @Test

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=1803953&r1=1803952&r2=1803953&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 Aug  3 06:27:41 2017
@@ -66,6 +66,7 @@ public class NRTIndexTest {
     public void setUp() throws IOException {
         indexCopier = new IndexCopier(sameThreadExecutor(), 
temporaryFolder.getRoot());
         indexFactory = new NRTIndexFactory(indexCopier, 
StatisticsProvider.NOOP);
+        //indexFactory.setAssertAllResourcesClosed(true);
         LuceneIndexEditorContext.configureUniqueId(builder);
     }
 


Reply via email to