From e2e00cd2450058384ce4bbdf16b130824a75f81a Mon Sep 17 00:00:00 2001
From: Andrey Ershov <andrershov@gmail.com>
Date: Wed, 23 Jan 2019 07:21:26 +0100
Subject: [PATCH] Fix MetaStateFormat tests
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

It's not safe to continue writing state using MetaDataStateFormat
after dirty WriteStateException occurred if it's not recovered by
successful subsequent state write.

We've encountered test failure of testFailRandomlyAndReadAnyState.
The test breaks in the following way. There are 3 state paths. And what
happens next

Successful write at the beginning of the test yields 0 0 0 state
files in the directories.
1st write in the loop is unsuccessful, but not dirty - 0 0 0.
2nd write in the loop is not successful and dirty (failure during
fsync), however before removing new files we have 1 1 1. But now during
deletion, the first deletion fails and we get - 1 0 0.
3rd write in the loop is unsuccessful, but not dirty - so we want to
keep old generation, which happens to be the 1st generation, so now we
have 1 x x in state folders. Now we assert that we either load 0 or 1
state from the state folders and select only 2rd and 3th folder to
emulate disk failures - this results in NPE because there is nothing in
these folders.
Fortunately, this won’t be a problem in real life, because if there is
a dirty exception, we shut down the node and make sure we perform a
successful write on the node startup.
---
 .../org/elasticsearch/gateway/GatewayMetaStateTests.java    | 6 ++++++
 .../org/elasticsearch/gateway/MetaDataStateFormatTests.java | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/server/src/test/java/org/elasticsearch/gateway/GatewayMetaStateTests.java b/server/src/test/java/org/elasticsearch/gateway/GatewayMetaStateTests.java
index 7a6b751de74..22259b919ec 100644
--- a/server/src/test/java/org/elasticsearch/gateway/GatewayMetaStateTests.java
+++ b/server/src/test/java/org/elasticsearch/gateway/GatewayMetaStateTests.java
@@ -412,6 +412,12 @@ public class GatewayMetaStateTests extends ESAllocationTestCase {
                 } catch (WriteStateException e) {
                     if (e.isDirty()) {
                         possibleMetaData.add(metaData);
+                        /*
+                         * If dirty WriteStateException occurred, it's only safe to proceed if there is subsequent
+                         * successful write of metadata and Manifest. We prefer to break here, not to over complicate test logic.
+                         * See also MetaDataStateFormat#testFailRandomlyAndReadAnyState, that does not break.
+                         */
+                        break;
                     }
                 }
             }
diff --git a/server/src/test/java/org/elasticsearch/gateway/MetaDataStateFormatTests.java b/server/src/test/java/org/elasticsearch/gateway/MetaDataStateFormatTests.java
index 2522452f727..40f3bd8a016 100644
--- a/server/src/test/java/org/elasticsearch/gateway/MetaDataStateFormatTests.java
+++ b/server/src/test/java/org/elasticsearch/gateway/MetaDataStateFormatTests.java
@@ -408,6 +408,12 @@ public class MetaDataStateFormatTests extends ESTestCase {
             Path[] randomPaths = randomSubsetOf(randomIntBetween(1, paths.length), paths).toArray(new Path[0]);
             DummyState stateOnDisk = format.loadLatestState(logger, NamedXContentRegistry.EMPTY, randomPaths);
             assertTrue(possibleStates.contains(stateOnDisk));
+            if (possibleStates.size() > 1) {
+                //if there was a WriteStateException we need to override current state before we continue
+                newState = writeAndReadStateSuccessfully(format, paths);
+                possibleStates.clear();
+                possibleStates.add(newState);
+            }
         }
 
         writeAndReadStateSuccessfully(format, paths);