From 900b098e7535f3584eab4848ced670505899fbeb Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Fri, 8 Feb 2019 11:57:10 +0100 Subject: [PATCH 1/2] Add parameter to force the closing of an index --- .../rest-api-spec/api/indices.close.json | 4 ++ .../test/indices.open/10_basic.yml | 17 +++++ .../CloseIndexClusterStateUpdateRequest.java | 10 +++ .../indices/close/CloseIndexRequest.java | 23 +++++++ .../close/CloseIndexRequestBuilder.java | 5 ++ .../close/TransportCloseIndexAction.java | 2 +- .../metadata/MetaDataIndexStateService.java | 14 +++-- .../admin/indices/RestCloseIndexAction.java | 1 + .../MetaDataIndexStateServiceTests.java | 62 ++++++++++++++++--- .../MetaDataIndexStateServiceUtils.java | 11 ++-- .../indices/cluster/ClusterStateChanges.java | 3 +- .../indices/state/CloseIndexIT.java | 57 +++++++++++++++++ 12 files changed, 189 insertions(+), 20 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/indices.close.json b/rest-api-spec/src/main/resources/rest-api-spec/api/indices.close.json index 4eaa93030ee7b..ad1f072c33585 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/indices.close.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/indices.close.json @@ -34,6 +34,10 @@ "options" : ["open","closed","none","all"], "default" : "open", "description" : "Whether to expand wildcard expression to concrete indices that are open, closed or both." + }, + "force": { + "type" : "boolean", + "description" : "Whether closing the index should be forced" } } }, diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.open/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.open/10_basic.yml index 64e59d5939287..f0e10a5b4bf89 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.open/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.open/10_basic.yml @@ -58,3 +58,20 @@ - match: { acknowledged: true } - match: { shards_acknowledged: true } +--- +"Close index with force flag": + - do: + indices.create: + index: test_index + body: + settings: + number_of_replicas: 0 + + - do: + cluster.health: + wait_for_status: green + + - do: + indices.close: + index: test_index + force: true diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/close/CloseIndexClusterStateUpdateRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/close/CloseIndexClusterStateUpdateRequest.java index bb0f98ac07b7e..913e3617a57ca 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/close/CloseIndexClusterStateUpdateRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/close/CloseIndexClusterStateUpdateRequest.java @@ -26,12 +26,22 @@ public class CloseIndexClusterStateUpdateRequest extends IndicesClusterStateUpdateRequest { private final long taskId; + private final boolean force; public CloseIndexClusterStateUpdateRequest(final long taskId) { + this(taskId, false); + } + + public CloseIndexClusterStateUpdateRequest(final long taskId, final boolean force) { this.taskId = taskId; + this.force = force; } public long taskId() { return taskId; } + + public boolean force() { + return force; + } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/close/CloseIndexRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/close/CloseIndexRequest.java index 272bae9425712..d01016c04a088 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/close/CloseIndexRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/close/CloseIndexRequest.java @@ -19,6 +19,7 @@ package org.elasticsearch.action.admin.indices.close; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.IndicesRequest; import org.elasticsearch.action.support.IndicesOptions; @@ -38,6 +39,7 @@ public class CloseIndexRequest extends AcknowledgedRequest im private String[] indices; private IndicesOptions indicesOptions = IndicesOptions.strictExpandOpen(); + private boolean force = false; public CloseIndexRequest() { } @@ -101,11 +103,29 @@ public CloseIndexRequest indicesOptions(IndicesOptions indicesOptions) { return this; } + /** + * Force the closing of indices without executing the pre-close sanity checks + */ + public boolean force() { + return force; + } + + /** + * Force the closing of indices without executing the pre-close sanity checks + */ + public CloseIndexRequest force(final boolean force) { + this.force = force; + return this; + } + @Override public void readFrom(StreamInput in) throws IOException { super.readFrom(in); indices = in.readStringArray(); indicesOptions = IndicesOptions.readIndicesOptions(in); + if (in.getVersion().onOrAfter(Version.V_8_0_0)) { + force = in.readBoolean(); + } } @Override @@ -113,5 +133,8 @@ public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); out.writeStringArray(indices); indicesOptions.writeIndicesOptions(out); + if (out.getVersion().onOrAfter(Version.V_8_0_0)) { + out.writeBoolean(force); + } } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/close/CloseIndexRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/admin/indices/close/CloseIndexRequestBuilder.java index e69c6fed87dcd..de03e2f98815c 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/close/CloseIndexRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/close/CloseIndexRequestBuilder.java @@ -60,4 +60,9 @@ public CloseIndexRequestBuilder setIndicesOptions(IndicesOptions indicesOptions) request.indicesOptions(indicesOptions); return this; } + + public CloseIndexRequestBuilder setForce(final boolean force) { + request.force(force); + return this; + } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/close/TransportCloseIndexAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/close/TransportCloseIndexAction.java index bb3db084b0c53..514cd6fbaa8e0 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/close/TransportCloseIndexAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/close/TransportCloseIndexAction.java @@ -111,7 +111,7 @@ protected void masterOperation(final Task task, final CloseIndexRequest request, return; } - final CloseIndexClusterStateUpdateRequest closeRequest = new CloseIndexClusterStateUpdateRequest(task.getId()) + final CloseIndexClusterStateUpdateRequest closeRequest = new CloseIndexClusterStateUpdateRequest(task.getId(), request.force()) .ackTimeout(request.timeout()) .masterNodeTimeout(request.masterNodeTimeout()) .indices(concreteIndices); diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexStateService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexStateService.java index 0781cab1fe757..90c2218ee7f01 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexStateService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexStateService.java @@ -125,6 +125,7 @@ public void closeIndices(final CloseIndexClusterStateUpdateRequest request, fina throw new IllegalArgumentException("Index name is required"); } + final boolean forced = request.force(); clusterService.submitStateUpdateTask("add-block-index-to-close " + Arrays.toString(concreteIndices), new ClusterStateUpdateTask(Priority.URGENT) { @@ -132,7 +133,7 @@ public void closeIndices(final CloseIndexClusterStateUpdateRequest request, fina @Override public ClusterState execute(final ClusterState currentState) { - return addIndexClosedBlocks(concreteIndices, blockedIndices, currentState); + return addIndexClosedBlocks(concreteIndices, blockedIndices, currentState, forced); } @Override @@ -199,8 +200,10 @@ public TimeValue timeout() { * block (or reuses an existing one) to every index to close in the cluster state. After the cluster state is published, the shards * should start to reject writing operations and we can proceed with step 2. */ - static ClusterState addIndexClosedBlocks(final Index[] indices, final Map blockedIndices, - final ClusterState currentState) { + static ClusterState addIndexClosedBlocks(final Index[] indices, + final Map blockedIndices, + final ClusterState currentState, + final boolean forced) { final MetaData.Builder metadata = MetaData.builder(currentState.metaData()); final Set indicesToClose = new HashSet<>(); @@ -223,9 +226,9 @@ static ClusterState addIndexClosedBlocks(final Index[] indices, final Map client.admin().indices().close(closeIndexRequest, new RestToXContentListener<>(channel)); } diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataIndexStateServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataIndexStateServiceTests.java index 56ee25ee5febb..04615a6da3408 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataIndexStateServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataIndexStateServiceTests.java @@ -109,6 +109,7 @@ public void testCloseRoutingTable() { assertIsClosed(blockedIndex.getName(), updatedState); } else { assertIsOpened(blockedIndex.getName(), updatedState); + assertHasBlock(blockedIndex.getName(), updatedState, blockedIndices.get(blockedIndex)); assertThat(updatedState.blocks().hasIndexBlockWithId(blockedIndex.getName(), INDEX_CLOSED_BLOCK_ID), is(true)); } } @@ -120,14 +121,15 @@ public void testAddIndexClosedBlocks() { final Map blockedIndices = new HashMap<>(); Index[] indices = new Index[]{new Index("_name", "_uid")}; expectThrows(IndexNotFoundException.class, () -> - MetaDataIndexStateService.addIndexClosedBlocks(indices, blockedIndices, initialState)); + MetaDataIndexStateService.addIndexClosedBlocks(indices, blockedIndices, initialState, randomBoolean())); assertTrue(blockedIndices.isEmpty()); } { final Map blockedIndices = new HashMap<>(); Index[] indices = Index.EMPTY_ARRAY; - ClusterState updatedState = MetaDataIndexStateService.addIndexClosedBlocks(indices, blockedIndices, initialState); + ClusterState updatedState = + MetaDataIndexStateService.addIndexClosedBlocks(indices, blockedIndices, initialState, randomBoolean()); assertSame(initialState, updatedState); assertTrue(blockedIndices.isEmpty()); } @@ -136,7 +138,7 @@ public void testAddIndexClosedBlocks() { ClusterState state = addClosedIndex("closed", randomIntBetween(1, 3), randomIntBetween(0, 3), initialState); Index[] indices = new Index[]{state.metaData().index("closed").getIndex()}; - ClusterState updatedState = MetaDataIndexStateService.addIndexClosedBlocks(indices, blockedIndices, state); + ClusterState updatedState = MetaDataIndexStateService.addIndexClosedBlocks(indices, blockedIndices, state, randomBoolean()); assertSame(state, updatedState); assertTrue(blockedIndices.isEmpty()); @@ -147,7 +149,7 @@ public void testAddIndexClosedBlocks() { state = addOpenedIndex("opened", randomIntBetween(1, 3), randomIntBetween(0, 3), state); Index[] indices = new Index[]{state.metaData().index("opened").getIndex(), state.metaData().index("closed").getIndex()}; - ClusterState updatedState = MetaDataIndexStateService.addIndexClosedBlocks(indices, blockedIndices, state); + ClusterState updatedState = MetaDataIndexStateService.addIndexClosedBlocks(indices, blockedIndices, state, false); assertNotSame(state, updatedState); Index opened = updatedState.metaData().index("opened").getIndex(); @@ -167,7 +169,7 @@ public void testAddIndexClosedBlocks() { state = addOpenedIndex("closed", randomIntBetween(1, 3), randomIntBetween(0, 3), state); } Index[] indices = new Index[]{state.metaData().index("restored").getIndex()}; - MetaDataIndexStateService.addIndexClosedBlocks(indices, unmodifiableMap(emptyMap()), state); + MetaDataIndexStateService.addIndexClosedBlocks(indices, unmodifiableMap(emptyMap()), state, randomBoolean()); }); assertThat(exception.getMessage(), containsString("Cannot close indices that are being restored: [[restored]]")); } @@ -181,7 +183,7 @@ public void testAddIndexClosedBlocks() { state = addOpenedIndex("closed", randomIntBetween(1, 3), randomIntBetween(0, 3), state); } Index[] indices = new Index[]{state.metaData().index("snapshotted").getIndex()}; - MetaDataIndexStateService.addIndexClosedBlocks(indices, unmodifiableMap(emptyMap()), state); + MetaDataIndexStateService.addIndexClosedBlocks(indices, unmodifiableMap(emptyMap()), state, randomBoolean()); }); assertThat(exception.getMessage(), containsString("Cannot close indices that are being snapshotted: [[snapshotted]]")); } @@ -204,7 +206,7 @@ public void testAddIndexClosedBlocks() { Index index3 = state.metaData().index("index-3").getIndex(); Index[] indices = new Index[]{index1, index2, index3}; - ClusterState updatedState = MetaDataIndexStateService.addIndexClosedBlocks(indices, blockedIndices, state); + ClusterState updatedState = MetaDataIndexStateService.addIndexClosedBlocks(indices, blockedIndices, state, randomBoolean()); assertNotSame(state, updatedState); for (Index index : indices) { @@ -218,6 +220,41 @@ public void testAddIndexClosedBlocks() { } } + public void testAddIndexClosedBlocksWhenForced() { + final ClusterState initialState = ClusterState.builder(new ClusterName("testAddIndexClosedBlocksWhenForced")).build(); + + final int nbOpenedIndices = randomIntBetween(1, 5); + final Index[] indices = new Index[nbOpenedIndices + 1]; + + ClusterState state = initialState; + for (int i = 0; i < nbOpenedIndices; i++) { + final String indexName = "opened-" + i; + state = addOpenedIndex(indexName, randomIntBetween(1, 3), randomIntBetween(0, 3), state); + indices[i] = state.metaData().index(indexName).getIndex(); + } + + state = addClosedIndex("closed", randomIntBetween(1, 3), randomIntBetween(0, 3), state); + indices[nbOpenedIndices] = state.metaData().index("closed").getIndex(); + state = addOpenedIndex("other", randomIntBetween(1, 3), randomIntBetween(0, 3), state); + + final Map blockedIndices = new HashMap<>(); + final ClusterState updatedState = MetaDataIndexStateService.addIndexClosedBlocks(indices, blockedIndices, state, true); + assertNotSame(state, updatedState); + + assertFalse(blockedIndices.containsKey(updatedState.metaData().index("closed").getIndex())); + assertIsClosed("closed", updatedState); + + assertFalse(blockedIndices.containsKey(updatedState.metaData().index("other").getIndex())); + assertIsOpened("other", updatedState); + assertNoBlock("other", updatedState); + + for (int i = 0; i < nbOpenedIndices; i++) { + final String indexName = "opened-" + i; + assertTrue(blockedIndices.containsKey(updatedState.metaData().index(indexName).getIndex())); + assertIsClosed(indexName, updatedState); // forced close directly closes indices + } + } + public void testAddIndexClosedBlocksReusesBlocks() { ClusterState state = ClusterState.builder(new ClusterName("testAddIndexClosedBlocksReuseBlocks")).build(); state = addOpenedIndex("test", randomIntBetween(1, 3), randomIntBetween(0, 3), state); @@ -226,13 +263,13 @@ public void testAddIndexClosedBlocksReusesBlocks() { Index[] indices = new Index[]{test}; final Map blockedIndices = new HashMap<>(); - state = MetaDataIndexStateService.addIndexClosedBlocks(indices, blockedIndices, state); + state = MetaDataIndexStateService.addIndexClosedBlocks(indices, blockedIndices, state, false); assertTrue(blockedIndices.containsKey(test)); assertHasBlock(test.getName(), state, blockedIndices.get(test)); final Map blockedIndices2 = new HashMap<>(); - state = MetaDataIndexStateService.addIndexClosedBlocks(indices, blockedIndices2, state); + state = MetaDataIndexStateService.addIndexClosedBlocks(indices, blockedIndices2, state, false); assertTrue(blockedIndices2.containsKey(test)); assertHasBlock(test.getName(), state, blockedIndices2.get(test)); @@ -385,4 +422,11 @@ private static void assertHasBlock(final String indexName, final ClusterState cl clusterState.blocks().indices().getOrDefault(indexName, emptySet()).stream() .filter(clusterBlock -> clusterBlock.id() == MetaDataIndexStateService.INDEX_CLOSED_BLOCK_ID).count(), equalTo(1L)); } + + private static void assertNoBlock(final String indexName, final ClusterState clusterState) { + assertThat(clusterState.blocks().hasIndexBlock(indexName, INDEX_CLOSED_BLOCK), is(false)); + assertThat("Index " + indexName + " must not have a block with [id=" + MetaDataIndexStateService.INDEX_CLOSED_BLOCK_ID + "]", + clusterState.blocks().indices().getOrDefault(indexName, emptySet()).stream() + .filter(clusterBlock -> clusterBlock.id() == MetaDataIndexStateService.INDEX_CLOSED_BLOCK_ID).count(), equalTo(0L)); + } } diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataIndexStateServiceUtils.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataIndexStateServiceUtils.java index 5ee6a7c60da3d..1d622c47f55c4 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataIndexStateServiceUtils.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataIndexStateServiceUtils.java @@ -31,11 +31,14 @@ private MetaDataIndexStateServiceUtils(){ } /** - * Allows to call {@link MetaDataIndexStateService#addIndexClosedBlocks(Index[], Map, ClusterState)} which is a protected method. + * Allows to call {@link MetaDataIndexStateService#addIndexClosedBlocks(Index[], Map, ClusterState, boolean)} + * which is a protected method. */ - public static ClusterState addIndexClosedBlocks(final Index[] indices, final Map blockedIndices, - final ClusterState state) { - return MetaDataIndexStateService.addIndexClosedBlocks(indices, blockedIndices, state); + public static ClusterState addIndexClosedBlocks(final Index[] indices, + final Map blockedIndices, + final ClusterState state, + final boolean forced) { + return MetaDataIndexStateService.addIndexClosedBlocks(indices, blockedIndices, state, forced); } /** diff --git a/server/src/test/java/org/elasticsearch/indices/cluster/ClusterStateChanges.java b/server/src/test/java/org/elasticsearch/indices/cluster/ClusterStateChanges.java index c1e32be9d29af..d9ca7cabab1f3 100644 --- a/server/src/test/java/org/elasticsearch/indices/cluster/ClusterStateChanges.java +++ b/server/src/test/java/org/elasticsearch/indices/cluster/ClusterStateChanges.java @@ -103,6 +103,7 @@ import java.util.stream.Collectors; import static com.carrotsearch.randomizedtesting.RandomizedTest.getRandom; +import static com.carrotsearch.randomizedtesting.RandomizedTest.rarely; import static org.elasticsearch.env.Environment.PATH_HOME_SETTING; import static org.hamcrest.Matchers.notNullValue; import static org.junit.Assert.assertThat; @@ -225,7 +226,7 @@ public ClusterState closeIndices(ClusterState state, CloseIndexRequest request) .map(index -> state.metaData().index(index).getIndex()).toArray(Index[]::new); final Map blockedIndices = new HashMap<>(); - ClusterState newState = MetaDataIndexStateServiceUtils.addIndexClosedBlocks(concreteIndices, blockedIndices, state); + ClusterState newState = MetaDataIndexStateServiceUtils.addIndexClosedBlocks(concreteIndices, blockedIndices, state, rarely()); newState = MetaDataIndexStateServiceUtils.closeRoutingTable(newState, blockedIndices, blockedIndices.keySet().stream() .collect(Collectors.toMap(Function.identity(), index -> new AcknowledgedResponse(true)))); diff --git a/server/src/test/java/org/elasticsearch/indices/state/CloseIndexIT.java b/server/src/test/java/org/elasticsearch/indices/state/CloseIndexIT.java index 1d32283c6cb94..a510446885436 100644 --- a/server/src/test/java/org/elasticsearch/indices/state/CloseIndexIT.java +++ b/server/src/test/java/org/elasticsearch/indices/state/CloseIndexIT.java @@ -18,20 +18,30 @@ */ package org.elasticsearch.indices.state; +import org.apache.lucene.store.Directory; +import org.apache.lucene.store.FSDirectory; import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.action.ActionRequestValidationException; +import org.elasticsearch.action.admin.cluster.node.stats.NodesStatsResponse; +import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.support.ActiveShardCount; +import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.block.ClusterBlockException; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.MetaDataIndexStateService; import org.elasticsearch.cluster.routing.ShardRouting; +import org.elasticsearch.common.io.PathUtils; +import org.elasticsearch.common.lucene.Lucene; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.indices.IndexClosedException; +import org.elasticsearch.monitor.fs.FsInfo; import org.elasticsearch.test.BackgroundIndexer; import org.elasticsearch.test.ESIntegTestCase; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.ArrayList; import java.util.List; import java.util.Locale; @@ -306,6 +316,53 @@ public void testConcurrentClosesAndOpens() throws Exception { indexer.totalIndexedDocs()); } + public void testForcedCloseCorruptedIndex() throws Exception { + final String indexName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT); + assertAcked(prepareCreate(indexName) + .setSettings(Settings.builder() + .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0) + .build())); + + final int nbDocs = scaledRandomIntBetween(100, 1000); + final IndexRequestBuilder[] builders = new IndexRequestBuilder[nbDocs]; + for (int i = 0; i < builders.length; i++) { + builders[i] = client().prepareIndex(indexName, "_doc").setSource("field", "value"); + } + indexRandom(randomBoolean(), builders); + ensureGreen(); + + final ClusterState clusterState = client().admin().cluster().prepareState().setIndices(indexName).get().getState(); + final ShardRouting primaryShard = clusterState.routingTable().index(indexName).shard(0).primaryShard(); + assertTrue(primaryShard.active()); + + final NodesStatsResponse nodeStats = client().admin().cluster().prepareNodesStats(primaryShard.currentNodeId()).setFs(true).get(); + final FsInfo fsInfo = nodeStats.getNodes().get(0).getFs(); + + Path segmentInfos = null; + for (FsInfo.Path path : fsInfo) { + final Path file = PathUtils.get(path.getPath()) + .resolve("indices") + .resolve(primaryShard.index().getUUID()) + .resolve(Integer.toString(primaryShard.getId())) + .resolve("index"); + if (Files.exists(file)) { + try (Directory dir = FSDirectory.open(file)) { + segmentInfos = file.resolve(Lucene.readSegmentInfos(dir).getSegmentsFileName()); + } + } + } + + assertNotNull(segmentInfos); + Files.delete(segmentInfos); + + AcknowledgedResponse closeIndexResponse = client().admin().indices().prepareClose(indexName).get(); + assertFalse(closeIndexResponse.isAcknowledged()); + + closeIndexResponse = client().admin().indices().prepareClose(indexName).setForce(true).get(); + assertTrue(closeIndexResponse.isAcknowledged()); + assertIndexIsClosed(indexName); + } + static void assertIndexIsClosed(final String... indices) { final ClusterState clusterState = client().admin().cluster().prepareState().get().getState(); for (String index : indices) { From b9e2836ece28da79af5c4206672dd408a4b84bcc Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Fri, 8 Feb 2019 12:37:26 +0100 Subject: [PATCH 2/2] Add deprecation warnings --- .../test/indices.open/10_basic.yml | 8 ++++ .../admin/indices/RestCloseIndexAction.java | 21 ++++++++- .../indices/RestCloseIndexActionTests.java | 46 +++++++++++++++++++ 3 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestCloseIndexActionTests.java diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.open/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.open/10_basic.yml index f0e10a5b4bf89..18af8b1ae5b87 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.open/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.open/10_basic.yml @@ -58,8 +58,14 @@ - match: { acknowledged: true } - match: { shards_acknowledged: true } + --- "Close index with force flag": + - skip: + version: " - 7.99.99" + reason: force parameter was added in 8.0.0 + features: "warnings" + - do: indices.create: index: test_index @@ -75,3 +81,5 @@ indices.close: index: test_index force: true + warnings: + - "parameter [force] is deprecated but was [true]" diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestCloseIndexAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestCloseIndexAction.java index d5dec51957813..871791c0f95ac 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestCloseIndexAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestCloseIndexAction.java @@ -19,10 +19,13 @@ package org.elasticsearch.rest.action.admin.indices; +import org.apache.logging.log4j.LogManager; import org.elasticsearch.action.admin.indices.close.CloseIndexRequest; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.common.Booleans; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestController; @@ -32,6 +35,9 @@ import java.io.IOException; public class RestCloseIndexAction extends BaseRestHandler { + + private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(RestCloseIndexAction.class)); + public RestCloseIndexAction(Settings settings, RestController controller) { super(settings); controller.registerHandler(RestRequest.Method.POST, "/_close", this); @@ -49,7 +55,20 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC closeIndexRequest.masterNodeTimeout(request.paramAsTime("master_timeout", closeIndexRequest.masterNodeTimeout())); closeIndexRequest.timeout(request.paramAsTime("timeout", closeIndexRequest.timeout())); closeIndexRequest.indicesOptions(IndicesOptions.fromRequest(request, closeIndexRequest.indicesOptions())); - closeIndexRequest.force(request.paramAsBoolean("force", closeIndexRequest.force())); + + final String forceParameter = request.param("force"); + final boolean forced; + if (forceParameter == null) { + forced = closeIndexRequest.force(); + } else { + deprecationLogger.deprecated("parameter [force] is deprecated but was [" + forceParameter + "]"); + if (forceParameter.length() == 0) { + forced = true; + } else { + forced = Booleans.parseBoolean(forceParameter); + } + } + closeIndexRequest.force(forced); return channel -> client.admin().indices().close(closeIndexRequest, new RestToXContentListener<>(channel)); } diff --git a/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestCloseIndexActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestCloseIndexActionTests.java new file mode 100644 index 0000000000000..725449e30aded --- /dev/null +++ b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestCloseIndexActionTests.java @@ -0,0 +1,46 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.rest.action.admin.indices; + +import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.rest.RestController; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.rest.FakeRestRequest; + +import java.util.Collections; + +import static org.mockito.Mockito.mock; + +public class RestCloseIndexActionTests extends ESTestCase { + + public void testForceParameterDeprecationWarnings() throws Exception { + final String forceParameter = randomFrom("true", "false"); + final FakeRestRequest request = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY) + .withParams(Collections.singletonMap("force", forceParameter)) + .withPath("index/_close") + .build(); + + final RestCloseIndexAction handler = new RestCloseIndexAction(Settings.EMPTY, mock(RestController.class)); + handler.prepareRequest(request, mock(NodeClient.class)); + assertWarnings("parameter [force] is deprecated but was [" + forceParameter + "]"); + } +}