Do not remove write block when unfreezing cold/frozen indices (#73368) (

#73409)

Unfreezing a snapshot backed index does not work well 
because the unfreeze action always removes the 
index.blocks.write block, causing shards to be failed 
when the cluster state is applied on data nodes. This 
is because searchable snapshots shards always 
expect the index.blocks.write to be set to true.

This commit changes the freeze/unfreeze action 
so that the write block is not removed when 
unfreezing searchable snapshots indices. It also 
changes the toggling of index.frozen and 
index.search.throttled settings so that they are 
just removed (instead of being turned to false) 
when unfreezing.

x-pack/plugin/frozen-indices/src/internalClusterTest/java/org/elasticsearch/index/engine/FrozenIndexTests.java

@@ -219,6 +219,8 @@ public void testFreezeAndUnfreeze() throws ExecutionException, InterruptedExcept
             Index index = resolveIndex("index");
             IndexService indexService = indexServices.indexServiceSafe(index);
             assertTrue(indexService.getIndexSettings().isSearchThrottled());
+            assertTrue(FrozenEngine.INDEX_FROZEN.get(indexService.getIndexSettings().getSettings()));
+            assertTrue(FrozenEngine.INDEX_FROZEN.exists(indexService.getIndexSettings().getSettings()));
             IndexShard shard = indexService.getShard(0);
             assertEquals(0, shard.refreshStats().getTotal());
             assertThat(indexService.getMetadata().getTimestampRange(), sameInstance(IndexLongFieldRange.UNKNOWN));
@@ -229,6 +231,8 @@ public void testFreezeAndUnfreeze() throws ExecutionException, InterruptedExcept
             Index index = resolveIndex("index");
             IndexService indexService = indexServices.indexServiceSafe(index);
             assertFalse(indexService.getIndexSettings().isSearchThrottled());
+            assertFalse(FrozenEngine.INDEX_FROZEN.get(indexService.getIndexSettings().getSettings()));
+            assertFalse(FrozenEngine.INDEX_FROZEN.exists(indexService.getIndexSettings().getSettings()));
             IndexShard shard = indexService.getShard(0);
             Engine engine = IndexShardTestCase.getEngine(shard);
             assertThat(engine, Matchers.instanceOf(InternalEngine.class));

x-pack/plugin/frozen-indices/src/main/java/org/elasticsearch/xpack/frozen/action/TransportFreezeIndexAction.java

@@ -41,6 +41,7 @@
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.TransportService;
 import org.elasticsearch.xpack.core.frozen.action.FreezeIndexAction;
+import org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -159,26 +160,28 @@ public ClusterState execute(ClusterState currentState) {
                 final Metadata.Builder builder = Metadata.builder(currentState.metadata());
                 ClusterBlocks.Builder blocks = ClusterBlocks.builder().blocks(currentState.blocks());
                 for (Index index : concreteIndices) {
-                    IndexMetadata meta = currentState.metadata().getIndexSafe(index);
-                    if (meta.getState() != IndexMetadata.State.CLOSE) {
+                    final IndexMetadata indexMetadata = currentState.metadata().getIndexSafe(index);
+                    if (indexMetadata.getState() != IndexMetadata.State.CLOSE) {
                         throw new IllegalStateException("index [" + index.getName() + "] is not closed");
                     }
-                    final IndexMetadata.Builder imdBuilder = IndexMetadata.builder(meta);
-                    imdBuilder.settingsVersion(meta.getSettingsVersion() + 1);
-                    final Settings.Builder settingsBuilder =
-                        Settings.builder()
-                            .put(currentState.metadata().index(index).getSettings())
-                            .put(FrozenEngine.INDEX_FROZEN.getKey(), request.freeze())
-                            .put(IndexSettings.INDEX_SEARCH_THROTTLED.getKey(), request.freeze());
+                    final Settings.Builder settingsBuilder = Settings.builder().put(indexMetadata.getSettings());
                     if (request.freeze()) {
+                        settingsBuilder.put(FrozenEngine.INDEX_FROZEN.getKey(), true);
+                        settingsBuilder.put(IndexSettings.INDEX_SEARCH_THROTTLED.getKey(), true);
                         settingsBuilder.put("index.blocks.write", true);
                         blocks.addIndexBlock(index.getName(), IndexMetadata.INDEX_WRITE_BLOCK);
                     } else {
-                        settingsBuilder.remove("index.blocks.write");
-                        blocks.removeIndexBlock(index.getName(), IndexMetadata.INDEX_WRITE_BLOCK);
+                        settingsBuilder.remove(FrozenEngine.INDEX_FROZEN.getKey());
+                        settingsBuilder.remove(IndexSettings.INDEX_SEARCH_THROTTLED.getKey());
+                        if (SearchableSnapshotsConstants.isSearchableSnapshotStore(indexMetadata.getSettings()) == false) {
+                            settingsBuilder.remove("index.blocks.write");
+                            blocks.removeIndexBlock(index.getName(), IndexMetadata.INDEX_WRITE_BLOCK);
+                        }
                     }
-                    imdBuilder.settings(settingsBuilder);
-                    builder.put(imdBuilder.build(), true);
+                    builder.put(IndexMetadata.builder(indexMetadata)
+                        .settingsVersion(indexMetadata.getSettingsVersion() + 1)
+                        .settings(settingsBuilder)
+                        .build(), true);
                 }
                 return ClusterState.builder(currentState).blocks(blocks).metadata(builder).build();
             }

x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/AbstractSearchableSnapshotsRestTestCase.java

@@ -41,6 +41,7 @@
 import static org.hamcrest.Matchers.greaterThanOrEqualTo;
 import static org.hamcrest.Matchers.lessThanOrEqualTo;
 import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.nullValue;
 
 public abstract class AbstractSearchableSnapshotsRestTestCase extends ESRestTestCase {
 
@@ -174,9 +175,20 @@ public void testSearchResultsWhenFrozen() throws Exception {
             final Request freezeRequest = new Request(HttpPost.METHOD_NAME, restoredIndexName + "/_freeze");
             assertOK(client().performRequest(freezeRequest));
             ensureGreen(restoredIndexName);
-            for (int i = 0; i < 10; i++) {
-                assertSearchResults(restoredIndexName, numDocs, Boolean.FALSE);
-            }
+            assertSearchResults(restoredIndexName, numDocs, Boolean.FALSE);
+            final Map<String, Object> frozenIndexSettings = indexSettings(restoredIndexName);
+            assertThat(Boolean.valueOf(extractValue(frozenIndexSettings, "index.frozen")), equalTo(true));
+            assertThat(Boolean.valueOf(extractValue(frozenIndexSettings, "index.search.throttled")), equalTo(true));
+            assertThat(Boolean.valueOf(extractValue(frozenIndexSettings, "index.blocks.write")), equalTo(true));
+
+            final Request unfreezeRequest = new Request(HttpPost.METHOD_NAME, restoredIndexName + "/_unfreeze");
+            assertOK(client().performRequest(unfreezeRequest));
+            ensureGreen(restoredIndexName);
+            assertSearchResults(restoredIndexName, numDocs, Boolean.FALSE);
+            final Map<String, Object> unfrozenIndexSettings = indexSettings(restoredIndexName);
+            assertThat(extractValue(unfrozenIndexSettings, "index.frozen"), nullValue());
+            assertThat(extractValue(unfrozenIndexSettings, "index.search.throttled"), nullValue());
+            assertThat(Boolean.valueOf(extractValue(frozenIndexSettings, "index.blocks.write")), equalTo(true));
         });
     }