Index: lucene/src/test/org/apache/lucene/index/TestDoc.java =================================================================== --- lucene/src/test/org/apache/lucene/index/TestDoc.java (revision 1220805) +++ lucene/src/test/org/apache/lucene/index/TestDoc.java (working copy) @@ -114,8 +114,7 @@ StringWriter sw = new StringWriter(); PrintWriter out = new PrintWriter(sw, true); - // TODO: why does this test trigger NRTCachingDirectory's assert? - Directory directory = newFSDirectory(indexDir, null, false); + Directory directory = newFSDirectory(indexDir, null); IndexWriter writer = new IndexWriter( directory, newIndexWriterConfig(TEST_VERSION_CURRENT, new MockAnalyzer(random)). @@ -143,14 +142,14 @@ directory.close(); out.close(); sw.close(); + String multiFileOutput = sw.getBuffer().toString(); //System.out.println(multiFileOutput); sw = new StringWriter(); out = new PrintWriter(sw, true); - // TODO: why does this test trigger NRTCachingDirectory's assert? - directory = newFSDirectory(indexDir, null, false); + directory = newFSDirectory(indexDir, null); writer = new IndexWriter( directory, newIndexWriterConfig(TEST_VERSION_CURRENT, new MockAnalyzer(random)). Index: lucene/src/test/org/apache/lucene/index/TestCrash.java =================================================================== --- lucene/src/test/org/apache/lucene/index/TestCrash.java (revision 1220805) +++ lucene/src/test/org/apache/lucene/index/TestCrash.java (working copy) @@ -30,9 +30,7 @@ public class TestCrash extends LuceneTestCase { private IndexWriter initIndex(Random random, boolean initialCommit) throws IOException { - // note: we pass 'false' here so our crashing/deleting won't trigger assertions in NRTCachingDir - // TODO: don't remember why this is ok... maybe we should check again that it really actually is. - return initIndex(random, newDirectory(random, false), initialCommit); + return initIndex(random, newDirectory(random), initialCommit); } private IndexWriter initIndex(Random random, MockDirectoryWrapper dir, boolean initialCommit) throws IOException { Index: lucene/src/java/org/apache/lucene/store/NRTCachingDirectory.java =================================================================== --- lucene/src/java/org/apache/lucene/store/NRTCachingDirectory.java (revision 1220805) +++ lucene/src/java/org/apache/lucene/store/NRTCachingDirectory.java (working copy) @@ -167,7 +167,7 @@ System.out.println("nrtdir.deleteFile name=" + name); } if (cache.fileExists(name)) { - assert !delegate.fileExists(name); + assert !delegate.fileExists(name): "name=" + name; cache.deleteFile(name); } else { delegate.deleteFile(name); @@ -196,8 +196,18 @@ if (VERBOSE) { System.out.println(" to cache"); } + try { + delegate.deleteFile(name); + } catch (IOException ioe) { + // This is fine: file may not exist + } return cache.createOutput(name, context); } else { + try { + cache.deleteFile(name); + } catch (IOException ioe) { + // This is fine: file may not exist + } return delegate.createOutput(name, context); } } @@ -247,6 +257,11 @@ * to the delegate and then closes the delegate. */ @Override public void close() throws IOException { + // NOTE: technically we shouldn't have to do this, ie, + // IndexWriter should have sync'd all files, but we do + // it for defensive reasons... or in case the app is + // doing something custom (creating outputs directly w/o + // using IndexWriter): for(String fileName : cache.listAll()) { unCache(fileName); } @@ -262,29 +277,40 @@ return !name.equals(IndexFileNames.SEGMENTS_GEN) && (merge == null || merge.estimatedMergeBytes <= maxMergeSizeBytes) && cache.sizeInBytes() <= maxCachedBytes; } + private final Object uncacheLock = new Object(); + private void unCache(String fileName) throws IOException { - final IndexOutput out; - IOContext context = IOContext.DEFAULT; - synchronized(this) { - if (!delegate.fileExists(fileName)) { - assert cache.fileExists(fileName); + // Only let one thread uncache at a time; this only + // happens during commit() or close(): + IndexOutput out = null; + IndexInput in = null; + try { + synchronized(uncacheLock) { + if (VERBOSE) { + System.out.println("nrtdir.unCache name=" + fileName); + } + if (!cache.fileExists(fileName)) { + // Another thread beat us... + return; + } + IOContext context = IOContext.DEFAULT; + if (delegate.fileExists(fileName)) { + throw new IOException("cannot uncache file=\"" + fileName + "\": it was separately also created in the delegate directory"); + } out = delegate.createOutput(fileName, context); - } else { - out = null; - } - } - if (out != null) { - IndexInput in = null; - try { in = cache.openInput(fileName, context); in.copyBytes(out, in.length()); - } finally { - IOUtils.close(in, out); + + // Lock order: uncacheLock -> this + synchronized(this) { + // Must sync here because other sync methods have + // if (cache.fileExists(name)) { ... } else { ... }: + cache.deleteFile(fileName); + } } - synchronized(this) { - cache.deleteFile(fileName); - } + } finally { + IOUtils.close(in, out); } } } Index: lucene/src/test-framework/java/org/apache/lucene/store/MockDirectoryWrapper.java =================================================================== --- lucene/src/test-framework/java/org/apache/lucene/store/MockDirectoryWrapper.java (revision 1220805) +++ lucene/src/test-framework/java/org/apache/lucene/store/MockDirectoryWrapper.java (working copy) @@ -220,9 +220,31 @@ } else if (damage == 2) { action = "partially truncated"; // Partially Truncate the file: - IndexOutput out = delegate.createOutput(name, LuceneTestCase.newIOContext(randomState)); - out.setLength(fileLength(name)/2); + + // First, make temp file and copy only half this + // file over: + String tempFileName; + while (true) { + tempFileName = ""+randomState.nextInt(); + if (!delegate.fileExists(tempFileName)) { + break; + } + } + final IndexOutput tempOut = delegate.createOutput(tempFileName, LuceneTestCase.newIOContext(randomState)); + IndexInput in = delegate.openInput(name, LuceneTestCase.newIOContext(randomState)); + tempOut.copyBytes(in, in.length()/2); + tempOut.close(); + in.close(); + + // Delete original and copy bytes back: + deleteFile(name, true); + + final IndexOutput out = delegate.createOutput(name, LuceneTestCase.newIOContext(randomState)); + in = delegate.openInput(tempFileName, LuceneTestCase.newIOContext(randomState)); + out.copyBytes(in, in.length()); out.close(); + in.close(); + deleteFile(tempFileName, true); } else if (damage == 3) { // The file survived intact: action = "didn't change"; Index: lucene/src/test-framework/java/org/apache/lucene/util/LuceneTestCase.java =================================================================== --- lucene/src/test-framework/java/org/apache/lucene/util/LuceneTestCase.java (revision 1220805) +++ lucene/src/test-framework/java/org/apache/lucene/util/LuceneTestCase.java (working copy) @@ -1007,18 +1007,8 @@ * See {@link #newDirectory()} for more information. */ public static MockDirectoryWrapper newDirectory(Random r) throws IOException { - return newDirectory(r, true); - } - - /** - * Returns a new Directory instance, using the specified random. You - * can specify maybeWrap as to whether the directory might be also - * wrapped by NRTCachingDirectory or FileSwitchDirectory - * See {@link #newDirectory()} for more information. - */ - public static MockDirectoryWrapper newDirectory(Random r, boolean maybeWrap) throws IOException { Directory impl = newDirectoryImpl(r, TEST_DIRECTORY); - MockDirectoryWrapper dir = new MockDirectoryWrapper(r, maybeWrap ? maybeNRTWrap(r, impl) : impl); + MockDirectoryWrapper dir = new MockDirectoryWrapper(r, maybeNRTWrap(r, impl)); stores.put(dir, Thread.currentThread().getStackTrace()); dir.setThrottling(TEST_THROTTLING); return dir; @@ -1030,7 +1020,7 @@ * information. */ public static MockDirectoryWrapper newDirectory(Directory d) throws IOException { - return newDirectory(random, d, true); + return newDirectory(random, d); } /** Returns a new FSDirectory instance over the given file, which must be a folder. */ @@ -1040,11 +1030,6 @@ /** Returns a new FSDirectory instance over the given file, which must be a folder. */ public static MockDirectoryWrapper newFSDirectory(File f, LockFactory lf) throws IOException { - return newFSDirectory(f, lf, true); - } - - /** Returns a new FSDirectory instance over the given file, which must be a folder. */ - public static MockDirectoryWrapper newFSDirectory(File f, LockFactory lf, boolean maybeWrap) throws IOException { String fsdirClass = TEST_DIRECTORY; if (fsdirClass.equals("random")) { fsdirClass = FS_DIRECTORIES[random.nextInt(FS_DIRECTORIES.length)]; @@ -1061,7 +1046,7 @@ } Directory fsdir = newFSDirectoryImpl(clazz, f); - MockDirectoryWrapper dir = new MockDirectoryWrapper(random, maybeWrap ? maybeNRTWrap(random, fsdir) : fsdir); + MockDirectoryWrapper dir = new MockDirectoryWrapper(random, maybeNRTWrap(random, fsdir)); if (lf != null) { dir.setLockFactory(lf); } @@ -1078,12 +1063,12 @@ * with contents copied from the provided directory. See * {@link #newDirectory()} for more information. */ - public static MockDirectoryWrapper newDirectory(Random r, Directory d, boolean maybeWrap) throws IOException { + public static MockDirectoryWrapper newDirectory(Random r, Directory d) throws IOException { Directory impl = newDirectoryImpl(r, TEST_DIRECTORY); for (String file : d.listAll()) { d.copy(impl, file, file, newIOContext(r)); } - MockDirectoryWrapper dir = new MockDirectoryWrapper(r, maybeWrap ? maybeNRTWrap(r, impl) : impl); + MockDirectoryWrapper dir = new MockDirectoryWrapper(r, maybeNRTWrap(r, impl)); stores.put(dir, Thread.currentThread().getStackTrace()); dir.setThrottling(TEST_THROTTLING); return dir;