Skip to content

Commit

Permalink
Handle no-op document level failures (#46083)
Browse files Browse the repository at this point in the history
Today we assume that document failures can not occur for no-ops. This
assumption is bogus, as they can fail for a variety of reasons such as
the Lucene index having reached the document limit. Because of this
assumption, we were asserting that such a document-level failure would
never happen. When this bogus assertion is violated, we fail the node, a
catastrophe. Instead, we need to treat this as a fatal engine exception.
  • Loading branch information
jasontedor committed Aug 28, 2019
1 parent 0e37fef commit 96dc774
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1514,9 +1514,14 @@ private NoOpResult innerNoOp(final NoOp noOp) throws IOException {
: "Noop tombstone document but _tombstone field is not set [" + doc + " ]";
doc.add(softDeletesField);
indexWriter.addDocument(doc);
} catch (Exception ex) {
} catch (final Exception ex) {
/*
* Document level failures when adding a no-op are unexpected, we likely hit something fatal such as the Lucene
* index being corrupt, or the Lucene document limit. We have already issued a sequence number here so this is
* fatal, fail the engine.
*/
if (ex instanceof AlreadyClosedException == false && indexWriter.getTragicException() == null) {
throw new AssertionError("noop operation should never fail at document level", ex);
failEngine("no-op origin[" + noOp.origin() + "] seq#[" + noOp.seqNo() + "] failed at document level", ex);
}
throw ex;
}
Expand Down Expand Up @@ -2862,4 +2867,5 @@ private void restoreVersionMapAndCheckpointTracker(DirectoryReader directoryRead
// remove live entries in the version map
refresh("restore_version_map_and_checkpoint_tracker", SearcherScope.INTERNAL, true);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -5997,4 +5997,35 @@ private Map<BytesRef, DeleteVersionValue> tombstonesInVersionMap(InternalEngine
.filter(e -> e.getValue() instanceof DeleteVersionValue)
.collect(Collectors.toMap(e -> e.getKey(), e -> (DeleteVersionValue) e.getValue()));
}

public void testNoOpFailure() throws IOException {
engine.close();
final Settings settings = Settings.builder()
.put(defaultSettings.getSettings())
.put(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), true).build();
final IndexSettings indexSettings = IndexSettingsModule.newIndexSettings(
IndexMetaData.builder(defaultSettings.getIndexMetaData()).settings(settings).build());
try (Store store = createStore();
Engine engine = createEngine(
(dir, iwc) -> new IndexWriter(dir, iwc) {

@Override
public long addDocument(Iterable<? extends IndexableField> doc) throws IOException {
throw new IllegalArgumentException("fatal");
}

},
null,
null,
config(indexSettings, store, createTempDir(), NoMergePolicy.INSTANCE, null))) {
final Engine.NoOp op = new Engine.NoOp(0, 0, PRIMARY, System.currentTimeMillis(), "test");
final IllegalArgumentException e = expectThrows(IllegalArgumentException. class, () -> engine.noOp(op));
assertThat(e.getMessage(), equalTo("fatal"));
assertTrue(engine.isClosed.get());
assertThat(engine.failedEngine.get(), not(nullValue()));
assertThat(engine.failedEngine.get(), instanceOf(IllegalArgumentException.class));
assertThat(engine.failedEngine.get().getMessage(), equalTo("fatal"));
}
}

}

0 comments on commit 96dc774

Please sign in to comment.