lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Simon Willnauer <simon.willna...@googlemail.com>
Subject Re: svn commit: r1163568 - in /lucene/dev/trunk/lucene: CHANGES.txt src/java/org/apache/lucene/index/IndexFileDeleter.java src/java/org/apache/lucene/index/IndexWriter.java src/test/org/apache/lucene/index/TestIndexWriter.java
Date Wed, 31 Aug 2011 10:58:32 GMT
On Wed, Aug 31, 2011 at 12:36 PM,  <mikemccand@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-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


Mime
View raw message