diff --git a/docs/changelog/87681.yaml b/docs/changelog/87681.yaml new file mode 100644 index 0000000000000..ab6bbe19da6b6 --- /dev/null +++ b/docs/changelog/87681.yaml @@ -0,0 +1,5 @@ +pr: 87681 +summary: Make `GetIndexAction` cancellable +area: Indices APIs +type: bug +issues: [] diff --git a/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/RestGetMappingsCancellationIT.java b/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/RestClusterInfoActionCancellationIT.java similarity index 90% rename from qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/RestGetMappingsCancellationIT.java rename to qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/RestClusterInfoActionCancellationIT.java index 15561d10a8f9e..9db7c6893c433 100644 --- a/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/RestGetMappingsCancellationIT.java +++ b/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/RestClusterInfoActionCancellationIT.java @@ -9,6 +9,7 @@ package org.elasticsearch.http; import org.apache.http.client.methods.HttpGet; +import org.elasticsearch.action.admin.indices.get.GetIndexAction; import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsAction; import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.action.support.master.AcknowledgedResponse; @@ -37,21 +38,28 @@ import static org.hamcrest.core.IsEqual.equalTo; @ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST, numDataNodes = 0, numClientNodes = 0) -public class RestGetMappingsCancellationIT extends HttpSmokeTestCase { +public class RestClusterInfoActionCancellationIT extends HttpSmokeTestCase { public void testGetMappingsCancellation() throws Exception { + runTest(GetMappingsAction.NAME, "/test/_mappings"); + } + + public void testGetIndicesCancellation() throws Exception { + runTest(GetIndexAction.NAME, "/test"); + } + + private void runTest(String actionName, String endpoint) throws Exception { internalCluster().startMasterOnlyNode(); internalCluster().startDataOnlyNode(); ensureStableCluster(2); createIndex("test"); ensureGreen("test"); - final String actionName = GetMappingsAction.NAME; // Add a retryable cluster block that would block the request execution updateClusterState(currentState -> { ClusterBlock clusterBlock = new ClusterBlock( 1000, - "Get mappings cancellation test cluster block", + actionName + " cancellation test cluster block", true, false, false, @@ -62,7 +70,7 @@ public void testGetMappingsCancellation() throws Exception { return ClusterState.builder(currentState).blocks(ClusterBlocks.builder().addGlobalBlock(clusterBlock).build()).build(); }); - final Request request = new Request(HttpGet.METHOD_NAME, "/test/_mappings"); + final Request request = new Request(HttpGet.METHOD_NAME, endpoint); final PlainActionFuture future = new PlainActionFuture<>(); final Cancellable cancellable = getRestClient().performRequestAsync(request, wrapAsRestResponseListener(future)); diff --git a/server/src/main/java/org/elasticsearch/action/ActionModule.java b/server/src/main/java/org/elasticsearch/action/ActionModule.java index b7f3c96d71617..1d0ceb6bea196 100644 --- a/server/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/server/src/main/java/org/elasticsearch/action/ActionModule.java @@ -734,7 +734,7 @@ public void initRestHandlers(Supplier nodesInCluster) { registerHandler.accept(new RestResetFeatureStateAction()); registerHandler.accept(new RestGetFeatureUpgradeStatusAction()); registerHandler.accept(new RestPostFeatureUpgradeAction()); - registerHandler.accept(new RestGetIndicesAction()); + registerHandler.accept(new RestGetIndicesAction(threadPool)); registerHandler.accept(new RestIndicesStatsAction()); registerHandler.accept(new RestIndicesSegmentsAction(threadPool)); registerHandler.accept(new RestIndicesShardStoresAction()); diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/get/GetIndexRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/get/GetIndexRequest.java index 8c821e90d9373..da865a27ba1c3 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/get/GetIndexRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/get/GetIndexRequest.java @@ -14,12 +14,16 @@ import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.util.ArrayUtils; import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.tasks.CancellableTask; +import org.elasticsearch.tasks.Task; +import org.elasticsearch.tasks.TaskId; import java.io.IOException; import java.util.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Set; /** @@ -161,4 +165,9 @@ public void writeTo(StreamOutput out) throws IOException { out.writeBoolean(humanReadable); out.writeBoolean(includeDefaults); } + + @Override + public Task createTask(long id, String type, String action, TaskId parentTaskId, Map headers) { + return new CancellableTask(id, type, action, getDescription(), parentTaskId, headers); + } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/get/TransportGetIndexAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/get/TransportGetIndexAction.java index 91e46931bea68..c84990642fa91 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/get/TransportGetIndexAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/get/TransportGetIndexAction.java @@ -17,7 +17,6 @@ import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.metadata.MappingMetadata; -import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.inject.Inject; @@ -25,6 +24,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.SettingsFilter; import org.elasticsearch.indices.IndicesService; +import org.elasticsearch.tasks.CancellableTask; import org.elasticsearch.tasks.Task; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; @@ -94,11 +94,12 @@ protected void doMasterOperation( boolean doneMappings = false; boolean doneSettings = false; for (Feature feature : features) { + checkCancellation(task); switch (feature) { case MAPPINGS: if (doneMappings == false) { mappingsResult = state.metadata() - .findMappings(concreteIndices, indicesService.getFieldFilter(), Metadata.ON_NEXT_INDEX_FIND_MAPPINGS_NOOP); + .findMappings(concreteIndices, indicesService.getFieldFilter(), () -> checkCancellation(task)); doneMappings = true; } break; @@ -113,6 +114,7 @@ protected void doMasterOperation( ImmutableOpenMap.Builder settingsMapBuilder = ImmutableOpenMap.builder(); ImmutableOpenMap.Builder defaultSettingsMapBuilder = ImmutableOpenMap.builder(); for (String index : concreteIndices) { + checkCancellation(task); Settings indexSettings = state.metadata().index(index).getSettings(); if (request.humanReadable()) { indexSettings = IndexMetadata.addHumanReadableSettings(indexSettings); @@ -137,4 +139,10 @@ protected void doMasterOperation( } listener.onResponse(new GetIndexResponse(concreteIndices, mappingsResult, aliasesResult, settings, defaultSettings, dataStreams)); } + + private static void checkCancellation(Task task) { + if (task instanceof CancellableTask cancellableTask) { + cancellableTask.ensureNotCancelled(); + } + } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/TransportGetMappingsAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/TransportGetMappingsAction.java index a9c6e01a71a0c..5504973cfd4b2 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/TransportGetMappingsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/TransportGetMappingsAction.java @@ -26,8 +26,6 @@ import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; -import java.util.concurrent.CancellationException; - public class TransportGetMappingsAction extends TransportClusterInfoAction { private static final Logger logger = LogManager.getLogger(TransportGetMappingsAction.class); @@ -75,8 +73,8 @@ protected void doMasterOperation( } private static void checkCancellation(Task task) { - if (task instanceof CancellableTask && ((CancellableTask) task).isCancelled()) { - throw new CancellationException("Task cancelled"); + if (task instanceof CancellableTask cancellableTask) { + cancellableTask.ensureNotCancelled(); } } } diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetIndicesAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetIndicesAction.java index 6bd1e35787ba5..0ece05ace9336 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetIndicesAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetIndicesAction.java @@ -17,7 +17,9 @@ import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestRequest; -import org.elasticsearch.rest.action.RestToXContentListener; +import org.elasticsearch.rest.action.DispatchingRestToXContentListener; +import org.elasticsearch.rest.action.RestCancellableNodeClient; +import org.elasticsearch.threadpool.ThreadPool; import java.io.IOException; import java.util.Collections; @@ -42,6 +44,12 @@ public class RestGetIndicesAction extends BaseRestHandler { .collect(Collectors.toSet()) ); + private final ThreadPool threadPool; + + public RestGetIndicesAction(ThreadPool threadPool) { + this.threadPool = threadPool; + } + @Override public List routes() { return List.of(new Route(GET, "/{index}"), new Route(HEAD, "/{index}")); @@ -70,7 +78,13 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC getIndexRequest.humanReadable(request.paramAsBoolean("human", false)); getIndexRequest.includeDefaults(request.paramAsBoolean("include_defaults", false)); getIndexRequest.features(GetIndexRequest.Feature.fromRequest(request)); - return channel -> client.admin().indices().getIndex(getIndexRequest, new RestToXContentListener<>(channel)); + final var httpChannel = request.getHttpChannel(); + return channel -> new RestCancellableNodeClient(client, httpChannel).admin() + .indices() + .getIndex( + getIndexRequest, + new DispatchingRestToXContentListener<>(threadPool.executor(ThreadPool.Names.MANAGEMENT), channel, request) + ); } /** diff --git a/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestGetIndicesActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestGetIndicesActionTests.java index 5885c6f8c9885..164fc38d15c44 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestGetIndicesActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestGetIndicesActionTests.java @@ -9,6 +9,7 @@ package org.elasticsearch.rest.action.admin.indices; import org.elasticsearch.client.internal.node.NodeClient; +import org.elasticsearch.common.util.concurrent.DeterministicTaskQueue; import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.test.ESTestCase; @@ -36,7 +37,7 @@ public void testIncludeTypeNamesWarning() throws IOException { Map.of("Content-Type", contentTypeHeader, "Accept", contentTypeHeader) ).withMethod(RestRequest.Method.GET).withPath("/some_index").withParams(params).build(); - RestGetIndicesAction handler = new RestGetIndicesAction(); + RestGetIndicesAction handler = new RestGetIndicesAction(new DeterministicTaskQueue().getThreadPool()); handler.prepareRequest(request, mock(NodeClient.class)); assertCriticalWarnings(RestGetIndicesAction.TYPES_DEPRECATION_MESSAGE); @@ -57,7 +58,7 @@ public void testIncludeTypeNamesWarningExists() throws IOException { Map.of("Content-Type", contentTypeHeader, "Accept", contentTypeHeader) ).withMethod(RestRequest.Method.HEAD).withPath("/some_index").withParams(params).build(); - RestGetIndicesAction handler = new RestGetIndicesAction(); + RestGetIndicesAction handler = new RestGetIndicesAction(new DeterministicTaskQueue().getThreadPool()); handler.prepareRequest(request, mock(NodeClient.class)); } }