Assert no exceptions during state application

Today we log and swallow exceptions during cluster state application, but such
an exception should not occur. This commit adds assertions of this fact, and
updates the Javadocs to explain it.

Relates elastic#47038

server/src/main/java/org/elasticsearch/cluster/ClusterStateApplier.java

@@ -28,7 +28,11 @@
 public interface ClusterStateApplier {
 
     /**
-     * Called when a new cluster state ({@link ClusterChangedEvent#state()} needs to be applied
+     * Called when a new cluster state ({@link ClusterChangedEvent#state()} needs to be applied. The cluster state to be applied is already
+     * committed when this method is called, so an applier must therefore be prepared to deal with any state it receives without throwing
+     * an exception. Throwing an exception from an applier is very bad because it will stop the application of this state before it has
+     * reached all the other appliers, and will likely result in another attempt to apply the same (or very similar) cluster state which
+     * might continue until this node is removed from the cluster.
      */
     void applyClusterState(ClusterChangedEvent event);
 }

server/src/main/java/org/elasticsearch/cluster/service/ClusterApplierService.java

@@ -390,7 +390,7 @@ private static boolean assertNotCalledFromClusterStateApplier(String reason) {
         return true;
     }
 
-    protected void runTask(UpdateTask task) {
+    private void runTask(UpdateTask task) {
         if (!lifecycle.started()) {
             logger.debug("processing [{}]: ignoring, cluster applier service not started", task.source);
             return;
@@ -447,6 +447,9 @@ protected void runTask(UpdateTask task) {
                             "failed to apply updated cluster state in [{}]:\nversion [{}], uuid [{}], source [{}]",
                             executionTime, newClusterState.version(), newClusterState.stateUUID(), task.source), e);
                 }
+                // failing to apply a cluster state with an exception indicates a bug in validation or in one of the appliers; if we
+                // continue we will retry with the same cluster state but that might not help.
+                assert false;
                 task.listener.onFailure(task.source, e);
             }
         }

server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java

@@ -193,7 +193,6 @@ public synchronized Settings applySettings(Settings newSettings) {
         } catch (Exception ex) {
             logger.warn("failed to apply settings", ex);
             throw ex;
-        } finally {
         }
         return lastSettingsApplied = newSettings;
     }

server/src/main/java/org/elasticsearch/indices/cluster/IndicesClusterStateService.java

@@ -586,23 +586,32 @@ private void updateIndices(ClusterChangedEvent event) {
             final IndexMetaData newIndexMetaData = state.metaData().index(index);
             assert newIndexMetaData != null : "index " + index + " should have been removed by deleteIndices";
             if (ClusterChangedEvent.indexMetaDataChanged(currentIndexMetaData, newIndexMetaData)) {
-                indexService.updateMetaData(currentIndexMetaData, newIndexMetaData);
+                String reason = null;
                 try {
+                    reason = "metadata update failed";
+                    try {
+                        indexService.updateMetaData(currentIndexMetaData, newIndexMetaData);
+                    } catch (Exception e) {
+                        assert false : e;
+                        throw e;
+                    }
+
+                    reason = "mapping update failed";
                     if (indexService.updateMapping(currentIndexMetaData, newIndexMetaData) && sendRefreshMapping) {
                         nodeMappingRefreshAction.nodeMappingRefresh(state.nodes().getMasterNode(),
                             new NodeMappingRefreshAction.NodeMappingRefreshRequest(newIndexMetaData.getIndex().getName(),
                                 newIndexMetaData.getIndexUUID(), state.nodes().getLocalNodeId())
                         );
                     }
                 } catch (Exception e) {
-                    indicesService.removeIndex(indexService.index(), FAILURE, "removing index (mapping update failed)");
+                    indicesService.removeIndex(indexService.index(), FAILURE, "removing index (" + reason + ")");
 
                     // fail shards that would be created or updated by createOrUpdateShards
                     RoutingNode localRoutingNode = state.getRoutingNodes().node(state.nodes().getLocalNodeId());
                     if (localRoutingNode != null) {
                         for (final ShardRouting shardRouting : localRoutingNode) {
                             if (shardRouting.index().equals(index) && failedShardsCache.containsKey(shardRouting.shardId()) == false) {
-                                sendFailShard(shardRouting, "failed to update mapping for index", e, state);
+                                sendFailShard(shardRouting, "failed to update index (" + reason + ")", e, state);
                             }
                         }
                     }