From 04afdb6442186d5b23d87505063487f590642efc Mon Sep 17 00:00:00 2001 From: Henning Andersen <33268011+henningandersen@users.noreply.github.com> Date: Tue, 26 Mar 2019 14:56:45 +0100 Subject: [PATCH] LUCENE-8735: Avoid FileAlreadyExistsException on windows. (#619) FilterDirectory.getPendingDeletions() did not delegate the call, which resulted in a new IndexWriter on same directory not considering pending delete files. This could in turn result in a FileAlreadyExistsException when running windows. --- lucene/CHANGES.txt | 4 ++ .../simpletext/SimpleTextCompoundFormat.java | 27 ++++++++----- .../lucene50/Lucene50CompoundReader.java | 6 +++ .../lucene/store/ByteBuffersDirectory.java | 7 ++++ .../org/apache/lucene/store/Directory.java | 5 +-- .../apache/lucene/store/FilterDirectory.java | 2 +- .../apache/lucene/index/TestIndexWriter.java | 39 +++++++++++++++++++ .../lucene/index/BaseMergePolicyTestCase.java | 4 ++ .../apache/solr/store/hdfs/HdfsDirectory.java | 9 ++++- 9 files changed, 87 insertions(+), 16 deletions(-) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 5638c56f5aa..70d0d03d367 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -7,6 +7,10 @@ http://s.apache.org/luceneversions API Changes +* LUCENE-8735: Directory.getPendingDeletions is now abstract to ensure + subclasses override it. FilterDirectory now delegates the call, ensuring + correct default behaviour for subclasses. (Henning Andersen) + * LUCENE-8474: RAMDirectory and associated deprecated classes have been removed. (Dawid Weiss) diff --git a/lucene/codecs/src/java/org/apache/lucene/codecs/simpletext/SimpleTextCompoundFormat.java b/lucene/codecs/src/java/org/apache/lucene/codecs/simpletext/SimpleTextCompoundFormat.java index bed0c07a5ab..d04e99cafe7 100644 --- a/lucene/codecs/src/java/org/apache/lucene/codecs/simpletext/SimpleTextCompoundFormat.java +++ b/lucene/codecs/src/java/org/apache/lucene/codecs/simpletext/SimpleTextCompoundFormat.java @@ -25,7 +25,9 @@ import java.text.DecimalFormatSymbols; import java.text.ParseException; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.Locale; +import java.util.Set; import org.apache.lucene.codecs.CompoundFormat; import org.apache.lucene.index.CorruptIndexException; @@ -86,23 +88,23 @@ public class SimpleTextCompoundFormat extends CompoundFormat { SimpleTextUtil.readLine(in, scratch); assert StringHelper.startsWith(scratch.get(), TABLENAME); fileNames[i] = si.name + IndexFileNames.stripSegmentName(stripPrefix(scratch, TABLENAME)); - + if (i > 0) { // files must be unique and in sorted order assert fileNames[i].compareTo(fileNames[i-1]) > 0; } - + SimpleTextUtil.readLine(in, scratch); assert StringHelper.startsWith(scratch.get(), TABLESTART); startOffsets[i] = Long.parseLong(stripPrefix(scratch, TABLESTART)); - + SimpleTextUtil.readLine(in, scratch); assert StringHelper.startsWith(scratch.get(), TABLEEND); endOffsets[i] = Long.parseLong(stripPrefix(scratch, TABLEEND)); } - + return new Directory() { - + private int getIndex(String name) throws IOException { int index = Arrays.binarySearch(fileNames, name); if (index < 0) { @@ -110,32 +112,37 @@ public class SimpleTextCompoundFormat extends CompoundFormat { } return index; } - + @Override public String[] listAll() throws IOException { ensureOpen(); return fileNames.clone(); } - + @Override public long fileLength(String name) throws IOException { ensureOpen(); int index = getIndex(name); return endOffsets[index] - startOffsets[index]; } - + @Override public IndexInput openInput(String name, IOContext context) throws IOException { ensureOpen(); int index = getIndex(name); return in.slice(name, startOffsets[index], endOffsets[index] - startOffsets[index]); } - + @Override public void close() throws IOException { in.close(); } - + + @Override + public Set getPendingDeletions() throws IOException { + return Collections.emptySet(); + } + // write methods: disabled @Override diff --git a/lucene/core/src/java/org/apache/lucene/codecs/lucene50/Lucene50CompoundReader.java b/lucene/core/src/java/org/apache/lucene/codecs/lucene50/Lucene50CompoundReader.java index 7526c88c202..59e42d4a4d2 100644 --- a/lucene/core/src/java/org/apache/lucene/codecs/lucene50/Lucene50CompoundReader.java +++ b/lucene/core/src/java/org/apache/lucene/codecs/lucene50/Lucene50CompoundReader.java @@ -23,6 +23,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.Set; import org.apache.lucene.codecs.CodecUtil; import org.apache.lucene.index.CorruptIndexException; @@ -212,4 +213,9 @@ final class Lucene50CompoundReader extends Directory { public String toString() { return "CompoundFileDirectory(segment=\"" + segmentName + "\" in dir=" + directory + ")"; } + + @Override + public Set getPendingDeletions() { + return Collections.emptySet(); + } } diff --git a/lucene/core/src/java/org/apache/lucene/store/ByteBuffersDirectory.java b/lucene/core/src/java/org/apache/lucene/store/ByteBuffersDirectory.java index 535d5582943..6e4de1923e3 100644 --- a/lucene/core/src/java/org/apache/lucene/store/ByteBuffersDirectory.java +++ b/lucene/core/src/java/org/apache/lucene/store/ByteBuffersDirectory.java @@ -23,9 +23,11 @@ import java.nio.file.FileAlreadyExistsException; import java.nio.file.NoSuchFileException; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Locale; import java.util.Objects; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicLong; import java.util.function.BiFunction; @@ -229,6 +231,11 @@ public final class ByteBuffersDirectory extends BaseDirectory { files.clear(); } + @Override + public Set getPendingDeletions() { + return Collections.emptySet(); + } + private final class FileEntry { private final String fileName; diff --git a/lucene/core/src/java/org/apache/lucene/store/Directory.java b/lucene/core/src/java/org/apache/lucene/store/Directory.java index 7dcf7097af7..13e8e904b2e 100644 --- a/lucene/core/src/java/org/apache/lucene/store/Directory.java +++ b/lucene/core/src/java/org/apache/lucene/store/Directory.java @@ -22,7 +22,6 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.nio.file.NoSuchFileException; import java.util.Collection; // for javadocs -import java.util.Collections; import java.util.Set; import org.apache.lucene.util.IOUtils; @@ -207,7 +206,5 @@ public abstract class Directory implements Closeable { * * @lucene.internal */ - public Set getPendingDeletions() throws IOException { - return Collections.emptySet(); - } + public abstract Set getPendingDeletions() throws IOException; } diff --git a/lucene/core/src/java/org/apache/lucene/store/FilterDirectory.java b/lucene/core/src/java/org/apache/lucene/store/FilterDirectory.java index 7b2a6c702d2..c2faaf4d0cb 100644 --- a/lucene/core/src/java/org/apache/lucene/store/FilterDirectory.java +++ b/lucene/core/src/java/org/apache/lucene/store/FilterDirectory.java @@ -117,6 +117,6 @@ public abstract class FilterDirectory extends Directory { @Override public Set getPendingDeletions() throws IOException { - return super.getPendingDeletions(); + return in.getPendingDeletions(); } } diff --git a/lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java b/lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java index 6733c2dd02f..45e12283cc3 100644 --- a/lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java +++ b/lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java @@ -2737,6 +2737,45 @@ public class TestIndexWriter extends LuceneTestCase { } + public void testPendingDeletionsRollbackWithReader() throws IOException { + // irony: currently we don't emulate windows well enough to work on windows! + assumeFalse("windows is not supported", Constants.WINDOWS); + + Path path = createTempDir(); + + // Use WindowsFS to prevent open files from being deleted: + FileSystem fs = new WindowsFS(path.getFileSystem()).getFileSystem(URI.create("file:///")); + Path root = new FilterPath(path, fs); + try (FSDirectory _dir = new SimpleFSDirectory(root)) { + Directory dir = new FilterDirectory(_dir) {}; + + IndexWriterConfig iwc = new IndexWriterConfig(new MockAnalyzer(random())); + IndexWriter w = new IndexWriter(dir, iwc); + Document d = new Document(); + d.add(new StringField("id", "1", Field.Store.YES)); + d.add(new NumericDocValuesField("numval", 1)); + w.addDocument(d); + w.commit(); + w.addDocument(d); + w.flush(); + DirectoryReader reader = DirectoryReader.open(w); + w.rollback(); + + // try-delete superfluous files (some will fail due to windows-fs) + IndexWriterConfig iwc2 = new IndexWriterConfig(new MockAnalyzer(random())); + new IndexWriter(dir, iwc2).close(); + + // test that we can index on top of pending deletions + IndexWriterConfig iwc3 = new IndexWriterConfig(new MockAnalyzer(random())); + w = new IndexWriter(dir, iwc3); + w.addDocument(d); + w.commit(); + + reader.close(); + w.close(); + } + } + public void testWithPendingDeletions() throws Exception { // irony: currently we don't emulate windows well enough to work on windows! assumeFalse("windows is not supported", Constants.WINDOWS); diff --git a/lucene/test-framework/src/java/org/apache/lucene/index/BaseMergePolicyTestCase.java b/lucene/test-framework/src/java/org/apache/lucene/index/BaseMergePolicyTestCase.java index b77c68dc2f4..9c763461e3b 100644 --- a/lucene/test-framework/src/java/org/apache/lucene/index/BaseMergePolicyTestCase.java +++ b/lucene/test-framework/src/java/org/apache/lucene/index/BaseMergePolicyTestCase.java @@ -277,6 +277,10 @@ public abstract class BaseMergePolicyTestCase extends LuceneTestCase { throw new UnsupportedOperationException(); } + @Override + public Set getPendingDeletions() throws IOException { + throw new UnsupportedOperationException(); + } }; /** diff --git a/solr/core/src/java/org/apache/solr/store/hdfs/HdfsDirectory.java b/solr/core/src/java/org/apache/solr/store/hdfs/HdfsDirectory.java index a186715df78..584cf2d5dde 100644 --- a/solr/core/src/java/org/apache/solr/store/hdfs/HdfsDirectory.java +++ b/solr/core/src/java/org/apache/solr/store/hdfs/HdfsDirectory.java @@ -21,7 +21,9 @@ import java.lang.invoke.MethodHandles; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.List; +import java.util.Set; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FSDataInputStream; @@ -195,7 +197,12 @@ public class HdfsDirectory extends BaseDirectory { public Configuration getConfiguration() { return configuration; } - + + @Override + public Set getPendingDeletions() { + return Collections.emptySet(); + } + public static class HdfsIndexInput extends CustomBufferedIndexInput { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());