Good idea, I'll fix! Mike McCandless
http://blog.mikemccandless.com On Wed, Aug 31, 2011 at 6:58 AM, Simon Willnauer <simon.willna...@googlemail.com> wrote: > 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
