On Wed, Aug 31, 2011 at 12:36 PM, <mikemcc...@apache.org> wrote:
> Author: mikemccand
> Date: Wed Aug 31 10:36:36 2011
> New Revision: 1163568
>
> URL: http://svn.apache.org/viewvc?rev=1163568&view=rev
> Log:
> LUCENE-3409: drop reader pool from IW.deleteAll
>
> Modified:
> lucene/dev/trunk/lucene/CHANGES.txt
>
> lucene/dev/trunk/lucene/src/java/org/apache/lucene/index/IndexFileDeleter.java
> lucene/dev/trunk/lucene/src/java/org/apache/lucene/index/IndexWriter.java
>
> lucene/dev/trunk/lucene/src/test/org/apache/lucene/index/TestIndexWriter.java
>
> Modified: lucene/dev/trunk/lucene/CHANGES.txt
> URL:
> http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/CHANGES.txt?rev=1163568&r1=1163567&r2=1163568&view=diff
> ==============================================================================
> --- lucene/dev/trunk/lucene/CHANGES.txt (original)
> +++ lucene/dev/trunk/lucene/CHANGES.txt Wed Aug 31 10:36:36 2011
> @@ -577,6 +577,10 @@ Bug fixes
> throw NoSuchDirectoryException when all files written so far have been
> written to one directory, but the other still has not yet been created on
> the
> filesystem. (Robert Muir)
> +
> +* LUCENE-3409: IndexWriter.deleteAll was failing to close pooled NRT
> + SegmentReaders, leading to unused files accumulating in the
> + Directory. (tal steier via Mike McCandless)
>
> New Features
>
>
> Modified:
> lucene/dev/trunk/lucene/src/java/org/apache/lucene/index/IndexFileDeleter.java
> URL:
> http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/src/java/org/apache/lucene/index/IndexFileDeleter.java?rev=1163568&r1=1163567&r2=1163568&view=diff
> ==============================================================================
> ---
> lucene/dev/trunk/lucene/src/java/org/apache/lucene/index/IndexFileDeleter.java
> (original)
> +++
> lucene/dev/trunk/lucene/src/java/org/apache/lucene/index/IndexFileDeleter.java
> Wed Aug 31 10:36:36 2011
> @@ -374,6 +374,10 @@ final class IndexFileDeleter {
> }
>
> public void refresh() throws IOException {
> + // Set to null so that we regenerate the list of pending
> + // files; else we can accumulate same file more than
> + // once
> + deletable = null;
> refresh(null);
> }
>
>
> Modified:
> lucene/dev/trunk/lucene/src/java/org/apache/lucene/index/IndexWriter.java
> URL:
> http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/src/java/org/apache/lucene/index/IndexWriter.java?rev=1163568&r1=1163567&r2=1163568&view=diff
> ==============================================================================
> --- lucene/dev/trunk/lucene/src/java/org/apache/lucene/index/IndexWriter.java
> (original)
> +++ lucene/dev/trunk/lucene/src/java/org/apache/lucene/index/IndexWriter.java
> Wed Aug 31 10:36:36 2011
> @@ -600,6 +600,23 @@ public class IndexWriter implements Clos
> drop(info, IOContext.Context.MERGE);
> }
>
> + public synchronized void dropAll() throws IOException {
> + Iterator<Map.Entry<SegmentCacheKey,SegmentReader>> iter =
> readerMap.entrySet().iterator();
> + while (iter.hasNext()) {
> +
> + final Map.Entry<SegmentCacheKey,SegmentReader> ent = iter.next();
> +
> + SegmentReader sr = ent.getValue();
> + sr.hasChanges = false;
> + iter.remove();
> +
> + // NOTE: it is allowed that this decRef does not
> + // actually close the SR; this can happen when a
> + // near real-time reader using this SR is still open
> + sr.decRef();
> + }
> + }
> +
just being a little picky here, can't we simply iterate over the
readerMap.values() and call readerMap.clear() afterwards like
<snip>
for (SegementReader sr : readerMap.values()) {
sr.hasChange = false;
sr.decRef();
}
readerMap.clear();
</snip>
the iter.remove() call does a key lookup each time which is totally
unnecessary (well this is super minor!) but it looks more readable,
its less code and slightly more efficient?
simon
> public synchronized void drop(SegmentInfo info, IOContext.Context
> context) throws IOException {
> final SegmentReader sr;
> if ((sr = readerMap.remove(new SegmentCacheKey(info, context))) !=
> null) {
> @@ -2141,7 +2158,7 @@ public class IndexWriter implements Clos
> deleter.refresh();
>
> // Don't bother saving any changes in our segmentInfos
> - readerPool.clear(null);
> + readerPool.dropAll();
>
> // Mark that the index has changed
> ++changeCount;
> @@ -3698,7 +3715,6 @@ public class IndexWriter implements Clos
>
> synchronized(this) {
> deleter.deleteFile(compoundFileName);
> -
> deleter.deleteFile(IndexFileNames.segmentFileName(mergedName,
> "", IndexFileNames.COMPOUND_FILE_ENTRIES_EXTENSION));
> deleter.deleteNewFiles(merge.info.files());
> }
>
> Modified:
> lucene/dev/trunk/lucene/src/test/org/apache/lucene/index/TestIndexWriter.java
> URL:
> http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/src/test/org/apache/lucene/index/TestIndexWriter.java?rev=1163568&r1=1163567&r2=1163568&view=diff
> ==============================================================================
> ---
> lucene/dev/trunk/lucene/src/test/org/apache/lucene/index/TestIndexWriter.java
> (original)
> +++
> lucene/dev/trunk/lucene/src/test/org/apache/lucene/index/TestIndexWriter.java
> Wed Aug 31 10:36:36 2011
> @@ -1849,4 +1849,28 @@ public class TestIndexWriter extends Luc
> writer.close();
> dir.close();
> }
> +
> + public void testDeleteAllNRTLeftoverFiles() throws Exception {
> +
> + Directory d = new MockDirectoryWrapper(random, new RAMDirectory());
> + IndexWriter w = new IndexWriter(d, new
> IndexWriterConfig(TEST_VERSION_CURRENT, new MockAnalyzer(random)));
> + Document doc = new Document();
> + for(int i = 0; i < 20; i++) {
> + for(int j = 0; j < 100; ++j) {
> + w.addDocument(doc);
> + }
> + w.commit();
> + IndexReader.open(w, true).close();
> +
> + w.deleteAll();
> + w.commit();
> +
> + // Make sure we accumulate no files except for empty
> + // segments_N and segments.gen:
> + assertTrue(d.listAll().length <= 2);
> + }
> +
> + w.close();
> + d.close();
> + }
> }
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
