You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by mi...@apache.org on 2013/10/04 13:55:23 UTC
svn commit: r1529136 - in /lucene/dev/branches/branch_4x: ./ lucene/
lucene/core/ lucene/core/src/java/org/apache/lucene/index/
lucene/core/src/java/org/apache/lucene/search/
lucene/core/src/test/org/apache/lucene/index/
lucene/core/src/test/org/apache...
Author: mikemccand
Date: Fri Oct 4 11:55:23 2013
New Revision: 1529136
URL: http://svn.apache.org/r1529136
Log:
LUCENE-5254: don't hold ref to original SR from SCR, to avoid bounded leak of things like live docs bitset
Modified:
lucene/dev/branches/branch_4x/ (props changed)
lucene/dev/branches/branch_4x/lucene/ (props changed)
lucene/dev/branches/branch_4x/lucene/CHANGES.txt (contents, props changed)
lucene/dev/branches/branch_4x/lucene/core/ (props changed)
lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/SegmentCoreReaders.java
lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/SegmentReader.java
lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/FieldCache.java
lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/FieldCacheImpl.java
lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/index/TestDocTermOrds.java
lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestFieldCache.java
lucene/dev/branches/branch_4x/lucene/test-framework/ (props changed)
lucene/dev/branches/branch_4x/lucene/test-framework/src/java/org/apache/lucene/search/QueryUtils.java
Modified: lucene/dev/branches/branch_4x/lucene/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/CHANGES.txt?rev=1529136&r1=1529135&r2=1529136&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/CHANGES.txt (original)
+++ lucene/dev/branches/branch_4x/lucene/CHANGES.txt Fri Oct 4 11:55:23 2013
@@ -47,6 +47,11 @@ Bug Fixes
its state, which could result in exceptions being thrown, as well as
incorrect ordinals returned from getParent. (Shai Erera)
+* LUCENE-5254: Fixed bounded memory leak, where objects like live
+ docs bitset were not freed from an starting reader after reopening
+ to a new reader and closing the original one. (Shai Erera, Mike
+ McCandless)
+
API Changes:
* LUCENE-5222: Add SortField.needsScores(). Previously it was not possible
Modified: lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/SegmentCoreReaders.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/SegmentCoreReaders.java?rev=1529136&r1=1529135&r2=1529136&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/SegmentCoreReaders.java (original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/SegmentCoreReaders.java Fri Oct 4 11:55:23 2013
@@ -60,7 +60,7 @@ final class SegmentCoreReaders {
final int termsIndexDivisor;
- private final SegmentReader owner;
+ private final Object ownerCoreCacheKey;
final StoredFieldsReader fieldsReaderOrig;
final TermVectorsReader termVectorsReaderOrig;
@@ -109,7 +109,12 @@ final class SegmentCoreReaders {
Collections.synchronizedSet(new LinkedHashSet<CoreClosedListener>());
SegmentCoreReaders(SegmentReader owner, Directory dir, SegmentInfoPerCommit si, IOContext context, int termsIndexDivisor) throws IOException {
-
+
+ // SegmentReader uses us as the coreCacheKey; we cannot
+ // call owner.getCoreCacheKey() because that will return
+ // null!:
+ this.ownerCoreCacheKey = this;
+
if (termsIndexDivisor == 0) {
throw new IllegalArgumentException("indexDivisor must be < 0 (don't load terms index) or greater than 0 (got 0)");
}
@@ -166,12 +171,6 @@ final class SegmentCoreReaders {
decRef();
}
}
-
- // Must assign this at the end -- if we hit an
- // exception above core, we don't want to attempt to
- // purge the FieldCache (will hit NPE because core is
- // not assigned yet).
- this.owner = owner;
}
void incRef() {
@@ -348,7 +347,7 @@ final class SegmentCoreReaders {
private void notifyCoreClosedListeners() {
synchronized(coreClosedListeners) {
for (CoreClosedListener listener : coreClosedListeners) {
- listener.onClose(owner);
+ listener.onClose(ownerCoreCacheKey);
}
}
}
@@ -369,8 +368,4 @@ final class SegmentCoreReaders {
((fieldsReaderOrig!=null)? fieldsReaderOrig.ramBytesUsed() : 0) +
((termVectorsReaderOrig!=null) ? termVectorsReaderOrig.ramBytesUsed() : 0);
}
- @Override
- public String toString() {
- return "SegmentCoreReader(owner=" + owner + ")";
- }
}
Modified: lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/SegmentReader.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/SegmentReader.java?rev=1529136&r1=1529135&r2=1529136&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/SegmentReader.java (original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/SegmentReader.java Fri Oct 4 11:55:23 2013
@@ -209,6 +209,8 @@ public final class SegmentReader extends
// same entry in the FieldCache. See LUCENE-1579.
@Override
public Object getCoreCacheKey() {
+ // NOTE: if this every changes, be sure to fix
+ // SegmentCoreReader's ownerCoreCacheKey to match!
return core;
}
@@ -273,9 +275,9 @@ public final class SegmentReader extends
* @lucene.experimental
*/
public static interface CoreClosedListener {
- /** Invoked when the shared core of the provided {@link
+ /** Invoked when the shared core of the original {@code
* SegmentReader} has closed. */
- public void onClose(SegmentReader owner);
+ public void onClose(Object ownerCoreCacheKey);
}
/** Expert: adds a CoreClosedListener to this reader's shared core */
Modified: lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/FieldCache.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/FieldCache.java?rev=1529136&r1=1529135&r2=1529136&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/FieldCache.java (original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/FieldCache.java Fri Oct 4 11:55:23 2013
@@ -29,6 +29,7 @@ import org.apache.lucene.document.Numeri
import org.apache.lucene.index.AtomicReader;
import org.apache.lucene.index.BinaryDocValues;
import org.apache.lucene.index.DocTermOrds;
+import org.apache.lucene.index.IndexReader; // javadocs
import org.apache.lucene.index.SortedDocValues;
import org.apache.lucene.index.SortedSetDocValues;
import org.apache.lucene.index.Terms;
@@ -503,7 +504,7 @@ public interface FieldCache {
*
* @see #getInts(AtomicReader, String, IntParser, boolean)
*/
- public Ints getInts (AtomicReader reader, String field, boolean setDocsWithField) throws IOException;
+ public Ints getInts(AtomicReader reader, String field, boolean setDocsWithField) throws IOException;
/**
* Returns an {@link Ints} over the values found in documents in the given
@@ -529,7 +530,7 @@ public interface FieldCache {
* @throws IOException
* If any error occurs.
*/
- public Ints getInts (AtomicReader reader, String field, IntParser parser, boolean setDocsWithField) throws IOException;
+ public Ints getInts(AtomicReader reader, String field, IntParser parser, boolean setDocsWithField) throws IOException;
/**
* Returns a {@link Floats} over the values found in documents in the given
@@ -537,7 +538,7 @@ public interface FieldCache {
*
* @see #getFloats(AtomicReader, String, FloatParser, boolean)
*/
- public Floats getFloats (AtomicReader reader, String field, boolean setDocsWithField) throws IOException;
+ public Floats getFloats(AtomicReader reader, String field, boolean setDocsWithField) throws IOException;
/**
* Returns a {@link Floats} over the values found in documents in the given
@@ -563,7 +564,7 @@ public interface FieldCache {
* @throws IOException
* If any error occurs.
*/
- public Floats getFloats (AtomicReader reader, String field, FloatParser parser, boolean setDocsWithField) throws IOException;
+ public Floats getFloats(AtomicReader reader, String field, FloatParser parser, boolean setDocsWithField) throws IOException;
/**
* Returns a {@link Longs} over the values found in documents in the given
@@ -644,14 +645,14 @@ public interface FieldCache {
* @return The values in the given field for each document.
* @throws IOException If any error occurs.
*/
- public BinaryDocValues getTerms (AtomicReader reader, String field, boolean setDocsWithField) throws IOException;
+ public BinaryDocValues getTerms(AtomicReader reader, String field, boolean setDocsWithField) throws IOException;
/** Expert: just like {@link #getTerms(AtomicReader,String,boolean)},
* but you can specify whether more RAM should be consumed in exchange for
* faster lookups (default is "true"). Note that the
* first call for a given reader and field "wins",
* subsequent calls will share the same cache entry. */
- public BinaryDocValues getTerms (AtomicReader reader, String field, boolean setDocsWithField, float acceptableOverheadRatio) throws IOException;
+ public BinaryDocValues getTerms(AtomicReader reader, String field, boolean setDocsWithField, float acceptableOverheadRatio) throws IOException;
/** Checks the internal cache for an appropriate entry, and if none
* is found, reads the term values in <code>field</code>
@@ -663,7 +664,7 @@ public interface FieldCache {
* @return The values in the given field for each document.
* @throws IOException If any error occurs.
*/
- public SortedDocValues getTermsIndex (AtomicReader reader, String field) throws IOException;
+ public SortedDocValues getTermsIndex(AtomicReader reader, String field) throws IOException;
/** Expert: just like {@link
* #getTermsIndex(AtomicReader,String)}, but you can specify
@@ -671,7 +672,7 @@ public interface FieldCache {
* faster lookups (default is "true"). Note that the
* first call for a given reader and field "wins",
* subsequent calls will share the same cache entry. */
- public SortedDocValues getTermsIndex (AtomicReader reader, String field, float acceptableOverheadRatio) throws IOException;
+ public SortedDocValues getTermsIndex(AtomicReader reader, String field, float acceptableOverheadRatio) throws IOException;
/**
* Checks the internal cache for an appropriate entry, and if none is found, reads the term values
@@ -776,7 +777,7 @@ public interface FieldCache {
* </p>
* @lucene.experimental
*/
- public abstract CacheEntry[] getCacheEntries();
+ public CacheEntry[] getCacheEntries();
/**
* <p>
@@ -789,16 +790,17 @@ public interface FieldCache {
* </p>
* @lucene.experimental
*/
- public abstract void purgeAllCaches();
+ public void purgeAllCaches();
/**
* Expert: drops all cache entries associated with this
- * reader. NOTE: this reader must precisely match the
- * reader that the cache entry is keyed on. If you pass a
- * top-level reader, it usually will have no effect as
- * Lucene now caches at the segment reader level.
+ * reader {@link IndexReader#getCoreCacheKey}. NOTE: this cache key must
+ * precisely match the reader that the cache entry is
+ * keyed on. If you pass a top-level reader, it usually
+ * will have no effect as Lucene now caches at the segment
+ * reader level.
*/
- public abstract void purge(AtomicReader r);
+ public void purgeByCacheKey(Object coreCacheKey);
/**
* If non-null, FieldCacheImpl will warn whenever
Modified: lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/FieldCacheImpl.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/FieldCacheImpl.java?rev=1529136&r1=1529135&r2=1529136&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/FieldCacheImpl.java (original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/FieldCacheImpl.java Fri Oct 4 11:55:23 2013
@@ -80,9 +80,9 @@ class FieldCacheImpl implements FieldCac
}
@Override
- public synchronized void purge(AtomicReader r) {
+ public synchronized void purgeByCacheKey(Object coreCacheKey) {
for(Cache c : caches.values()) {
- c.purge(r);
+ c.purgeByCacheKey(coreCacheKey);
}
}
@@ -112,8 +112,8 @@ class FieldCacheImpl implements FieldCac
// per-segment fieldcaches don't purge until the shared core closes.
final SegmentReader.CoreClosedListener purgeCore = new SegmentReader.CoreClosedListener() {
@Override
- public void onClose(SegmentReader owner) {
- FieldCacheImpl.this.purge(owner);
+ public void onClose(Object ownerCoreCacheKey) {
+ FieldCacheImpl.this.purgeByCacheKey(ownerCoreCacheKey);
}
};
@@ -122,7 +122,7 @@ class FieldCacheImpl implements FieldCac
@Override
public void onClose(IndexReader owner) {
assert owner instanceof AtomicReader;
- FieldCacheImpl.this.purge((AtomicReader) owner);
+ FieldCacheImpl.this.purgeByCacheKey(((AtomicReader) owner).getCoreCacheKey());
}
};
@@ -157,10 +157,9 @@ class FieldCacheImpl implements FieldCac
throws IOException;
/** Remove this reader from the cache, if present. */
- public void purge(AtomicReader r) {
- Object readerKey = r.getCoreCacheKey();
+ public void purgeByCacheKey(Object coreCacheKey) {
synchronized(readerCache) {
- readerCache.remove(readerKey);
+ readerCache.remove(coreCacheKey);
}
}
Modified: lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/index/TestDocTermOrds.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/index/TestDocTermOrds.java?rev=1529136&r1=1529135&r2=1529136&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/index/TestDocTermOrds.java (original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/index/TestDocTermOrds.java Fri Oct 4 11:55:23 2013
@@ -172,7 +172,7 @@ public class TestDocTermOrds extends Luc
AtomicReader slowR = SlowCompositeReaderWrapper.wrap(r);
verify(slowR, idToOrds, termsArray, null);
- FieldCache.DEFAULT.purge(slowR);
+ FieldCache.DEFAULT.purgeByCacheKey(slowR.getCoreCacheKey());
r.close();
dir.close();
@@ -291,7 +291,7 @@ public class TestDocTermOrds extends Luc
verify(slowR, idToOrdsPrefix, termsArray, prefixRef);
}
- FieldCache.DEFAULT.purge(slowR);
+ FieldCache.DEFAULT.purgeByCacheKey(slowR.getCoreCacheKey());
r.close();
dir.close();
Modified: lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestFieldCache.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestFieldCache.java?rev=1529136&r1=1529135&r2=1529136&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestFieldCache.java (original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestFieldCache.java Fri Oct 4 11:55:23 2013
@@ -304,7 +304,7 @@ public class TestFieldCache extends Luce
termOrds = cache.getDocTermOrds(reader, "bogusfield");
assertTrue(termOrds.getValueCount() == 0);
- FieldCache.DEFAULT.purge(reader);
+ FieldCache.DEFAULT.purgeByCacheKey(reader.getCoreCacheKey());
}
public void testEmptyIndex() throws Exception {
@@ -315,7 +315,7 @@ public class TestFieldCache extends Luce
AtomicReader reader = SlowCompositeReaderWrapper.wrap(r);
FieldCache.DEFAULT.getTerms(reader, "foobar", true);
FieldCache.DEFAULT.getTermsIndex(reader, "foobar");
- FieldCache.DEFAULT.purge(reader);
+ FieldCache.DEFAULT.purgeByCacheKey(reader.getCoreCacheKey());
r.close();
dir.close();
}
Modified: lucene/dev/branches/branch_4x/lucene/test-framework/src/java/org/apache/lucene/search/QueryUtils.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/test-framework/src/java/org/apache/lucene/search/QueryUtils.java?rev=1529136&r1=1529135&r2=1529136&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/test-framework/src/java/org/apache/lucene/search/QueryUtils.java (original)
+++ lucene/dev/branches/branch_4x/lucene/test-framework/src/java/org/apache/lucene/search/QueryUtils.java Fri Oct 4 11:55:23 2013
@@ -132,7 +132,7 @@ public class QueryUtils {
public static void purgeFieldCache(IndexReader r) throws IOException {
// this is just a hack, to get an atomic reader that contains all subreaders for insanity checks
- FieldCache.DEFAULT.purge(SlowCompositeReaderWrapper.wrap(r));
+ FieldCache.DEFAULT.purgeByCacheKey(SlowCompositeReaderWrapper.wrap(r).getCoreCacheKey());
}
/** This is a MultiReader that can be used for randomly wrapping other readers