documentation and minor fixes for engine level index/delete operations

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 location == null || operationFailure == null
-                assert operationFailure == null : "expected no failures when translog location is not 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 {

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<Boolean> 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);
         }
     }
 

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)
+            isDocumentFailure = maybeFailEngine(operation.operationType().getLowercase(), failure) == false;
-                    && maybeFailEngine(operation.operationType().getLowercase(), failure));
         } 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 <T extends Throwable> void rethrow(Throwable t) throws T {
+    private static <T extends Throwable> void rethrow(Throwable t) throws T {
         throw (T) t;
     }
 

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
+     * Called after the indexing operation occurred. Note that this is
-     * check {@link Engine.IndexResult#hasFailure()} for operation failures
+     * also called when indexing a document did not succeed due to document
-     * and delegate to {@link #postIndex(Engine.Index, Exception)} with
+     * related failures. See {@link #postIndex(Engine.Index, Exception)}
-     * {@link Engine.IndexResult#getFailure()} if appropriate
+     * for engine level failures
      */
     default void postIndex(Engine.Index index, Engine.IndexResult result) {}
 
     /**
-     * Called after the indexing operation occurred with exception that
+     * Called after the indexing operation occurred with engine level exception.
-     * is not specific to the {@link Engine.Index} i.e. persistent engine
+     * See {@link #postIndex(Engine.Index, Engine.IndexResult)} for document
-     * failures etc.
+     * 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
+     * Called after the delete operation occurred. Note that this is
-     * check {@link Engine.DeleteResult#hasFailure()} for operation failures
+     * also called when deleting a document did not succeed due to document
-     * and delegate to {@link #postDelete(Engine.Delete, Exception)} with
+     * related failures. See {@link #postDelete(Engine.Delete, Exception)}
-     * {@link Engine.DeleteResult#getFailure()} if appropriate
+     * for engine level failures
      */
     default void postDelete(Engine.Delete delete, Engine.DeleteResult result) {}
 
     /**
-     * Called after the delete operation occurred with exception that
+     * Called after the delete operation occurred with engine level exception.
-     * is not specific to the {@link Engine.Delete} i.e. persistent engine
+     * See {@link #postDelete(Engine.Delete, Engine.DeleteResult)} for document
-     * failures etc.
+     * related failures
      */
     default void postDelete(Engine.Delete delete, Exception ex) {}