lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael McCandless <luc...@mikemccandless.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 11:53:47 GMT
Good idea, I'll fix!

Mike McCandless

http://blog.mikemccandless.com

On Wed, Aug 31, 2011 at 6:58 AM, Simon Willnauer
<simon.willnauer@googlemail.com> wrote:
> 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