From 70951cfb55f1204542342ff44184fd5d53c123be Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Mon, 21 Nov 2016 16:51:29 -0500 Subject: [PATCH 1/3] Die with dignity on the Lucene layer When a fatal error tragically closes an index writer, such an error never makes its way to the uncaught exception handler. This prevents the node from being torn down if an out of memory error or other fatal error is thrown in the Lucene layer. This commit ensures that such events bubble their way up to the uncaught exception handler. --- .../elasticsearch/index/engine/InternalEngine.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/engine/InternalEngine.java b/core/src/main/java/org/elasticsearch/index/engine/InternalEngine.java index 127d8a2c98eb1..ee7b7b453a231 100644 --- a/core/src/main/java/org/elasticsearch/index/engine/InternalEngine.java +++ b/core/src/main/java/org/elasticsearch/index/engine/InternalEngine.java @@ -1102,10 +1102,15 @@ private void failOnTragicEvent(AlreadyClosedException ex) { // we need to fail the engine. it might have already been failed before // but we are double-checking it's failed and closed if (indexWriter.isOpen() == false && indexWriter.getTragicException() != null) { - final Exception tragedy = indexWriter.getTragicException() instanceof Exception ? - (Exception) indexWriter.getTragicException() : - new Exception(indexWriter.getTragicException()); - failEngine("already closed by tragic event on the index writer", tragedy); + if (indexWriter.getTragicException() instanceof Error) { + try { + logger.error("tragic event in index writer", ex); + } finally { + throw (Error) indexWriter.getTragicException(); + } + } else { + failEngine("already closed by tragic event on the index writer", (Exception) indexWriter.getTragicException()); + } } else if (translog.isOpen() == false && translog.getTragicException() != null) { failEngine("already closed by tragic event on the translog", translog.getTragicException()); } else if (failedEngine.get() == null) { // we are closed but the engine is not failed yet? From 309d97a1c9df53b06abde19438e73030d0a6df43 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Mon, 21 Nov 2016 21:29:02 -0500 Subject: [PATCH 2/3] Add test for index writer fatal error bubbling This commit adds a unit test that when an index writer is closed tragically, if it is due to a fatal error then that error bubbles up. --- .../index/engine/InternalEngine.java | 1 + .../index/engine/InternalEngineTests.java | 43 ++++++++++++++++++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/elasticsearch/index/engine/InternalEngine.java b/core/src/main/java/org/elasticsearch/index/engine/InternalEngine.java index ee7b7b453a231..c3a80d16aa8af 100644 --- a/core/src/main/java/org/elasticsearch/index/engine/InternalEngine.java +++ b/core/src/main/java/org/elasticsearch/index/engine/InternalEngine.java @@ -1097,6 +1097,7 @@ public IndexCommit acquireIndexCommit(final boolean flushFirst) throws EngineExc } } + @SuppressWarnings("finally") private void failOnTragicEvent(AlreadyClosedException ex) { // if we are already closed due to some tragic exception // we need to fail the engine. it might have already been failed before diff --git a/core/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java b/core/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java index 19b50c3717cb5..ba850fa04a79f 100644 --- a/core/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java +++ b/core/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java @@ -26,6 +26,8 @@ import org.apache.logging.log4j.core.LogEvent; import org.apache.logging.log4j.core.appender.AbstractAppender; import org.apache.logging.log4j.core.filter.RegexFilter; +import org.apache.lucene.analysis.Analyzer; +import org.apache.lucene.analysis.Tokenizer; import org.apache.lucene.analysis.standard.StandardAnalyzer; import org.apache.lucene.codecs.Codec; import org.apache.lucene.document.Field; @@ -125,6 +127,7 @@ import org.junit.After; import org.junit.Before; +import java.io.IOError; import java.io.IOException; import java.io.InputStream; import java.nio.charset.Charset; @@ -229,8 +232,12 @@ public void setUp() throws Exception { } public EngineConfig copy(EngineConfig config, EngineConfig.OpenMode openMode) { + return copy(config, openMode, config.getAnalyzer()); + } + + public EngineConfig copy(EngineConfig config, EngineConfig.OpenMode openMode, Analyzer analyzer) { return new EngineConfig(openMode, config.getShardId(), config.getThreadPool(), config.getIndexSettings(), config.getWarmer(), - config.getStore(), config.getDeletionPolicy(), config.getMergePolicy(), config.getAnalyzer(), config.getSimilarity(), + config.getStore(), config.getDeletionPolicy(), config.getMergePolicy(), analyzer, config.getSimilarity(), new CodecService(null, logger), config.getEventListener(), config.getTranslogRecoveryPerformer(), config.getQueryCache(), config.getQueryCachingPolicy(), config.getTranslogConfig(), config.getFlushMergesAfter(), config.getRefreshListeners(), config.getMaxUnsafeAutoIdTimestamp()); @@ -2849,4 +2856,38 @@ public void afterRefresh(boolean didRefresh) throws IOException { assertTrue(internalEngine.failedEngine.get() instanceof MockDirectoryWrapper.FakeIOException); } } + + public void testTragicEventErrorBubblesUp() throws IOException { + engine.close(); + final AtomicBoolean failWithFatalError = new AtomicBoolean(true); + final VirtualMachineError error = randomFrom( + new InternalError(), + new OutOfMemoryError(), + new StackOverflowError(), + new UnknownError()); + engine = new InternalEngine(copy(engine.config(), EngineConfig.OpenMode.OPEN_INDEX_AND_TRANSLOG, new Analyzer() { + @Override + protected TokenStreamComponents createComponents(String fieldName) { + return new TokenStreamComponents(new Tokenizer() { + @Override + public boolean incrementToken() throws IOException { + if (failWithFatalError.get()) { + throw error; + } else { + throw new AssertionError("should not get to this point"); + } + } + }); + } + })); + final Document document = testDocument(); + document.add(new TextField("value", "test", Field.Store.YES)); + final ParsedDocument doc = testParsedDocument("1", "1", "test", null, -1, -1, document, B_1, null); + final Engine.Index first = new Engine.Index(newUid("1"), doc); + expectThrows(error.getClass(), () -> engine.index(first)); + failWithFatalError.set(false); + expectThrows(error.getClass(), () -> engine.index(first)); + assertNull(engine.failedEngine.get()); + } + } From f4492b5922618dccf21fb0cbff2fdbc926f8a00c Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 22 Nov 2016 11:20:01 -0500 Subject: [PATCH 3/3] Remove extra space in InternalEngineTests This commit removes an extra space in InternalEngineTests#testTragicEventErrorBubblesUp. --- .../org/elasticsearch/index/engine/InternalEngineTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java b/core/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java index ba850fa04a79f..5d5a2e7b1ceaf 100644 --- a/core/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java +++ b/core/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java @@ -2884,7 +2884,7 @@ public boolean incrementToken() throws IOException { document.add(new TextField("value", "test", Field.Store.YES)); final ParsedDocument doc = testParsedDocument("1", "1", "test", null, -1, -1, document, B_1, null); final Engine.Index first = new Engine.Index(newUid("1"), doc); - expectThrows(error.getClass(), () -> engine.index(first)); + expectThrows(error.getClass(), () -> engine.index(first)); failWithFatalError.set(false); expectThrows(error.getClass(), () -> engine.index(first)); assertNull(engine.failedEngine.get());