From 77adad6ebafeee10df4343dcda587001c25cdddb Mon Sep 17 00:00:00 2001 From: William Brafford Date: Fri, 29 Jan 2021 17:14:07 -0500 Subject: [PATCH 01/25] Stub out classes for reset feature states API --- .../elasticsearch/action/ActionModule.java | 3 + .../features/ResetFeatureStateAction.java | 34 ++++++++++ .../features/ResetFeatureStateRequest.java | 48 +++++++++++++ .../features/ResetFeatureStateResponse.java | 51 ++++++++++++++ .../TransportResetFeatureStateAction.java | 68 +++++++++++++++++++ .../cluster/RestResetFeatureStateAction.java | 46 +++++++++++++ 6 files changed, 250 insertions(+) create mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateAction.java create mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateRequest.java create mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java create mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java create mode 100644 server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java diff --git a/server/src/main/java/org/elasticsearch/action/ActionModule.java b/server/src/main/java/org/elasticsearch/action/ActionModule.java index d4ff96be8db45..73bb826766bb5 100644 --- a/server/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/server/src/main/java/org/elasticsearch/action/ActionModule.java @@ -58,7 +58,9 @@ import org.elasticsearch.action.admin.cluster.snapshots.create.TransportCreateSnapshotAction; import org.elasticsearch.action.admin.cluster.snapshots.delete.DeleteSnapshotAction; import org.elasticsearch.action.admin.cluster.snapshots.delete.TransportDeleteSnapshotAction; +import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateAction; import org.elasticsearch.action.admin.cluster.snapshots.features.SnapshottableFeaturesAction; +import org.elasticsearch.action.admin.cluster.snapshots.features.TransportResetFeatureStateAction; import org.elasticsearch.action.admin.cluster.snapshots.features.TransportSnapshottableFeaturesAction; import org.elasticsearch.action.admin.cluster.snapshots.get.GetSnapshotsAction; import org.elasticsearch.action.admin.cluster.snapshots.get.TransportGetSnapshotsAction; @@ -501,6 +503,7 @@ public void reg actions.register(RestoreSnapshotAction.INSTANCE, TransportRestoreSnapshotAction.class); actions.register(SnapshotsStatusAction.INSTANCE, TransportSnapshotsStatusAction.class); actions.register(SnapshottableFeaturesAction.INSTANCE, TransportSnapshottableFeaturesAction.class); + actions.register(ResetFeatureStateAction.INSTANCE, TransportResetFeatureStateAction.class); actions.register(IndicesStatsAction.INSTANCE, TransportIndicesStatsAction.class); actions.register(IndicesSegmentsAction.INSTANCE, TransportIndicesSegmentsAction.class); diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateAction.java new file mode 100644 index 0000000000000..cbeda5df7105d --- /dev/null +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateAction.java @@ -0,0 +1,34 @@ +/* + * 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.action.admin.cluster.snapshots.features; + +import org.elasticsearch.action.ActionType; + +public class ResetFeatureStateAction extends ActionType { + + public static final ResetFeatureStateAction INSTANCE = new ResetFeatureStateAction(); + // TODO[wrb]: seems like a bad action name... + // are features getting bigger than snapshots as an abstraction? + public static final String NAME = "cluster:admin/snapshot/features/reset"; + + private ResetFeatureStateAction() { + super(NAME, ResetFeatureStateResponse::new); + } +} diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateRequest.java new file mode 100644 index 0000000000000..d8c58f832f0ce --- /dev/null +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateRequest.java @@ -0,0 +1,48 @@ +/* + * 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.action.admin.cluster.snapshots.features; + +import org.elasticsearch.action.ActionRequestValidationException; +import org.elasticsearch.action.support.master.MasterNodeRequest; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; + +import java.io.IOException; + +public class ResetFeatureStateRequest extends MasterNodeRequest { + + public ResetFeatureStateRequest() { + // TODO[wrb] + } + + public ResetFeatureStateRequest(StreamInput in) throws IOException { + super(in); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); + } + + @Override + public ActionRequestValidationException validate() { + return null; + } +} diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java new file mode 100644 index 0000000000000..cdb80a7383ec6 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java @@ -0,0 +1,51 @@ +/* + * 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.action.admin.cluster.snapshots.features; + +import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.xcontent.ToXContentObject; +import org.elasticsearch.common.xcontent.XContentBuilder; + +import java.io.IOException; + +public class ResetFeatureStateResponse extends ActionResponse implements ToXContentObject { + + public ResetFeatureStateResponse() { + // TODO[wrb] + } + + public ResetFeatureStateResponse(StreamInput in) throws IOException { + super(in); + // TODO[wrb] + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + // TODO[wrb] + return null; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + // TODO[wrb] + } +} diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java new file mode 100644 index 0000000000000..643df87940727 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java @@ -0,0 +1,68 @@ +/* + * 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.action.admin.cluster.snapshots.features; + +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.action.support.master.TransportMasterNodeAction; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.block.ClusterBlockException; +import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.tasks.Task; +import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.TransportService; + +public class TransportResetFeatureStateAction extends TransportMasterNodeAction { + + @Inject + protected TransportResetFeatureStateAction( + String actionName, + TransportService transportService, + ClusterService clusterService, + ThreadPool threadPool, + ActionFilters actionFilters, + Writeable.Reader request, + IndexNameExpressionResolver indexNameExpressionResolver, + Writeable.Reader response, + String executor + ) { + super(actionName, transportService, clusterService, threadPool, actionFilters, + request, indexNameExpressionResolver, response, executor); + } + + @Override + protected void masterOperation( + Task task, + ResetFeatureStateRequest request, + ClusterState state, + ActionListener listener + ) throws Exception { + // TODO[wrb] + } + + @Override + protected ClusterBlockException checkBlock(ResetFeatureStateRequest request, ClusterState state) { + // TODO[wrb] + return null; + } +} diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java new file mode 100644 index 0000000000000..7e089a0f51508 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.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.cluster; + +import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.rest.BaseRestHandler; +import org.elasticsearch.rest.RestRequest; + +import java.io.IOException; +import java.util.List; + +public class RestResetFeatureStateAction extends BaseRestHandler { + + @Override + public List routes() { + return List.of(new Route(RestRequest.Method.POST, "/_reset_feature_state")); + } + + @Override + public String getName() { + return "reset_feature_state"; + } + + @Override + protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { + // TODO[wrb] + return null; + } +} From 0a55d99382e01c95b530120ac3e4188dd61a9a68 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Tue, 2 Feb 2021 16:24:48 -0500 Subject: [PATCH 02/25] Add enough code to get an integration test working --- .../snapshots/SystemIndexResetApiIT.java | 99 +++++++++++++++++++ .../elasticsearch/action/ActionModule.java | 2 + .../TransportResetFeatureStateAction.java | 77 ++++++++++++--- .../plugins/SystemIndexPlugin.java | 6 ++ .../cluster/RestResetFeatureStateAction.java | 11 ++- 5 files changed, 182 insertions(+), 13 deletions(-) create mode 100644 server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java new file mode 100644 index 0000000000000..b9400994888b5 --- /dev/null +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java @@ -0,0 +1,99 @@ +/* + * 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.snapshots; + +import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateAction; +import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateRequest; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.index.IndexNotFoundException; +import org.elasticsearch.indices.SystemIndexDescriptor; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.plugins.SystemIndexPlugin; +import org.elasticsearch.test.ESIntegTestCase; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; + +import static org.hamcrest.Matchers.containsString; + +public class SystemIndexResetApiIT extends ESIntegTestCase { + + @Override + protected Collection> nodePlugins() { + List> plugins = new ArrayList<>(super.nodePlugins()); + plugins.add(SystemIndexTestPlugin.class); + return plugins; + } + + public void testResetSystemIndices() throws Exception { + // put a document in a system index + indexDoc(SystemIndexTestPlugin.SYSTEM_INDEX_PATTERN, "1", "purpose", "pre-snapshot doc"); + refresh(SystemIndexTestPlugin.SYSTEM_INDEX_PATTERN); + + // put a document in associated index + indexDoc(SystemIndexTestPlugin.ASSOCIATED_INDEX_NAME, "1", "purpose", "pre-snapshot doc"); + refresh(SystemIndexTestPlugin.ASSOCIATED_INDEX_NAME); + + // call the reset API + client().execute(ResetFeatureStateAction.INSTANCE, new ResetFeatureStateRequest()).get(); + + // verify that both indices are gone + Exception e1 = expectThrows(IndexNotFoundException.class, () -> client().admin().indices().prepareGetIndex() + .addIndices(SystemIndexTestPlugin.SYSTEM_INDEX_PATTERN) + .get()); + + assertThat(e1.getMessage(), containsString("no such index")); + + Exception e2 = expectThrows(IndexNotFoundException.class, () -> client().admin().indices().prepareGetIndex() + .addIndices(SystemIndexTestPlugin.ASSOCIATED_INDEX_NAME) + .get()); + + assertThat(e2.getMessage(), containsString("no such index")); + } + + public static class SystemIndexTestPlugin extends Plugin implements SystemIndexPlugin { + + public static final String SYSTEM_INDEX_PATTERN = ".test-system-idx"; + public static final String ASSOCIATED_INDEX_NAME = ".associated-idx"; + + @Override + public Collection getSystemIndexDescriptors(Settings settings) { + return Collections.singletonList(new SystemIndexDescriptor(SYSTEM_INDEX_PATTERN + "*", "System indices for tests")); + } + + @Override + public Collection getAssociatedIndexPatterns() { + return Collections.singletonList(ASSOCIATED_INDEX_NAME); + } + + @Override + public String getFeatureName() { + return SystemIndicesSnapshotIT.SystemIndexTestPlugin.class.getSimpleName(); + } + + @Override + public String getFeatureDescription() { + return "A simple test plugin"; + } + } + +} diff --git a/server/src/main/java/org/elasticsearch/action/ActionModule.java b/server/src/main/java/org/elasticsearch/action/ActionModule.java index 73bb826766bb5..95e1d6a634ee2 100644 --- a/server/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/server/src/main/java/org/elasticsearch/action/ActionModule.java @@ -284,6 +284,7 @@ import org.elasticsearch.rest.action.admin.cluster.RestPutStoredScriptAction; import org.elasticsearch.rest.action.admin.cluster.RestReloadSecureSettingsAction; import org.elasticsearch.rest.action.admin.cluster.RestRemoteClusterInfoAction; +import org.elasticsearch.rest.action.admin.cluster.RestResetFeatureStateAction; import org.elasticsearch.rest.action.admin.cluster.RestRestoreSnapshotAction; import org.elasticsearch.rest.action.admin.cluster.RestSnapshotsStatusAction; import org.elasticsearch.rest.action.admin.cluster.RestSnapshottableFeaturesAction; @@ -654,6 +655,7 @@ public void initRestHandlers(Supplier nodesInCluster) { registerHandler.accept(new RestDeleteSnapshotAction()); registerHandler.accept(new RestSnapshotsStatusAction()); registerHandler.accept(new RestSnapshottableFeaturesAction()); + registerHandler.accept(new RestResetFeatureStateAction()); registerHandler.accept(new RestGetIndicesAction()); registerHandler.accept(new RestIndicesStatsAction()); registerHandler.accept(new RestIndicesSegmentsAction()); diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java index 643df87940727..6788a84fc31e8 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java @@ -20,34 +20,54 @@ package org.elasticsearch.action.admin.cluster.snapshots.features; import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.admin.indices.delete.DeleteIndexClusterStateUpdateRequest; import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.action.support.master.TransportMasterNodeAction; +import org.elasticsearch.client.Client; +import org.elasticsearch.cluster.ClusterChangedEvent; import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.ClusterStateUpdateTask; +import org.elasticsearch.cluster.ack.ClusterStateUpdateRequest; import org.elasticsearch.cluster.block.ClusterBlockException; +import org.elasticsearch.cluster.block.ClusterBlockLevel; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.cluster.metadata.MetadataDeleteIndexService; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.util.set.Sets; +import org.elasticsearch.index.Index; +import org.elasticsearch.indices.SystemIndexDescriptor; +import org.elasticsearch.indices.SystemIndices; import org.elasticsearch.tasks.Task; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; +import java.util.Arrays; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; + public class TransportResetFeatureStateAction extends TransportMasterNodeAction { + private final SystemIndices systemIndices; + private final MetadataDeleteIndexService deleteIndexService; + @Inject - protected TransportResetFeatureStateAction( - String actionName, + public TransportResetFeatureStateAction( TransportService transportService, ClusterService clusterService, ThreadPool threadPool, ActionFilters actionFilters, - Writeable.Reader request, IndexNameExpressionResolver indexNameExpressionResolver, - Writeable.Reader response, - String executor + SystemIndices systemIndices, + MetadataDeleteIndexService deleteIndexService ) { - super(actionName, transportService, clusterService, threadPool, actionFilters, - request, indexNameExpressionResolver, response, executor); + super(ResetFeatureStateAction.NAME, transportService, clusterService, threadPool, actionFilters, + ResetFeatureStateRequest::new, indexNameExpressionResolver, ResetFeatureStateResponse::new, + ThreadPool.Names.SAME); + this.systemIndices = systemIndices; + this.deleteIndexService = deleteIndexService; } @Override @@ -57,12 +77,47 @@ protected void masterOperation( ClusterState state, ActionListener listener ) throws Exception { - // TODO[wrb] + // TODO[wrb] - temporary implementation, not how we want to do things. + Set concreteSystemIndices = systemIndices.getFeatures().values().stream() + .flatMap(feature -> feature.getIndexDescriptors().stream()) + .map(SystemIndexDescriptor::getIndexPattern) + .flatMap(pattern -> Arrays.stream( + indexNameExpressionResolver.concreteIndices( + state, + IndicesOptions.LENIENT_EXPAND_OPEN_CLOSED_HIDDEN, + pattern))) + .collect(Collectors.toSet()); + + Set concreteAssociatedIndices = systemIndices.getFeatures().values().stream() + .flatMap(feature -> feature.getAssociatedIndexPatterns().stream()) + .flatMap(pattern -> Arrays.stream( + indexNameExpressionResolver.concreteIndices( + state, + IndicesOptions.LENIENT_EXPAND_OPEN_CLOSED_HIDDEN, + pattern))) + .collect(Collectors.toSet()); + + clusterService.submitStateUpdateTask("reset_feature_states", new ClusterStateUpdateTask(request.masterNodeTimeout()) { + @Override + public ClusterState execute(ClusterState currentState) throws Exception { + return deleteIndexService.deleteIndices(currentState, Sets.union(concreteSystemIndices, concreteAssociatedIndices)); + } + + @Override + public void onFailure(String source, Exception e) { + listener.onFailure(e); + } + + @Override + public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) { + listener.onResponse(new ResetFeatureStateResponse()); + } + }); } @Override protected ClusterBlockException checkBlock(ResetFeatureStateRequest request, ClusterState state) { - // TODO[wrb] - return null; + // TODO[wrb] - what if there is a block on a system index? + return state.blocks().globalBlockedException(ClusterBlockLevel.WRITE); } } diff --git a/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java b/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java index c3a4a56f24ba1..0512f9cebe8af 100644 --- a/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java @@ -8,6 +8,8 @@ package org.elasticsearch.plugins; +import org.elasticsearch.client.Client; +import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.indices.SystemIndexDescriptor; @@ -49,4 +51,8 @@ default Collection getSystemIndexDescriptors(Settings set default Collection getAssociatedIndexPatterns() { return Collections.emptyList(); } + + default void cleanUpFeature(ClusterService clusterService, Client client) { + // do nothing + } } diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java index 7e089a0f51508..7dd70e7524da0 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java @@ -19,9 +19,12 @@ package org.elasticsearch.rest.action.admin.cluster; +import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateRequest; +import org.elasticsearch.action.admin.cluster.snapshots.features.SnapshottableFeaturesAction; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.rest.action.RestToXContentListener; import java.io.IOException; import java.util.List; @@ -40,7 +43,11 @@ public String getName() { @Override protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { - // TODO[wrb] - return null; + final ResetFeatureStateRequest req = new ResetFeatureStateRequest(); + req.masterNodeTimeout(request.paramAsTime("master_timeout", req.masterNodeTimeout())); + + return restChannel -> { + client.execute(SnapshottableFeaturesAction.INSTANCE, req, new RestToXContentListener<>(restChannel)); + }; } } From 51c8f8a64f3cf39309c0d4cbc382dde26c4f731e Mon Sep 17 00:00:00 2001 From: William Brafford Date: Tue, 2 Feb 2021 16:30:04 -0500 Subject: [PATCH 03/25] Improve unit test --- .../snapshots/SystemIndexResetApiIT.java | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java index b9400994888b5..ad74b9f384ec3 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java @@ -21,6 +21,7 @@ import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateAction; import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateRequest; +import org.elasticsearch.action.admin.indices.get.GetIndexResponse; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.indices.SystemIndexDescriptor; @@ -33,6 +34,7 @@ import java.util.Collections; import java.util.List; +import static org.hamcrest.Matchers.arrayContaining; import static org.hamcrest.Matchers.containsString; public class SystemIndexResetApiIT extends ESIntegTestCase { @@ -46,13 +48,17 @@ protected Collection> nodePlugins() { public void testResetSystemIndices() throws Exception { // put a document in a system index - indexDoc(SystemIndexTestPlugin.SYSTEM_INDEX_PATTERN, "1", "purpose", "pre-snapshot doc"); + indexDoc(SystemIndexTestPlugin.SYSTEM_INDEX_PATTERN, "1", "purpose", "system index doc"); refresh(SystemIndexTestPlugin.SYSTEM_INDEX_PATTERN); // put a document in associated index - indexDoc(SystemIndexTestPlugin.ASSOCIATED_INDEX_NAME, "1", "purpose", "pre-snapshot doc"); + indexDoc(SystemIndexTestPlugin.ASSOCIATED_INDEX_NAME, "1", "purpose", "associated index doc"); refresh(SystemIndexTestPlugin.ASSOCIATED_INDEX_NAME); + // put a document in a normal index + indexDoc("my_index", "1", "purpose", "normal index doc"); + refresh("my_index"); + // call the reset API client().execute(ResetFeatureStateAction.INSTANCE, new ResetFeatureStateRequest()).get(); @@ -68,6 +74,12 @@ public void testResetSystemIndices() throws Exception { .get()); assertThat(e2.getMessage(), containsString("no such index")); + + GetIndexResponse response = client().admin().indices().prepareGetIndex() + .addIndices("my_index") + .get(); + + assertThat(response.getIndices(), arrayContaining("my_index")); } public static class SystemIndexTestPlugin extends Plugin implements SystemIndexPlugin { From 581cd0aaa9b29dfcf07d66f9685d3462cc8722fc Mon Sep 17 00:00:00 2001 From: William Brafford Date: Wed, 3 Feb 2021 17:13:16 -0500 Subject: [PATCH 04/25] Add rough version of callback to SystemIndexPlugin --- .../snapshots/SystemIndexResetApiIT.java | 12 +++++++++ .../TransportResetFeatureStateAction.java | 26 ++++++++++++++----- .../elasticsearch/indices/SystemIndices.java | 19 ++++++++++++-- .../java/org/elasticsearch/node/Node.java | 3 ++- .../plugins/SystemIndexPlugin.java | 18 ++++++++++--- 5 files changed, 66 insertions(+), 12 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java index ad74b9f384ec3..d064d1d4fef0f 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java @@ -22,7 +22,10 @@ import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateAction; import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateRequest; import org.elasticsearch.action.admin.indices.get.GetIndexResponse; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.metadata.MetadataDeleteIndexService; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.indices.SystemIndexDescriptor; import org.elasticsearch.plugins.Plugin; @@ -33,6 +36,7 @@ import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Set; import static org.hamcrest.Matchers.arrayContaining; import static org.hamcrest.Matchers.containsString; @@ -106,6 +110,14 @@ public String getFeatureName() { public String getFeatureDescription() { return "A simple test plugin"; } + + @Override + public ClusterState cleanUpFeature( + Set indices, + MetadataDeleteIndexService deleteIndexService, + ClusterState state) { + return deleteIndexService.deleteIndices(state, indices); + } } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java index 6788a84fc31e8..42d0d81bd4ab6 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java @@ -20,15 +20,11 @@ package org.elasticsearch.action.admin.cluster.snapshots.features; import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.admin.indices.delete.DeleteIndexClusterStateUpdateRequest; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.action.support.master.TransportMasterNodeAction; -import org.elasticsearch.client.Client; -import org.elasticsearch.cluster.ClusterChangedEvent; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.ClusterStateUpdateTask; -import org.elasticsearch.cluster.ack.ClusterStateUpdateRequest; import org.elasticsearch.cluster.block.ClusterBlockException; import org.elasticsearch.cluster.block.ClusterBlockLevel; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; @@ -44,9 +40,9 @@ import org.elasticsearch.transport.TransportService; import java.util.Arrays; -import java.util.List; import java.util.Set; import java.util.stream.Collectors; +import java.util.stream.Stream; public class TransportResetFeatureStateAction extends TransportMasterNodeAction { @@ -100,7 +96,25 @@ protected void masterOperation( clusterService.submitStateUpdateTask("reset_feature_states", new ClusterStateUpdateTask(request.masterNodeTimeout()) { @Override public ClusterState execute(ClusterState currentState) throws Exception { - return deleteIndexService.deleteIndices(currentState, Sets.union(concreteSystemIndices, concreteAssociatedIndices)); + ClusterState state = currentState; + for (SystemIndices.Feature feature : systemIndices.getFeatures().values()) { + + ClusterState intermediateState = state; + Set concreteSystemIndices = Stream.concat( + feature.getIndexDescriptors().stream().map(SystemIndexDescriptor::getIndexPattern), + feature.getAssociatedIndexPatterns().stream()) + .flatMap(pattern -> Arrays.stream( + indexNameExpressionResolver.concreteIndices( + intermediateState, + IndicesOptions.LENIENT_EXPAND_OPEN_CLOSED_HIDDEN, + pattern))) + .collect(Collectors.toSet()); + + state = feature.getCleanUpFunction().apply( + Sets.union(concreteSystemIndices, concreteAssociatedIndices), + deleteIndexService, state); + } + return state; } @Override diff --git a/server/src/main/java/org/elasticsearch/indices/SystemIndices.java b/server/src/main/java/org/elasticsearch/indices/SystemIndices.java index 9eda85e963db5..42d903ddfedb5 100644 --- a/server/src/main/java/org/elasticsearch/indices/SystemIndices.java +++ b/server/src/main/java/org/elasticsearch/indices/SystemIndices.java @@ -13,10 +13,14 @@ import org.apache.lucene.util.automaton.CharacterRunAutomaton; import org.apache.lucene.util.automaton.MinimizationOperations; import org.apache.lucene.util.automaton.Operations; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.metadata.MetadataDeleteIndexService; import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.TriFunction; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.index.Index; import org.elasticsearch.snapshots.SnapshotsService; +import org.elasticsearch.snapshots.SnapshotsService; import java.util.Collection; import java.util.Collections; @@ -25,6 +29,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.stream.Collectors; import static java.util.stream.Collectors.toUnmodifiableList; @@ -202,15 +207,21 @@ public static class Feature { private final String description; private final Collection indexDescriptors; private final Collection associatedIndexPatterns; + private final TriFunction, MetadataDeleteIndexService, ClusterState, ClusterState> cleanUpFunction; - public Feature(String description, Collection indexDescriptors, Collection associatedIndexPatterns) { + public Feature( + String description, + Collection indexDescriptors, + Collection associatedIndexPatterns, + TriFunction, MetadataDeleteIndexService, ClusterState, ClusterState> cleanUpFunction) { this.description = description; this.indexDescriptors = indexDescriptors; this.associatedIndexPatterns = associatedIndexPatterns; + this.cleanUpFunction = cleanUpFunction; } public Feature(String description, Collection indexDescriptors) { - this(description, indexDescriptors, Collections.emptyList()); + this(description, indexDescriptors, Collections.emptyList(), (a, b, c) -> c); } public String getDescription() { @@ -224,5 +235,9 @@ public Collection getIndexDescriptors() { public Collection getAssociatedIndexPatterns() { return associatedIndexPatterns; } + + public TriFunction, MetadataDeleteIndexService, ClusterState, ClusterState> getCleanUpFunction() { + return cleanUpFunction; + } } } diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index cb51b0d7ccae8..ce534996e042f 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -504,7 +504,8 @@ protected Node(final Environment initialEnvironment, plugin -> new SystemIndices.Feature( plugin.getFeatureDescription(), plugin.getSystemIndexDescriptors(settings), - plugin.getAssociatedIndexPatterns() + plugin.getAssociatedIndexPatterns(), + plugin.getCleanUpFunction() )) ); final SystemIndices systemIndices = new SystemIndices(featuresMap); diff --git a/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java b/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java index 0512f9cebe8af..68677d85683d9 100644 --- a/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java @@ -8,13 +8,16 @@ package org.elasticsearch.plugins; -import org.elasticsearch.client.Client; -import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.metadata.MetadataDeleteIndexService; +import org.elasticsearch.common.TriFunction; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.index.Index; import org.elasticsearch.indices.SystemIndexDescriptor; import java.util.Collection; import java.util.Collections; +import java.util.Set; /** * Plugin for defining system indices. Extends {@link ActionPlugin} because system indices must be accessed via APIs @@ -52,7 +55,16 @@ default Collection getAssociatedIndexPatterns() { return Collections.emptyList(); } - default void cleanUpFeature(ClusterService clusterService, Client client) { + default ClusterState cleanUpFeature( + Set indices, + MetadataDeleteIndexService deleteIndexService, + ClusterState state) { // do nothing + return state; + } + + default TriFunction, MetadataDeleteIndexService, ClusterState, ClusterState> getCleanUpFunction() { + // return (indices, deleteIndexService, state) -> state; + return this::cleanUpFeature; } } From d6fa39cc55c1089cc03e882e0124d86347fb413e Mon Sep 17 00:00:00 2001 From: William Brafford Date: Thu, 4 Feb 2021 14:36:21 -0500 Subject: [PATCH 05/25] License changes --- .../snapshots/SystemIndexResetApiIT.java | 21 +++++-------------- .../features/ResetFeatureStateAction.java | 21 +++++-------------- .../features/ResetFeatureStateRequest.java | 21 +++++-------------- .../features/ResetFeatureStateResponse.java | 21 +++++-------------- .../TransportResetFeatureStateAction.java | 21 +++++-------------- .../cluster/RestResetFeatureStateAction.java | 21 +++++-------------- 6 files changed, 30 insertions(+), 96 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java index d064d1d4fef0f..90f865d797865 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java @@ -1,20 +1,9 @@ /* - * 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. + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. */ package org.elasticsearch.snapshots; diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateAction.java index cbeda5df7105d..e45cd17e7bd7e 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateAction.java @@ -1,20 +1,9 @@ /* - * 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. + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. */ package org.elasticsearch.action.admin.cluster.snapshots.features; diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateRequest.java index d8c58f832f0ce..29fbd89d4a3da 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateRequest.java @@ -1,20 +1,9 @@ /* - * 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. + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. */ package org.elasticsearch.action.admin.cluster.snapshots.features; diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java index cdb80a7383ec6..23d99c92cf1e1 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java @@ -1,20 +1,9 @@ /* - * 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. + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. */ package org.elasticsearch.action.admin.cluster.snapshots.features; diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java index 42d0d81bd4ab6..5339c52ac73fa 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java @@ -1,20 +1,9 @@ /* - * 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. + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. */ package org.elasticsearch.action.admin.cluster.snapshots.features; diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java index 7dd70e7524da0..197140f4a09c4 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java @@ -1,20 +1,9 @@ /* - * 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. + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. */ package org.elasticsearch.rest.action.admin.cluster; From ca50e12cc65fefd91d8247dac2395454e665b9aa Mon Sep 17 00:00:00 2001 From: William Brafford Date: Fri, 5 Feb 2021 13:48:07 -0500 Subject: [PATCH 06/25] Use a HandledTransportAction for reset action My initial cut used a TransportMasterNodeAction, which requires code that carefully manipulates cluster state. At least for the first cut and testing, it seems like it will be much easier to use a client within a HandledTransportAction, which effectively makes the TransportResetFeatureStateAction a class that dispatches other transport actions to do the real work. This could leave cleanup vulnerable to interference from other actions, e.g., indices deleted between lookup and deletion actions, but I can try to tighten that up at a later stage if I need to. --- .../snapshots/SystemIndexResetApiIT.java | 11 -- .../features/ResetFeatureStateRequest.java | 4 +- .../TransportResetFeatureStateAction.java | 105 +++--------------- .../elasticsearch/indices/SystemIndices.java | 16 +-- .../plugins/SystemIndexPlugin.java | 37 ++++-- .../cluster/RestResetFeatureStateAction.java | 1 - 6 files changed, 54 insertions(+), 120 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java index 90f865d797865..9bcdc56bff009 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java @@ -11,10 +11,7 @@ import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateAction; import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateRequest; import org.elasticsearch.action.admin.indices.get.GetIndexResponse; -import org.elasticsearch.cluster.ClusterState; -import org.elasticsearch.cluster.metadata.MetadataDeleteIndexService; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.indices.SystemIndexDescriptor; import org.elasticsearch.plugins.Plugin; @@ -25,7 +22,6 @@ import java.util.Collection; import java.util.Collections; import java.util.List; -import java.util.Set; import static org.hamcrest.Matchers.arrayContaining; import static org.hamcrest.Matchers.containsString; @@ -100,13 +96,6 @@ public String getFeatureDescription() { return "A simple test plugin"; } - @Override - public ClusterState cleanUpFeature( - Set indices, - MetadataDeleteIndexService deleteIndexService, - ClusterState state) { - return deleteIndexService.deleteIndices(state, indices); - } } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateRequest.java index 29fbd89d4a3da..59f4c430ccf8f 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateRequest.java @@ -8,14 +8,14 @@ package org.elasticsearch.action.admin.cluster.snapshots.features; +import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionRequestValidationException; -import org.elasticsearch.action.support.master.MasterNodeRequest; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import java.io.IOException; -public class ResetFeatureStateRequest extends MasterNodeRequest { +public class ResetFeatureStateRequest extends ActionRequest { public ResetFeatureStateRequest() { // TODO[wrb] diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java index 5339c52ac73fa..2ad6ac6a30fd2 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java @@ -10,117 +10,48 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.ActionFilters; -import org.elasticsearch.action.support.IndicesOptions; -import org.elasticsearch.action.support.master.TransportMasterNodeAction; -import org.elasticsearch.cluster.ClusterState; -import org.elasticsearch.cluster.ClusterStateUpdateTask; -import org.elasticsearch.cluster.block.ClusterBlockException; -import org.elasticsearch.cluster.block.ClusterBlockLevel; -import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.action.support.HandledTransportAction; +import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.cluster.metadata.MetadataDeleteIndexService; -import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.common.util.set.Sets; -import org.elasticsearch.index.Index; -import org.elasticsearch.indices.SystemIndexDescriptor; import org.elasticsearch.indices.SystemIndices; import org.elasticsearch.tasks.Task; -import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; -import java.util.Arrays; import java.util.Set; -import java.util.stream.Collectors; -import java.util.stream.Stream; -public class TransportResetFeatureStateAction extends TransportMasterNodeAction { +public class TransportResetFeatureStateAction extends HandledTransportAction { private final SystemIndices systemIndices; private final MetadataDeleteIndexService deleteIndexService; + private final NodeClient client; @Inject public TransportResetFeatureStateAction( TransportService transportService, - ClusterService clusterService, - ThreadPool threadPool, ActionFilters actionFilters, - IndexNameExpressionResolver indexNameExpressionResolver, SystemIndices systemIndices, - MetadataDeleteIndexService deleteIndexService + MetadataDeleteIndexService deleteIndexService, + NodeClient client ) { - super(ResetFeatureStateAction.NAME, transportService, clusterService, threadPool, actionFilters, - ResetFeatureStateRequest::new, indexNameExpressionResolver, ResetFeatureStateResponse::new, - ThreadPool.Names.SAME); + super(ResetFeatureStateAction.NAME, transportService, actionFilters, + ResetFeatureStateRequest::new); this.systemIndices = systemIndices; this.deleteIndexService = deleteIndexService; + this.client = client; } @Override - protected void masterOperation( + protected void doExecute( Task task, ResetFeatureStateRequest request, - ClusterState state, - ActionListener listener - ) throws Exception { - // TODO[wrb] - temporary implementation, not how we want to do things. - Set concreteSystemIndices = systemIndices.getFeatures().values().stream() - .flatMap(feature -> feature.getIndexDescriptors().stream()) - .map(SystemIndexDescriptor::getIndexPattern) - .flatMap(pattern -> Arrays.stream( - indexNameExpressionResolver.concreteIndices( - state, - IndicesOptions.LENIENT_EXPAND_OPEN_CLOSED_HIDDEN, - pattern))) - .collect(Collectors.toSet()); - - Set concreteAssociatedIndices = systemIndices.getFeatures().values().stream() - .flatMap(feature -> feature.getAssociatedIndexPatterns().stream()) - .flatMap(pattern -> Arrays.stream( - indexNameExpressionResolver.concreteIndices( - state, - IndicesOptions.LENIENT_EXPAND_OPEN_CLOSED_HIDDEN, - pattern))) - .collect(Collectors.toSet()); - - clusterService.submitStateUpdateTask("reset_feature_states", new ClusterStateUpdateTask(request.masterNodeTimeout()) { - @Override - public ClusterState execute(ClusterState currentState) throws Exception { - ClusterState state = currentState; - for (SystemIndices.Feature feature : systemIndices.getFeatures().values()) { - - ClusterState intermediateState = state; - Set concreteSystemIndices = Stream.concat( - feature.getIndexDescriptors().stream().map(SystemIndexDescriptor::getIndexPattern), - feature.getAssociatedIndexPatterns().stream()) - .flatMap(pattern -> Arrays.stream( - indexNameExpressionResolver.concreteIndices( - intermediateState, - IndicesOptions.LENIENT_EXPAND_OPEN_CLOSED_HIDDEN, - pattern))) - .collect(Collectors.toSet()); - - state = feature.getCleanUpFunction().apply( - Sets.union(concreteSystemIndices, concreteAssociatedIndices), - deleteIndexService, state); - } - return state; - } - - @Override - public void onFailure(String source, Exception e) { - listener.onFailure(e); - } - - @Override - public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) { - listener.onResponse(new ResetFeatureStateResponse()); - } - }); - } - - @Override - protected ClusterBlockException checkBlock(ResetFeatureStateRequest request, ClusterState state) { - // TODO[wrb] - what if there is a block on a system index? - return state.blocks().globalBlockedException(ClusterBlockLevel.WRITE); + ActionListener listener) { + + for (SystemIndices.Feature feature : systemIndices.getFeatures().values()) { + feature.getCleanUpFunction().apply( + Set.of(".test-system-idx", ".associated-idx"), + client, + listener); + } } } diff --git a/server/src/main/java/org/elasticsearch/indices/SystemIndices.java b/server/src/main/java/org/elasticsearch/indices/SystemIndices.java index 42d903ddfedb5..b195fb2c917a5 100644 --- a/server/src/main/java/org/elasticsearch/indices/SystemIndices.java +++ b/server/src/main/java/org/elasticsearch/indices/SystemIndices.java @@ -13,14 +13,14 @@ import org.apache.lucene.util.automaton.CharacterRunAutomaton; import org.apache.lucene.util.automaton.MinimizationOperations; import org.apache.lucene.util.automaton.Operations; -import org.elasticsearch.cluster.ClusterState; -import org.elasticsearch.cluster.metadata.MetadataDeleteIndexService; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateResponse; +import org.elasticsearch.client.Client; import org.elasticsearch.common.Nullable; -import org.elasticsearch.common.TriFunction; +import org.elasticsearch.common.TriConsumer; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.index.Index; import org.elasticsearch.snapshots.SnapshotsService; -import org.elasticsearch.snapshots.SnapshotsService; import java.util.Collection; import java.util.Collections; @@ -207,13 +207,13 @@ public static class Feature { private final String description; private final Collection indexDescriptors; private final Collection associatedIndexPatterns; - private final TriFunction, MetadataDeleteIndexService, ClusterState, ClusterState> cleanUpFunction; + private final TriConsumer, Client, ActionListener> cleanUpFunction; public Feature( String description, Collection indexDescriptors, Collection associatedIndexPatterns, - TriFunction, MetadataDeleteIndexService, ClusterState, ClusterState> cleanUpFunction) { + TriConsumer, Client, ActionListener> cleanUpFunction) { this.description = description; this.indexDescriptors = indexDescriptors; this.associatedIndexPatterns = associatedIndexPatterns; @@ -221,7 +221,7 @@ public Feature( } public Feature(String description, Collection indexDescriptors) { - this(description, indexDescriptors, Collections.emptyList(), (a, b, c) -> c); + this(description, indexDescriptors, Collections.emptyList(), (a, b, c) -> {}); } public String getDescription() { @@ -236,7 +236,7 @@ public Collection getAssociatedIndexPatterns() { return associatedIndexPatterns; } - public TriFunction, MetadataDeleteIndexService, ClusterState, ClusterState> getCleanUpFunction() { + public TriConsumer, Client, ActionListener> getCleanUpFunction() { return cleanUpFunction; } } diff --git a/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java b/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java index 68677d85683d9..5d4d8478aaf26 100644 --- a/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java @@ -8,11 +8,14 @@ package org.elasticsearch.plugins; -import org.elasticsearch.cluster.ClusterState; -import org.elasticsearch.cluster.metadata.MetadataDeleteIndexService; -import org.elasticsearch.common.TriFunction; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateResponse; +import org.elasticsearch.action.admin.indices.delete.DeleteIndexAction; +import org.elasticsearch.action.admin.indices.delete.DeleteIndexRequest; +import org.elasticsearch.action.support.master.AcknowledgedResponse; +import org.elasticsearch.client.Client; +import org.elasticsearch.common.TriConsumer; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.index.Index; import org.elasticsearch.indices.SystemIndexDescriptor; import java.util.Collection; @@ -55,15 +58,27 @@ default Collection getAssociatedIndexPatterns() { return Collections.emptyList(); } - default ClusterState cleanUpFeature( - Set indices, - MetadataDeleteIndexService deleteIndexService, - ClusterState state) { - // do nothing - return state; + default void cleanUpFeature( + Set indices, + Client client, + ActionListener listener) { + + DeleteIndexRequest deleteIndexRequest = new DeleteIndexRequest(); + deleteIndexRequest.indices(indices.toArray(String[]::new)); + client.execute(DeleteIndexAction.INSTANCE, deleteIndexRequest, new ActionListener<>() { + @Override + public void onResponse(AcknowledgedResponse acknowledgedResponse) { + listener.onResponse(new ResetFeatureStateResponse()); + } + + @Override + public void onFailure(Exception e) { + listener.onFailure(e); + } + }); } - default TriFunction, MetadataDeleteIndexService, ClusterState, ClusterState> getCleanUpFunction() { + default TriConsumer, Client, ActionListener> getCleanUpFunction() { // return (indices, deleteIndexService, state) -> state; return this::cleanUpFeature; } diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java index 197140f4a09c4..edeb6c0d96b77 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java @@ -33,7 +33,6 @@ public String getName() { @Override protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { final ResetFeatureStateRequest req = new ResetFeatureStateRequest(); - req.masterNodeTimeout(request.paramAsTime("master_timeout", req.masterNodeTimeout())); return restChannel -> { client.execute(SnapshottableFeaturesAction.INSTANCE, req, new RestToXContentListener<>(restChannel)); From bf5c86aa578b9308392da0cb9be76a95d5bbb0d2 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Fri, 5 Feb 2021 15:20:19 -0500 Subject: [PATCH 07/25] Just use clusterService, client, and listener in callback --- .../TransportResetFeatureStateAction.java | 16 +++++-------- .../elasticsearch/indices/SystemIndices.java | 8 +++---- .../plugins/SystemIndexPlugin.java | 23 +++++++++++++++---- 3 files changed, 28 insertions(+), 19 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java index 2ad6ac6a30fd2..8ff704c2ae28f 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java @@ -13,32 +13,31 @@ import org.elasticsearch.action.support.HandledTransportAction; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.cluster.metadata.MetadataDeleteIndexService; +import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.indices.SystemIndices; import org.elasticsearch.tasks.Task; import org.elasticsearch.transport.TransportService; -import java.util.Set; - public class TransportResetFeatureStateAction extends HandledTransportAction { private final SystemIndices systemIndices; - private final MetadataDeleteIndexService deleteIndexService; private final NodeClient client; + private final ClusterService clusterService; @Inject public TransportResetFeatureStateAction( TransportService transportService, ActionFilters actionFilters, SystemIndices systemIndices, - MetadataDeleteIndexService deleteIndexService, - NodeClient client + NodeClient client, + ClusterService clusterService ) { super(ResetFeatureStateAction.NAME, transportService, actionFilters, ResetFeatureStateRequest::new); this.systemIndices = systemIndices; - this.deleteIndexService = deleteIndexService; this.client = client; + this.clusterService = clusterService; } @Override @@ -48,10 +47,7 @@ protected void doExecute( ActionListener listener) { for (SystemIndices.Feature feature : systemIndices.getFeatures().values()) { - feature.getCleanUpFunction().apply( - Set.of(".test-system-idx", ".associated-idx"), - client, - listener); + feature.getCleanUpFunction().apply(clusterService, client, listener); } } } diff --git a/server/src/main/java/org/elasticsearch/indices/SystemIndices.java b/server/src/main/java/org/elasticsearch/indices/SystemIndices.java index b195fb2c917a5..48f91a38cadab 100644 --- a/server/src/main/java/org/elasticsearch/indices/SystemIndices.java +++ b/server/src/main/java/org/elasticsearch/indices/SystemIndices.java @@ -16,6 +16,7 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateResponse; import org.elasticsearch.client.Client; +import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.TriConsumer; import org.elasticsearch.common.collect.Tuple; @@ -29,7 +30,6 @@ import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.Set; import java.util.stream.Collectors; import static java.util.stream.Collectors.toUnmodifiableList; @@ -207,13 +207,13 @@ public static class Feature { private final String description; private final Collection indexDescriptors; private final Collection associatedIndexPatterns; - private final TriConsumer, Client, ActionListener> cleanUpFunction; + private final TriConsumer> cleanUpFunction; public Feature( String description, Collection indexDescriptors, Collection associatedIndexPatterns, - TriConsumer, Client, ActionListener> cleanUpFunction) { + TriConsumer> cleanUpFunction) { this.description = description; this.indexDescriptors = indexDescriptors; this.associatedIndexPatterns = associatedIndexPatterns; @@ -236,7 +236,7 @@ public Collection getAssociatedIndexPatterns() { return associatedIndexPatterns; } - public TriConsumer, Client, ActionListener> getCleanUpFunction() { + public TriConsumer> getCleanUpFunction() { return cleanUpFunction; } } diff --git a/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java b/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java index 5d4d8478aaf26..43ec41a07714a 100644 --- a/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java @@ -14,13 +14,17 @@ import org.elasticsearch.action.admin.indices.delete.DeleteIndexRequest; import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.client.Client; +import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.TriConsumer; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.indices.SystemIndexDescriptor; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.Set; +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.Stream; /** * Plugin for defining system indices. Extends {@link ActionPlugin} because system indices must be accessed via APIs @@ -59,12 +63,21 @@ default Collection getAssociatedIndexPatterns() { } default void cleanUpFeature( - Set indices, - Client client, + ClusterService clusterService, Client client, ActionListener listener) { + List systemIndices = getSystemIndexDescriptors(clusterService.getSettings()).stream() + .map(sid -> sid.getMatchingIndices(clusterService.state().getMetadata())) + .flatMap(List::stream) + .collect(Collectors.toList()); + + List associatedIndices = new ArrayList<>(getAssociatedIndexPatterns()); + + List allIndices = Stream.concat(systemIndices.stream(), associatedIndices.stream()) + .collect(Collectors.toList()); + DeleteIndexRequest deleteIndexRequest = new DeleteIndexRequest(); - deleteIndexRequest.indices(indices.toArray(String[]::new)); + deleteIndexRequest.indices(allIndices.toArray(String[]::new)); client.execute(DeleteIndexAction.INSTANCE, deleteIndexRequest, new ActionListener<>() { @Override public void onResponse(AcknowledgedResponse acknowledgedResponse) { @@ -78,7 +91,7 @@ public void onFailure(Exception e) { }); } - default TriConsumer, Client, ActionListener> getCleanUpFunction() { + default TriConsumer> getCleanUpFunction() { // return (indices, deleteIndexService, state) -> state; return this::cleanUpFeature; } From 88ec70d995c08ad4e45072afbc843b217b6e4204 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Fri, 5 Feb 2021 16:34:37 -0500 Subject: [PATCH 08/25] Add javadoc and update tests --- .../snapshots/SystemIndexResetApiIT.java | 29 +++++++++++-------- .../features/ResetFeatureStateAction.java | 1 + .../features/ResetFeatureStateRequest.java | 4 ++- .../features/ResetFeatureStateResponse.java | 5 ++-- .../TransportResetFeatureStateAction.java | 4 ++- .../java/org/elasticsearch/node/Node.java | 2 +- .../plugins/SystemIndexPlugin.java | 13 +++++---- 7 files changed, 35 insertions(+), 23 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java index 9bcdc56bff009..14a05fb944a27 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java @@ -35,14 +35,18 @@ protected Collection> nodePlugins() { return plugins; } + /** Check that the reset method cleans up a feature */ public void testResetSystemIndices() throws Exception { + String sysindex = ".test-system-idx-1"; + String associndex = ".associated-idx-1"; + // put a document in a system index - indexDoc(SystemIndexTestPlugin.SYSTEM_INDEX_PATTERN, "1", "purpose", "system index doc"); - refresh(SystemIndexTestPlugin.SYSTEM_INDEX_PATTERN); + indexDoc(sysindex, "1", "purpose", "system index doc"); + refresh(sysindex); // put a document in associated index - indexDoc(SystemIndexTestPlugin.ASSOCIATED_INDEX_NAME, "1", "purpose", "associated index doc"); - refresh(SystemIndexTestPlugin.ASSOCIATED_INDEX_NAME); + indexDoc(associndex, "1", "purpose", "associated index doc"); + refresh(associndex); // put a document in a normal index indexDoc("my_index", "1", "purpose", "normal index doc"); @@ -53,13 +57,13 @@ public void testResetSystemIndices() throws Exception { // verify that both indices are gone Exception e1 = expectThrows(IndexNotFoundException.class, () -> client().admin().indices().prepareGetIndex() - .addIndices(SystemIndexTestPlugin.SYSTEM_INDEX_PATTERN) + .addIndices(sysindex) .get()); assertThat(e1.getMessage(), containsString("no such index")); Exception e2 = expectThrows(IndexNotFoundException.class, () -> client().admin().indices().prepareGetIndex() - .addIndices(SystemIndexTestPlugin.ASSOCIATED_INDEX_NAME) + .addIndices(associndex) .get()); assertThat(e2.getMessage(), containsString("no such index")); @@ -71,19 +75,22 @@ public void testResetSystemIndices() throws Exception { assertThat(response.getIndices(), arrayContaining("my_index")); } + /** + * A test plugin with patterns for system indices and associated indices. + */ public static class SystemIndexTestPlugin extends Plugin implements SystemIndexPlugin { - public static final String SYSTEM_INDEX_PATTERN = ".test-system-idx"; - public static final String ASSOCIATED_INDEX_NAME = ".associated-idx"; + public static final String SYSTEM_INDEX_PATTERN = ".test-system-idx*"; + public static final String ASSOCIATED_INDEX_PATTERN = ".associated-idx*"; @Override public Collection getSystemIndexDescriptors(Settings settings) { - return Collections.singletonList(new SystemIndexDescriptor(SYSTEM_INDEX_PATTERN + "*", "System indices for tests")); + return Collections.singletonList(new SystemIndexDescriptor(SYSTEM_INDEX_PATTERN, "System indices for tests")); } @Override public Collection getAssociatedIndexPatterns() { - return Collections.singletonList(ASSOCIATED_INDEX_NAME); + return Collections.singletonList(ASSOCIATED_INDEX_PATTERN); } @Override @@ -95,7 +102,5 @@ public String getFeatureName() { public String getFeatureDescription() { return "A simple test plugin"; } - } - } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateAction.java index e45cd17e7bd7e..9e4141c10620c 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateAction.java @@ -10,6 +10,7 @@ import org.elasticsearch.action.ActionType; +/** Action for resetting feature states, mostly meaning system indices */ public class ResetFeatureStateAction extends ActionType { public static final ResetFeatureStateAction INSTANCE = new ResetFeatureStateAction(); diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateRequest.java index 59f4c430ccf8f..82ed910ba7901 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateRequest.java @@ -15,10 +15,12 @@ import java.io.IOException; +/** Request for resetting feature state */ public class ResetFeatureStateRequest extends ActionRequest { public ResetFeatureStateRequest() { - // TODO[wrb] + // TODO[wrb] - We might need to let this request take a list of + // feature state names, but not for the initial implementation } public ResetFeatureStateRequest(StreamInput in) throws IOException { diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java index 23d99c92cf1e1..98131c58b1398 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java @@ -16,10 +16,11 @@ import java.io.IOException; +/** Response to a feature state reset request. */ public class ResetFeatureStateResponse extends ActionResponse implements ToXContentObject { public ResetFeatureStateResponse() { - // TODO[wrb] + // TODO[wrb] - we should return a list or map of feature states and their responses } public ResetFeatureStateResponse(StreamInput in) throws IOException { @@ -30,7 +31,7 @@ public ResetFeatureStateResponse(StreamInput in) throws IOException { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { // TODO[wrb] - return null; + return builder; } @Override diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java index 8ff704c2ae28f..b17e174ca343c 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java @@ -12,13 +12,15 @@ import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.HandledTransportAction; import org.elasticsearch.client.node.NodeClient; -import org.elasticsearch.cluster.metadata.MetadataDeleteIndexService; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.indices.SystemIndices; import org.elasticsearch.tasks.Task; import org.elasticsearch.transport.TransportService; +/** + * Transport action for cleaning up feature index state. + */ public class TransportResetFeatureStateAction extends HandledTransportAction { private final SystemIndices systemIndices; diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index ce534996e042f..ad2e5b884704f 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -505,7 +505,7 @@ protected Node(final Environment initialEnvironment, plugin.getFeatureDescription(), plugin.getSystemIndexDescriptors(settings), plugin.getAssociatedIndexPatterns(), - plugin.getCleanUpFunction() + plugin::cleanUpFeature )) ); final SystemIndices systemIndices = new SystemIndices(featuresMap); diff --git a/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java b/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java index 43ec41a07714a..dbd1f2d5a2733 100644 --- a/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java @@ -15,7 +15,6 @@ import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.client.Client; import org.elasticsearch.cluster.service.ClusterService; -import org.elasticsearch.common.TriConsumer; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.indices.SystemIndexDescriptor; @@ -62,6 +61,13 @@ default Collection getAssociatedIndexPatterns() { return Collections.emptyList(); } + /** + * Cleans up the state of the feature by deleting system indices and associated indices. + * Override to do more for cleanup (e.g. cancelling tasks). + * @param clusterService Cluster service to provide cluster state + * @param client A client, for executing actions + * @param listener Listener for post-cleanup result TODO[wrb]: need to gather results over features + */ default void cleanUpFeature( ClusterService clusterService, Client client, ActionListener listener) { @@ -90,9 +96,4 @@ public void onFailure(Exception e) { } }); } - - default TriConsumer> getCleanUpFunction() { - // return (indices, deleteIndexService, state) -> state; - return this::cleanUpFeature; - } } From eb039550303382cd25324efb3550dcb6268f32b3 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Mon, 8 Feb 2021 16:46:39 -0500 Subject: [PATCH 09/25] Fill out ResetFeatureStateResponse class --- .../features/ResetFeatureStateResponse.java | 101 +++++++++++++++++- .../plugins/SystemIndexPlugin.java | 3 +- .../ResetFeatureStateResponseTests.java | 49 +++++++++ 3 files changed, 148 insertions(+), 5 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponseTests.java diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java index 98131c58b1398..b5d7aa78e6564 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java @@ -11,31 +11,124 @@ import org.elasticsearch.action.ActionResponse; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Objects; /** Response to a feature state reset request. */ public class ResetFeatureStateResponse extends ActionResponse implements ToXContentObject { - public ResetFeatureStateResponse() { + List itemList; + + public ResetFeatureStateResponse(Map statusMap) { + // TODO[wrb] - we should return a list or map of feature states and their responses + itemList = new ArrayList<>(); + for (Map.Entry entry : statusMap.entrySet()) { + itemList.add(new Item(entry.getKey(), entry.getValue())); + } + } + + public ResetFeatureStateResponse(List statuslist) { // TODO[wrb] - we should return a list or map of feature states and their responses + itemList = new ArrayList<>(); + itemList.addAll(statuslist); } public ResetFeatureStateResponse(StreamInput in) throws IOException { super(in); - // TODO[wrb] + this.itemList = in.readList(Item::new); + } + + public List getItemList() { + return this.itemList; } @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - // TODO[wrb] + builder.startObject(); + { + builder.startArray("features"); + for (Item item: this.itemList) { + builder.value(item); + } + builder.endArray(); + } + builder.endObject(); return builder; } @Override public void writeTo(StreamOutput out) throws IOException { - // TODO[wrb] + out.writeList(this.itemList); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ResetFeatureStateResponse that = (ResetFeatureStateResponse) o; + return Objects.equals(itemList, that.itemList); + } + + @Override + public int hashCode() { + return Objects.hash(itemList); + } + + public static class Item implements Writeable, ToXContentObject { + private String featureName; + private String status; + + Item(String featureName, String status) { + this.featureName = featureName; + this.status = status; + } + + Item(StreamInput in) throws IOException { + this.featureName = in.readString(); + this.status = in.readString(); + } + + public String getFeatureName() { + return this.featureName; + } + + public String getStatus() { + return this.status; + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(); + builder.field("feature_name", this.featureName); + builder.field("status", this.status); + builder.endObject(); + return builder; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeString(this.featureName); + out.writeString(this.status); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Item item = (Item) o; + return Objects.equals(featureName, item.featureName) && Objects.equals(status, item.status); + } + + @Override + public int hashCode() { + return Objects.hash(featureName, status); + } } } diff --git a/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java b/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java index dbd1f2d5a2733..298c0a63384a5 100644 --- a/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java @@ -21,6 +21,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -87,7 +88,7 @@ default void cleanUpFeature( client.execute(DeleteIndexAction.INSTANCE, deleteIndexRequest, new ActionListener<>() { @Override public void onResponse(AcknowledgedResponse acknowledgedResponse) { - listener.onResponse(new ResetFeatureStateResponse()); + listener.onResponse(new ResetFeatureStateResponse(new HashMap<>())); } @Override diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponseTests.java new file mode 100644 index 0000000000000..c0c105f8030cd --- /dev/null +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponseTests.java @@ -0,0 +1,49 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.action.admin.cluster.snapshots.features; + +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.test.AbstractWireSerializingTestCase; + +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; + +public class ResetFeatureStateResponseTests extends AbstractWireSerializingTestCase { + + @Override + protected Writeable.Reader instanceReader() { + return ResetFeatureStateResponse::new; + } + + @Override + protected ResetFeatureStateResponse createTestInstance() { + Map responses = new HashMap<>(); + responses.put("feature_one", randomFrom("SUCCESS", "FAILURE")); + responses.put("feature_two", randomFrom("SUCCESS", "FAILURE")); + return new ResetFeatureStateResponse(responses); + } + + @Override + protected ResetFeatureStateResponse mutateInstance(ResetFeatureStateResponse instance) throws IOException { + int minSize = 0; + if (instance.getItemList().size() == 0) { + minSize = 1; + } + Set existingFeatureNames = instance.getItemList().stream() + .map(item -> item.getFeatureName()) + .collect(Collectors.toSet()); + return new ResetFeatureStateResponse(randomList(minSize, 10, + () -> new ResetFeatureStateResponse.Item( + randomValueOtherThanMany(existingFeatureNames::contains, () -> randomAlphaOfLengthBetween(4, 10)), + randomAlphaOfLengthBetween(5, 10)))); + } +} From c5e82bc0ee958eebdf1361a468fc2c43ad062bbf Mon Sep 17 00:00:00 2001 From: William Brafford Date: Mon, 8 Feb 2021 17:43:28 -0500 Subject: [PATCH 10/25] Hook up response class in TransportAction --- .../snapshots/SystemIndexResetApiIT.java | 7 ++++++- .../features/ResetFeatureStateResponse.java | 17 ++++++++++++++- .../TransportResetFeatureStateAction.java | 21 ++++++++++++++++++- .../elasticsearch/indices/SystemIndices.java | 6 +++--- .../plugins/SystemIndexPlugin.java | 7 +++---- 5 files changed, 48 insertions(+), 10 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java index 14a05fb944a27..d9439a14958c8 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java @@ -10,6 +10,7 @@ import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateAction; import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateRequest; +import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateResponse; import org.elasticsearch.action.admin.indices.get.GetIndexResponse; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.IndexNotFoundException; @@ -24,7 +25,9 @@ import java.util.List; import static org.hamcrest.Matchers.arrayContaining; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; public class SystemIndexResetApiIT extends ESIntegTestCase { @@ -53,7 +56,9 @@ public void testResetSystemIndices() throws Exception { refresh("my_index"); // call the reset API - client().execute(ResetFeatureStateAction.INSTANCE, new ResetFeatureStateRequest()).get(); + ResetFeatureStateResponse apiResponse = client().execute(ResetFeatureStateAction.INSTANCE, new ResetFeatureStateRequest()).get(); + assertThat(apiResponse.getItemList(), contains( + new ResetFeatureStateResponse.Item("SystemIndexTestPlugin", "SUCCESS"))); // verify that both indices are gone Exception e1 = expectThrows(IndexNotFoundException.class, () -> client().admin().indices().prepareGetIndex() diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java index b5d7aa78e6564..d13055ae6edf6 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java @@ -81,11 +81,18 @@ public int hashCode() { return Objects.hash(itemList); } + @Override + public String toString() { + return "ResetFeatureStateResponse{" + + "itemList=" + itemList + + '}'; + } + public static class Item implements Writeable, ToXContentObject { private String featureName; private String status; - Item(String featureName, String status) { + public Item(String featureName, String status) { this.featureName = featureName; this.status = status; } @@ -130,5 +137,13 @@ public boolean equals(Object o) { public int hashCode() { return Objects.hash(featureName, status); } + + @Override + public String toString() { + return "Item{" + + "featureName='" + featureName + '\'' + + ", status='" + status + '\'' + + '}'; + } } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java index b17e174ca343c..038449ad8f5b1 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java @@ -14,10 +14,15 @@ import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.util.concurrent.CountDown; import org.elasticsearch.indices.SystemIndices; import org.elasticsearch.tasks.Task; import org.elasticsearch.transport.TransportService; +import java.util.Collections; +import java.util.SortedMap; +import java.util.TreeMap; + /** * Transport action for cleaning up feature index state. */ @@ -48,8 +53,22 @@ protected void doExecute( ResetFeatureStateRequest request, ActionListener listener) { + final int features = systemIndices.getFeatures().size(); + final CountDown completionCounter = new CountDown(features - 1); + final SortedMap responses = Collections.synchronizedSortedMap(new TreeMap<>()); + final Runnable terminalHandler = () -> { + if (completionCounter.countDown()) { + listener.onResponse(new ResetFeatureStateResponse(responses)); + } + }; + for (SystemIndices.Feature feature : systemIndices.getFeatures().values()) { - feature.getCleanUpFunction().apply(clusterService, client, listener); + feature.getCleanUpFunction().apply(clusterService, client, ActionListener.wrap( + response -> { + responses.put(response.getFeatureName(), response.getStatus()); + terminalHandler.run(); + }, failure -> { terminalHandler.run(); } + )); } } } diff --git a/server/src/main/java/org/elasticsearch/indices/SystemIndices.java b/server/src/main/java/org/elasticsearch/indices/SystemIndices.java index 48f91a38cadab..6c4512e505adc 100644 --- a/server/src/main/java/org/elasticsearch/indices/SystemIndices.java +++ b/server/src/main/java/org/elasticsearch/indices/SystemIndices.java @@ -207,13 +207,13 @@ public static class Feature { private final String description; private final Collection indexDescriptors; private final Collection associatedIndexPatterns; - private final TriConsumer> cleanUpFunction; + private final TriConsumer> cleanUpFunction; public Feature( String description, Collection indexDescriptors, Collection associatedIndexPatterns, - TriConsumer> cleanUpFunction) { + TriConsumer> cleanUpFunction) { this.description = description; this.indexDescriptors = indexDescriptors; this.associatedIndexPatterns = associatedIndexPatterns; @@ -236,7 +236,7 @@ public Collection getAssociatedIndexPatterns() { return associatedIndexPatterns; } - public TriConsumer> getCleanUpFunction() { + public TriConsumer> getCleanUpFunction() { return cleanUpFunction; } } diff --git a/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java b/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java index 298c0a63384a5..6894870068655 100644 --- a/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java @@ -21,7 +21,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -71,7 +70,7 @@ default Collection getAssociatedIndexPatterns() { */ default void cleanUpFeature( ClusterService clusterService, Client client, - ActionListener listener) { + ActionListener listener) { List systemIndices = getSystemIndexDescriptors(clusterService.getSettings()).stream() .map(sid -> sid.getMatchingIndices(clusterService.state().getMetadata())) @@ -88,12 +87,12 @@ default void cleanUpFeature( client.execute(DeleteIndexAction.INSTANCE, deleteIndexRequest, new ActionListener<>() { @Override public void onResponse(AcknowledgedResponse acknowledgedResponse) { - listener.onResponse(new ResetFeatureStateResponse(new HashMap<>())); + listener.onResponse(new ResetFeatureStateResponse.Item(getFeatureName(), "SUCCESS")); } @Override public void onFailure(Exception e) { - listener.onFailure(e); + listener.onResponse(new ResetFeatureStateResponse.Item(getFeatureName(), "FAILURE")); } }); } From 8cafd70721124019229d22122f52c81c388f941a Mon Sep 17 00:00:00 2001 From: William Brafford Date: Mon, 8 Feb 2021 21:35:43 -0500 Subject: [PATCH 11/25] Rename inner class --- .../snapshots/SystemIndexResetApiIT.java | 3 +- .../features/ResetFeatureStateResponse.java | 44 +++++++++---------- .../TransportResetFeatureStateAction.java | 1 + .../elasticsearch/indices/SystemIndices.java | 8 ++-- .../plugins/SystemIndexPlugin.java | 6 +-- .../ResetFeatureStateResponseTests.java | 2 +- 6 files changed, 31 insertions(+), 33 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java index d9439a14958c8..73d261fff3d4a 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java @@ -27,7 +27,6 @@ import static org.hamcrest.Matchers.arrayContaining; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.equalTo; public class SystemIndexResetApiIT extends ESIntegTestCase { @@ -58,7 +57,7 @@ public void testResetSystemIndices() throws Exception { // call the reset API ResetFeatureStateResponse apiResponse = client().execute(ResetFeatureStateAction.INSTANCE, new ResetFeatureStateRequest()).get(); assertThat(apiResponse.getItemList(), contains( - new ResetFeatureStateResponse.Item("SystemIndexTestPlugin", "SUCCESS"))); + new ResetFeatureStateResponse.ResetFeatureStateStatus("SystemIndexTestPlugin", "SUCCESS"))); // verify that both indices are gone Exception e1 = expectThrows(IndexNotFoundException.class, () -> client().admin().indices().prepareGetIndex() diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java index d13055ae6edf6..74fa9e73f95bc 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java @@ -24,29 +24,27 @@ /** Response to a feature state reset request. */ public class ResetFeatureStateResponse extends ActionResponse implements ToXContentObject { - List itemList; + List resetFeatureStateStatusList; public ResetFeatureStateResponse(Map statusMap) { - // TODO[wrb] - we should return a list or map of feature states and their responses - itemList = new ArrayList<>(); + resetFeatureStateStatusList = new ArrayList<>(); for (Map.Entry entry : statusMap.entrySet()) { - itemList.add(new Item(entry.getKey(), entry.getValue())); + resetFeatureStateStatusList.add(new ResetFeatureStateStatus(entry.getKey(), entry.getValue())); } } - public ResetFeatureStateResponse(List statuslist) { - // TODO[wrb] - we should return a list or map of feature states and their responses - itemList = new ArrayList<>(); - itemList.addAll(statuslist); + public ResetFeatureStateResponse(List statusList) { + resetFeatureStateStatusList = new ArrayList<>(); + resetFeatureStateStatusList.addAll(statusList); } public ResetFeatureStateResponse(StreamInput in) throws IOException { super(in); - this.itemList = in.readList(Item::new); + this.resetFeatureStateStatusList = in.readList(ResetFeatureStateStatus::new); } - public List getItemList() { - return this.itemList; + public List getItemList() { + return this.resetFeatureStateStatusList; } @Override @@ -54,8 +52,8 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.startObject(); { builder.startArray("features"); - for (Item item: this.itemList) { - builder.value(item); + for (ResetFeatureStateStatus resetFeatureStateStatus : this.resetFeatureStateStatusList) { + builder.value(resetFeatureStateStatus); } builder.endArray(); } @@ -65,7 +63,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws @Override public void writeTo(StreamOutput out) throws IOException { - out.writeList(this.itemList); + out.writeList(this.resetFeatureStateStatusList); } @Override @@ -73,31 +71,31 @@ public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; ResetFeatureStateResponse that = (ResetFeatureStateResponse) o; - return Objects.equals(itemList, that.itemList); + return Objects.equals(resetFeatureStateStatusList, that.resetFeatureStateStatusList); } @Override public int hashCode() { - return Objects.hash(itemList); + return Objects.hash(resetFeatureStateStatusList); } @Override public String toString() { return "ResetFeatureStateResponse{" + - "itemList=" + itemList + + "resetFeatureStateStatusList=" + resetFeatureStateStatusList + '}'; } - public static class Item implements Writeable, ToXContentObject { + public static class ResetFeatureStateStatus implements Writeable, ToXContentObject { private String featureName; private String status; - public Item(String featureName, String status) { + public ResetFeatureStateStatus(String featureName, String status) { this.featureName = featureName; this.status = status; } - Item(StreamInput in) throws IOException { + ResetFeatureStateStatus(StreamInput in) throws IOException { this.featureName = in.readString(); this.status = in.readString(); } @@ -129,8 +127,8 @@ public void writeTo(StreamOutput out) throws IOException { public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; - Item item = (Item) o; - return Objects.equals(featureName, item.featureName) && Objects.equals(status, item.status); + ResetFeatureStateStatus that = (ResetFeatureStateStatus) o; + return Objects.equals(featureName, that.featureName) && Objects.equals(status, that.status); } @Override @@ -140,7 +138,7 @@ public int hashCode() { @Override public String toString() { - return "Item{" + + return "ResetFeatureStateStatus{" + "featureName='" + featureName + '\'' + ", status='" + status + '\'' + '}'; diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java index 038449ad8f5b1..303caf84c4907 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java @@ -55,6 +55,7 @@ protected void doExecute( final int features = systemIndices.getFeatures().size(); final CountDown completionCounter = new CountDown(features - 1); + // TODO[wrb] use concurrent list rather than map final SortedMap responses = Collections.synchronizedSortedMap(new TreeMap<>()); final Runnable terminalHandler = () -> { if (completionCounter.countDown()) { diff --git a/server/src/main/java/org/elasticsearch/indices/SystemIndices.java b/server/src/main/java/org/elasticsearch/indices/SystemIndices.java index 6c4512e505adc..607d072fe6e17 100644 --- a/server/src/main/java/org/elasticsearch/indices/SystemIndices.java +++ b/server/src/main/java/org/elasticsearch/indices/SystemIndices.java @@ -14,7 +14,7 @@ import org.apache.lucene.util.automaton.MinimizationOperations; import org.apache.lucene.util.automaton.Operations; import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateResponse; +import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateResponse.ResetFeatureStateStatus; import org.elasticsearch.client.Client; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Nullable; @@ -207,13 +207,13 @@ public static class Feature { private final String description; private final Collection indexDescriptors; private final Collection associatedIndexPatterns; - private final TriConsumer> cleanUpFunction; + private final TriConsumer> cleanUpFunction; public Feature( String description, Collection indexDescriptors, Collection associatedIndexPatterns, - TriConsumer> cleanUpFunction) { + TriConsumer> cleanUpFunction) { this.description = description; this.indexDescriptors = indexDescriptors; this.associatedIndexPatterns = associatedIndexPatterns; @@ -236,7 +236,7 @@ public Collection getAssociatedIndexPatterns() { return associatedIndexPatterns; } - public TriConsumer> getCleanUpFunction() { + public TriConsumer> getCleanUpFunction() { return cleanUpFunction; } } diff --git a/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java b/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java index 6894870068655..2886f71171eee 100644 --- a/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java @@ -70,7 +70,7 @@ default Collection getAssociatedIndexPatterns() { */ default void cleanUpFeature( ClusterService clusterService, Client client, - ActionListener listener) { + ActionListener listener) { List systemIndices = getSystemIndexDescriptors(clusterService.getSettings()).stream() .map(sid -> sid.getMatchingIndices(clusterService.state().getMetadata())) @@ -87,12 +87,12 @@ default void cleanUpFeature( client.execute(DeleteIndexAction.INSTANCE, deleteIndexRequest, new ActionListener<>() { @Override public void onResponse(AcknowledgedResponse acknowledgedResponse) { - listener.onResponse(new ResetFeatureStateResponse.Item(getFeatureName(), "SUCCESS")); + listener.onResponse(new ResetFeatureStateResponse.ResetFeatureStateStatus(getFeatureName(), "SUCCESS")); } @Override public void onFailure(Exception e) { - listener.onResponse(new ResetFeatureStateResponse.Item(getFeatureName(), "FAILURE")); + listener.onResponse(new ResetFeatureStateResponse.ResetFeatureStateStatus(getFeatureName(), "FAILURE")); } }); } diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponseTests.java index c0c105f8030cd..7dc7948ccc1b8 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponseTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponseTests.java @@ -42,7 +42,7 @@ protected ResetFeatureStateResponse mutateInstance(ResetFeatureStateResponse ins .map(item -> item.getFeatureName()) .collect(Collectors.toSet()); return new ResetFeatureStateResponse(randomList(minSize, 10, - () -> new ResetFeatureStateResponse.Item( + () -> new ResetFeatureStateResponse.ResetFeatureStateStatus( randomValueOtherThanMany(existingFeatureNames::contains, () -> randomAlphaOfLengthBetween(4, 10)), randomAlphaOfLengthBetween(5, 10)))); } From e2af327d45e3e49665b2932f19eab8b646ae96c3 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Wed, 10 Feb 2021 17:18:14 -0500 Subject: [PATCH 12/25] Add a second plugin to integration tests --- .../snapshots/SystemIndexResetApiIT.java | 61 +++++++++++++++---- .../features/ResetFeatureStateResponse.java | 10 +-- .../TransportResetFeatureStateAction.java | 16 ++--- .../ResetFeatureStateResponseTests.java | 18 +++--- 4 files changed, 71 insertions(+), 34 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java index 73d261fff3d4a..f2b766c626891 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java @@ -25,7 +25,7 @@ import java.util.List; import static org.hamcrest.Matchers.arrayContaining; -import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; public class SystemIndexResetApiIT extends ESIntegTestCase { @@ -34,21 +34,27 @@ public class SystemIndexResetApiIT extends ESIntegTestCase { protected Collection> nodePlugins() { List> plugins = new ArrayList<>(super.nodePlugins()); plugins.add(SystemIndexTestPlugin.class); + plugins.add(SecondSystemIndexTestPlugin.class); return plugins; } /** Check that the reset method cleans up a feature */ public void testResetSystemIndices() throws Exception { - String sysindex = ".test-system-idx-1"; - String associndex = ".associated-idx-1"; + String systemIndex1 = ".test-system-idx-1"; + String systemIndex2 = ".second-test-system-idx-1"; + String associatedIndex = ".associated-idx-1"; // put a document in a system index - indexDoc(sysindex, "1", "purpose", "system index doc"); - refresh(sysindex); + indexDoc(systemIndex1, "1", "purpose", "system index doc"); + refresh(systemIndex1); + + // put a document in a second system index + indexDoc(systemIndex2, "1", "purpose", "second system index doc"); + refresh(systemIndex2); // put a document in associated index - indexDoc(associndex, "1", "purpose", "associated index doc"); - refresh(associndex); + indexDoc(associatedIndex, "1", "purpose", "associated index doc"); + refresh(associatedIndex); // put a document in a normal index indexDoc("my_index", "1", "purpose", "normal index doc"); @@ -56,22 +62,30 @@ public void testResetSystemIndices() throws Exception { // call the reset API ResetFeatureStateResponse apiResponse = client().execute(ResetFeatureStateAction.INSTANCE, new ResetFeatureStateRequest()).get(); - assertThat(apiResponse.getItemList(), contains( - new ResetFeatureStateResponse.ResetFeatureStateStatus("SystemIndexTestPlugin", "SUCCESS"))); + assertThat(apiResponse.getItemList(), containsInAnyOrder( + new ResetFeatureStateResponse.ResetFeatureStateStatus("SystemIndexTestPlugin", "SUCCESS"), + new ResetFeatureStateResponse.ResetFeatureStateStatus("SecondSystemIndexTestPlugin", "SUCCESS") + )); // verify that both indices are gone Exception e1 = expectThrows(IndexNotFoundException.class, () -> client().admin().indices().prepareGetIndex() - .addIndices(sysindex) + .addIndices(systemIndex1) .get()); assertThat(e1.getMessage(), containsString("no such index")); Exception e2 = expectThrows(IndexNotFoundException.class, () -> client().admin().indices().prepareGetIndex() - .addIndices(associndex) + .addIndices(associatedIndex) .get()); assertThat(e2.getMessage(), containsString("no such index")); + Exception e3 = expectThrows(IndexNotFoundException.class, () -> client().admin().indices().prepareGetIndex() + .addIndices(systemIndex2) + .get()); + + assertThat(e3.getMessage(), containsString("no such index")); + GetIndexResponse response = client().admin().indices().prepareGetIndex() .addIndices("my_index") .get(); @@ -99,7 +113,7 @@ public Collection getAssociatedIndexPatterns() { @Override public String getFeatureName() { - return SystemIndicesSnapshotIT.SystemIndexTestPlugin.class.getSimpleName(); + return SystemIndexTestPlugin.class.getSimpleName(); } @Override @@ -107,4 +121,27 @@ public String getFeatureDescription() { return "A simple test plugin"; } } + + /** + * A second test plugin with a patterns for system indices. + */ + public static class SecondSystemIndexTestPlugin extends Plugin implements SystemIndexPlugin { + + public static final String SYSTEM_INDEX_PATTERN = ".second-test-system-idx*"; + + @Override + public Collection getSystemIndexDescriptors(Settings settings) { + return Collections.singletonList(new SystemIndexDescriptor(SYSTEM_INDEX_PATTERN, "System indices for tests")); + } + + @Override + public String getFeatureName() { + return SecondSystemIndexTestPlugin.class.getSimpleName(); + } + + @Override + public String getFeatureDescription() { + return "A second test plugin"; + } + } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java index 74fa9e73f95bc..68710c8b9129f 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java @@ -17,8 +17,8 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.Comparator; import java.util.List; -import java.util.Map; import java.util.Objects; /** Response to a feature state reset request. */ @@ -26,16 +26,10 @@ public class ResetFeatureStateResponse extends ActionResponse implements ToXCont List resetFeatureStateStatusList; - public ResetFeatureStateResponse(Map statusMap) { - resetFeatureStateStatusList = new ArrayList<>(); - for (Map.Entry entry : statusMap.entrySet()) { - resetFeatureStateStatusList.add(new ResetFeatureStateStatus(entry.getKey(), entry.getValue())); - } - } - public ResetFeatureStateResponse(List statusList) { resetFeatureStateStatusList = new ArrayList<>(); resetFeatureStateStatusList.addAll(statusList); + resetFeatureStateStatusList.sort(Comparator.comparing(ResetFeatureStateStatus::getFeatureName)); } public ResetFeatureStateResponse(StreamInput in) throws IOException { diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java index 303caf84c4907..2bc98c52c06ca 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java @@ -19,9 +19,9 @@ import org.elasticsearch.tasks.Task; import org.elasticsearch.transport.TransportService; +import java.util.ArrayList; import java.util.Collections; -import java.util.SortedMap; -import java.util.TreeMap; +import java.util.List; /** * Transport action for cleaning up feature index state. @@ -54,22 +54,24 @@ protected void doExecute( ActionListener listener) { final int features = systemIndices.getFeatures().size(); - final CountDown completionCounter = new CountDown(features - 1); - // TODO[wrb] use concurrent list rather than map - final SortedMap responses = Collections.synchronizedSortedMap(new TreeMap<>()); + final CountDown completionCounter = new CountDown(features); + final List resetStatusList = Collections.synchronizedList(new ArrayList<>()); final Runnable terminalHandler = () -> { if (completionCounter.countDown()) { - listener.onResponse(new ResetFeatureStateResponse(responses)); + listener.onResponse(new ResetFeatureStateResponse(resetStatusList)); } }; for (SystemIndices.Feature feature : systemIndices.getFeatures().values()) { feature.getCleanUpFunction().apply(clusterService, client, ActionListener.wrap( response -> { - responses.put(response.getFeatureName(), response.getStatus()); + resetStatusList.add(new ResetFeatureStateResponse.ResetFeatureStateStatus(response.getFeatureName(), + response.getStatus())); terminalHandler.run(); }, failure -> { terminalHandler.run(); } )); } + + terminalHandler.run(); } } diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponseTests.java index 7dc7948ccc1b8..b326b07e20994 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponseTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponseTests.java @@ -12,8 +12,8 @@ import org.elasticsearch.test.AbstractWireSerializingTestCase; import java.io.IOException; -import java.util.HashMap; -import java.util.Map; +import java.util.ArrayList; +import java.util.List; import java.util.Set; import java.util.stream.Collectors; @@ -26,10 +26,14 @@ protected Writeable.Reader instanceReader() { @Override protected ResetFeatureStateResponse createTestInstance() { - Map responses = new HashMap<>(); - responses.put("feature_one", randomFrom("SUCCESS", "FAILURE")); - responses.put("feature_two", randomFrom("SUCCESS", "FAILURE")); - return new ResetFeatureStateResponse(responses); + List resetStatuses = new ArrayList<>(); + String feature1 = randomAlphaOfLengthBetween(4, 10); + String feature2 = randomValueOtherThan(feature1, () -> randomAlphaOfLengthBetween(4, 10)); + resetStatuses.add(new ResetFeatureStateResponse.ResetFeatureStateStatus( + feature1, randomFrom("SUCCESS", "FAILURE"))); + resetStatuses.add(new ResetFeatureStateResponse.ResetFeatureStateStatus( + feature2, randomFrom("SUCCESS", "FAILURE"))); + return new ResetFeatureStateResponse(resetStatuses); } @Override @@ -39,7 +43,7 @@ protected ResetFeatureStateResponse mutateInstance(ResetFeatureStateResponse ins minSize = 1; } Set existingFeatureNames = instance.getItemList().stream() - .map(item -> item.getFeatureName()) + .map(ResetFeatureStateResponse.ResetFeatureStateStatus::getFeatureName) .collect(Collectors.toSet()); return new ResetFeatureStateResponse(randomList(minSize, 10, () -> new ResetFeatureStateResponse.ResetFeatureStateStatus( From 1e4c3bd2a0725e986a288d6b8a1ec3e9e1af313e Mon Sep 17 00:00:00 2001 From: William Brafford Date: Thu, 11 Feb 2021 17:36:59 -0500 Subject: [PATCH 13/25] Clean up code by using a GroupedActionListener --- .../snapshots/SystemIndexResetApiIT.java | 3 +- .../TransportResetFeatureStateAction.java | 31 ++++++++----------- .../elasticsearch/indices/SystemIndices.java | 3 +- 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java index f2b766c626891..f58457b03ca65 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java @@ -64,7 +64,8 @@ public void testResetSystemIndices() throws Exception { ResetFeatureStateResponse apiResponse = client().execute(ResetFeatureStateAction.INSTANCE, new ResetFeatureStateRequest()).get(); assertThat(apiResponse.getItemList(), containsInAnyOrder( new ResetFeatureStateResponse.ResetFeatureStateStatus("SystemIndexTestPlugin", "SUCCESS"), - new ResetFeatureStateResponse.ResetFeatureStateStatus("SecondSystemIndexTestPlugin", "SUCCESS") + new ResetFeatureStateResponse.ResetFeatureStateStatus("SecondSystemIndexTestPlugin", "SUCCESS"), + new ResetFeatureStateResponse.ResetFeatureStateStatus("tasks", "SUCCESS") )); // verify that both indices are gone diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java index 2bc98c52c06ca..488c29f339d74 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/TransportResetFeatureStateAction.java @@ -10,18 +10,17 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.action.support.GroupedActionListener; import org.elasticsearch.action.support.HandledTransportAction; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.common.util.concurrent.CountDown; import org.elasticsearch.indices.SystemIndices; import org.elasticsearch.tasks.Task; import org.elasticsearch.transport.TransportService; import java.util.ArrayList; import java.util.Collections; -import java.util.List; /** * Transport action for cleaning up feature index state. @@ -53,25 +52,21 @@ protected void doExecute( ResetFeatureStateRequest request, ActionListener listener) { + if (systemIndices.getFeatures().size() == 0) { + listener.onResponse(new ResetFeatureStateResponse(Collections.emptyList())); + } + final int features = systemIndices.getFeatures().size(); - final CountDown completionCounter = new CountDown(features); - final List resetStatusList = Collections.synchronizedList(new ArrayList<>()); - final Runnable terminalHandler = () -> { - if (completionCounter.countDown()) { - listener.onResponse(new ResetFeatureStateResponse(resetStatusList)); - } - }; + GroupedActionListener groupedActionListener = new GroupedActionListener<>( + listener.map(responses -> { + assert features == responses.size(); + return new ResetFeatureStateResponse(new ArrayList<>(responses)); + }), + systemIndices.getFeatures().size() + ); for (SystemIndices.Feature feature : systemIndices.getFeatures().values()) { - feature.getCleanUpFunction().apply(clusterService, client, ActionListener.wrap( - response -> { - resetStatusList.add(new ResetFeatureStateResponse.ResetFeatureStateStatus(response.getFeatureName(), - response.getStatus())); - terminalHandler.run(); - }, failure -> { terminalHandler.run(); } - )); + feature.getCleanUpFunction().apply(clusterService, client, groupedActionListener); } - - terminalHandler.run(); } } diff --git a/server/src/main/java/org/elasticsearch/indices/SystemIndices.java b/server/src/main/java/org/elasticsearch/indices/SystemIndices.java index 607d072fe6e17..f2e6b2bdbadc8 100644 --- a/server/src/main/java/org/elasticsearch/indices/SystemIndices.java +++ b/server/src/main/java/org/elasticsearch/indices/SystemIndices.java @@ -221,7 +221,8 @@ public Feature( } public Feature(String description, Collection indexDescriptors) { - this(description, indexDescriptors, Collections.emptyList(), (a, b, c) -> {}); + this(description, indexDescriptors, Collections.emptyList(), + (clusterService, client, listener) -> listener.onResponse(new ResetFeatureStateStatus(TASKS_FEATURE_NAME, "SUCCESS"))); } public String getDescription() { From 237bc63e4ddc087d6a7f0c3511f9e23fa116eb9d Mon Sep 17 00:00:00 2001 From: William Brafford Date: Tue, 23 Feb 2021 15:09:20 -0500 Subject: [PATCH 14/25] Add reset to list of non-operator actions --- .../org/elasticsearch/xpack/security/operator/Constants.java | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/Constants.java b/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/Constants.java index 9784c203ba1f2..6212524913b04 100644 --- a/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/Constants.java +++ b/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/Constants.java @@ -82,6 +82,7 @@ public class Constants { "cluster:admin/snapshot/status", "cluster:admin/snapshot/status[nodes]", "cluster:admin/snapshot/features/get", + "cluster:admin/snapshot/features/reset", "cluster:admin/tasks/cancel", "cluster:admin/transform/delete", "cluster:admin/transform/preview", From 02a5937aee264792f7512cb1c014f633e6cfb894 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Wed, 24 Feb 2021 16:38:56 -0500 Subject: [PATCH 15/25] Consolidate default deletion logic in SystemIndices.feature --- .../elasticsearch/indices/SystemIndices.java | 57 +++++++++++++++++-- .../java/org/elasticsearch/node/Node.java | 3 +- .../plugins/SystemIndexPlugin.java | 41 ++++--------- .../get/TransportGetAliasesActionTests.java | 2 +- .../action/bulk/TransportBulkActionTests.java | 2 +- .../action/support/AutoCreateIndexTests.java | 2 +- .../MetadataCreateIndexServiceTests.java | 3 +- .../indices/SystemIndexManagerTests.java | 10 ++-- .../indices/SystemIndicesTests.java | 21 +++---- 9 files changed, 86 insertions(+), 55 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/indices/SystemIndices.java b/server/src/main/java/org/elasticsearch/indices/SystemIndices.java index f2e6b2bdbadc8..7ecc10ee70654 100644 --- a/server/src/main/java/org/elasticsearch/indices/SystemIndices.java +++ b/server/src/main/java/org/elasticsearch/indices/SystemIndices.java @@ -15,6 +15,9 @@ import org.apache.lucene.util.automaton.Operations; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateResponse.ResetFeatureStateStatus; +import org.elasticsearch.action.admin.indices.delete.DeleteIndexAction; +import org.elasticsearch.action.admin.indices.delete.DeleteIndexRequest; +import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.client.Client; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Nullable; @@ -23,6 +26,7 @@ import org.elasticsearch.index.Index; import org.elasticsearch.snapshots.SnapshotsService; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.Comparator; @@ -31,6 +35,7 @@ import java.util.Map; import java.util.Optional; import java.util.stream.Collectors; +import java.util.stream.Stream; import static java.util.stream.Collectors.toUnmodifiableList; import static org.elasticsearch.tasks.TaskResultsService.TASKS_DESCRIPTOR; @@ -43,7 +48,7 @@ */ public class SystemIndices { private static final Map SERVER_SYSTEM_INDEX_DESCRIPTORS = Map.of( - TASKS_FEATURE_NAME, new Feature("Manages task results", List.of(TASKS_DESCRIPTOR)) + TASKS_FEATURE_NAME, new Feature(TASKS_FEATURE_NAME, "Manages task results", List.of(TASKS_DESCRIPTOR)) ); private final CharacterRunAutomaton runAutomaton; @@ -204,25 +209,31 @@ public static void validateFeatureName(String name, String plugin) { } public static class Feature { + private final String name; private final String description; private final Collection indexDescriptors; private final Collection associatedIndexPatterns; private final TriConsumer> cleanUpFunction; public Feature( + String name, String description, Collection indexDescriptors, Collection associatedIndexPatterns, TriConsumer> cleanUpFunction) { + this.name = name; this.description = description; this.indexDescriptors = indexDescriptors; this.associatedIndexPatterns = associatedIndexPatterns; this.cleanUpFunction = cleanUpFunction; } - public Feature(String description, Collection indexDescriptors) { - this(description, indexDescriptors, Collections.emptyList(), - (clusterService, client, listener) -> listener.onResponse(new ResetFeatureStateStatus(TASKS_FEATURE_NAME, "SUCCESS"))); + public Feature(String name, String description, Collection indexDescriptors) { + this(name, description, indexDescriptors, Collections.emptyList(), + (clusterService, client, listener) -> { + cleanUpFeature(indexDescriptors, Collections.emptyList(), name, clusterService, client, listener); + } + ); } public String getDescription() { @@ -240,5 +251,43 @@ public Collection getAssociatedIndexPatterns() { public TriConsumer> getCleanUpFunction() { return cleanUpFunction; } + + public static void cleanUpFeature( + Collection indexDescriptors, + Collection associatedIndexPatterns, + String name, + ClusterService clusterService, + Client client, + ActionListener listener) { + List systemIndices = indexDescriptors.stream() + .map(sid -> sid.getMatchingIndices(clusterService.state().getMetadata())) + .flatMap(List::stream) + .collect(Collectors.toList()); + + List associatedIndices = new ArrayList<>(associatedIndexPatterns); + + List allIndices = Stream.concat(systemIndices.stream(), associatedIndices.stream()) + .collect(Collectors.toList()); + + if (allIndices.isEmpty()) { + // if no actual indices match the pattern, we can stop here + listener.onResponse(new ResetFeatureStateStatus(name, "SUCCESS")); + return; + } + + DeleteIndexRequest deleteIndexRequest = new DeleteIndexRequest(); + deleteIndexRequest.indices(allIndices.toArray(String[]::new)); + client.execute(DeleteIndexAction.INSTANCE, deleteIndexRequest, new ActionListener<>() { + @Override + public void onResponse(AcknowledgedResponse acknowledgedResponse) { + listener.onResponse(new ResetFeatureStateStatus(name, "SUCCESS")); + } + + @Override + public void onFailure(Exception e) { + listener.onResponse(new ResetFeatureStateStatus(name, "FAILURE: " + e.getMessage())); + } + }); + } } } diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index ad2e5b884704f..2654401cba7f7 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -500,8 +500,9 @@ protected Node(final Environment initialEnvironment, .stream() .peek(plugin -> SystemIndices.validateFeatureName(plugin.getFeatureName(), plugin.getClass().getCanonicalName())) .collect(Collectors.toUnmodifiableMap( - plugin -> plugin.getFeatureName(), + SystemIndexPlugin::getFeatureName, plugin -> new SystemIndices.Feature( + plugin.getFeatureName(), plugin.getFeatureDescription(), plugin.getSystemIndexDescriptors(settings), plugin.getAssociatedIndexPatterns(), diff --git a/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java b/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java index 2886f71171eee..9724b7eeb182c 100644 --- a/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java @@ -10,20 +10,14 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateResponse; -import org.elasticsearch.action.admin.indices.delete.DeleteIndexAction; -import org.elasticsearch.action.admin.indices.delete.DeleteIndexRequest; -import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.client.Client; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.indices.SystemIndexDescriptor; +import org.elasticsearch.indices.SystemIndices; -import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.List; -import java.util.stream.Collectors; -import java.util.stream.Stream; /** * Plugin for defining system indices. Extends {@link ActionPlugin} because system indices must be accessed via APIs @@ -66,34 +60,19 @@ default Collection getAssociatedIndexPatterns() { * Override to do more for cleanup (e.g. cancelling tasks). * @param clusterService Cluster service to provide cluster state * @param client A client, for executing actions - * @param listener Listener for post-cleanup result TODO[wrb]: need to gather results over features + * @param listener Listener for post-cleanup result */ default void cleanUpFeature( ClusterService clusterService, Client client, ActionListener listener) { - List systemIndices = getSystemIndexDescriptors(clusterService.getSettings()).stream() - .map(sid -> sid.getMatchingIndices(clusterService.state().getMetadata())) - .flatMap(List::stream) - .collect(Collectors.toList()); - - List associatedIndices = new ArrayList<>(getAssociatedIndexPatterns()); - - List allIndices = Stream.concat(systemIndices.stream(), associatedIndices.stream()) - .collect(Collectors.toList()); - - DeleteIndexRequest deleteIndexRequest = new DeleteIndexRequest(); - deleteIndexRequest.indices(allIndices.toArray(String[]::new)); - client.execute(DeleteIndexAction.INSTANCE, deleteIndexRequest, new ActionListener<>() { - @Override - public void onResponse(AcknowledgedResponse acknowledgedResponse) { - listener.onResponse(new ResetFeatureStateResponse.ResetFeatureStateStatus(getFeatureName(), "SUCCESS")); - } - - @Override - public void onFailure(Exception e) { - listener.onResponse(new ResetFeatureStateResponse.ResetFeatureStateStatus(getFeatureName(), "FAILURE")); - } - }); + SystemIndices.Feature.cleanUpFeature( + getSystemIndexDescriptors(clusterService.getSettings()), + getAssociatedIndexPatterns(), + getFeatureName(), + clusterService, + client, + listener + ); } } diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesActionTests.java index e85cafdca2fa0..92320bf8382f9 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesActionTests.java @@ -153,7 +153,7 @@ public void testDeprecationWarningEmittedWhenRequestingNonExistingAliasInSystemP ClusterState state = systemIndexTestClusterState(); SystemIndices systemIndices = new SystemIndices(Collections.singletonMap( this.getTestName(), - new SystemIndices.Feature("test feature", + new SystemIndices.Feature(this.getTestName(), "test feature", Collections.singletonList(new SystemIndexDescriptor(".y", "an index that doesn't exist"))))); GetAliasesRequest request = new GetAliasesRequest(".y"); diff --git a/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTests.java b/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTests.java index 000b1ce3b4e14..74a5ffc8a3701 100644 --- a/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTests.java @@ -244,7 +244,7 @@ public void testOnlySystem() { indicesLookup.put(".bar", new Index(IndexMetadata.builder(".bar").settings(settings).system(true).numberOfShards(1).numberOfReplicas(0).build())); SystemIndices systemIndices = new SystemIndices( - Map.of("plugin", new SystemIndices.Feature("test feature", List.of(new SystemIndexDescriptor(".test", ""))))); + Map.of("plugin", new SystemIndices.Feature("plugin", "test feature", List.of(new SystemIndexDescriptor(".test", ""))))); List onlySystem = List.of(".foo", ".bar"); assertTrue(bulkAction.isOnlySystem(buildBulkRequest(onlySystem), indicesLookup, systemIndices)); diff --git a/server/src/test/java/org/elasticsearch/action/support/AutoCreateIndexTests.java b/server/src/test/java/org/elasticsearch/action/support/AutoCreateIndexTests.java index 808eaa6d614e4..78c7408d2e9b9 100644 --- a/server/src/test/java/org/elasticsearch/action/support/AutoCreateIndexTests.java +++ b/server/src/test/java/org/elasticsearch/action/support/AutoCreateIndexTests.java @@ -301,7 +301,7 @@ private static ClusterState buildClusterState(String... indices) { private AutoCreateIndex newAutoCreateIndex(Settings settings) { SystemIndices systemIndices = new SystemIndices(Map.of( - "plugin", new SystemIndices.Feature("test feature", List.of(new SystemIndexDescriptor(TEST_SYSTEM_INDEX_NAME, ""))))); + "plugin", new SystemIndices.Feature("plugin", "test feature", List.of(new SystemIndexDescriptor(TEST_SYSTEM_INDEX_NAME, ""))))); return new AutoCreateIndex(settings, new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), new IndexNameExpressionResolver(new ThreadContext(Settings.EMPTY)), systemIndices); } diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexServiceTests.java index 0f2e77fb18b54..2c45512a01a38 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexServiceTests.java @@ -517,7 +517,8 @@ public void testValidateDotIndex() { null, threadPool, null, - new SystemIndices(Collections.singletonMap("foo", new SystemIndices.Feature("test feature", systemIndexDescriptors))), + new SystemIndices(Collections.singletonMap("foo", new SystemIndices.Feature("foo", "test feature", + systemIndexDescriptors))), false ); // Check deprecations diff --git a/server/src/test/java/org/elasticsearch/indices/SystemIndexManagerTests.java b/server/src/test/java/org/elasticsearch/indices/SystemIndexManagerTests.java index 32e8d8afc8d8f..ea94a08abd108 100644 --- a/server/src/test/java/org/elasticsearch/indices/SystemIndexManagerTests.java +++ b/server/src/test/java/org/elasticsearch/indices/SystemIndexManagerTests.java @@ -72,7 +72,7 @@ public class SystemIndexManagerTests extends ESTestCase { .setOrigin("FAKE_ORIGIN") .build(); - private static final SystemIndices.Feature FEATURE = new SystemIndices.Feature("a test feature", List.of(DESCRIPTOR)); + private static final SystemIndices.Feature FEATURE = new SystemIndices.Feature("foo", "a test feature", List.of(DESCRIPTOR)); private Client client; @@ -101,8 +101,8 @@ public void testManagerSkipsDescriptorsThatAreNotManaged() { .build(); SystemIndices systemIndices = new SystemIndices(Map.of( - "index 1", new SystemIndices.Feature("index 1 feature", List.of(d1)), - "index 2", new SystemIndices.Feature("index 2 feature", List.of(d2)))); + "index 1", new SystemIndices.Feature("index 1", "index 1 feature", List.of(d1)), + "index 2", new SystemIndices.Feature("index 2", "index 2 feature", List.of(d2)))); SystemIndexManager manager = new SystemIndexManager(systemIndices, client); final List eligibleDescriptors = manager.getEligibleDescriptors( @@ -139,8 +139,8 @@ public void testManagerSkipsDescriptorsForIndicesThatDoNotExist() { .build(); SystemIndices systemIndices = new SystemIndices(Map.of( - "index 1", new SystemIndices.Feature("index 1 feature", List.of(d1)), - "index 2", new SystemIndices.Feature("index 2 feature", List.of(d2))));; + "index 1", new SystemIndices.Feature("index 1", "index 1 feature", List.of(d1)), + "index 2", new SystemIndices.Feature("index 2", "index 2 feature", List.of(d2))));; SystemIndexManager manager = new SystemIndexManager(systemIndices, client); final List eligibleDescriptors = manager.getEligibleDescriptors( diff --git a/server/src/test/java/org/elasticsearch/indices/SystemIndicesTests.java b/server/src/test/java/org/elasticsearch/indices/SystemIndicesTests.java index 23b48b4284035..999f09b9d1f60 100644 --- a/server/src/test/java/org/elasticsearch/indices/SystemIndicesTests.java +++ b/server/src/test/java/org/elasticsearch/indices/SystemIndicesTests.java @@ -34,9 +34,9 @@ public void testBasicOverlappingPatterns() { String broadPatternSource = "AAA" + randomAlphaOfLength(5); String otherSource = "ZZZ" + randomAlphaOfLength(6); Map descriptors = new HashMap<>(); - descriptors.put(broadPatternSource, new SystemIndices.Feature("test feature", List.of(broadPattern))); + descriptors.put(broadPatternSource, new SystemIndices.Feature(broadPatternSource, "test feature", List.of(broadPattern))); descriptors.put(otherSource, - new SystemIndices.Feature("test 2", List.of(notOverlapping, overlapping1, overlapping2, overlapping3))); + new SystemIndices.Feature(otherSource, "test 2", List.of(notOverlapping, overlapping1, overlapping2, overlapping3))); IllegalStateException exception = expectThrows(IllegalStateException.class, () -> SystemIndices.checkForOverlappingPatterns(descriptors)); @@ -62,8 +62,8 @@ public void testComplexOverlappingPatterns() { String source1 = "AAA" + randomAlphaOfLength(5); String source2 = "ZZZ" + randomAlphaOfLength(6); Map descriptors = new HashMap<>(); - descriptors.put(source1, new SystemIndices.Feature("test", List.of(pattern1))); - descriptors.put(source2, new SystemIndices.Feature("test", List.of(pattern2))); + descriptors.put(source1, new SystemIndices.Feature(source1, "test", List.of(pattern1))); + descriptors.put(source2, new SystemIndices.Feature(source2, "test", List.of(pattern2))); IllegalStateException exception = expectThrows(IllegalStateException.class, () -> SystemIndices.checkForOverlappingPatterns(descriptors)); @@ -84,7 +84,8 @@ public void testBuiltInSystemIndices() { public void testPluginCannotOverrideBuiltInSystemIndex() { Map pluginMap = Map.of( - TASKS_FEATURE_NAME, new SystemIndices.Feature("test", List.of(new SystemIndexDescriptor(TASK_INDEX, "Task Result Index"))) + TASKS_FEATURE_NAME, new SystemIndices.Feature(TASKS_FEATURE_NAME, "test", List.of(new SystemIndexDescriptor(TASK_INDEX, "Task" + + " Result Index"))) ); IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new SystemIndices(pluginMap)); assertThat(e.getMessage(), containsString("plugin or module attempted to define the same source")); @@ -93,7 +94,7 @@ public void testPluginCannotOverrideBuiltInSystemIndex() { public void testPatternWithSimpleRange() { final SystemIndices systemIndices = new SystemIndices(Map.of( - "test", new SystemIndices.Feature("test feature", List.of(new SystemIndexDescriptor(".test-[abc]", ""))) + "test", new SystemIndices.Feature("test", "test feature", List.of(new SystemIndexDescriptor(".test-[abc]", ""))) )); assertThat(systemIndices.isSystemIndex(".test-a"), equalTo(true)); @@ -108,7 +109,7 @@ public void testPatternWithSimpleRange() { public void testPatternWithSimpleRangeAndRepeatOperator() { final SystemIndices systemIndices = new SystemIndices(Map.of( - "test", new SystemIndices.Feature("test feature", List.of(new SystemIndexDescriptor(".test-[a]+", ""))) + "test", new SystemIndices.Feature("test", "test feature", List.of(new SystemIndexDescriptor(".test-[a]+", ""))) )); assertThat(systemIndices.isSystemIndex(".test-a"), equalTo(true)); @@ -120,7 +121,7 @@ public void testPatternWithSimpleRangeAndRepeatOperator() { public void testPatternWithComplexRange() { final SystemIndices systemIndices = new SystemIndices(Map.of( - "test", new SystemIndices.Feature("test feature", List.of(new SystemIndexDescriptor(".test-[a-c]", ""))) + "test", new SystemIndices.Feature("test", "test feature", List.of(new SystemIndexDescriptor(".test-[a-c]", ""))) )); assertThat(systemIndices.isSystemIndex(".test-a"), equalTo(true)); @@ -141,8 +142,8 @@ public void testOverlappingDescriptorsWithRanges() { SystemIndexDescriptor pattern2 = new SystemIndexDescriptor(".test-a*", ""); Map descriptors = new HashMap<>(); - descriptors.put(source1, new SystemIndices.Feature("source 1", List.of(pattern1))); - descriptors.put(source2, new SystemIndices.Feature("source 2", List.of(pattern2))); + descriptors.put(source1, new SystemIndices.Feature(source1, "source 1", List.of(pattern1))); + descriptors.put(source2, new SystemIndices.Feature(source2, "source 2", List.of(pattern2))); IllegalStateException exception = expectThrows(IllegalStateException.class, () -> SystemIndices.checkForOverlappingPatterns(descriptors)); From 83817e4d917c10eb2d7c0a8f2a7357173273c49c Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Tue, 23 Feb 2021 14:00:11 -0700 Subject: [PATCH 16/25] ML feature state cleaner --- .../cluster/RestResetFeatureStateAction.java | 8 ++- .../xpack/ml/MachineLearning.java | 60 +++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java index edeb6c0d96b77..a09db0686c515 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java @@ -8,8 +8,8 @@ package org.elasticsearch.rest.action.admin.cluster; +import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateAction; import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateRequest; -import org.elasticsearch.action.admin.cluster.snapshots.features.SnapshottableFeaturesAction; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestRequest; @@ -20,6 +20,10 @@ public class RestResetFeatureStateAction extends BaseRestHandler { + @Override public boolean allowSystemIndexAccessByDefault() { + return true; + } + @Override public List routes() { return List.of(new Route(RestRequest.Method.POST, "/_reset_feature_state")); @@ -35,7 +39,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli final ResetFeatureStateRequest req = new ResetFeatureStateRequest(); return restChannel -> { - client.execute(SnapshottableFeaturesAction.INSTANCE, req, new RestToXContentListener<>(restChannel)); + client.execute(ResetFeatureStateAction.INSTANCE, req, new RestToXContentListener<>(restChannel)); }; } } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java index 095af50fc9106..5f042551f61ba 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java @@ -10,8 +10,10 @@ import org.apache.logging.log4j.Logger; import org.apache.lucene.util.SetOnce; import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateResponse; import org.elasticsearch.action.support.ActionFilter; import org.elasticsearch.client.Client; import org.elasticsearch.client.OriginSettingClient; @@ -361,8 +363,10 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.function.Supplier; import java.util.function.UnaryOperator; +import java.util.stream.Collectors; import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; @@ -1227,6 +1231,62 @@ public String getFeatureDescription() { return "Provides anomaly detection and forecasting functionality"; } + @Override public void cleanUpFeature( + ClusterService clusterService, + Client client, + ActionListener listener) { + + Map results = new ConcurrentHashMap<>(); + + ActionListener afterDataframesStopped = ActionListener.wrap(dataFrameStopResponse -> { + // Handle the response + results.put("data_frame/analytics", dataFrameStopResponse.isStopped()); + + if (results.values().stream().allMatch(b -> b)) { + // Call into the original listener to clean up the indices + SystemIndexPlugin.super.cleanUpFeature(clusterService, client, listener); + } else { + final List failedComponents = results.entrySet().stream() + .filter(result -> result.getValue() == false) + .map(Map.Entry::getKey) + .collect(Collectors.toList()); + listener.onFailure(new RuntimeException("Some components failed to reset: " + failedComponents)); + } + }, listener::onFailure); + + + ActionListener afterAnomalyDetectionClosed = ActionListener.wrap(closeJobResponse -> { + // Handle the response + results.put("anomaly_detectors", closeJobResponse.isClosed()); + + // Stop data frame analytics + StopDataFrameAnalyticsAction.Request stopDataFramesReq = new StopDataFrameAnalyticsAction.Request("_all"); + stopDataFramesReq.setForce(true); + stopDataFramesReq.setAllowNoMatch(true); + client.execute(StopDataFrameAnalyticsAction.INSTANCE, stopDataFramesReq, afterDataframesStopped); + }, listener::onFailure); + + // Close anomaly detection jobs + ActionListener afterDataFeedsStopped = ActionListener.wrap(datafeedResponse -> { + // Handle the response + results.put("datafeeds", datafeedResponse.isStopped()); + + // Close anomaly detection jobs + CloseJobAction.Request closeJobsRequest = new CloseJobAction.Request(); + closeJobsRequest.setForce(true); + closeJobsRequest.setAllowNoMatch(true); + closeJobsRequest.setJobId("_all"); + client.execute(CloseJobAction.INSTANCE, closeJobsRequest, afterAnomalyDetectionClosed); + }, listener::onFailure); + + // Stop data feeds + StopDatafeedAction.Request stopDatafeedsReq = new StopDatafeedAction.Request("_all"); + stopDatafeedsReq.setAllowNoMatch(true); + stopDatafeedsReq.setForce(true); + client.execute(StopDatafeedAction.INSTANCE, stopDatafeedsReq, + afterDataFeedsStopped); + } + @Override public BreakerSettings getCircuitBreaker(Settings settings) { return BreakerSettings.updateFromSettings( From 5ea27d6e0b2fd44f2455038f3415b18a2a86a695 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Thu, 25 Feb 2021 13:47:58 -0700 Subject: [PATCH 17/25] Implement Transform feature state reset --- .../xpack/transform/Transform.java | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/Transform.java b/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/Transform.java index 3c36c03186518..772fd52ffcc90 100644 --- a/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/Transform.java +++ b/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/Transform.java @@ -10,11 +10,14 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.lucene.util.SetOnce; +import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.action.admin.cluster.snapshots.features.ResetFeatureStateResponse; import org.elasticsearch.client.Client; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.metadata.IndexTemplateMetadata; +import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNodeRole; import org.elasticsearch.cluster.node.DiscoveryNodes; @@ -371,6 +374,35 @@ public Collection getSystemIndexDescriptors(Settings sett } } + @Override + public void cleanUpFeature( + ClusterService clusterService, + Client client, + ActionListener listener + ) { + ActionListener afterStoppingTransforms = ActionListener.wrap(stopTransformsResponse -> { + if (stopTransformsResponse.isAcknowledged() + && stopTransformsResponse.getTaskFailures().isEmpty() + && stopTransformsResponse.getNodeFailures().isEmpty()) { + + SystemIndexPlugin.super.cleanUpFeature(clusterService, client, listener); + } else { + String errMsg = "Failed to reset Transform: " + + (stopTransformsResponse.isAcknowledged() ? "" : "not acknowledged ") + + (stopTransformsResponse.getNodeFailures().isEmpty() + ? "" + : "node failures: " + stopTransformsResponse.getNodeFailures() + " ") + + (stopTransformsResponse.getTaskFailures().isEmpty() + ? "" + : "task failures: " + stopTransformsResponse.getTaskFailures()); + listener.onResponse(new ResetFeatureStateResponse.ResetFeatureStateStatus(this.getFeatureName(), errMsg)); + } + }, listener::onFailure); + + StopTransformAction.Request stopTransformsRequest = new StopTransformAction.Request(Metadata.ALL, true, true, null, true, false); + client.execute(StopTransformAction.INSTANCE, stopTransformsRequest, afterStoppingTransforms); + } + @Override public String getFeatureName() { return "transform"; From db50f4d669adfb04eb9445e21881c8919182ef7a Mon Sep 17 00:00:00 2001 From: William Brafford Date: Tue, 2 Mar 2021 14:57:12 -0500 Subject: [PATCH 18/25] Change endpoint to _feature/reset --- ...SystemIndexResetApiIT.java => FeatureStateResetApiIT.java} | 2 +- .../cluster/snapshots/features/ResetFeatureStateAction.java | 4 +--- .../action/admin/cluster/RestResetFeatureStateAction.java | 2 +- .../org/elasticsearch/xpack/security/operator/Constants.java | 2 +- 4 files changed, 4 insertions(+), 6 deletions(-) rename server/src/internalClusterTest/java/org/elasticsearch/snapshots/{SystemIndexResetApiIT.java => FeatureStateResetApiIT.java} (98%) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/FeatureStateResetApiIT.java similarity index 98% rename from server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java rename to server/src/internalClusterTest/java/org/elasticsearch/snapshots/FeatureStateResetApiIT.java index f58457b03ca65..24bddd4958215 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndexResetApiIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/FeatureStateResetApiIT.java @@ -28,7 +28,7 @@ import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; -public class SystemIndexResetApiIT extends ESIntegTestCase { +public class FeatureStateResetApiIT extends ESIntegTestCase { @Override protected Collection> nodePlugins() { diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateAction.java index 9e4141c10620c..6953fc00b481a 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateAction.java @@ -14,9 +14,7 @@ public class ResetFeatureStateAction extends ActionType { public static final ResetFeatureStateAction INSTANCE = new ResetFeatureStateAction(); - // TODO[wrb]: seems like a bad action name... - // are features getting bigger than snapshots as an abstraction? - public static final String NAME = "cluster:admin/snapshot/features/reset"; + public static final String NAME = "cluster:admin/features/reset"; private ResetFeatureStateAction() { super(NAME, ResetFeatureStateResponse::new); diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java index a09db0686c515..c899814c48a6d 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java @@ -26,7 +26,7 @@ public class RestResetFeatureStateAction extends BaseRestHandler { @Override public List routes() { - return List.of(new Route(RestRequest.Method.POST, "/_reset_feature_state")); + return List.of(new Route(RestRequest.Method.POST, "/_feature/reset")); } @Override diff --git a/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/Constants.java b/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/Constants.java index 13e99a8a31dd9..2e3eb18f384a8 100644 --- a/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/Constants.java +++ b/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/Constants.java @@ -81,8 +81,8 @@ public class Constants { "cluster:admin/snapshot/restore", "cluster:admin/snapshot/status", "cluster:admin/snapshot/status[nodes]", - "cluster:admin/snapshot/features/reset", "cluster:admin/features/get", + "cluster:admin/features/reset", "cluster:admin/tasks/cancel", "cluster:admin/transform/delete", "cluster:admin/transform/preview", From 6544c2554ab96e58b0aa4976e41d817b685d81b3 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Tue, 2 Mar 2021 16:52:42 -0500 Subject: [PATCH 19/25] Add javadoc and cleanup java style --- .../features/ResetFeatureStateResponse.java | 14 +++++- .../elasticsearch/indices/SystemIndices.java | 45 ++++++++++++++++--- .../java/org/elasticsearch/node/Node.java | 1 - .../cluster/RestResetFeatureStateAction.java | 5 +-- 4 files changed, 52 insertions(+), 13 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java index 68710c8b9129f..492b4f934f0d3 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateResponse.java @@ -26,6 +26,12 @@ public class ResetFeatureStateResponse extends ActionResponse implements ToXCont List resetFeatureStateStatusList; + /** + * Create a response showing which features have had state reset and success + * or failure status. + * + * @param statusList A list of status responses + */ public ResetFeatureStateResponse(List statusList) { resetFeatureStateStatusList = new ArrayList<>(); resetFeatureStateStatusList.addAll(statusList); @@ -80,9 +86,13 @@ public String toString() { '}'; } + /** + * An object with the name of a feature and a message indicating success or + * failure. + */ public static class ResetFeatureStateStatus implements Writeable, ToXContentObject { - private String featureName; - private String status; + private final String featureName; + private final String status; public ResetFeatureStateStatus(String featureName, String status) { this.featureName = featureName; diff --git a/server/src/main/java/org/elasticsearch/indices/SystemIndices.java b/server/src/main/java/org/elasticsearch/indices/SystemIndices.java index b57f38638f34a..d0c8deb8ca4db 100644 --- a/server/src/main/java/org/elasticsearch/indices/SystemIndices.java +++ b/server/src/main/java/org/elasticsearch/indices/SystemIndices.java @@ -58,6 +58,11 @@ TASKS_FEATURE_NAME, new Feature(TASKS_FEATURE_NAME, "Manages task results", List private final Map featureDescriptors; private final Map productToSystemIndicesMatcher; + /** + * Initialize the SystemIndices object + * @param pluginAndModulesDescriptors A map of this node's feature names to + * feature objects. + */ public SystemIndices(Map pluginAndModulesDescriptors) { featureDescriptors = buildSystemIndexDescriptorMap(pluginAndModulesDescriptors); checkForOverlappingPatterns(featureDescriptors); @@ -246,6 +251,11 @@ Collection getSystemIndexDescriptors() { .collect(Collectors.toList()); } + /** + * Check that a feature name is not reserved + * @param name Name of feature + * @param plugin Name of plugin providing the feature + */ public static void validateFeatureName(String name, String plugin) { if (SnapshotsService.NO_FEATURE_STATES_VALUE.equalsIgnoreCase(name)) { throw new IllegalArgumentException("feature name cannot be reserved name [\"" + SnapshotsService.NO_FEATURE_STATES_VALUE + @@ -253,31 +263,43 @@ public static void validateFeatureName(String name, String plugin) { } } + /** + * Class holding a description of a stateful feature. + */ public static class Feature { - private final String name; private final String description; private final Collection indexDescriptors; private final Collection associatedIndexPatterns; private final TriConsumer> cleanUpFunction; + /** + * Construct a Feature with a custom cleanup function + * @param description Description of the feature + * @param indexDescriptors Patterns describing system indices for this feature + * @param associatedIndexPatterns Patterns describing associated indices + * @param cleanUpFunction A function that will clean up the feature's state + */ public Feature( - String name, String description, Collection indexDescriptors, Collection associatedIndexPatterns, TriConsumer> cleanUpFunction) { - this.name = name; this.description = description; this.indexDescriptors = indexDescriptors; this.associatedIndexPatterns = associatedIndexPatterns; this.cleanUpFunction = cleanUpFunction; } + /** + * Construct a Feature using the default clean-up function + * @param name Name of the feature, used in logging + * @param description Description of the feature + * @param indexDescriptors Patterns describing system indices for this feature + */ public Feature(String name, String description, Collection indexDescriptors) { - this(name, description, indexDescriptors, Collections.emptyList(), - (clusterService, client, listener) -> { - cleanUpFeature(indexDescriptors, Collections.emptyList(), name, clusterService, client, listener); - } + this(description, indexDescriptors, Collections.emptyList(), + (clusterService, client, listener) -> + cleanUpFeature(indexDescriptors, Collections.emptyList(), name, clusterService, client, listener) ); } @@ -297,6 +319,15 @@ public TriConsumer indexDescriptors, Collection associatedIndexPatterns, diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index a452fe5fb0e83..9c44eea0052be 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -410,7 +410,6 @@ protected Node(final Environment initialEnvironment, .collect(Collectors.toUnmodifiableMap( plugin -> plugin.getFeatureName(), plugin -> new SystemIndices.Feature( - plugin.getFeatureName(), plugin.getFeatureDescription(), plugin.getSystemIndexDescriptors(settings), plugin.getAssociatedIndexPatterns(), diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java index c899814c48a6d..6219ec694db7a 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java @@ -18,6 +18,7 @@ import java.io.IOException; import java.util.List; +/** Rest handler for feature state reset requests */ public class RestResetFeatureStateAction extends BaseRestHandler { @Override public boolean allowSystemIndexAccessByDefault() { @@ -38,8 +39,6 @@ public String getName() { protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { final ResetFeatureStateRequest req = new ResetFeatureStateRequest(); - return restChannel -> { - client.execute(ResetFeatureStateAction.INSTANCE, req, new RestToXContentListener<>(restChannel)); - }; + return restChannel -> client.execute(ResetFeatureStateAction.INSTANCE, req, new RestToXContentListener<>(restChannel)); } } From 44d0917f6e86e8a26eb384bd78a04cf3f6588c13 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Wed, 3 Mar 2021 17:34:46 -0500 Subject: [PATCH 20/25] Add stub for API definition --- .../api/features.reset_features.json | 29 +++++++++++++++++++ .../test/features.reset_features/10_basic.yml | 8 +++++ 2 files changed, 37 insertions(+) create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/api/features.reset_features.json create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/features.reset_features/10_basic.yml diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/features.reset_features.json b/rest-api-spec/src/main/resources/rest-api-spec/api/features.reset_features.json new file mode 100644 index 0000000000000..cfe284c47b850 --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/features.reset_features.json @@ -0,0 +1,29 @@ +{ + "features.reset_features":{ + "documentation":{ + "url":"https://www.elastic.co/guide/en/elasticsearch/reference/master/modules-snapshots.html", + "description":"Resets the internal state of features, usually by deleting system indices" + }, + "stability":"experimental", + "visibility":"public", + "headers":{ + "accept": [ "application/json"] + }, + "url":{ + "paths":[ + { + "path":"/_features/reset", + "methods":[ + "POST" + ] + } + ] + }, + "params":{ + "master_timeout":{ + "type":"time", + "description":"Explicit operation timeout for connection to master node" + } + } + } +} diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/features.reset_features/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/features.reset_features/10_basic.yml new file mode 100644 index 0000000000000..5aa33ec5e4255 --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/features.reset_features/10_basic.yml @@ -0,0 +1,8 @@ +--- +"Get Features": + - skip: + features: contains + version: " - 7.99.99" # Adjust this after backport + reason: "This API was added in 7.13.0" + - do: { features.get_features: {}} + - contains: {'features': {'name': 'tasks'}} From 3369131d5098028b71d8e74f8b241c1c332c9d92 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Fri, 5 Mar 2021 17:36:41 -0500 Subject: [PATCH 21/25] Add stub for RHLC feature resetting --- .../elasticsearch/client/FeaturesClient.java | 32 +++++++++++++++++-- .../client/FeaturesRequestConverters.java | 14 +++++++- .../GetFeaturesRequest.java | 2 +- .../GetFeaturesResponse.java | 2 +- .../client/feature/ResetFeaturesRequest.java | 14 ++++++++ .../client/feature/ResetFeaturesResponse.java | 12 +++++++ .../org/elasticsearch/client/FeaturesIT.java | 4 +-- .../snapshots/GetFeaturesResponseTests.java | 1 + 8 files changed, 74 insertions(+), 7 deletions(-) rename client/rest-high-level/src/main/java/org/elasticsearch/client/{snapshots => feature}/GetFeaturesRequest.java (92%) rename client/rest-high-level/src/main/java/org/elasticsearch/client/{snapshots => feature}/GetFeaturesResponse.java (98%) create mode 100644 client/rest-high-level/src/main/java/org/elasticsearch/client/feature/ResetFeaturesRequest.java create mode 100644 client/rest-high-level/src/main/java/org/elasticsearch/client/feature/ResetFeaturesResponse.java diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/FeaturesClient.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/FeaturesClient.java index 4ba0613c8e6cd..59ce2dd8d7c4f 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/FeaturesClient.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/FeaturesClient.java @@ -9,8 +9,10 @@ package org.elasticsearch.client; import org.elasticsearch.action.ActionListener; -import org.elasticsearch.client.snapshots.GetFeaturesRequest; -import org.elasticsearch.client.snapshots.GetFeaturesResponse; +import org.elasticsearch.client.feature.GetFeaturesRequest; +import org.elasticsearch.client.feature.GetFeaturesResponse; +import org.elasticsearch.client.feature.ResetFeaturesRequest; +import org.elasticsearch.client.feature.ResetFeaturesResponse; import java.io.IOException; @@ -71,4 +73,30 @@ public Cancellable getFeaturesAsync( emptySet() ); } + + // reset features + public ResetFeaturesResponse resetFeatures(ResetFeaturesRequest resetFeaturesRequest, RequestOptions options) + throws IOException { + return restHighLevelClient.performRequestAndParseEntity( + resetFeaturesRequest, + FeaturesRequestConverters::resetFeatures, + options, + e -> new ResetFeaturesResponse(), // TODO[wrb]: actual parse method + emptySet() + ); + } + + // async reset features + public Cancellable resetFeaturesAsync( + ResetFeaturesRequest resetFeaturesRequest, RequestOptions options, + ActionListener listener) { + return restHighLevelClient.performRequestAsyncAndParseEntity( + resetFeaturesRequest, + FeaturesRequestConverters::resetFeatures, + options, + e -> new ResetFeaturesResponse(), // TODO[wrb]: actual parse method + listener, + emptySet() + ); + } } diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/FeaturesRequestConverters.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/FeaturesRequestConverters.java index 34dd1e095ba59..5de17e412dc38 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/FeaturesRequestConverters.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/FeaturesRequestConverters.java @@ -9,7 +9,9 @@ package org.elasticsearch.client; import org.apache.http.client.methods.HttpGet; -import org.elasticsearch.client.snapshots.GetFeaturesRequest; +import org.apache.http.client.methods.HttpPost; +import org.elasticsearch.client.feature.GetFeaturesRequest; +import org.elasticsearch.client.feature.ResetFeaturesRequest; public class FeaturesRequestConverters { @@ -23,4 +25,14 @@ static Request getFeatures(GetFeaturesRequest getFeaturesRequest) { request.addParameters(parameters.asMap()); return request; } + + // reset features post + static Request resetFeatures(ResetFeaturesRequest resetFeaturesRequest) { + String endpoint = "/_features/reset"; + Request request = new Request(HttpPost.METHOD_NAME, endpoint); + RequestConverters.Params parameters = new RequestConverters.Params(); + parameters.withMasterTimeout(resetFeaturesRequest.masterNodeTimeout()); + request.addParameters(parameters.asMap()); + return request; + } } diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/snapshots/GetFeaturesRequest.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/feature/GetFeaturesRequest.java similarity index 92% rename from client/rest-high-level/src/main/java/org/elasticsearch/client/snapshots/GetFeaturesRequest.java rename to client/rest-high-level/src/main/java/org/elasticsearch/client/feature/GetFeaturesRequest.java index 65f4826ed7977..71ff178585cf1 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/snapshots/GetFeaturesRequest.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/feature/GetFeaturesRequest.java @@ -6,7 +6,7 @@ * Side Public License, v 1. */ -package org.elasticsearch.client.snapshots; +package org.elasticsearch.client.feature; import org.elasticsearch.client.TimedRequest; diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/snapshots/GetFeaturesResponse.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/feature/GetFeaturesResponse.java similarity index 98% rename from client/rest-high-level/src/main/java/org/elasticsearch/client/snapshots/GetFeaturesResponse.java rename to client/rest-high-level/src/main/java/org/elasticsearch/client/feature/GetFeaturesResponse.java index 03c22e1018928..fb533da2e63bb 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/snapshots/GetFeaturesResponse.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/feature/GetFeaturesResponse.java @@ -6,7 +6,7 @@ * Side Public License, v 1. */ -package org.elasticsearch.client.snapshots; +package org.elasticsearch.client.feature; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.xcontent.ConstructingObjectParser; diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/feature/ResetFeaturesRequest.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/feature/ResetFeaturesRequest.java new file mode 100644 index 0000000000000..7e49a562c9a4e --- /dev/null +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/feature/ResetFeaturesRequest.java @@ -0,0 +1,14 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.client.feature; + +import org.elasticsearch.client.TimedRequest; + +public class ResetFeaturesRequest extends TimedRequest { +} diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/feature/ResetFeaturesResponse.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/feature/ResetFeaturesResponse.java new file mode 100644 index 0000000000000..e3c365d51cf7a --- /dev/null +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/feature/ResetFeaturesResponse.java @@ -0,0 +1,12 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.client.feature; + +public class ResetFeaturesResponse { +} diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/FeaturesIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/FeaturesIT.java index 42757d77db2a4..195bf4ee77f8c 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/FeaturesIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/FeaturesIT.java @@ -8,8 +8,8 @@ package org.elasticsearch.client; -import org.elasticsearch.client.snapshots.GetFeaturesRequest; -import org.elasticsearch.client.snapshots.GetFeaturesResponse; +import org.elasticsearch.client.feature.GetFeaturesRequest; +import org.elasticsearch.client.feature.GetFeaturesResponse; import java.io.IOException; diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/snapshots/GetFeaturesResponseTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/snapshots/GetFeaturesResponseTests.java index 09d9f508dd418..473587ea3d70b 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/snapshots/GetFeaturesResponseTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/snapshots/GetFeaturesResponseTests.java @@ -9,6 +9,7 @@ package org.elasticsearch.client.snapshots; import org.elasticsearch.client.AbstractResponseTestCase; +import org.elasticsearch.client.feature.GetFeaturesResponse; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; From b18ff947b6060b3b9b4773653742823d2a644cb1 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Wed, 10 Mar 2021 10:38:09 -0500 Subject: [PATCH 22/25] Implement client feature reset response parser --- .../elasticsearch/client/FeaturesClient.java | 4 +- .../client/feature/ResetFeaturesResponse.java | 68 +++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/FeaturesClient.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/FeaturesClient.java index 59ce2dd8d7c4f..17b000817bcfa 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/FeaturesClient.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/FeaturesClient.java @@ -81,7 +81,7 @@ public ResetFeaturesResponse resetFeatures(ResetFeaturesRequest resetFeaturesReq resetFeaturesRequest, FeaturesRequestConverters::resetFeatures, options, - e -> new ResetFeaturesResponse(), // TODO[wrb]: actual parse method + ResetFeaturesResponse::parse, emptySet() ); } @@ -94,7 +94,7 @@ public Cancellable resetFeaturesAsync( resetFeaturesRequest, FeaturesRequestConverters::resetFeatures, options, - e -> new ResetFeaturesResponse(), // TODO[wrb]: actual parse method + ResetFeaturesResponse::parse, listener, emptySet() ); diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/feature/ResetFeaturesResponse.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/feature/ResetFeaturesResponse.java index e3c365d51cf7a..2a89d167f056a 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/feature/ResetFeaturesResponse.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/feature/ResetFeaturesResponse.java @@ -8,5 +8,73 @@ package org.elasticsearch.client.feature; +import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.xcontent.ConstructingObjectParser; +import org.elasticsearch.common.xcontent.ObjectParser; +import org.elasticsearch.common.xcontent.XContentParser; + +import java.util.List; + public class ResetFeaturesResponse { + private final List features; + + private static final ParseField FEATURES = new ParseField("features"); + + @SuppressWarnings("unchecked") + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( + "snapshottable_features_response", true, + (a, ctx) -> new ResetFeaturesResponse((List) a[0]) + ); + + static { + PARSER.declareObjectArray(ConstructingObjectParser.constructorArg(), GetFeaturesResponse.SnapshottableFeature::parse, FEATURES); + } + + public ResetFeaturesResponse(List features) { + this.features = features; + } + + public List getFeatures() { + return features; + } + + public static ResetFeaturesResponse parse(XContentParser parser) { + return PARSER.apply(parser, null); + } + + public static class ResetFeatureStateStatus { + private final String featureName; + private final String status; + + private static final ParseField FEATURE_NAME = new ParseField("feature_name"); + private static final ParseField STATUS = new ParseField("status"); + + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( + "feature", true, (a, ctx) -> new ResetFeatureStateStatus((String) a[0], (String) a[1]) + ); + + static { + PARSER.declareField(ConstructingObjectParser.constructorArg(), + (p, c) -> p.text(), FEATURE_NAME, ObjectParser.ValueType.STRING); + PARSER.declareField(ConstructingObjectParser.constructorArg(), + (p, c) -> p.text(), STATUS, ObjectParser.ValueType.STRING); + } + + ResetFeatureStateStatus(String featureName, String status) { + this.featureName = featureName; + this.status = status; + } + + public static ResetFeatureStateStatus parse(XContentParser parser, Void ctx) { + return PARSER.apply(parser, ctx); + } + + public String getFeatureName() { + return featureName; + } + + public String getStatus() { + return status; + } + } } From d07d8c68e05ebd5f8af77b633d2d0b44514b61bb Mon Sep 17 00:00:00 2001 From: William Brafford Date: Mon, 15 Mar 2021 15:56:33 -0400 Subject: [PATCH 23/25] Respond to PR feedback --- .../elasticsearch/client/FeaturesClient.java | 24 +++++++++++++++++-- .../client/FeaturesRequestConverters.java | 7 +----- .../client/feature/ResetFeaturesResponse.java | 4 ++-- .../org/elasticsearch/client/FeaturesIT.java | 15 ++++++++++++ .../features/ResetFeatureStateRequest.java | 2 -- .../elasticsearch/indices/SystemIndices.java | 12 ++++------ .../cluster/RestResetFeatureStateAction.java | 2 +- 7 files changed, 46 insertions(+), 20 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/FeaturesClient.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/FeaturesClient.java index 17b000817bcfa..a26e1dcc8843d 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/FeaturesClient.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/FeaturesClient.java @@ -74,7 +74,17 @@ public Cancellable getFeaturesAsync( ); } - // reset features + /** + * Reset the state of Elasticsearch features, deleting system indices and performing other + * cleanup operations. + * See Rest + * Features API on elastic.co + * + * @param resetFeaturesRequest the request + * @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized + * @return the response + * @throws IOException in case there is a problem sending the request or parsing back the response + */ public ResetFeaturesResponse resetFeatures(ResetFeaturesRequest resetFeaturesRequest, RequestOptions options) throws IOException { return restHighLevelClient.performRequestAndParseEntity( @@ -86,7 +96,17 @@ public ResetFeaturesResponse resetFeatures(ResetFeaturesRequest resetFeaturesReq ); } - // async reset features + /** + * Asynchronously reset the state of Elasticsearch features, deleting system indices and performing other + * cleanup operations. + * See Get Snapshottable + * Features API on elastic.co + * + * @param resetFeaturesRequest the request + * @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized + * @param listener the listener to be notified upon request completion + * @return cancellable that may be used to cancel the request + */ public Cancellable resetFeaturesAsync( ResetFeaturesRequest resetFeaturesRequest, RequestOptions options, ActionListener listener) { diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/FeaturesRequestConverters.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/FeaturesRequestConverters.java index 5de17e412dc38..8832af5eb63a0 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/FeaturesRequestConverters.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/FeaturesRequestConverters.java @@ -26,13 +26,8 @@ static Request getFeatures(GetFeaturesRequest getFeaturesRequest) { return request; } - // reset features post static Request resetFeatures(ResetFeaturesRequest resetFeaturesRequest) { String endpoint = "/_features/reset"; - Request request = new Request(HttpPost.METHOD_NAME, endpoint); - RequestConverters.Params parameters = new RequestConverters.Params(); - parameters.withMasterTimeout(resetFeaturesRequest.masterNodeTimeout()); - request.addParameters(parameters.asMap()); - return request; + return new Request(HttpPost.METHOD_NAME, endpoint); } } diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/feature/ResetFeaturesResponse.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/feature/ResetFeaturesResponse.java index 2a89d167f056a..f33a50e9b3f68 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/feature/ResetFeaturesResponse.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/feature/ResetFeaturesResponse.java @@ -27,7 +27,7 @@ public class ResetFeaturesResponse { ); static { - PARSER.declareObjectArray(ConstructingObjectParser.constructorArg(), GetFeaturesResponse.SnapshottableFeature::parse, FEATURES); + PARSER.declareObjectArray(ConstructingObjectParser.constructorArg(), ResetFeaturesResponse.ResetFeatureStateStatus::parse, FEATURES); } public ResetFeaturesResponse(List features) { @@ -50,7 +50,7 @@ public static class ResetFeatureStateStatus { private static final ParseField STATUS = new ParseField("status"); private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( - "feature", true, (a, ctx) -> new ResetFeatureStateStatus((String) a[0], (String) a[1]) + "features", true, (a, ctx) -> new ResetFeatureStateStatus((String) a[0], (String) a[1]) ); static { diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/FeaturesIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/FeaturesIT.java index 195bf4ee77f8c..e8c3463762992 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/FeaturesIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/FeaturesIT.java @@ -10,6 +10,8 @@ import org.elasticsearch.client.feature.GetFeaturesRequest; import org.elasticsearch.client.feature.GetFeaturesResponse; +import org.elasticsearch.client.feature.ResetFeaturesRequest; +import org.elasticsearch.client.feature.ResetFeaturesResponse; import java.io.IOException; @@ -28,4 +30,17 @@ public void testGetFeatures() throws IOException { assertThat(response.getFeatures().size(), greaterThan(1)); assertTrue(response.getFeatures().stream().anyMatch(feature -> "tasks".equals(feature.getFeatureName()))); } + + public void testResetFeatures() throws IOException { + ResetFeaturesRequest request = new ResetFeaturesRequest(); + + ResetFeaturesResponse response = execute(request, + highLevelClient().features()::resetFeatures, highLevelClient().features()::resetFeaturesAsync); + + assertThat(response, notNullValue()); + assertThat(response.getFeatures(), notNullValue()); + assertThat(response.getFeatures().size(), greaterThan(1)); + assertTrue(response.getFeatures().stream().anyMatch( + feature -> "tasks".equals(feature.getFeatureName()) && "SUCCESS".equals(feature.getStatus()))); + } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateRequest.java index 82ed910ba7901..62a2b7d78c320 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/features/ResetFeatureStateRequest.java @@ -19,8 +19,6 @@ public class ResetFeatureStateRequest extends ActionRequest { public ResetFeatureStateRequest() { - // TODO[wrb] - We might need to let this request take a list of - // feature state names, but not for the initial implementation } public ResetFeatureStateRequest(StreamInput in) throws IOException { diff --git a/server/src/main/java/org/elasticsearch/indices/SystemIndices.java b/server/src/main/java/org/elasticsearch/indices/SystemIndices.java index d0c8deb8ca4db..a31d907cb7cfc 100644 --- a/server/src/main/java/org/elasticsearch/indices/SystemIndices.java +++ b/server/src/main/java/org/elasticsearch/indices/SystemIndices.java @@ -22,6 +22,7 @@ import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.TriConsumer; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.index.Index; @@ -335,14 +336,11 @@ public static void cleanUpFeature( ClusterService clusterService, Client client, ActionListener listener) { - List systemIndices = indexDescriptors.stream() + Stream systemIndices = indexDescriptors.stream() .map(sid -> sid.getMatchingIndices(clusterService.state().getMetadata())) - .flatMap(List::stream) - .collect(Collectors.toList()); - - List associatedIndices = new ArrayList<>(associatedIndexPatterns); + .flatMap(List::stream); - List allIndices = Stream.concat(systemIndices.stream(), associatedIndices.stream()) + List allIndices = Stream.concat(systemIndices, associatedIndexPatterns.stream()) .collect(Collectors.toList()); if (allIndices.isEmpty()) { @@ -352,7 +350,7 @@ public static void cleanUpFeature( } DeleteIndexRequest deleteIndexRequest = new DeleteIndexRequest(); - deleteIndexRequest.indices(allIndices.toArray(String[]::new)); + deleteIndexRequest.indices(allIndices.toArray(Strings.EMPTY_ARRAY)); client.execute(DeleteIndexAction.INSTANCE, deleteIndexRequest, new ActionListener<>() { @Override public void onResponse(AcknowledgedResponse acknowledgedResponse) { diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java index 6219ec694db7a..c58d7c393e947 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java @@ -27,7 +27,7 @@ public class RestResetFeatureStateAction extends BaseRestHandler { @Override public List routes() { - return List.of(new Route(RestRequest.Method.POST, "/_feature/reset")); + return List.of(new Route(RestRequest.Method.POST, "/_features/reset")); } @Override From 746f43cf55435f730d9406818ea930502de91e94 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Mon, 15 Mar 2021 16:05:16 -0400 Subject: [PATCH 24/25] Change _features/reset path to _features/_reset Out of an abundance of caution, I think the "reset" part of this path should have a leading underscore, so that if there's ever a reason to implement "GET _features/" we won't have to worry about distinguishing "reset" from a feature name. --- .../elasticsearch/client/FeaturesRequestConverters.java | 2 +- .../rest-api-spec/api/features.reset_features.json | 8 +------- .../action/admin/cluster/RestResetFeatureStateAction.java | 2 +- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/FeaturesRequestConverters.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/FeaturesRequestConverters.java index 8832af5eb63a0..bb2b8be43cf3b 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/FeaturesRequestConverters.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/FeaturesRequestConverters.java @@ -27,7 +27,7 @@ static Request getFeatures(GetFeaturesRequest getFeaturesRequest) { } static Request resetFeatures(ResetFeaturesRequest resetFeaturesRequest) { - String endpoint = "/_features/reset"; + String endpoint = "/_features/_reset"; return new Request(HttpPost.METHOD_NAME, endpoint); } } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/features.reset_features.json b/rest-api-spec/src/main/resources/rest-api-spec/api/features.reset_features.json index cfe284c47b850..1a7f944e88079 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/features.reset_features.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/features.reset_features.json @@ -12,18 +12,12 @@ "url":{ "paths":[ { - "path":"/_features/reset", + "path":"/_features/_reset", "methods":[ "POST" ] } ] - }, - "params":{ - "master_timeout":{ - "type":"time", - "description":"Explicit operation timeout for connection to master node" - } } } } diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java index c58d7c393e947..d3fedbc372760 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestResetFeatureStateAction.java @@ -27,7 +27,7 @@ public class RestResetFeatureStateAction extends BaseRestHandler { @Override public List routes() { - return List.of(new Route(RestRequest.Method.POST, "/_features/reset")); + return List.of(new Route(RestRequest.Method.POST, "/_features/_reset")); } @Override From 8c97adb264de58b36f2277b9ab3d2098a18bb478 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Mon, 15 Mar 2021 16:27:58 -0400 Subject: [PATCH 25/25] Checkstyle fixes --- .../elasticsearch/client/feature/ResetFeaturesResponse.java | 4 +++- .../main/java/org/elasticsearch/indices/SystemIndices.java | 1 - 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/feature/ResetFeaturesResponse.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/feature/ResetFeaturesResponse.java index f33a50e9b3f68..72d004021d6be 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/feature/ResetFeaturesResponse.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/feature/ResetFeaturesResponse.java @@ -27,7 +27,9 @@ public class ResetFeaturesResponse { ); static { - PARSER.declareObjectArray(ConstructingObjectParser.constructorArg(), ResetFeaturesResponse.ResetFeatureStateStatus::parse, FEATURES); + PARSER.declareObjectArray( + ConstructingObjectParser.constructorArg(), + ResetFeaturesResponse.ResetFeatureStateStatus::parse, FEATURES); } public ResetFeaturesResponse(List features) { diff --git a/server/src/main/java/org/elasticsearch/indices/SystemIndices.java b/server/src/main/java/org/elasticsearch/indices/SystemIndices.java index a31d907cb7cfc..f1a5c852c6bfb 100644 --- a/server/src/main/java/org/elasticsearch/indices/SystemIndices.java +++ b/server/src/main/java/org/elasticsearch/indices/SystemIndices.java @@ -28,7 +28,6 @@ import org.elasticsearch.index.Index; import org.elasticsearch.snapshots.SnapshotsService; -import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.Comparator;