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 ch...@apache.org on 2017/08/03 06:27:42 UTC
svn commit: r1803953 - in /jackrabbit/oak/trunk/oak-lucene/src:
main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/
test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/
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);
}