diff --git a/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java b/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java index a625c235cb600..1ed0f5d6f7994 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java @@ -92,6 +92,7 @@ import java.util.Collection; import java.util.HashMap; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -1464,14 +1465,21 @@ private DeleteResult deleteInLucene(Delete delete, DeletionStrategy plan) throws } return new DeleteResult( plan.versionOfDeletion, getPrimaryTerm(), plan.seqNoOfDeletion, plan.currentlyDeleted == false); - } catch (Exception ex) { - if (indexWriter.getTragicException() == null) { - // there is no tragic event and such it must be a document level failure - return new DeleteResult( - ex, plan.versionOfDeletion, delete.primaryTerm(), plan.seqNoOfDeletion, plan.currentlyDeleted == false); - } else { - throw ex; + } catch (final Exception ex) { + /* + * Document level failures when deleting 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) { + final String reason = String.format( + Locale.ROOT, + "delete id[%s] origin [%s] seq#[%d] failed at the document level", + delete.id(), + delete.origin(), + delete.seqNo()); + failEngine(reason, ex); } + throw ex; } } diff --git a/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java b/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java index 42175ae21d1b5..b5e34b7218417 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java @@ -77,6 +77,7 @@ import org.elasticsearch.cluster.routing.ShardRouting; import org.elasticsearch.cluster.routing.ShardRoutingState; import org.elasticsearch.cluster.routing.TestShardRouting; +import org.elasticsearch.common.CheckedBiConsumer; import org.elasticsearch.common.CheckedRunnable; import org.elasticsearch.common.Randomness; import org.elasticsearch.common.Strings; @@ -3318,8 +3319,8 @@ public void testHandleDocumentFailure() throws Exception { AtomicReference throwingIndexWriter = new AtomicReference<>(); try (InternalEngine engine = createEngine(defaultSettings, store, createTempDir(), NoMergePolicy.INSTANCE, (directory, iwc) -> { - throwingIndexWriter.set(new ThrowingIndexWriter(directory, iwc)); - return throwingIndexWriter.get(); + throwingIndexWriter.set(new ThrowingIndexWriter(directory, iwc)); + return throwingIndexWriter.get(); }) ) { // test document failure while indexing @@ -3343,22 +3344,6 @@ public void testHandleDocumentFailure() throws Exception { assertNotNull(indexResult.getTranslogLocation()); engine.index(indexForDoc(doc2)); - // test failure while deleting - // all these simulated exceptions are not fatal to the IW so we treat them as document failures - final Engine.DeleteResult deleteResult; - if (randomBoolean()) { - throwingIndexWriter.get().setThrowFailure(() -> new IOException("simulated")); - deleteResult = engine.delete(new Engine.Delete("test", "1", newUid(doc1), primaryTerm.get())); - assertThat(deleteResult.getFailure(), instanceOf(IOException.class)); - } else { - throwingIndexWriter.get().setThrowFailure(() -> new IllegalArgumentException("simulated max token length")); - deleteResult = engine.delete(new Engine.Delete("test", "1", newUid(doc1), primaryTerm.get())); - assertThat(deleteResult.getFailure(), - instanceOf(IllegalArgumentException.class)); - } - assertThat(deleteResult.getVersion(), equalTo(2L)); - assertThat(deleteResult.getSeqNo(), equalTo(3L)); - // test non document level failure is thrown if (randomBoolean()) { // simulate close by corruption @@ -5815,4 +5800,48 @@ public long addDocument(Iterable doc) throws IOExcepti } } + public void testDeleteFailureSoftDeletesEnabledDocAlreadyDeleted() throws IOException { + runTestDeleteFailure(true, InternalEngine::delete); + } + + public void testDeleteFailureSoftDeletesEnabled() throws IOException { + runTestDeleteFailure(true, (engine, op) -> {}); + } + + public void testDeleteFailureSoftDeletesDisabled() throws IOException { + runTestDeleteFailure(false, (engine, op) -> {}); + } + + private void runTestDeleteFailure( + final boolean softDeletesEnabled, + final CheckedBiConsumer consumer) throws IOException { + engine.close(); + final Settings settings = Settings.builder() + .put(defaultSettings.getSettings()) + .put(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), softDeletesEnabled).build(); + final IndexSettings indexSettings = IndexSettingsModule.newIndexSettings( + IndexMetaData.builder(defaultSettings.getIndexMetaData()).settings(settings).build()); + final AtomicReference iw = new AtomicReference<>(); + try (Store store = createStore(); + InternalEngine engine = createEngine( + (dir, iwc) -> { + iw.set(new ThrowingIndexWriter(dir, iwc)); + return iw.get(); + }, + null, + null, + config(indexSettings, store, createTempDir(), NoMergePolicy.INSTANCE, null))) { + engine.index(new Engine.Index(newUid("0"), primaryTerm.get(), InternalEngineTests.createParsedDoc("0", null))); + final Engine.Delete op = new Engine.Delete("_doc", "0", newUid("0"), primaryTerm.get()); + consumer.accept(engine, op); + iw.get().setThrowFailure(() -> new IllegalArgumentException("fatal")); + final IllegalArgumentException e = expectThrows(IllegalArgumentException. class, () -> engine.delete(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")); + } + } + }