diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index a2ab0414096..5a34e690b6b 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -564,6 +564,11 @@ Bug fixes * LUCENE-3365: Create or Append mode determined before obtaining write lock can cause IndexWriter overriding an existing index. (Geoff Cooney via Simon Willnauer) + +* LUCENE-3380: Fixed a bug where FileSwitchDirectory's listAll() would wrongly + 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) New Features diff --git a/lucene/src/java/org/apache/lucene/store/FileSwitchDirectory.java b/lucene/src/java/org/apache/lucene/store/FileSwitchDirectory.java index 5b76e46a47a..f94af04f361 100644 --- a/lucene/src/java/org/apache/lucene/store/FileSwitchDirectory.java +++ b/lucene/src/java/org/apache/lucene/store/FileSwitchDirectory.java @@ -78,11 +78,38 @@ public class FileSwitchDirectory extends Directory { @Override public String[] listAll() throws IOException { Set files = new HashSet(); - for(String f : primaryDir.listAll()) { - files.add(f); + // LUCENE-3380: either or both of our dirs could be FSDirs, + // but if one underlying delegate is an FSDir and mkdirs() has not + // yet been called, because so far everything is written to the other, + // in this case, we don't want to throw a NoSuchDirectoryException + NoSuchDirectoryException exc = null; + try { + for(String f : primaryDir.listAll()) { + files.add(f); + } + } catch (NoSuchDirectoryException e) { + exc = e; } - for(String f : secondaryDir.listAll()) { - files.add(f); + try { + for(String f : secondaryDir.listAll()) { + files.add(f); + } + } catch (NoSuchDirectoryException e) { + // we got NoSuchDirectoryException from both dirs + // rethrow the first. + if (exc != null) { + throw exc; + } + // we got NoSuchDirectoryException from the secondary, + // and the primary is empty. + if (files.isEmpty()) { + throw e; + } + } + // we got NoSuchDirectoryException from the primary, + // and the secondary is empty. + if (exc != null && files.isEmpty()) { + throw exc; } return files.toArray(new String[files.size()]); } @@ -150,13 +177,19 @@ public class FileSwitchDirectory extends Directory { return getDirectory(name).openInput(name, context); } + // final due to LUCENE-3380: currently CFS backdoors the directory to create CFE + // by using the basic implementation and not delegating, we ensure that all + // openInput/createOutput requests come thru NRTCachingDirectory. @Override - public CompoundFileDirectory openCompoundInput(String name, IOContext context) throws IOException { - return getDirectory(name).openCompoundInput(name, context); + public final CompoundFileDirectory openCompoundInput(String name, IOContext context) throws IOException { + return super.openCompoundInput(name, context); } + // final due to LUCENE-3380: currently CFS backdoors the directory to create CFE + // by using the basic implementation and not delegating, we ensure that all + // openInput/createOutput requests come thru NRTCachingDirectory. @Override - public CompoundFileDirectory createCompoundOutput(String name, IOContext context) throws IOException { - return getDirectory(name).createCompoundOutput(name, context); + public final CompoundFileDirectory createCompoundOutput(String name, IOContext context) throws IOException { + return super.createCompoundOutput(name, context); } } diff --git a/lucene/src/test/org/apache/lucene/store/TestFileSwitchDirectory.java b/lucene/src/test/org/apache/lucene/store/TestFileSwitchDirectory.java index 75090b17072..15ac64a8b30 100644 --- a/lucene/src/test/org/apache/lucene/store/TestFileSwitchDirectory.java +++ b/lucene/src/test/org/apache/lucene/store/TestFileSwitchDirectory.java @@ -18,6 +18,8 @@ package org.apache.lucene.store; */ import java.io.IOException; +import java.util.Arrays; +import java.util.Collections; import java.util.HashSet; import java.util.Set; @@ -28,6 +30,7 @@ import org.apache.lucene.index.IndexWriter; import org.apache.lucene.index.IndexWriterConfig; import org.apache.lucene.index.TestIndexWriterReader; import org.apache.lucene.util.LuceneTestCase; +import org.apache.lucene.util._TestUtil; public class TestFileSwitchDirectory extends LuceneTestCase { /** @@ -77,4 +80,76 @@ public class TestFileSwitchDirectory extends LuceneTestCase { } fsd.close(); } + + private Directory newFSSwitchDirectory(Set primaryExtensions) throws IOException { + Directory a = new SimpleFSDirectory(_TestUtil.getTempDir("foo")); + Directory b = new SimpleFSDirectory(_TestUtil.getTempDir("bar")); + FileSwitchDirectory switchDir = new FileSwitchDirectory(primaryExtensions, a, b, true); + return new MockDirectoryWrapper(random, switchDir); + } + + // LUCENE-3380 -- make sure we get exception if the directory really does not exist. + public void testNoDir() throws Throwable { + Directory dir = newFSSwitchDirectory(Collections.emptySet()); + try { + IndexReader.open(dir, true); + fail("did not hit expected exception"); + } catch (NoSuchDirectoryException nsde) { + // expected + } + dir.close(); + } + + // LUCENE-3380 test that we can add a file, and then when we call list() we get it back + public void testDirectoryFilter() throws IOException { + Directory dir = newFSSwitchDirectory(Collections.emptySet()); + String name = "file"; + try { + dir.createOutput(name, newIOContext(random)).close(); + assertTrue(dir.fileExists(name)); + assertTrue(Arrays.asList(dir.listAll()).contains(name)); + } finally { + dir.close(); + } + } + + // LUCENE-3380 test that delegate compound files correctly. + public void testCompoundFileAppendTwice() throws IOException { + Directory newDir = newFSSwitchDirectory(Collections.singleton("cfs")); + CompoundFileDirectory csw = newDir.createCompoundOutput("d.cfs", newIOContext(random)); + createSequenceFile(newDir, "d1", (byte) 0, 15); + IndexOutput out = csw.createOutput("d.xyz", newIOContext(random)); + out.writeInt(0); + try { + newDir.copy(csw, "d1", "d1", newIOContext(random)); + fail("file does already exist"); + } catch (IOException e) { + // + } + out.close(); + assertEquals(1, csw.listAll().length); + assertEquals("d.xyz", csw.listAll()[0]); + + csw.close(); + + CompoundFileDirectory cfr = newDir.openCompoundInput("d.cfs", newIOContext(random)); + assertEquals(1, cfr.listAll().length); + assertEquals("d.xyz", cfr.listAll()[0]); + cfr.close(); + newDir.close(); + } + + /** Creates a file of the specified size with sequential data. The first + * byte is written as the start byte provided. All subsequent bytes are + * computed as start + offset where offset is the number of the byte. + */ + private void createSequenceFile(Directory dir, String name, byte start, int size) throws IOException { + IndexOutput os = dir.createOutput(name, newIOContext(random)); + for (int i=0; i < size; i++) { + os.writeByte(start); + start ++; + } + os.close(); + } + }