Author: chetanm
Date: Thu Aug 3 06:28:06 2017
New Revision: 1803955
URL: http://svn.apache.org/viewvc?rev=1803955&view=rev
Log:
OAK-6500 - NRTIndex leaks file handles due to unclosed IndexReader
Actual fix by using ref counting for IndexReader instances
Modified:
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNodeManager.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/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/HybridIndexTest.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/IndexNodeManager.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNodeManager.java?rev=1803955&r1=1803954&r2=1803955&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNodeManager.java
(original)
+++
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNodeManager.java
Thu Aug 3 06:28:06 2017
@@ -22,6 +22,7 @@ import static com.google.common.base.Pre
import java.io.IOException;
import java.util.Collections;
import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
@@ -86,7 +87,7 @@ public class IndexNodeManager {
private final ReadWriteLock lock = new ReentrantReadWriteLock();
- private volatile IndexSearcher indexSearcher;
+ private volatile SearcherHolder searcherHolder;
private final NRTIndex nrtIndex;
@@ -101,8 +102,6 @@ public class IndexNodeManager {
private boolean closed = false;
- private List<LuceneIndexReader> nrtReaders;
-
private final int indexNodeId = INDEX_NODE_COUNTER.incrementAndGet();
IndexNodeManager(String name, IndexDefinition definition,
List<LuceneIndexReader> readers, @Nullable NRTIndex nrtIndex)
@@ -112,8 +111,7 @@ public class IndexNodeManager {
this.definition = definition;
this.readers = readers;
this.nrtIndex = nrtIndex;
- this.nrtReaders = getNRTReaders();
- this.indexSearcher = new IndexSearcher(createReader(nrtReaders));
+ this.searcherHolder = createHolder(getNRTReaders());
this.refreshPolicy = nrtIndex != null ? nrtIndex.getRefreshPolicy() :
ReaderRefreshPolicy.NEVER;
}
@@ -135,6 +133,7 @@ public class IndexNodeManager {
return readers.isEmpty() ? null : getDefaultReader().getLookup();
}
+ @CheckForNull
IndexNode acquire() {
lock.readLock().lock();
if (closed) {
@@ -145,7 +144,7 @@ public class IndexNodeManager {
try {
refreshPolicy.refreshOnReadIfRequired(refreshCallback);
success = true;
- return new IndexNodeImpl(indexSearcher);
+ return new IndexNodeImpl(searcherHolder);
} finally {
if (!success) {
lock.readLock().unlock();
@@ -171,12 +170,8 @@ public class IndexNodeManager {
lock.writeLock().unlock();
}
- //Do not close the NRTIndex here as it might be in use
- //by newer IndexNode. Just close the readers obtained from
- //them
- for (LuceneIndexReader reader : Iterables.concat(readers, nrtReaders)){
- reader.close();
- }
+ releaseHolder(searcherHolder);
+ closeReaders(readers);
}
private List<LuceneIndexReader> getPrimaryReaders() {
@@ -197,9 +192,10 @@ public class IndexNodeManager {
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));
+ if (newNRTReaders != searcherHolder.nrtReaders) {
+ SearcherHolder old = searcherHolder;
+ searcherHolder = createHolder(newNRTReaders);
+ releaseHolder(old);
PERF_LOGGER.end(start, 0, "Refreshed reader for index [{}]",
definition);
}
}
@@ -210,39 +206,96 @@ public class IndexNodeManager {
}
private IndexReader createReader(List<LuceneIndexReader> nrtReaders) {
+ //Increment count by 1. MultiReader does it for all readers
+ //So no need for an explicit increment for MultiReader
+
if (readers.size() == 1 && nrtReaders.isEmpty()){
- return readers.get(0).getReader();
+ IndexReader reader = readers.get(0).getReader();
+ reader.incRef();
+ return reader;
}
if (nrtReaders.size() == 1 && readers.isEmpty()){
- return nrtReaders.get(0).getReader();
+ IndexReader reader = nrtReaders.get(0).getReader();
+ reader.incRef();
+ return reader;
}
+
IndexReader[] readerArr = new IndexReader[readers.size() +
nrtReaders.size()];
int i = 0;
for (LuceneIndexReader r : Iterables.concat(readers, nrtReaders)){
readerArr[i++] = r.getReader();
}
- return new MultiReader(readerArr, true);
+ return new MultiReader(readerArr, false);
}
private List<LuceneIndexReader> getNRTReaders() {
return nrtIndex != null ? nrtIndex.getReaders() :
Collections.<LuceneIndexReader>emptyList();
}
- private class IndexNodeImpl implements IndexNode {
- private final IndexSearcher searcher;
+ private SearcherHolder createHolder(List<LuceneIndexReader> newNRTReaders)
{
+ return new SearcherHolder(new
IndexSearcher(createReader(newNRTReaders)), newNRTReaders);
+ }
+
+ private void closeReaders(Iterable<LuceneIndexReader> readers) {
+ for (LuceneIndexReader r : readers){
+ try {
+ r.close();
+ } catch (IOException e) {
+ log.warn("Error occurred while releasing reader for index
[{}]", definition.getIndexPath(), e);
+ }
+ }
+ }
+
+ private void releaseHolder(SearcherHolder holder) {
+ decrementSearcherUsageCount(holder.searcher);
+ }
- private IndexNodeImpl(IndexSearcher searcher) {
+ private static void incrementSearcherUsageCount(IndexSearcher searcher) {
+ searcher.getIndexReader().incRef();
+ }
+
+ private void decrementSearcherUsageCount(IndexSearcher searcher) {
+ try {
+ //Decrement the count by 1 as we increased it while creating the
searcher
+ //in createReader
+ searcher.getIndexReader().decRef();
+ } catch (IOException e) {
+ log.warn("Error occurred while releasing reader for index [{}]",
definition.getIndexPath(), e);
+ }
+ }
+
+ private static class SearcherHolder {
+ final IndexSearcher searcher;
+ final List<LuceneIndexReader> nrtReaders;
+
+ public SearcherHolder(IndexSearcher searcher, List<LuceneIndexReader>
nrtReaders) {
this.searcher = searcher;
+ this.nrtReaders = nrtReaders;
+ }
+ }
+
+ private class IndexNodeImpl implements IndexNode {
+ private final SearcherHolder holder;
+ private final AtomicBoolean released = new AtomicBoolean();
+
+ private IndexNodeImpl(SearcherHolder searcherHolder) {
+ this.holder = searcherHolder;
+ //Increment on each acquire
+ incrementSearcherUsageCount(holder.searcher);
}
@Override
public void release() {
- IndexNodeManager.this.release();
+ if (released.compareAndSet(false, true)) {
+ //Decrement on each release
+ decrementSearcherUsageCount(holder.searcher);
+ IndexNodeManager.this.release();
+ }
}
@Override
public IndexSearcher getSearcher() {
- return searcher;
+ return holder.searcher;
}
@Override
@@ -262,7 +315,7 @@ public class IndexNodeManager {
@Override
public List<LuceneIndexReader> getNRTReaders() {
- return IndexNodeManager.this.nrtReaders;
+ return holder.nrtReaders;
}
@Override
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=1803955&r1=1803954&r2=1803955&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:28:06 2017
@@ -110,8 +110,12 @@ public class NRTIndex implements Closeab
@CheckForNull
LuceneIndexReader getPrimaryReader() {
- DirectoryReader reader = createReader();
- return reader != null ? new NRTReader(reader, directory) : null;
+ DirectoryReader latestReader = createReader();
+ if (latestReader != dirReader) {
+ decrementReaderUseCount(dirReader);
+ dirReader = latestReader;
+ }
+ return latestReader != null ? new NRTReader(latestReader, directory) :
null;
}
public LuceneIndexWriter getWriter() throws IOException {
@@ -145,6 +149,9 @@ public class NRTIndex implements Closeab
if (previousReader != null) {
newReaders.add(previousReader);
}
+
+ decrementReaderUseCount(dirReader);
+
dirReader = latestReader;
readers = ImmutableList.copyOf(newReaders);
return readers;
@@ -161,6 +168,10 @@ public class NRTIndex implements Closeab
log.debug("[{}] Closing NRTIndex [{}]", definition.getIndexPath(),
getName());
+ if (dirReader != null){
+ dirReader.close();
+ }
+
assertAllReadersAreClosed();
if (indexWriter != null) {
@@ -212,6 +223,17 @@ public class NRTIndex implements Closeab
}
}
+ private void decrementReaderUseCount(IndexReader reader) {
+ try {
+ if (reader != null) {
+ reader.decRef();
+ }
+ } catch (IOException e) {
+ log.warn("[{}] Error occurred while releasing reader instance {}",
+ definition.getIndexPath(), toString(), e);
+ }
+ }
+
/**
* If index was updated then a new reader would be returned otherwise
* existing reader would be returned
Modified:
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/HybridIndexTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/HybridIndexTest.java?rev=1803955&r1=1803954&r2=1803955&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/HybridIndexTest.java
(original)
+++
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/HybridIndexTest.java
Thu Aug 3 06:28:06 2017
@@ -138,8 +138,7 @@ public class HybridIndexTest extends Abs
MountInfoProvider mip = defaultMountInfoProvider();
nrtIndexFactory = new NRTIndexFactory(copier, clock,
TimeUnit.MILLISECONDS.toSeconds(refreshDelta), StatisticsProvider.NOOP);
- //TODO OAK-6500
- //nrtIndexFactory.setAssertAllResourcesClosed(true);
+ nrtIndexFactory.setAssertAllResourcesClosed(true);
LuceneIndexReaderFactory indexReaderFactory = new
DefaultIndexReaderFactory(mip, copier);
IndexTracker tracker = new
IndexTracker(indexReaderFactory,nrtIndexFactory);
luceneIndexProvider = new LuceneIndexProvider(tracker);
@@ -351,7 +350,6 @@ public class HybridIndexTest extends Abs
assertQuery(query, of("/b", "/c"));
}
- @Ignore("OAK-6500")
@Test
public void noFileLeaks() throws Exception{
nrtIndexFactory.setDirectoryFactory(new NRTDirectoryFactory() {
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=1803955&r1=1803954&r2=1803955&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:28:06 2017
@@ -66,7 +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);
+ indexFactory.setAssertAllResourcesClosed(true);
LuceneIndexEditorContext.configureUniqueId(builder);
}
