diff --git a/docs/changelog/86405.yaml b/docs/changelog/86405.yaml new file mode 100644 index 0000000000000..37953eeb4e535 --- /dev/null +++ b/docs/changelog/86405.yaml @@ -0,0 +1,6 @@ +pr: 86405 +summary: Faster GET _cluster/settings API +area: Infra/Core +type: enhancement +issues: + - 82342 diff --git a/server/src/main/java/org/elasticsearch/action/ActionModule.java b/server/src/main/java/org/elasticsearch/action/ActionModule.java index 33906c5b634fe..c1fd9cc721ce5 100644 --- a/server/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/server/src/main/java/org/elasticsearch/action/ActionModule.java @@ -59,7 +59,9 @@ import org.elasticsearch.action.admin.cluster.repositories.verify.VerifyRepositoryAction; import org.elasticsearch.action.admin.cluster.reroute.ClusterRerouteAction; import org.elasticsearch.action.admin.cluster.reroute.TransportClusterRerouteAction; +import org.elasticsearch.action.admin.cluster.settings.ClusterGetSettingsAction; import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsAction; +import org.elasticsearch.action.admin.cluster.settings.TransportClusterGetSettingsAction; import org.elasticsearch.action.admin.cluster.settings.TransportClusterUpdateSettingsAction; import org.elasticsearch.action.admin.cluster.shards.ClusterSearchShardsAction; import org.elasticsearch.action.admin.cluster.shards.TransportClusterSearchShardsAction; @@ -547,6 +549,7 @@ public void reg actions.register(ClusterStateAction.INSTANCE, TransportClusterStateAction.class); actions.register(ClusterHealthAction.INSTANCE, TransportClusterHealthAction.class); actions.register(ClusterUpdateSettingsAction.INSTANCE, TransportClusterUpdateSettingsAction.class); + actions.register(ClusterGetSettingsAction.INSTANCE, TransportClusterGetSettingsAction.class); actions.register(ClusterRerouteAction.INSTANCE, TransportClusterRerouteAction.class); actions.register(ClusterSearchShardsAction.INSTANCE, TransportClusterSearchShardsAction.class); actions.register(PendingClusterTasksAction.INSTANCE, TransportPendingClusterTasksAction.class); @@ -709,7 +712,7 @@ public void initRestHandlers(Supplier nodesInCluster) { registerHandler.accept(new RestClusterStateAction(settingsFilter, threadPool)); registerHandler.accept(new RestClusterHealthAction()); registerHandler.accept(new RestClusterUpdateSettingsAction()); - registerHandler.accept(new RestClusterGetSettingsAction(settings, clusterSettings, settingsFilter)); + registerHandler.accept(new RestClusterGetSettingsAction(settings, clusterSettings, settingsFilter, nodesInCluster)); registerHandler.accept(new RestClusterRerouteAction(settingsFilter)); registerHandler.accept(new RestClusterSearchShardsAction()); registerHandler.accept(new RestPendingClusterTasksAction()); diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/ClusterGetSettingsAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/ClusterGetSettingsAction.java new file mode 100644 index 0000000000000..71767728e57bc --- /dev/null +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/ClusterGetSettingsAction.java @@ -0,0 +1,112 @@ +/* + * 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.settings; + +import org.elasticsearch.Version; +import org.elasticsearch.action.ActionRequestValidationException; +import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.action.ActionType; +import org.elasticsearch.action.support.master.MasterNodeReadRequest; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.settings.Settings; + +import java.io.IOException; +import java.util.Objects; + +public class ClusterGetSettingsAction extends ActionType { + + public static final ClusterGetSettingsAction INSTANCE = new ClusterGetSettingsAction(); + public static final String NAME = "cluster:monitor/settings"; + + public ClusterGetSettingsAction() { + super(NAME, Response::new); + } + + /** + * Request to retrieve the cluster settings + */ + public static class Request extends MasterNodeReadRequest { + public Request() {} + + public Request(StreamInput in) throws IOException { + super(in); + assert in.getVersion().onOrAfter(Version.V_8_3_0); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + assert out.getVersion().onOrAfter(Version.V_8_3_0); + super.writeTo(out); + } + + @Override + public ActionRequestValidationException validate() { + return null; + } + } + + /** + * Response for cluster settings + */ + public static class Response extends ActionResponse { + private final Settings persistentSettings; + private final Settings transientSettings; + private final Settings settings; + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Response response = (Response) o; + return Objects.equals(persistentSettings, response.persistentSettings) + && Objects.equals(transientSettings, response.transientSettings) + && Objects.equals(settings, response.settings); + } + + @Override + public int hashCode() { + return Objects.hash(persistentSettings, transientSettings, settings); + } + + public Response(StreamInput in) throws IOException { + super(in); + assert in.getVersion().onOrAfter(Version.V_8_3_0); + persistentSettings = Settings.readSettingsFromStream(in); + transientSettings = Settings.readSettingsFromStream(in); + settings = Settings.readSettingsFromStream(in); + } + + public Response(Settings persistentSettings, Settings transientSettings, Settings settings) { + this.persistentSettings = Objects.requireNonNullElse(persistentSettings, Settings.EMPTY); + this.transientSettings = Objects.requireNonNullElse(transientSettings, Settings.EMPTY); + this.settings = Objects.requireNonNullElse(settings, Settings.EMPTY); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + assert out.getVersion().onOrAfter(Version.V_8_3_0); + Settings.writeSettingsToStream(persistentSettings, out); + Settings.writeSettingsToStream(transientSettings, out); + Settings.writeSettingsToStream(settings, out); + } + + public Settings persistentSettings() { + return persistentSettings; + } + + public Settings transientSettings() { + return transientSettings; + } + + public Settings settings() { + return settings; + } + } +} diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/TransportClusterGetSettingsAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/TransportClusterGetSettingsAction.java new file mode 100644 index 0000000000000..2ef9ad7def5ee --- /dev/null +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/TransportClusterGetSettingsAction.java @@ -0,0 +1,77 @@ +/* + * 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.settings; + +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.action.support.master.TransportMasterNodeReadAction; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.block.ClusterBlockException; +import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.cluster.metadata.Metadata; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.settings.SettingsFilter; +import org.elasticsearch.tasks.Task; +import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.TransportService; + +public class TransportClusterGetSettingsAction extends TransportMasterNodeReadAction< + ClusterGetSettingsAction.Request, + ClusterGetSettingsAction.Response> { + + private final SettingsFilter settingsFilter; + + @Inject + public TransportClusterGetSettingsAction( + TransportService transportService, + ClusterService clusterService, + ThreadPool threadPool, + SettingsFilter settingsFilter, + ActionFilters actionFilters, + IndexNameExpressionResolver indexNameExpressionResolver + ) { + super( + ClusterGetSettingsAction.NAME, + false, + transportService, + clusterService, + threadPool, + actionFilters, + ClusterGetSettingsAction.Request::new, + indexNameExpressionResolver, + ClusterGetSettingsAction.Response::new, + ThreadPool.Names.SAME + ); + + this.settingsFilter = settingsFilter; + } + + @Override + protected void masterOperation( + Task task, + ClusterGetSettingsAction.Request request, + ClusterState state, + ActionListener listener + ) throws Exception { + Metadata metadata = state.metadata(); + listener.onResponse( + new ClusterGetSettingsAction.Response( + settingsFilter.filter(metadata.persistentSettings()), + settingsFilter.filter(metadata.transientSettings()), + settingsFilter.filter(metadata.settings()) + ) + ); + } + + @Override + protected ClusterBlockException checkBlock(ClusterGetSettingsAction.Request request, ClusterState state) { + return null; + } +} diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestClusterGetSettingsAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestClusterGetSettingsAction.java index f71ba965e60be..14a18ffeb04ab 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestClusterGetSettingsAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestClusterGetSettingsAction.java @@ -8,11 +8,14 @@ package org.elasticsearch.rest.action.admin.cluster; +import org.elasticsearch.Version; +import org.elasticsearch.action.admin.cluster.settings.ClusterGetSettingsAction; import org.elasticsearch.action.admin.cluster.settings.RestClusterGetSettingsResponse; import org.elasticsearch.action.admin.cluster.state.ClusterStateRequest; +import org.elasticsearch.action.support.master.MasterNodeReadRequest; import org.elasticsearch.client.internal.Requests; import org.elasticsearch.client.internal.node.NodeClient; -import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.SettingsFilter; @@ -23,6 +26,7 @@ import java.io.IOException; import java.util.List; import java.util.Set; +import java.util.function.Supplier; import static org.elasticsearch.rest.RestRequest.Method.GET; @@ -31,11 +35,18 @@ public class RestClusterGetSettingsAction extends BaseRestHandler { private final Settings settings; private final ClusterSettings clusterSettings; private final SettingsFilter settingsFilter; + private final Supplier nodesInCluster; - public RestClusterGetSettingsAction(Settings settings, ClusterSettings clusterSettings, SettingsFilter settingsFilter) { + public RestClusterGetSettingsAction( + Settings settings, + ClusterSettings clusterSettings, + SettingsFilter settingsFilter, + Supplier nodesInCluster + ) { this.settings = settings; this.clusterSettings = clusterSettings; this.settingsFilter = settingsFilter; + this.nodesInCluster = nodesInCluster; } @Override @@ -48,18 +59,51 @@ public String getName() { return "cluster_get_settings_action"; } + private void setUpRequestParams(MasterNodeReadRequest clusterRequest, RestRequest request) { + clusterRequest.local(request.paramAsBoolean("local", clusterRequest.local())); + clusterRequest.masterNodeTimeout(request.paramAsTime("master_timeout", clusterRequest.masterNodeTimeout())); + } + @Override public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { - ClusterStateRequest clusterStateRequest = Requests.clusterStateRequest().routingTable(false).nodes(false); final boolean renderDefaults = request.paramAsBoolean("include_defaults", false); - clusterStateRequest.local(request.paramAsBoolean("local", clusterStateRequest.local())); - clusterStateRequest.masterNodeTimeout(request.paramAsTime("master_timeout", clusterStateRequest.masterNodeTimeout())); + + if (nodesInCluster.get().getMinNodeVersion().before(Version.V_8_3_0)) { + return prepareLegacyRequest(request, client, renderDefaults); + } + + ClusterGetSettingsAction.Request clusterSettingsRequest = new ClusterGetSettingsAction.Request(); + + setUpRequestParams(clusterSettingsRequest, request); + + return channel -> client.execute( + ClusterGetSettingsAction.INSTANCE, + clusterSettingsRequest, + new RestToXContentListener(channel).map( + r -> response(r, renderDefaults, settingsFilter, clusterSettings, settings) + ) + ); + } + + private RestChannelConsumer prepareLegacyRequest(final RestRequest request, final NodeClient client, final boolean renderDefaults) { + ClusterStateRequest clusterStateRequest = Requests.clusterStateRequest().routingTable(false).nodes(false); + setUpRequestParams(clusterStateRequest, request); return channel -> client.admin() .cluster() .state( clusterStateRequest, new RestToXContentListener(channel).map( - response -> response(response.getState(), renderDefaults, settingsFilter, clusterSettings, settings) + r -> response( + new ClusterGetSettingsAction.Response( + r.getState().metadata().persistentSettings(), + r.getState().metadata().transientSettings(), + r.getState().metadata().settings() + ), + renderDefaults, + settingsFilter, + clusterSettings, + settings + ) ) ); } @@ -75,16 +119,16 @@ public boolean canTripCircuitBreaker() { } static RestClusterGetSettingsResponse response( - final ClusterState state, + final ClusterGetSettingsAction.Response response, final boolean renderDefaults, final SettingsFilter settingsFilter, final ClusterSettings clusterSettings, final Settings settings ) { return new RestClusterGetSettingsResponse( - settingsFilter.filter(state.metadata().persistentSettings()), - settingsFilter.filter(state.metadata().transientSettings()), - renderDefaults ? settingsFilter.filter(clusterSettings.diff(state.metadata().settings(), settings)) : Settings.EMPTY + settingsFilter.filter(response.persistentSettings()), + settingsFilter.filter(response.transientSettings()), + renderDefaults ? settingsFilter.filter(clusterSettings.diff(response.settings(), settings)) : Settings.EMPTY ); } diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/settings/ClusterGetSettingsSerializationTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/settings/ClusterGetSettingsSerializationTests.java new file mode 100644 index 0000000000000..2f53e713d6fca --- /dev/null +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/settings/ClusterGetSettingsSerializationTests.java @@ -0,0 +1,48 @@ +/* + * 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.settings; + +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.test.AbstractWireSerializingTestCase; + +public class ClusterGetSettingsSerializationTests extends AbstractWireSerializingTestCase { + @Override + protected Writeable.Reader instanceReader() { + return ClusterGetSettingsAction.Response::new; + } + + @Override + protected ClusterGetSettingsAction.Response createTestInstance() { + final Settings persistentSettings = Settings.builder() + .put("persistent.foo.filtered", "bar") + .put("persistent.foo.non_filtered", "baz") + .build(); + + final Settings transientSettings = Settings.builder() + .put("transient.foo.filtered", "bar") + .put("transient.foo.non_filtered", "baz") + .build(); + + final Settings allSettings = Settings.builder().put(persistentSettings).put(transientSettings).build(); + + return new ClusterGetSettingsAction.Response(persistentSettings, transientSettings, allSettings); + } + + @Override + protected ClusterGetSettingsAction.Response mutateInstance(ClusterGetSettingsAction.Response instance) { + final Settings otherSettings = Settings.builder().put("random.setting", randomAlphaOfLength(randomIntBetween(1, 12))).build(); + return switch (between(0, 2)) { + case 0 -> new ClusterGetSettingsAction.Response(otherSettings, instance.transientSettings(), instance.settings()); + case 1 -> new ClusterGetSettingsAction.Response(instance.persistentSettings(), otherSettings, instance.settings()); + case 2 -> new ClusterGetSettingsAction.Response(instance.persistentSettings(), instance.transientSettings(), otherSettings); + default -> throw new IllegalStateException("Unexpected switch value"); + }; + } +} diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/settings/ClusterGetSettingsTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/settings/ClusterGetSettingsTests.java new file mode 100644 index 0000000000000..9e757894bcdda --- /dev/null +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/settings/ClusterGetSettingsTests.java @@ -0,0 +1,84 @@ +/* + * 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.settings; + +import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.action.support.PlainActionFuture; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.cluster.metadata.Metadata; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.settings.SettingsFilter; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.TransportService; + +import java.util.List; + +import static org.mockito.Mockito.mock; + +public class ClusterGetSettingsTests extends ESTestCase { + + public void testRequestConstruction() { + final Settings persistentSettings = Settings.builder() + .put("persistent.foo.filtered", "bar") + .put("persistent.foo.non_filtered", "baz") + .build(); + + final Settings transientSettings = Settings.builder() + .put("transient.foo.filtered", "bar") + .put("transient.foo.non_filtered", "baz") + .build(); + + ClusterGetSettingsAction.Response response = new ClusterGetSettingsAction.Response(persistentSettings, transientSettings, null); + + assertEquals(persistentSettings, response.persistentSettings()); + assertEquals(transientSettings, response.transientSettings()); + assertEquals(Settings.EMPTY, response.settings()); + } + + public void testTransportFilters() throws Exception { + final SettingsFilter filter = new SettingsFilter(List.of("persistent.foo.filtered", "transient.foo.filtered")); + + TransportClusterGetSettingsAction action = new TransportClusterGetSettingsAction( + mock(TransportService.class), + mock(ClusterService.class), + mock(ThreadPool.class), + filter, + mock(ActionFilters.class), + mock(IndexNameExpressionResolver.class) + ); + + final Settings persistentSettings = Settings.builder() + .put("persistent.foo.filtered", "bar") + .put("persistent.foo.non_filtered", "baz") + .build(); + + final Settings transientSettings = Settings.builder() + .put("transient.foo.filtered", "bar") + .put("transient.foo.non_filtered", "baz") + .build(); + + final Metadata metadata = Metadata.builder().persistentSettings(persistentSettings).transientSettings(transientSettings).build(); + final ClusterState clusterState = ClusterState.builder(ClusterState.EMPTY_STATE).metadata(metadata).build(); + + final PlainActionFuture future = new PlainActionFuture<>(); + action.masterOperation(null, null, clusterState, future); + assertTrue(future.isDone()); + + final ClusterGetSettingsAction.Response response = future.get(); + + assertFalse(response.persistentSettings().hasValue("persistent.foo.filtered")); + assertTrue(response.persistentSettings().hasValue("persistent.foo.non_filtered")); + + assertFalse(response.transientSettings().hasValue("transient.foo.filtered")); + assertTrue(response.transientSettings().hasValue("transient.foo.non_filtered")); + } +} diff --git a/server/src/test/java/org/elasticsearch/rest/action/admin/cluster/RestClusterGetSettingsActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/admin/cluster/RestClusterGetSettingsActionTests.java index c5d17f22f860f..2edc50e9dc90c 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/admin/cluster/RestClusterGetSettingsActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/admin/cluster/RestClusterGetSettingsActionTests.java @@ -8,6 +8,7 @@ package org.elasticsearch.rest.action.admin.cluster; +import org.elasticsearch.action.admin.cluster.settings.ClusterGetSettingsAction; import org.elasticsearch.action.admin.cluster.settings.RestClusterGetSettingsResponse; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.Metadata; @@ -52,8 +53,13 @@ private void runTestFilterSettingsTest( ) ).collect(Collectors.toSet()); final ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, settingsSet); + final ClusterState clusterState = builder.build(); final RestClusterGetSettingsResponse response = RestClusterGetSettingsAction.response( - builder.build(), + new ClusterGetSettingsAction.Response( + clusterState.metadata().persistentSettings(), + clusterState.metadata().transientSettings(), + clusterState.metadata().settings() + ), randomBoolean(), filter, clusterSettings, 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 5dbe2fdb61a4b..8a3c1a57dbd33 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 @@ -264,6 +264,7 @@ public class Constants { "cluster:monitor/nodes/stats", "cluster:monitor/nodes/usage", "cluster:monitor/remote/info", + "cluster:monitor/settings", "cluster:monitor/state", "cluster:monitor/stats", "cluster:monitor/task",