From cf3e2d1aa816132c70afb0d8d6b80fedd5adad33 Mon Sep 17 00:00:00 2001 From: Areek Zillur Date: Tue, 1 Nov 2016 15:31:28 -0400 Subject: [PATCH] documentation and minor fixes for engine level index/delete operations --- .../replication/TransportWriteAction.java | 6 ++-- .../elasticsearch/index/engine/Engine.java | 28 +++++++++++++++---- .../index/engine/InternalEngine.java | 7 +++-- .../shard/IndexingOperationListener.java | 28 +++++++++---------- 4 files changed, 43 insertions(+), 26 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/action/support/replication/TransportWriteAction.java b/core/src/main/java/org/elasticsearch/action/support/replication/TransportWriteAction.java index 111edf5606b..15f269c46f5 100644 --- a/core/src/main/java/org/elasticsearch/action/support/replication/TransportWriteAction.java +++ b/core/src/main/java/org/elasticsearch/action/support/replication/TransportWriteAction.java @@ -89,9 +89,9 @@ public abstract class TransportWriteAction< @Nullable Location location, @Nullable Exception operationFailure, IndexShard primary) { super(request, finalResponse, operationFailure); - if (location != null) { - assert operationFailure == null : "expected no failures when translog location is not null"; - } + assert location == null || operationFailure == null + : "expected either failure to be null or translog location to be null, " + + "but found: [" + location + "] translog location and [" + operationFailure + "] failure"; if (operationFailure != null) { this.finishedAsyncActions = true; } else { diff --git a/core/src/main/java/org/elasticsearch/index/engine/Engine.java b/core/src/main/java/org/elasticsearch/index/engine/Engine.java index 0df026ad617..0da96def3ef 100644 --- a/core/src/main/java/org/elasticsearch/index/engine/Engine.java +++ b/core/src/main/java/org/elasticsearch/index/engine/Engine.java @@ -278,9 +278,25 @@ public abstract class Engine implements Closeable { } } - public abstract IndexResult index(Index operation); + /** + * Perform document index operation on the engine + * @param index operation to perform + * @return {@link IndexResult} containing updated translog location, version and + * document specific failures + * + * Note: engine level failures (i.e. persistent engine failures) are thrown + */ + public abstract IndexResult index(final Index index); - public abstract DeleteResult delete(Delete delete); + /** + * Perform document delete operation on the engine + * @param delete operation to perform + * @return {@link DeleteResult} containing updated translog location, version and + * document specific failures + * + * Note: engine level failures (i.e. persistent engine failures) are thrown + */ + public abstract DeleteResult delete(final Delete delete); /** * Base class for index and delete operation results @@ -291,9 +307,9 @@ public abstract class Engine implements Closeable { private final Operation.TYPE operationType; private final long version; private final Exception failure; + private final SetOnce freeze = new SetOnce<>(); private Translog.Location translogLocation; private long took; - private boolean freeze; protected Result(Operation.TYPE operationType, Exception failure, long version) { this.operationType = operationType; @@ -335,7 +351,7 @@ public abstract class Engine implements Closeable { } void setTranslogLocation(Translog.Location translogLocation) { - if (freeze == false) { + if (freeze.get() == null) { assert failure == null : "failure has to be null to set translog location"; this.translogLocation = translogLocation; } else { @@ -344,7 +360,7 @@ public abstract class Engine implements Closeable { } void setTook(long took) { - if (freeze == false) { + if (freeze.get() == null) { this.took = took; } else { throw new IllegalStateException("result is already frozen"); @@ -352,7 +368,7 @@ public abstract class Engine implements Closeable { } void freeze() { - this.freeze = true; + freeze.set(true); } } 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 e993e464764..36d5f195905 100644 --- a/core/src/main/java/org/elasticsearch/index/engine/InternalEngine.java +++ b/core/src/main/java/org/elasticsearch/index/engine/InternalEngine.java @@ -424,8 +424,7 @@ public class InternalEngine extends Engine { // errors and returns true if that is the case. We use that to indicate a document level failure // and set the error in operation.setFailure. In case of environment related errors, the failure // is bubbled up - isDocumentFailure = !((failure instanceof IllegalStateException || failure instanceof IOException) - && maybeFailEngine(operation.operationType().getLowercase(), failure)); + isDocumentFailure = maybeFailEngine(operation.operationType().getLowercase(), failure) == false; } catch (Exception inner) { // we failed checking whether the failure can fail the engine, treat it as a persistent engine failure isDocumentFailure = false; @@ -434,13 +433,15 @@ public class InternalEngine extends Engine { if (isDocumentFailure) { return failure; } else { + // throw original exception in case the exception caused the engine to fail rethrow(failure); return null; } } + // hack to rethrow original exception in case of engine level failures during index/delete operation @SuppressWarnings("unchecked") - static void rethrow(Throwable t) throws T { + private static void rethrow(Throwable t) throws T { throw (T) t; } diff --git a/core/src/main/java/org/elasticsearch/index/shard/IndexingOperationListener.java b/core/src/main/java/org/elasticsearch/index/shard/IndexingOperationListener.java index e0114a918ff..36f2765222a 100644 --- a/core/src/main/java/org/elasticsearch/index/shard/IndexingOperationListener.java +++ b/core/src/main/java/org/elasticsearch/index/shard/IndexingOperationListener.java @@ -38,17 +38,17 @@ public interface IndexingOperationListener { } /** - * Called after the indexing operation occurred. Implementations should - * check {@link Engine.IndexResult#hasFailure()} for operation failures - * and delegate to {@link #postIndex(Engine.Index, Exception)} with - * {@link Engine.IndexResult#getFailure()} if appropriate + * Called after the indexing operation occurred. Note that this is + * also called when indexing a document did not succeed due to document + * related failures. See {@link #postIndex(Engine.Index, Exception)} + * for engine level failures */ default void postIndex(Engine.Index index, Engine.IndexResult result) {} /** - * Called after the indexing operation occurred with exception that - * is not specific to the {@link Engine.Index} i.e. persistent engine - * failures etc. + * Called after the indexing operation occurred with engine level exception. + * See {@link #postIndex(Engine.Index, Engine.IndexResult)} for document + * related failures */ default void postIndex(Engine.Index index, Exception ex) {} @@ -61,17 +61,17 @@ public interface IndexingOperationListener { /** - * Called after the delete operation occurred. Implementations should - * check {@link Engine.DeleteResult#hasFailure()} for operation failures - * and delegate to {@link #postDelete(Engine.Delete, Exception)} with - * {@link Engine.DeleteResult#getFailure()} if appropriate + * Called after the delete operation occurred. Note that this is + * also called when deleting a document did not succeed due to document + * related failures. See {@link #postDelete(Engine.Delete, Exception)} + * for engine level failures */ default void postDelete(Engine.Delete delete, Engine.DeleteResult result) {} /** - * Called after the delete operation occurred with exception that - * is not specific to the {@link Engine.Delete} i.e. persistent engine - * failures etc. + * Called after the delete operation occurred with engine level exception. + * See {@link #postDelete(Engine.Delete, Engine.DeleteResult)} for document + * related failures */ default void postDelete(Engine.Delete delete, Exception ex) {}