Fail invalid incremental cluster state writes (#61030)

It is disastrous if we commit an incremental cluster state update without having written the full state first. We assert that this doesn't happen, but it is hard to fully test the myriad ways that things might fail in a messy production environment. Given the disastrous consequences it is worth erring on the side of caution in this area. This commit fails invalid writes even if assertions are disabled.
elastic · Aug 12, 2020 · 0ba66dc · 0ba66dc
1 parent ec1c2bd
commit 0ba66dc
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 3 deletions.
diff --git a/server/src/main/java/org/elasticsearch/gateway/GatewayMetaState.java b/server/src/main/java/org/elasticsearch/gateway/GatewayMetaState.java
@@ -455,7 +455,7 @@ public void setCurrentTerm(long currentTerm) {
                     getWriterSafe().writeFullStateAndCommit(currentTerm, lastAcceptedState);
                     writeNextStateFully = false;
                 } else {
-                    getWriterSafe().commit(currentTerm, lastAcceptedState.version());
+                    getWriterSafe().writeIncrementalTermUpdateAndCommit(currentTerm, lastAcceptedState.version());
                 }
             } catch (Exception e) {
                 handleExceptionOnWrite(e);

diff --git a/server/src/main/java/org/elasticsearch/gateway/PersistedClusterStateService.java b/server/src/main/java/org/elasticsearch/gateway/PersistedClusterStateService.java
@@ -609,7 +609,8 @@ public void writeFullStateAndCommit(long currentTerm, ClusterState clusterState)
         void writeIncrementalStateAndCommit(long currentTerm, ClusterState previousClusterState,
                                             ClusterState clusterState) throws IOException {
             ensureOpen();
-            assert fullStateWritten : "Need to write full state first before doing incremental writes";
+            ensureFullStateWritten();
+
             try {
                 final long startTimeMillis = relativeTimeMillisSupplier.getAsLong();
                 final WriterStats stats = updateMetadata(previousClusterState.metadata(), clusterState.metadata());
@@ -631,6 +632,15 @@ void writeIncrementalStateAndCommit(long currentTerm, ClusterState previousClust
             }
         }
 
+        private void ensureFullStateWritten() {
+            assert fullStateWritten : "Need to write full state first before doing incremental writes";
+            //noinspection ConstantConditions to catch this even if assertions are disabled
+            if (fullStateWritten == false) {
+                logger.error("cannot write incremental state");
+                throw new IllegalStateException("cannot write incremental state");
+            }
+        }
+
         /**
          * Update the persisted metadata to match the given cluster state by removing any stale or unnecessary documents and adding any
          * updated documents.
@@ -730,7 +740,13 @@ private WriterStats addMetadata(Metadata metadata) throws IOException {
             return new WriterStats(true, metadata.indices().size(), 0);
         }
 
-        public void commit(long currentTerm, long lastAcceptedVersion) throws IOException {
+        public void writeIncrementalTermUpdateAndCommit(long currentTerm, long lastAcceptedVersion) throws IOException {
+            ensureOpen();
+            ensureFullStateWritten();
+            commit(currentTerm, lastAcceptedVersion);
+        }
+
+        void commit(long currentTerm, long lastAcceptedVersion) throws IOException {
             ensureOpen();
             try {
                 for (MetadataIndexWriter metadataIndexWriter : metadataIndexWriters) {