From da095bc7da6ff5f8e2005d55156605ac5144ebda Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Mon, 24 Aug 2020 20:19:44 +0200 Subject: [PATCH] LUCENE-9477: Don't leave potentially broken segments file behind (#1777) If we fail to rollback an already renamed pending segments file during commit due to a failure in directory syncing we might not fully roll back to a proper state if we hit a failure during rollback which leaves the index in a broken state. This is a best effort approach to remove the renamed file in the case of a failure during sync. --- .../org/apache/lucene/index/SegmentInfos.java | 17 +++-- .../index/TestIndexWriterExceptions.java | 62 +++++++++++++++++++ .../index/TestIndexWriterOnVMError.java | 4 +- .../lucene/store/MockDirectoryWrapper.java | 2 +- 4 files changed, 78 insertions(+), 7 deletions(-) diff --git a/lucene/core/src/java/org/apache/lucene/index/SegmentInfos.java b/lucene/core/src/java/org/apache/lucene/index/SegmentInfos.java index dc379ab3cd9..745b3d74aad 100644 --- a/lucene/core/src/java/org/apache/lucene/index/SegmentInfos.java +++ b/lucene/core/src/java/org/apache/lucene/index/SegmentInfos.java @@ -786,7 +786,6 @@ public final class SegmentInfos implements Cloneable, Iterable random().nextBoolean() && maybeFailDelete.get(); + dir.failOn(new MockDirectoryWrapper.Failure() { + @Override + public void eval(MockDirectoryWrapper dir) { + if (callStackContains(MockDirectoryWrapper.class, "syncMetaData") + && callStackContains(SegmentInfos.class, "finishCommit")) { + throw new RuntimeException("boom"); + } else if (failDelete.getAsBoolean() && + callStackContains(IndexWriter.class, "rollbackInternalNoCommit") && callStackContains(IndexFileDeleter.class, "deleteFiles")) { + throw new RuntimeException("bang"); + } + }}); + for (int i = 0; i < 5; i++) { + Document doc = new Document(); + doc.add(newStringField("id", Integer.toString(i), Field.Store.NO)); + doc.add(new NumericDocValuesField("dv", i)); + doc.add(new BinaryDocValuesField("dv2", new BytesRef(Integer.toString(i)))); + doc.add(new SortedDocValuesField("dv3", new BytesRef(Integer.toString(i)))); + doc.add(new SortedSetDocValuesField("dv4", new BytesRef(Integer.toString(i)))); + doc.add(new SortedSetDocValuesField("dv4", new BytesRef(Integer.toString(i - 1)))); + doc.add(new SortedNumericDocValuesField("dv5", i)); + doc.add(new SortedNumericDocValuesField("dv5", i - 1)); + doc.add(newTextField("text1", TestUtil.randomAnalysisString(random(), 20, true), Field.Store.NO)); + // ensure we store something + doc.add(new StoredField("stored1", "foo")); + doc.add(new StoredField("stored1", "bar")); + // ensure we get some payloads + doc.add(newTextField("text_payloads", TestUtil.randomAnalysisString(random(), 6, true), Field.Store.NO)); + // ensure we get some vectors + FieldType ft = new FieldType(TextField.TYPE_NOT_STORED); + ft.setStoreTermVectors(true); + doc.add(newField("text_vectors", TestUtil.randomAnalysisString(random(), 6, true), ft)); + doc.add(new IntPoint("point", random().nextInt())); + doc.add(new IntPoint("point2d", random().nextInt(), random().nextInt())); + writer.addDocument(new Document()); + } + try { + writer.commit(); + fail(); + } catch (RuntimeException e) { + assertEquals("boom", e.getMessage()); + } + try { + maybeFailDelete.set(true); + writer.rollback(); + } catch (RuntimeException e) { + assertEquals("bang", e.getMessage()); + } + maybeFailDelete.set(false); + assertTrue(writer.isClosed()); + assertTrue(DirectoryReader.indexExists(dir)); + DirectoryReader.open(dir).close(); + } + } } diff --git a/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterOnVMError.java b/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterOnVMError.java index 73704dd6a17..7c4ccacf508 100644 --- a/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterOnVMError.java +++ b/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterOnVMError.java @@ -224,7 +224,9 @@ public class TestIndexWriterOnVMError extends LuceneTestCase { // assertTrue("hit OOM but writer is still open, WTF: ", writer.isClosed()); try { writer.rollback(); - } catch (Throwable t) {} + } catch (Throwable t) { + t.printStackTrace(log); + } return (VirtualMachineError) e; } else { Rethrow.rethrow(disaster); diff --git a/lucene/test-framework/src/java/org/apache/lucene/store/MockDirectoryWrapper.java b/lucene/test-framework/src/java/org/apache/lucene/store/MockDirectoryWrapper.java index 12d5075527c..c0cbe57b235 100644 --- a/lucene/test-framework/src/java/org/apache/lucene/store/MockDirectoryWrapper.java +++ b/lucene/test-framework/src/java/org/apache/lucene/store/MockDirectoryWrapper.java @@ -249,7 +249,7 @@ public class MockDirectoryWrapper extends BaseDirectoryWrapper { maybeYield(); maybeThrowDeterministicException(); if (crashed) { - throw new IOException("cannot rename after crash"); + throw new IOException("cannot sync metadata after crash"); } in.syncMetaData(); }