From d49788298a7b95ee2b869bce46a61823b6718c90 Mon Sep 17 00:00:00 2001 From: Ruirui Zhang Date: Wed, 10 Jul 2024 12:18:52 -0700 Subject: [PATCH 01/14] Add Get QueryGroup API Logic Signed-off-by: Ruirui Zhang --- .../wlm/action/GetQueryGroupAction.java | 37 ++++ .../wlm/action/GetQueryGroupRequest.java | 66 ++++++ .../wlm/action/GetQueryGroupResponse.java | 82 ++++++++ .../action/TransportGetQueryGroupAction.java | 58 +++++ .../wlm/action/WorkloadManagementPlugin.java | 62 ++++++ .../WorkloadManagementPluginModule.java | 32 +++ .../action/rest/RestGetQueryGroupAction.java | 69 ++++++ .../plugin/wlm/action/rest/package-info.java | 12 ++ .../wlm/action/service/Persistable.java | 24 +++ .../service/QueryGroupPersistenceService.java | 126 +++++++++++ .../wlm/action/service/package-info.java | 12 ++ .../wlm/action/GetQueryGroupRequestTests.java | 38 ++++ .../action/GetQueryGroupResponseTests.java | 65 ++++++ .../wlm/action/QueryGroupTestUtils.java | 116 ++++++++++ .../QueryGroupPersistenceServiceTests.java | 76 +++++++ .../common/settings/ClusterSettings.java | 1 + .../QueryGroupServiceSettings.java | 198 ++++++++++++++++++ .../search/query_group/package-info.java | 12 ++ 18 files changed, 1086 insertions(+) create mode 100644 plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupAction.java create mode 100644 plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequest.java create mode 100644 plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponse.java create mode 100644 plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java create mode 100644 plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/WorkloadManagementPlugin.java create mode 100644 plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/WorkloadManagementPluginModule.java create mode 100644 plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/rest/RestGetQueryGroupAction.java create mode 100644 plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/rest/package-info.java create mode 100644 plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/Persistable.java create mode 100644 plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceService.java create mode 100644 plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/package-info.java create mode 100644 plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequestTests.java create mode 100644 plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponseTests.java create mode 100644 plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/QueryGroupTestUtils.java create mode 100644 plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceServiceTests.java create mode 100644 server/src/main/java/org/opensearch/search/query_group/QueryGroupServiceSettings.java create mode 100644 server/src/main/java/org/opensearch/search/query_group/package-info.java diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupAction.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupAction.java new file mode 100644 index 0000000000000..fb9365196f828 --- /dev/null +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupAction.java @@ -0,0 +1,37 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.plugin.wlm.action; + +import org.opensearch.action.ActionType; + +/** + * Transport action to get QueryGroup + * + * @opensearch.api + */ +public class GetQueryGroupAction extends ActionType { + + /** + /** + * An instance of GetQueryGroupAction + */ + public static final GetQueryGroupAction INSTANCE = new GetQueryGroupAction(); + + /** + * Name for GetQueryGroupAction + */ + public static final String NAME = "cluster:admin/opensearch/query_group/wlm/_get"; + + /** + * Default constructor + */ + private GetQueryGroupAction() { + super(NAME, GetQueryGroupResponse::new); + } +} diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequest.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequest.java new file mode 100644 index 0000000000000..ee394fdc36440 --- /dev/null +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequest.java @@ -0,0 +1,66 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.plugin.wlm.action; + +import org.opensearch.action.ActionRequest; +import org.opensearch.action.ActionRequestValidationException; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.core.common.io.stream.Writeable; + +import java.io.IOException; + +/** + * A request for get QueryGroup + * + * @opensearch.internal + */ +public class GetQueryGroupRequest extends ActionRequest implements Writeable.Reader { + String name; + + /** + * Default constructor for GetQueryGroupRequest + * @param name - name for the QueryGroup to get + */ + public GetQueryGroupRequest(String name) { + this.name = name; + } + + /** + * Constructor for GetQueryGroupRequest + * @param in - A {@link StreamInput} object + */ + public GetQueryGroupRequest(StreamInput in) throws IOException { + super(in); + name = in.readOptionalString(); + } + + @Override + public GetQueryGroupRequest read(StreamInput in) throws IOException { + return new GetQueryGroupRequest(in); + } + + @Override + public ActionRequestValidationException validate() { + return null; + } + + /** + * Name getter + */ + public String getName() { + return name; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); + out.writeOptionalString(name); + } +} diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponse.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponse.java new file mode 100644 index 0000000000000..3355ffc936360 --- /dev/null +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponse.java @@ -0,0 +1,82 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.plugin.wlm.action; + +import org.opensearch.cluster.metadata.QueryGroup; +import org.opensearch.core.action.ActionResponse; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.core.rest.RestStatus; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.core.xcontent.ToXContentObject; +import org.opensearch.core.xcontent.XContentBuilder; + +import java.io.IOException; +import java.util.List; + +/** + * Response for the get API for QueryGroup + * + * @opensearch.internal + */ +public class GetQueryGroupResponse extends ActionResponse implements ToXContent, ToXContentObject { + private final List queryGroups; + private RestStatus restStatus; + + /** + * Constructor for GetQueryGroupResponse + * @param queryGroups - The QueryGroup list to be fetched + * @param restStatus - The rest status of the request + */ + public GetQueryGroupResponse(final List queryGroups, RestStatus restStatus) { + this.queryGroups = queryGroups; + this.restStatus = restStatus; + } + + /** + * Constructor for GetQueryGroupResponse + * @param in - A {@link StreamInput} object + */ + public GetQueryGroupResponse(StreamInput in) throws IOException { + this.queryGroups = in.readList(QueryGroup::new); + restStatus = RestStatus.readFrom(in); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeList(queryGroups); + RestStatus.writeTo(out, restStatus); + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(); + builder.startArray("query_groups"); + for (QueryGroup group : queryGroups) { + group.toXContent(builder, params); + } + builder.endArray(); + builder.endObject(); + return builder; + } + + /** + * queryGroups getter + */ + public List getQueryGroups() { + return queryGroups; + } + + /** + * restStatus getter + */ + public RestStatus getRestStatus() { + return restStatus; + } +} diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java new file mode 100644 index 0000000000000..df34da9e9e248 --- /dev/null +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java @@ -0,0 +1,58 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.plugin.wlm.action; + +import org.opensearch.action.support.ActionFilters; +import org.opensearch.action.support.HandledTransportAction; +import org.opensearch.cluster.metadata.QueryGroup; +import org.opensearch.common.inject.Inject; +import org.opensearch.core.action.ActionListener; +import org.opensearch.plugin.wlm.action.service.Persistable; +import org.opensearch.tasks.Task; +import org.opensearch.threadpool.ThreadPool; +import org.opensearch.transport.TransportService; + +/** + * Transport action for get QueryGroup + * + * @opensearch.internal + */ +public class TransportGetQueryGroupAction extends HandledTransportAction { + + private final ThreadPool threadPool; + private final Persistable queryGroupPersistenceService; + + /** + * Constructor for TransportGetQueryGroupAction + * + * @param actionName - action name + * @param transportService - a {@link TransportService} object + * @param actionFilters - a {@link ActionFilters} object + * @param threadPool - a {@link ThreadPool} object + * @param queryGroupPersistenceService - a {@link Persistable} object + */ + @Inject + public TransportGetQueryGroupAction( + String actionName, + TransportService transportService, + ActionFilters actionFilters, + ThreadPool threadPool, + Persistable queryGroupPersistenceService + ) { + super(GetQueryGroupAction.NAME, transportService, actionFilters, GetQueryGroupRequest::new); + this.threadPool = threadPool; + this.queryGroupPersistenceService = queryGroupPersistenceService; + } + + @Override + protected void doExecute(Task task, GetQueryGroupRequest request, ActionListener listener) { + String name = request.getName(); + threadPool.executor(ThreadPool.Names.GENERIC).execute(() -> queryGroupPersistenceService.get(name, listener)); + } +} diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/WorkloadManagementPlugin.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/WorkloadManagementPlugin.java new file mode 100644 index 0000000000000..8860e9aae91dc --- /dev/null +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/WorkloadManagementPlugin.java @@ -0,0 +1,62 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.plugin.wlm.action; + +import org.opensearch.action.ActionRequest; +import org.opensearch.cluster.metadata.IndexNameExpressionResolver; +import org.opensearch.cluster.node.DiscoveryNodes; +import org.opensearch.common.inject.Module; +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.IndexScopedSettings; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.settings.SettingsFilter; +import org.opensearch.core.action.ActionResponse; +import org.opensearch.plugin.wlm.action.rest.RestGetQueryGroupAction; +import org.opensearch.plugins.ActionPlugin; +import org.opensearch.plugins.Plugin; +import org.opensearch.rest.RestController; +import org.opensearch.rest.RestHandler; + +import java.util.Collection; +import java.util.List; +import java.util.function.Supplier; + +/** + * Plugin class for WorkloadManagement + */ +public class WorkloadManagementPlugin extends Plugin implements ActionPlugin { + + /** + * Default constructor + */ + public WorkloadManagementPlugin() {} + + @Override + public List> getActions() { + return List.of(new ActionPlugin.ActionHandler<>(GetQueryGroupAction.INSTANCE, TransportGetQueryGroupAction.class)); + } + + @Override + public List getRestHandlers( + Settings settings, + RestController restController, + ClusterSettings clusterSettings, + IndexScopedSettings indexScopedSettings, + SettingsFilter settingsFilter, + IndexNameExpressionResolver indexNameExpressionResolver, + Supplier nodesInCluster + ) { + return List.of(new RestGetQueryGroupAction()); + } + + @Override + public Collection createGuiceModules() { + return List.of(new WorkloadManagementPluginModule()); + } +} diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/WorkloadManagementPluginModule.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/WorkloadManagementPluginModule.java new file mode 100644 index 0000000000000..65f92a59a576b --- /dev/null +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/WorkloadManagementPluginModule.java @@ -0,0 +1,32 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.plugin.wlm.action; + +import org.opensearch.cluster.metadata.QueryGroup; +import org.opensearch.common.inject.AbstractModule; +import org.opensearch.common.inject.TypeLiteral; +import org.opensearch.plugin.wlm.action.service.Persistable; +import org.opensearch.plugin.wlm.action.service.QueryGroupPersistenceService; + +/** + * Guice Module to manage WorkloadManagement related objects + */ +public class WorkloadManagementPluginModule extends AbstractModule { + + /** + * Constructor for WorkloadManagementPluginModule + */ + public WorkloadManagementPluginModule() {} + + @Override + protected void configure() { + bind(new TypeLiteral>() { + }).to(QueryGroupPersistenceService.class).asEagerSingleton(); + } +} diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/rest/RestGetQueryGroupAction.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/rest/RestGetQueryGroupAction.java new file mode 100644 index 0000000000000..162c2b2b304cb --- /dev/null +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/rest/RestGetQueryGroupAction.java @@ -0,0 +1,69 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.plugin.wlm.action.rest; + +import org.opensearch.client.node.NodeClient; +import org.opensearch.core.rest.RestStatus; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.plugin.wlm.action.GetQueryGroupAction; +import org.opensearch.plugin.wlm.action.GetQueryGroupRequest; +import org.opensearch.plugin.wlm.action.GetQueryGroupResponse; +import org.opensearch.rest.BaseRestHandler; +import org.opensearch.rest.BytesRestResponse; +import org.opensearch.rest.RestChannel; +import org.opensearch.rest.RestRequest; +import org.opensearch.rest.RestResponse; +import org.opensearch.rest.action.RestResponseListener; + +import java.io.IOException; +import java.util.List; + +import static org.opensearch.rest.RestRequest.Method.GET; + +/** + * Rest action to get a QueryGroup0 + * + * @opensearch.api + */ +public class RestGetQueryGroupAction extends BaseRestHandler { + + /** + * Constructor for RestGetQueryGroupAction + */ + public RestGetQueryGroupAction() {} + + @Override + public String getName() { + return "get_query_group"; + } + + /** + * The list of {@link Route}s that this RestHandler is responsible for handling. + */ + @Override + public List routes() { + return List.of(new Route(GET, "_query_group/{name}"), new Route(GET, "_query_group/")); + } + + @Override + protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { + String name = request.param("name"); + GetQueryGroupRequest getQueryGroupRequest = new GetQueryGroupRequest(name); + return channel -> client.execute(GetQueryGroupAction.INSTANCE, getQueryGroupRequest, getQueryGroupResponse(channel)); + } + + private RestResponseListener getQueryGroupResponse(final RestChannel channel) { + return new RestResponseListener<>(channel) { + @Override + public RestResponse buildResponse(final GetQueryGroupResponse response) throws Exception { + return new BytesRestResponse(RestStatus.OK, response.toXContent(channel.newBuilder(), ToXContent.EMPTY_PARAMS)); + } + }; + } +} diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/rest/package-info.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/rest/package-info.java new file mode 100644 index 0000000000000..6d468b26c5649 --- /dev/null +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/rest/package-info.java @@ -0,0 +1,12 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** + * Base Package of CRUD API of QueryGroup + */ +package org.opensearch.plugin.wlm.action.rest; diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/Persistable.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/Persistable.java new file mode 100644 index 0000000000000..eae172fe6f3f9 --- /dev/null +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/Persistable.java @@ -0,0 +1,24 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.plugin.wlm.action.service; + +import org.opensearch.core.action.ActionListener; +import org.opensearch.plugin.wlm.action.GetQueryGroupResponse; + +/** + * This interface defines the key APIs for implementing QueruGroup persistence + */ +public interface Persistable { + /** + * fetch the QueryGroup in a durable storage + * @param name + * @param listener + */ + void get(String name, ActionListener listener); +} diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceService.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceService.java new file mode 100644 index 0000000000000..5d481dc87d8bc --- /dev/null +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceService.java @@ -0,0 +1,126 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.plugin.wlm.action.service; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.metadata.QueryGroup; +import org.opensearch.cluster.service.ClusterManagerTaskThrottler.ThrottlingKey; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.inject.Inject; +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Settings; +import org.opensearch.core.action.ActionListener; +import org.opensearch.core.rest.RestStatus; +import org.opensearch.plugin.wlm.action.GetQueryGroupResponse; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.DoubleAdder; + +import static org.opensearch.search.query_group.QueryGroupServiceSettings.MAX_QUERY_GROUP_COUNT; + +/** + * This class defines the functions for QueryGroup persistence + */ +public class QueryGroupPersistenceService implements Persistable { + private static final Logger logger = LogManager.getLogger(QueryGroupPersistenceService.class); + private final ClusterService clusterService; + private static final String SOURCE = "query-group-persistence-service"; + private static final String CREATE_QUERY_GROUP_THROTTLING_KEY = "create-query-group"; + private static final String UPDATE_QUERY_GROUP_THROTTLING_KEY = "update-query-group"; + private static final String DELETE_QUERY_GROUP_THROTTLING_KEY = "delete-query-group"; + private final AtomicInteger inflightCreateQueryGroupRequestCount; + private final Map inflightResourceLimitValues; + private volatile int maxQueryGroupCount; + final ThrottlingKey createQueryGroupThrottlingKey; + final ThrottlingKey updateQueryGroupThrottlingKey; + final ThrottlingKey deleteQueryGroupThrottlingKey; + + /** + * Constructor for QueryGroupPersistenceService + * + * @param clusterService {@link ClusterService} - The cluster service to be used by QueryGroupPersistenceService + * @param settings {@link Settings} - The settings to be used by QueryGroupPersistenceService + * @param clusterSettings {@link ClusterSettings} - The cluster settings to be used by QueryGroupPersistenceService + */ + @Inject + public QueryGroupPersistenceService( + final ClusterService clusterService, + final Settings settings, + final ClusterSettings clusterSettings + ) { + this.clusterService = clusterService; + this.createQueryGroupThrottlingKey = clusterService.registerClusterManagerTask(CREATE_QUERY_GROUP_THROTTLING_KEY, true); + this.deleteQueryGroupThrottlingKey = clusterService.registerClusterManagerTask(DELETE_QUERY_GROUP_THROTTLING_KEY, true); + this.updateQueryGroupThrottlingKey = clusterService.registerClusterManagerTask(UPDATE_QUERY_GROUP_THROTTLING_KEY, true); + maxQueryGroupCount = MAX_QUERY_GROUP_COUNT.get(settings); + clusterSettings.addSettingsUpdateConsumer(MAX_QUERY_GROUP_COUNT, this::setMaxQueryGroupCount); + inflightCreateQueryGroupRequestCount = new AtomicInteger(); + inflightResourceLimitValues = new HashMap<>(); + } + + /** + * Set maxQueryGroupCount to be newMaxQueryGroupCount + * @param newMaxQueryGroupCount - the max number of QueryGroup allowed + */ + public void setMaxQueryGroupCount(int newMaxQueryGroupCount) { + if (newMaxQueryGroupCount < 0) { + throw new IllegalArgumentException("node.query_group.max_count can't be negative"); + } + this.maxQueryGroupCount = newMaxQueryGroupCount; + } + + @Override + public void get(String name, ActionListener listener) { + ClusterState currentState = clusterService.state(); + List resultGroups = getFromClusterStateMetadata(name, currentState); + if (resultGroups.isEmpty() && name != null && !name.isEmpty()) { + logger.warn("No QueryGroup exists with the provided name: {}", name); + Exception e = new RuntimeException("No QueryGroup exists with the provided name: " + name); + listener.onFailure(e); + return; + } + GetQueryGroupResponse response = new GetQueryGroupResponse(resultGroups, RestStatus.OK); + listener.onResponse(response); + } + + List getFromClusterStateMetadata(String name, ClusterState currentState) { + Map currentGroups = currentState.getMetadata().queryGroups(); + if (name == null || name.isEmpty()) { + return new ArrayList<>(currentGroups.values()); + } + List resultGroups = new ArrayList<>(); + for (QueryGroup group : currentGroups.values()) { + if (group.getName().equals(name)) { + resultGroups.add(group); + break; + } + } + return resultGroups; + } + + /** + * inflightCreateQueryGroupRequestCount getter + */ + public AtomicInteger getInflightCreateQueryGroupRequestCount() { + return inflightCreateQueryGroupRequestCount; + } + + /** + * inflightResourceLimitValues getter + */ + public Map getInflightResourceLimitValues() { + return inflightResourceLimitValues; + } +} diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/package-info.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/package-info.java new file mode 100644 index 0000000000000..e70ac3afb81b5 --- /dev/null +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/package-info.java @@ -0,0 +1,12 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** + * Package for the service classes for QueryGroup CRUD operations + */ +package org.opensearch.plugin.wlm.action.service; diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequestTests.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequestTests.java new file mode 100644 index 0000000000000..107c4d085ff46 --- /dev/null +++ b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequestTests.java @@ -0,0 +1,38 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.plugin.wlm.action; + +import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.test.OpenSearchTestCase; + +import java.io.IOException; + +public class GetQueryGroupRequestTests extends OpenSearchTestCase { + + public void testSerialization() throws IOException { + GetQueryGroupRequest request = new GetQueryGroupRequest(QueryGroupTestUtils.NAME_ONE); + assertEquals(QueryGroupTestUtils.NAME_ONE, request.getName()); + BytesStreamOutput out = new BytesStreamOutput(); + request.writeTo(out); + StreamInput streamInput = out.bytes().streamInput(); + GetQueryGroupRequest otherRequest = new GetQueryGroupRequest(streamInput); + assertEquals(request.getName(), otherRequest.getName()); + } + + public void testSerializationWithNull() throws IOException { + GetQueryGroupRequest request = new GetQueryGroupRequest((String) null); + assertNull(request.getName()); + BytesStreamOutput out = new BytesStreamOutput(); + request.writeTo(out); + StreamInput streamInput = out.bytes().streamInput(); + GetQueryGroupRequest otherRequest = new GetQueryGroupRequest(streamInput); + assertEquals(request.getName(), otherRequest.getName()); + } +} diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponseTests.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponseTests.java new file mode 100644 index 0000000000000..1d706362377c6 --- /dev/null +++ b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponseTests.java @@ -0,0 +1,65 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.plugin.wlm.action; + +import org.opensearch.cluster.metadata.QueryGroup; +import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.rest.RestStatus; +import org.opensearch.test.OpenSearchTestCase; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +public class GetQueryGroupResponseTests extends OpenSearchTestCase { + + public void testSerializationSingleQueryGroup() throws IOException { + List list = new ArrayList<>(); + list.add(QueryGroupTestUtils.queryGroupOne); + GetQueryGroupResponse response = new GetQueryGroupResponse(list, RestStatus.OK); + assertEquals(response.getQueryGroups(), list); + + BytesStreamOutput out = new BytesStreamOutput(); + response.writeTo(out); + StreamInput streamInput = out.bytes().streamInput(); + + GetQueryGroupResponse otherResponse = new GetQueryGroupResponse(streamInput); + assertEquals(response.getRestStatus(), otherResponse.getRestStatus()); + QueryGroupTestUtils.compareQueryGroups(response.getQueryGroups(), otherResponse.getQueryGroups()); + } + + public void testSerializationMultipleQueryGroup() throws IOException { + GetQueryGroupResponse response = new GetQueryGroupResponse(QueryGroupTestUtils.queryGroupList(), RestStatus.OK); + assertEquals(response.getQueryGroups(), QueryGroupTestUtils.queryGroupList()); + + BytesStreamOutput out = new BytesStreamOutput(); + response.writeTo(out); + StreamInput streamInput = out.bytes().streamInput(); + + GetQueryGroupResponse otherResponse = new GetQueryGroupResponse(streamInput); + assertEquals(response.getRestStatus(), otherResponse.getRestStatus()); + assertEquals(2, otherResponse.getQueryGroups().size()); + QueryGroupTestUtils.compareQueryGroups(response.getQueryGroups(), otherResponse.getQueryGroups()); + } + + public void testSerializationNull() throws IOException { + List list = new ArrayList<>(); + GetQueryGroupResponse response = new GetQueryGroupResponse(list, RestStatus.OK); + assertEquals(response.getQueryGroups(), list); + + BytesStreamOutput out = new BytesStreamOutput(); + response.writeTo(out); + StreamInput streamInput = out.bytes().streamInput(); + + GetQueryGroupResponse otherResponse = new GetQueryGroupResponse(streamInput); + assertEquals(response.getRestStatus(), otherResponse.getRestStatus()); + assertEquals(0, otherResponse.getQueryGroups().size()); + } +} diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/QueryGroupTestUtils.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/QueryGroupTestUtils.java new file mode 100644 index 0000000000000..ef167a1589115 --- /dev/null +++ b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/QueryGroupTestUtils.java @@ -0,0 +1,116 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.plugin.wlm.action; + +import org.opensearch.cluster.ClusterName; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.metadata.Metadata; +import org.opensearch.cluster.metadata.QueryGroup; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Settings; +import org.opensearch.plugin.wlm.action.service.QueryGroupPersistenceService; +import org.opensearch.threadpool.ThreadPool; + +import java.util.ArrayList; +import java.util.Comparator; +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.DoubleAdder; + +import static org.opensearch.cluster.metadata.QueryGroup.builder; +import static org.opensearch.search.ResourceType.fromName; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; + +public class QueryGroupTestUtils { + public static final String NAME_ONE = "query_group_one"; + public static final String NAME_TWO = "query_group_two"; + public static final String _ID_ONE = "AgfUO5Ja9yfsYlONlYi3TQ=="; + public static final String _ID_TWO = "G5iIqHy4g7eK1qIAAAAIH53=1"; + public static final String NAME_NONE_EXISTED = "query_group_none_existed"; + public static final String MEMORY_STRING = "memory"; + public static final String MONITOR_STRING = "monitor"; + public static final long TIMESTAMP_ONE = 4513232413L; + public static final long TIMESTAMP_TWO = 4513232415L; + public static final QueryGroup queryGroupOne = builder().name(NAME_ONE) + ._id(_ID_ONE) + .mode(MONITOR_STRING) + .resourceLimits(Map.of(fromName(MEMORY_STRING), 0.3)) + .updatedAt(TIMESTAMP_ONE) + .build(); + + public static final QueryGroup queryGroupTwo = builder().name(NAME_TWO) + ._id(_ID_TWO) + .mode(MONITOR_STRING) + .resourceLimits(Map.of(fromName(MEMORY_STRING), 0.6)) + .updatedAt(TIMESTAMP_TWO) + .build(); + + public static final Map queryGroupMap = Map.of(NAME_ONE, queryGroupOne, NAME_TWO, queryGroupTwo); + + public static List queryGroupList() { + List list = new ArrayList<>(); + list.add(queryGroupOne); + list.add(queryGroupTwo); + return list; + } + + public static ClusterState clusterState() { + final Metadata metadata = Metadata.builder().queryGroups(Map.of(NAME_ONE, queryGroupOne, NAME_TWO, queryGroupTwo)).build(); + return ClusterState.builder(new ClusterName("_name")).metadata(metadata).build(); + } + + public static Settings settings() { + return Settings.builder().build(); + } + + public static ClusterSettings clusterSettings() { + return new ClusterSettings(settings(), ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + } + + public static QueryGroupPersistenceService queryGroupPersistenceService() { + ClusterService clusterService = new ClusterService(settings(), clusterSettings(), mock(ThreadPool.class)); + return new QueryGroupPersistenceService(clusterService, settings(), clusterSettings()); + } + + public static List preparePersistenceServiceSetup(Map queryGroups) { + Metadata metadata = Metadata.builder().queryGroups(queryGroups).build(); + Settings settings = Settings.builder().build(); + ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + ClusterService clusterService = new ClusterService(settings, clusterSettings, mock(ThreadPool.class)); + ClusterState clusterState = ClusterState.builder(new ClusterName("_name")).metadata(metadata).build(); + QueryGroupPersistenceService queryGroupPersistenceService = new QueryGroupPersistenceService( + clusterService, + settings, + clusterSettings + ); + return List.of(queryGroupPersistenceService, clusterState); + } + + public static void compareQueryGroups(List listOne, List listTwo) { + assertEquals(listOne.size(), listTwo.size()); + listOne.sort(Comparator.comparing(QueryGroup::getName)); + listTwo.sort(Comparator.comparing(QueryGroup::getName)); + for (int i = 0; i < listOne.size(); i++) { + assertTrue(listOne.get(i).equals(listTwo.get(i))); + } + } + + public static void assertInflightValuesAreZero(QueryGroupPersistenceService queryGroupPersistenceService) { + assertEquals(0, queryGroupPersistenceService.getInflightCreateQueryGroupRequestCount().get()); + Map inflightResourceMap = queryGroupPersistenceService.getInflightResourceLimitValues(); + if (inflightResourceMap != null) { + for (String resourceName : inflightResourceMap.keySet()) { + assertEquals(0, inflightResourceMap.get(resourceName).intValue()); + } + } + } +} diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceServiceTests.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceServiceTests.java new file mode 100644 index 0000000000000..2861f42d4cd40 --- /dev/null +++ b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceServiceTests.java @@ -0,0 +1,76 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.plugin.wlm.action.service; + +import org.opensearch.cluster.metadata.QueryGroup; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Settings; +import org.opensearch.test.OpenSearchTestCase; + +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; + +import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.NAME_NONE_EXISTED; +import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.NAME_ONE; +import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.NAME_TWO; +import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.assertInflightValuesAreZero; +import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.clusterState; +import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.compareQueryGroups; +import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.queryGroupList; +import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.queryGroupOne; +import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.queryGroupPersistenceService; +import static org.mockito.Mockito.mock; + +public class QueryGroupPersistenceServiceTests extends OpenSearchTestCase { + + public void testGetSingleQueryGroup() { + List groups = queryGroupPersistenceService().getFromClusterStateMetadata(NAME_ONE, clusterState()); + assertEquals(1, groups.size()); + QueryGroup queryGroup = groups.get(0); + List listOne = new ArrayList<>(); + List listTwo = new ArrayList<>(); + listOne.add(queryGroupOne); + listTwo.add(queryGroup); + compareQueryGroups(listOne, listTwo); + assertInflightValuesAreZero(queryGroupPersistenceService()); + } + + public void testGetAllQueryGroups() { + assertEquals(2, clusterState().metadata().queryGroups().size()); + List res = queryGroupPersistenceService().getFromClusterStateMetadata(null, clusterState()); + assertEquals(2, res.size()); + Set currentNAME = res.stream().map(QueryGroup::getName).collect(Collectors.toSet()); + assertTrue(currentNAME.contains(NAME_ONE)); + assertTrue(currentNAME.contains(NAME_TWO)); + compareQueryGroups(queryGroupList(), res); + assertInflightValuesAreZero(queryGroupPersistenceService()); + } + + public void testGetZeroQueryGroups() { + Settings settings = Settings.builder().build(); + ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + QueryGroupPersistenceService queryGroupPersistenceService = new QueryGroupPersistenceService( + mock(ClusterService.class), + settings, + clusterSettings + ); + List res = queryGroupPersistenceService.getFromClusterStateMetadata(NAME_NONE_EXISTED, clusterState()); + assertEquals(0, res.size()); + assertInflightValuesAreZero(queryGroupPersistenceService()); + } + + public void testGetNonExistedQueryGroups() { + List groups = queryGroupPersistenceService().getFromClusterStateMetadata(NAME_NONE_EXISTED, clusterState()); + assertEquals(0, groups.size()); + assertInflightValuesAreZero(queryGroupPersistenceService()); + } +} diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index 7baae17dd77cd..03c18caf08506 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -160,6 +160,7 @@ import org.opensearch.search.backpressure.settings.SearchShardTaskSettings; import org.opensearch.search.backpressure.settings.SearchTaskSettings; import org.opensearch.search.fetch.subphase.highlight.FastVectorHighlighter; +import org.opensearch.search.query_group.QueryGroupServiceSettings; import org.opensearch.snapshots.InternalSnapshotsInfoService; import org.opensearch.snapshots.SnapshotsService; import org.opensearch.tasks.TaskCancellationMonitoringSettings; diff --git a/server/src/main/java/org/opensearch/search/query_group/QueryGroupServiceSettings.java b/server/src/main/java/org/opensearch/search/query_group/QueryGroupServiceSettings.java new file mode 100644 index 0000000000000..7f6e4955cf22f --- /dev/null +++ b/server/src/main/java/org/opensearch/search/query_group/QueryGroupServiceSettings.java @@ -0,0 +1,198 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.search.query_group; + +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Setting; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.unit.TimeValue; + +/** + * Main class to declare the QueryGroup feature related settings + */ +public class QueryGroupServiceSettings { + private static final Long DEFAULT_RUN_INTERVAL_MILLIS = 1000l; + private static final Double DEFAULT_NODE_LEVEL_REJECTION_THRESHOLD = 0.8; + private static final Double DEFAULT_NODE_LEVEL_CANCELLATION_THRESHOLD = 0.9; + /** + * default max queryGroup count on any node at any given point in time + */ + public static final int DEFAULT_MAX_QUERY_GROUP_COUNT_VALUE = 100; + + public static final String QUERY_GROUP_COUNT_SETTING_NAME = "node.query_group.max_count"; + public static final double NODE_LEVEL_CANCELLATION_THRESHOLD_MAX_VALUE = 0.95; + public static final double NODE_LEVEL_REJECTION_THRESHOLD_MAX_VALUE = 0.90; + + private TimeValue runIntervalMillis; + private Double nodeLevelMemoryCancellationThreshold; + private Double nodeLevelMemoryRejectionThreshold; + private volatile int maxQueryGroupCount; + /** + * max QueryGroup count setting + */ + public static final Setting MAX_QUERY_GROUP_COUNT = Setting.intSetting( + QUERY_GROUP_COUNT_SETTING_NAME, + DEFAULT_MAX_QUERY_GROUP_COUNT_VALUE, + 0, + (newVal) -> { + if (newVal > 100 || newVal < 1) throw new IllegalArgumentException( + QUERY_GROUP_COUNT_SETTING_NAME + " should be in range [1-100]" + ); + }, + Setting.Property.Dynamic, + Setting.Property.NodeScope + ); + /** + * Setting name for default QueryGroup count + */ + public static final String SERVICE_RUN_INTERVAL_MILLIS_SETTING_NAME = "query_group.service.run_interval_millis"; + /** + * Setting to control the run interval of QSB service + */ + private static final Setting QUERY_GROUP_RUN_INTERVAL_SETTING = Setting.longSetting( + SERVICE_RUN_INTERVAL_MILLIS_SETTING_NAME, + DEFAULT_RUN_INTERVAL_MILLIS, + 1, + Setting.Property.Dynamic, + Setting.Property.NodeScope + ); + + /** + * Setting name for node level rejection threshold for QSB + */ + public static final String NODE_REJECTION_THRESHOLD_SETTING_NAME = "query_group.node.rejection_threshold"; + /** + * Setting to control the rejection threshold + */ + public static final Setting NODE_LEVEL_REJECTION_THRESHOLD = Setting.doubleSetting( + NODE_REJECTION_THRESHOLD_SETTING_NAME, + DEFAULT_NODE_LEVEL_REJECTION_THRESHOLD, + Setting.Property.Dynamic, + Setting.Property.NodeScope + ); + /** + * Setting name for node level cancellation threshold + */ + public static final String NODE_CANCELLATION_THRESHOLD_SETTING_NAME = "query_group.node.cancellation_threshold"; + /** + * Setting name for node level cancellation threshold + */ + public static final Setting NODE_LEVEL_CANCELLATION_THRESHOLD = Setting.doubleSetting( + NODE_CANCELLATION_THRESHOLD_SETTING_NAME, + DEFAULT_NODE_LEVEL_CANCELLATION_THRESHOLD, + Setting.Property.Dynamic, + Setting.Property.NodeScope + ); + + /** + * QueryGroup service settings constructor + * @param settings + * @param clusterSettings + */ + public QueryGroupServiceSettings(Settings settings, ClusterSettings clusterSettings) { + runIntervalMillis = new TimeValue(QUERY_GROUP_RUN_INTERVAL_SETTING.get(settings)); + nodeLevelMemoryCancellationThreshold = NODE_LEVEL_CANCELLATION_THRESHOLD.get(settings); + nodeLevelMemoryRejectionThreshold = NODE_LEVEL_REJECTION_THRESHOLD.get(settings); + maxQueryGroupCount = MAX_QUERY_GROUP_COUNT.get(settings); + + ensureRejectionThresholdIsLessThanCancellation(nodeLevelMemoryRejectionThreshold, nodeLevelMemoryCancellationThreshold); + + clusterSettings.addSettingsUpdateConsumer(MAX_QUERY_GROUP_COUNT, this::setMaxQueryGroupCount); + clusterSettings.addSettingsUpdateConsumer(NODE_LEVEL_CANCELLATION_THRESHOLD, this::setNodeLevelMemoryCancellationThreshold); + clusterSettings.addSettingsUpdateConsumer(NODE_LEVEL_REJECTION_THRESHOLD, this::setNodeLevelMemoryRejectionThreshold); + } + + /** + * Method to get runInterval for QSB + * @return runInterval in milliseconds for QSB Service + */ + public TimeValue getRunIntervalMillis() { + return runIntervalMillis; + } + + /** + * Method to set the new QueryGroup count + * @param newMaxQueryGroupCount is the new maxQueryGroupCount per node + */ + public void setMaxQueryGroupCount(int newMaxQueryGroupCount) { + if (newMaxQueryGroupCount < 0) { + throw new IllegalArgumentException("node.node.query_group.max_count can't be negative"); + } + this.maxQueryGroupCount = newMaxQueryGroupCount; + } + + /** + * Method to get the node level cancellation threshold + * @return current node level cancellation threshold + */ + public Double getNodeLevelMemoryCancellationThreshold() { + return nodeLevelMemoryCancellationThreshold; + } + + /** + * Method to set the node level cancellation threshold + * @param nodeLevelMemoryCancellationThreshold sets the new node level cancellation threshold + * @throws IllegalArgumentException if the value is > 0.95 and cancellation < rejection threshold + */ + public void setNodeLevelMemoryCancellationThreshold(Double nodeLevelMemoryCancellationThreshold) { + if (Double.compare(nodeLevelMemoryCancellationThreshold, NODE_LEVEL_CANCELLATION_THRESHOLD_MAX_VALUE) > 0) { + throw new IllegalArgumentException( + NODE_CANCELLATION_THRESHOLD_SETTING_NAME + " value should not be greater than 0.95 as it pose a threat of node drop" + ); + } + + ensureRejectionThresholdIsLessThanCancellation(nodeLevelMemoryRejectionThreshold, nodeLevelMemoryCancellationThreshold); + + this.nodeLevelMemoryCancellationThreshold = nodeLevelMemoryCancellationThreshold; + } + + /** + * Method to get the node level rejection threshold + * @return the current node level rejection threshold + */ + public Double getNodeLevelMemoryRejectionThreshold() { + return nodeLevelMemoryRejectionThreshold; + } + + /** + * Method to set the node level rejection threshold + * @param nodeLevelMemoryRejectionThreshold sets the new rejection threshold + * @throws IllegalArgumentException if rejection > 0.90 and rejection < cancellation threshold + */ + public void setNodeLevelMemoryRejectionThreshold(Double nodeLevelMemoryRejectionThreshold) { + if (Double.compare(nodeLevelMemoryRejectionThreshold, NODE_LEVEL_REJECTION_THRESHOLD_MAX_VALUE) > 0) { + throw new IllegalArgumentException( + NODE_REJECTION_THRESHOLD_SETTING_NAME + " value not be greater than 0.90 as it pose a threat of node drop" + ); + } + + ensureRejectionThresholdIsLessThanCancellation(nodeLevelMemoryRejectionThreshold, nodeLevelMemoryCancellationThreshold); + + this.nodeLevelMemoryRejectionThreshold = nodeLevelMemoryRejectionThreshold; + } + + private void ensureRejectionThresholdIsLessThanCancellation( + Double nodeLevelMemoryRejectionThreshold, + Double nodeLevelMemoryCancellationThreshold + ) { + if (Double.compare(nodeLevelMemoryCancellationThreshold, nodeLevelMemoryRejectionThreshold) < 0) { + throw new IllegalArgumentException( + NODE_CANCELLATION_THRESHOLD_SETTING_NAME + " value should not be less than " + NODE_REJECTION_THRESHOLD_SETTING_NAME + ); + } + } + + /** + * Method to get the current QueryGroup count + * @return the current max QueryGroup count + */ + public int getMaxQueryGroupCount() { + return maxQueryGroupCount; + } +} diff --git a/server/src/main/java/org/opensearch/search/query_group/package-info.java b/server/src/main/java/org/opensearch/search/query_group/package-info.java new file mode 100644 index 0000000000000..00b68b0d3306c --- /dev/null +++ b/server/src/main/java/org/opensearch/search/query_group/package-info.java @@ -0,0 +1,12 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** + * QueryGroup related artifacts + */ +package org.opensearch.search.query_group; From 61953f0c70670d28fa94634a9beb14907da19895 Mon Sep 17 00:00:00 2001 From: Ruirui Zhang Date: Wed, 10 Jul 2024 12:28:52 -0700 Subject: [PATCH 02/14] add to changelog Signed-off-by: Ruirui Zhang --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 511dd77c24e22..5ba172cc78a83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [Concurrent Segment Search] Support composite aggregations with scripting ([#15072](https://github.com/opensearch-project/OpenSearch/pull/15072)) - Add `rangeQuery` and `regexpQuery` for `constant_keyword` field type ([#14711](https://github.com/opensearch-project/OpenSearch/pull/14711)) - Add took time to request nodes stats ([#15054](https://github.com/opensearch-project/OpenSearch/pull/15054)) +- [Workload Management] Add Get QueryGroup API Logic ([14709](https://github.com/opensearch-project/OpenSearch/pull/14709)) - [Workload Management] QueryGroup resource tracking framework changes ([#13897](https://github.com/opensearch-project/OpenSearch/pull/13897)) - Add slice execution listeners to SearchOperationListener interface ([#15153](https://github.com/opensearch-project/OpenSearch/pull/15153)) From 82fc7be99610bced998c4fa7f2b0615b2b643903 Mon Sep 17 00:00:00 2001 From: Ruirui Zhang Date: Wed, 10 Jul 2024 15:22:51 -0700 Subject: [PATCH 03/14] fix javadoc Signed-off-by: Ruirui Zhang --- .../org/opensearch/plugin/wlm/action/service/Persistable.java | 4 ++-- .../search/query_group/QueryGroupServiceSettings.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/Persistable.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/Persistable.java index eae172fe6f3f9..cb6870599d306 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/Persistable.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/Persistable.java @@ -17,8 +17,8 @@ public interface Persistable { /** * fetch the QueryGroup in a durable storage - * @param name - * @param listener + * @param name - name of the QueryGroup to get + * @param listener - ActionListener for GetQueryGroupResponse */ void get(String name, ActionListener listener); } diff --git a/server/src/main/java/org/opensearch/search/query_group/QueryGroupServiceSettings.java b/server/src/main/java/org/opensearch/search/query_group/QueryGroupServiceSettings.java index 7f6e4955cf22f..425916ce3e924 100644 --- a/server/src/main/java/org/opensearch/search/query_group/QueryGroupServiceSettings.java +++ b/server/src/main/java/org/opensearch/search/query_group/QueryGroupServiceSettings.java @@ -92,8 +92,8 @@ public class QueryGroupServiceSettings { /** * QueryGroup service settings constructor - * @param settings - * @param clusterSettings + * @param settings - QueryGroup service settings + * @param clusterSettings - QueryGroup cluster settings */ public QueryGroupServiceSettings(Settings settings, ClusterSettings clusterSettings) { runIntervalMillis = new TimeValue(QUERY_GROUP_RUN_INTERVAL_SETTING.get(settings)); From f08004f9215b740af3e321c9d15774bb03387c38 Mon Sep 17 00:00:00 2001 From: Ruirui Zhang Date: Wed, 10 Jul 2024 16:21:34 -0700 Subject: [PATCH 04/14] change GetQueryGroupAction NAME and add more tests Signed-off-by: Ruirui Zhang --- .../wlm/action/GetQueryGroupAction.java | 2 +- .../action/GetQueryGroupResponseTests.java | 71 ++++++++++++++++++- .../wlm/action/QueryGroupTestUtils.java | 4 +- 3 files changed, 72 insertions(+), 5 deletions(-) diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupAction.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupAction.java index fb9365196f828..a2a3ad1876316 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupAction.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupAction.java @@ -26,7 +26,7 @@ public class GetQueryGroupAction extends ActionType { /** * Name for GetQueryGroupAction */ - public static final String NAME = "cluster:admin/opensearch/query_group/wlm/_get"; + public static final String NAME = "cluster:admin/opensearch/wlm/query_group/_get"; /** * Default constructor diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponseTests.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponseTests.java index 1d706362377c6..6de9a6194eba5 100644 --- a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponseTests.java +++ b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponseTests.java @@ -10,19 +10,26 @@ import org.opensearch.cluster.metadata.QueryGroup; import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.common.xcontent.json.JsonXContent; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.rest.RestStatus; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; import java.util.ArrayList; import java.util.List; +import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.queryGroupOne; +import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.queryGroupTwo; +import static org.mockito.Mockito.mock; + public class GetQueryGroupResponseTests extends OpenSearchTestCase { public void testSerializationSingleQueryGroup() throws IOException { List list = new ArrayList<>(); - list.add(QueryGroupTestUtils.queryGroupOne); + list.add(queryGroupOne); GetQueryGroupResponse response = new GetQueryGroupResponse(list, RestStatus.OK); assertEquals(response.getQueryGroups(), list); @@ -62,4 +69,66 @@ public void testSerializationNull() throws IOException { assertEquals(response.getRestStatus(), otherResponse.getRestStatus()); assertEquals(0, otherResponse.getQueryGroups().size()); } + + public void testToXContentGetSingleQueryGroup() throws IOException { + List queryGroupList = new ArrayList<>(); + queryGroupList.add(queryGroupOne); + XContentBuilder builder = JsonXContent.contentBuilder().prettyPrint(); + GetQueryGroupResponse response = new GetQueryGroupResponse(queryGroupList, RestStatus.OK); + String actual = response.toXContent(builder, mock(ToXContent.Params.class)).toString(); + String expected = "{\n" + + " \"query_groups\" : [\n" + + " {\n" + + " \"_id\" : \"AgfUO5Ja9yfsYlONlYi3TQ==\",\n" + + " \"name\" : \"query_group_one\",\n" + + " \"resiliency_mode\" : \"monitor\",\n" + + " \"updatedAt\" : 4513232413,\n" + + " \"resourceLimits\" : {\n" + + " \"memory\" : 0.3\n" + + " }\n" + + " }\n" + + " ]\n" + + "}"; + assertEquals(expected, actual); + } + + public void testToXContentGetMultipleQueryGroup() throws IOException { + List queryGroupList = new ArrayList<>(); + queryGroupList.add(queryGroupOne); + queryGroupList.add(queryGroupTwo); + XContentBuilder builder = JsonXContent.contentBuilder().prettyPrint(); + GetQueryGroupResponse response = new GetQueryGroupResponse(queryGroupList, RestStatus.OK); + String actual = response.toXContent(builder, mock(ToXContent.Params.class)).toString(); + String expected = "{\n" + + " \"query_groups\" : [\n" + + " {\n" + + " \"_id\" : \"AgfUO5Ja9yfsYlONlYi3TQ==\",\n" + + " \"name\" : \"query_group_one\",\n" + + " \"resiliency_mode\" : \"monitor\",\n" + + " \"updatedAt\" : 4513232413,\n" + + " \"resourceLimits\" : {\n" + + " \"memory\" : 0.3\n" + + " }\n" + + " },\n" + + " {\n" + + " \"_id\" : \"G5iIqHy4g7eK1qIAAAAIH53=1\",\n" + + " \"name\" : \"query_group_two\",\n" + + " \"resiliency_mode\" : \"monitor\",\n" + + " \"updatedAt\" : 4513232415,\n" + + " \"resourceLimits\" : {\n" + + " \"memory\" : 0.6\n" + + " }\n" + + " }\n" + + " ]\n" + + "}"; + assertEquals(expected, actual); + } + + public void testToXContentGetZeroQueryGroup() throws IOException { + XContentBuilder builder = JsonXContent.contentBuilder().prettyPrint(); + GetQueryGroupResponse otherResponse = new GetQueryGroupResponse(new ArrayList<>(), RestStatus.OK); + String actual = otherResponse.toXContent(builder, mock(ToXContent.Params.class)).toString(); + String expected = "{\n" + " \"query_groups\" : [ ]\n" + "}"; + assertEquals(expected, actual); + } } diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/QueryGroupTestUtils.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/QueryGroupTestUtils.java index ef167a1589115..2e7b2f7a9dda6 100644 --- a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/QueryGroupTestUtils.java +++ b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/QueryGroupTestUtils.java @@ -54,8 +54,6 @@ public class QueryGroupTestUtils { .updatedAt(TIMESTAMP_TWO) .build(); - public static final Map queryGroupMap = Map.of(NAME_ONE, queryGroupOne, NAME_TWO, queryGroupTwo); - public static List queryGroupList() { List list = new ArrayList<>(); list.add(queryGroupOne); @@ -64,7 +62,7 @@ public static List queryGroupList() { } public static ClusterState clusterState() { - final Metadata metadata = Metadata.builder().queryGroups(Map.of(NAME_ONE, queryGroupOne, NAME_TWO, queryGroupTwo)).build(); + final Metadata metadata = Metadata.builder().queryGroups(Map.of(_ID_ONE, queryGroupOne, _ID_TWO, queryGroupTwo)).build(); return ClusterState.builder(new ClusterName("_name")).metadata(metadata).build(); } From 0752ed8ae466584ac7802d954b711791611b71fc Mon Sep 17 00:00:00 2001 From: Ruirui Zhang Date: Thu, 11 Jul 2024 18:04:03 -0700 Subject: [PATCH 05/14] add more unit tests Signed-off-by: Ruirui Zhang --- .../service/QueryGroupPersistenceService.java | 7 ++++ .../wlm/action/QueryGroupTestUtils.java | 32 +++++++++++++++++-- .../QueryGroupPersistenceServiceTests.java | 18 +++++++++++ 3 files changed, 54 insertions(+), 3 deletions(-) diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceService.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceService.java index 5d481dc87d8bc..d01a3f31541a1 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceService.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceService.java @@ -123,4 +123,11 @@ public AtomicInteger getInflightCreateQueryGroupRequestCount() { public Map getInflightResourceLimitValues() { return inflightResourceLimitValues; } + + /** + * maxQueryGroupCount getter + */ + public int getMaxQueryGroupCount() { + return maxQueryGroupCount; + } } diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/QueryGroupTestUtils.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/QueryGroupTestUtils.java index 2e7b2f7a9dda6..fe6eb3410869b 100644 --- a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/QueryGroupTestUtils.java +++ b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/QueryGroupTestUtils.java @@ -12,6 +12,8 @@ import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.metadata.Metadata; import org.opensearch.cluster.metadata.QueryGroup; +import org.opensearch.cluster.service.ClusterApplierService; +import org.opensearch.cluster.service.ClusterManagerService; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; @@ -75,16 +77,40 @@ public static ClusterSettings clusterSettings() { } public static QueryGroupPersistenceService queryGroupPersistenceService() { - ClusterService clusterService = new ClusterService(settings(), clusterSettings(), mock(ThreadPool.class)); + ClusterApplierService clusterApplierService = new ClusterApplierService( + "name", + settings(), + clusterSettings(), + mock(ThreadPool.class) + ); + clusterApplierService.setInitialState(clusterState()); + ClusterService clusterService = new ClusterService( + settings(), + clusterSettings(), + mock(ClusterManagerService.class), + clusterApplierService + ); return new QueryGroupPersistenceService(clusterService, settings(), clusterSettings()); } public static List preparePersistenceServiceSetup(Map queryGroups) { Metadata metadata = Metadata.builder().queryGroups(queryGroups).build(); Settings settings = Settings.builder().build(); - ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - ClusterService clusterService = new ClusterService(settings, clusterSettings, mock(ThreadPool.class)); ClusterState clusterState = ClusterState.builder(new ClusterName("_name")).metadata(metadata).build(); + ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + ClusterApplierService clusterApplierService = new ClusterApplierService( + "name", + settings(), + clusterSettings(), + mock(ThreadPool.class) + ); + clusterApplierService.setInitialState(clusterState); + ClusterService clusterService = new ClusterService( + settings(), + clusterSettings(), + mock(ClusterManagerService.class), + clusterApplierService + ); QueryGroupPersistenceService queryGroupPersistenceService = new QueryGroupPersistenceService( clusterService, settings, diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceServiceTests.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceServiceTests.java index 2861f42d4cd40..f48b6bc65dcd5 100644 --- a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceServiceTests.java +++ b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceServiceTests.java @@ -12,6 +12,8 @@ import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; +import org.opensearch.core.action.ActionListener; +import org.opensearch.plugin.wlm.action.GetQueryGroupResponse; import org.opensearch.test.OpenSearchTestCase; import java.util.ArrayList; @@ -73,4 +75,20 @@ public void testGetNonExistedQueryGroups() { assertEquals(0, groups.size()); assertInflightValuesAreZero(queryGroupPersistenceService()); } + + @SuppressWarnings("unchecked") + public void testGet() { + QueryGroupPersistenceService queryGroupPersistenceService = queryGroupPersistenceService(); + ActionListener mockListener = mock(ActionListener.class); + queryGroupPersistenceService.get(NAME_ONE, mockListener); + queryGroupPersistenceService.get(NAME_NONE_EXISTED, mockListener); + } + + public void testMaxQueryGroupCount() { + assertThrows(IllegalArgumentException.class, () -> queryGroupPersistenceService().setMaxQueryGroupCount(-1)); + QueryGroupPersistenceService queryGroupPersistenceService = queryGroupPersistenceService(); + queryGroupPersistenceService.setMaxQueryGroupCount(50); + assertEquals(50, queryGroupPersistenceService.getMaxQueryGroupCount()); + + } } From 9d23d08a73249b53d65068fe9d61e300dc0740eb Mon Sep 17 00:00:00 2001 From: Ruirui Zhang Date: Tue, 16 Jul 2024 15:17:50 -0700 Subject: [PATCH 06/14] fix spotlessapply Signed-off-by: Ruirui Zhang --- .../wlm/action/GetQueryGroupAction.java | 2 +- .../wlm/action/GetQueryGroupRequest.java | 2 +- .../wlm/action/GetQueryGroupResponse.java | 2 +- .../action/TransportGetQueryGroupAction.java | 2 +- .../action/rest/RestGetQueryGroupAction.java | 4 ++-- .../service/QueryGroupPersistenceService.java | 21 ------------------- .../wlm/action/QueryGroupTestUtils.java | 11 ---------- .../QueryGroupPersistenceServiceTests.java | 5 ----- 8 files changed, 6 insertions(+), 43 deletions(-) diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupAction.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupAction.java index a2a3ad1876316..a4c199cf1da61 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupAction.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupAction.java @@ -13,7 +13,7 @@ /** * Transport action to get QueryGroup * - * @opensearch.api + * @opensearch.experimental */ public class GetQueryGroupAction extends ActionType { diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequest.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequest.java index ee394fdc36440..4633d710dcb52 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequest.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequest.java @@ -19,7 +19,7 @@ /** * A request for get QueryGroup * - * @opensearch.internal + * @opensearch.experimental */ public class GetQueryGroupRequest extends ActionRequest implements Writeable.Reader { String name; diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponse.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponse.java index 3355ffc936360..c6b6dbc4a967f 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponse.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponse.java @@ -23,7 +23,7 @@ /** * Response for the get API for QueryGroup * - * @opensearch.internal + * @opensearch.experimental */ public class GetQueryGroupResponse extends ActionResponse implements ToXContent, ToXContentObject { private final List queryGroups; diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java index df34da9e9e248..32cb865e769fb 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java @@ -21,7 +21,7 @@ /** * Transport action for get QueryGroup * - * @opensearch.internal + * @opensearch.experimental */ public class TransportGetQueryGroupAction extends HandledTransportAction { diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/rest/RestGetQueryGroupAction.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/rest/RestGetQueryGroupAction.java index 162c2b2b304cb..209a3cf19be8f 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/rest/RestGetQueryGroupAction.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/rest/RestGetQueryGroupAction.java @@ -29,7 +29,7 @@ /** * Rest action to get a QueryGroup0 * - * @opensearch.api + * @opensearch.experimental */ public class RestGetQueryGroupAction extends BaseRestHandler { @@ -48,7 +48,7 @@ public String getName() { */ @Override public List routes() { - return List.of(new Route(GET, "_query_group/{name}"), new Route(GET, "_query_group/")); + return List.of(new Route(GET, "_wlm/query_group/{name}"), new Route(GET, "_wlm/query_group/")); } @Override diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceService.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceService.java index d01a3f31541a1..54735044355ac 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceService.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceService.java @@ -22,11 +22,8 @@ import org.opensearch.plugin.wlm.action.GetQueryGroupResponse; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.atomic.DoubleAdder; import static org.opensearch.search.query_group.QueryGroupServiceSettings.MAX_QUERY_GROUP_COUNT; @@ -40,8 +37,6 @@ public class QueryGroupPersistenceService implements Persistable { private static final String CREATE_QUERY_GROUP_THROTTLING_KEY = "create-query-group"; private static final String UPDATE_QUERY_GROUP_THROTTLING_KEY = "update-query-group"; private static final String DELETE_QUERY_GROUP_THROTTLING_KEY = "delete-query-group"; - private final AtomicInteger inflightCreateQueryGroupRequestCount; - private final Map inflightResourceLimitValues; private volatile int maxQueryGroupCount; final ThrottlingKey createQueryGroupThrottlingKey; final ThrottlingKey updateQueryGroupThrottlingKey; @@ -66,8 +61,6 @@ public QueryGroupPersistenceService( this.updateQueryGroupThrottlingKey = clusterService.registerClusterManagerTask(UPDATE_QUERY_GROUP_THROTTLING_KEY, true); maxQueryGroupCount = MAX_QUERY_GROUP_COUNT.get(settings); clusterSettings.addSettingsUpdateConsumer(MAX_QUERY_GROUP_COUNT, this::setMaxQueryGroupCount); - inflightCreateQueryGroupRequestCount = new AtomicInteger(); - inflightResourceLimitValues = new HashMap<>(); } /** @@ -110,20 +103,6 @@ List getFromClusterStateMetadata(String name, ClusterState currentSt return resultGroups; } - /** - * inflightCreateQueryGroupRequestCount getter - */ - public AtomicInteger getInflightCreateQueryGroupRequestCount() { - return inflightCreateQueryGroupRequestCount; - } - - /** - * inflightResourceLimitValues getter - */ - public Map getInflightResourceLimitValues() { - return inflightResourceLimitValues; - } - /** * maxQueryGroupCount getter */ diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/QueryGroupTestUtils.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/QueryGroupTestUtils.java index fe6eb3410869b..96831919d5516 100644 --- a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/QueryGroupTestUtils.java +++ b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/QueryGroupTestUtils.java @@ -24,7 +24,6 @@ import java.util.Comparator; import java.util.List; import java.util.Map; -import java.util.concurrent.atomic.DoubleAdder; import static org.opensearch.cluster.metadata.QueryGroup.builder; import static org.opensearch.search.ResourceType.fromName; @@ -127,14 +126,4 @@ public static void compareQueryGroups(List listOne, List assertTrue(listOne.get(i).equals(listTwo.get(i))); } } - - public static void assertInflightValuesAreZero(QueryGroupPersistenceService queryGroupPersistenceService) { - assertEquals(0, queryGroupPersistenceService.getInflightCreateQueryGroupRequestCount().get()); - Map inflightResourceMap = queryGroupPersistenceService.getInflightResourceLimitValues(); - if (inflightResourceMap != null) { - for (String resourceName : inflightResourceMap.keySet()) { - assertEquals(0, inflightResourceMap.get(resourceName).intValue()); - } - } - } } diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceServiceTests.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceServiceTests.java index f48b6bc65dcd5..10128afb884ee 100644 --- a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceServiceTests.java +++ b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceServiceTests.java @@ -24,7 +24,6 @@ import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.NAME_NONE_EXISTED; import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.NAME_ONE; import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.NAME_TWO; -import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.assertInflightValuesAreZero; import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.clusterState; import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.compareQueryGroups; import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.queryGroupList; @@ -43,7 +42,6 @@ public void testGetSingleQueryGroup() { listOne.add(queryGroupOne); listTwo.add(queryGroup); compareQueryGroups(listOne, listTwo); - assertInflightValuesAreZero(queryGroupPersistenceService()); } public void testGetAllQueryGroups() { @@ -54,7 +52,6 @@ public void testGetAllQueryGroups() { assertTrue(currentNAME.contains(NAME_ONE)); assertTrue(currentNAME.contains(NAME_TWO)); compareQueryGroups(queryGroupList(), res); - assertInflightValuesAreZero(queryGroupPersistenceService()); } public void testGetZeroQueryGroups() { @@ -67,13 +64,11 @@ public void testGetZeroQueryGroups() { ); List res = queryGroupPersistenceService.getFromClusterStateMetadata(NAME_NONE_EXISTED, clusterState()); assertEquals(0, res.size()); - assertInflightValuesAreZero(queryGroupPersistenceService()); } public void testGetNonExistedQueryGroups() { List groups = queryGroupPersistenceService().getFromClusterStateMetadata(NAME_NONE_EXISTED, clusterState()); assertEquals(0, groups.size()); - assertInflightValuesAreZero(queryGroupPersistenceService()); } @SuppressWarnings("unchecked") From 186380cb8f14a3565a9f2c08e32ac297159c2fc8 Mon Sep 17 00:00:00 2001 From: Ruirui Zhang Date: Tue, 23 Jul 2024 16:24:46 -0700 Subject: [PATCH 07/14] addressed comments Signed-off-by: Ruirui Zhang --- .../plugin/wlm/WorkloadManagementPlugin.java | 10 +- .../WorkloadManagementPluginModule.java | 11 +- .../action/TransportGetQueryGroupAction.java | 12 +- .../wlm/action/WorkloadManagementPlugin.java | 62 ------ .../plugin/wlm/action/rest/package-info.java | 12 -- .../wlm/action/service/Persistable.java | 24 --- .../service/QueryGroupPersistenceService.java | 112 ---------- .../wlm/action/service/package-info.java | 12 -- .../opensearch/plugin/wlm/package-info.java | 4 + .../rest/RestGetQueryGroupAction.java | 2 +- .../service/QueryGroupPersistenceService.java | 36 ++++ .../wlm/action/GetQueryGroupRequestTests.java | 1 + .../action/GetQueryGroupResponseTests.java | 15 +- .../wlm/action/QueryGroupTestUtils.java | 129 ------------ .../QueryGroupPersistenceServiceTests.java | 89 -------- .../QueryGroupPersistenceServiceTests.java | 63 ++++++ .../opensearch/cluster/metadata/Metadata.java | 4 +- .../QueryGroupServiceSettings.java | 197 ++++++++++++++---- 18 files changed, 288 insertions(+), 507 deletions(-) rename plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/{action => }/WorkloadManagementPluginModule.java (54%) delete mode 100644 plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/WorkloadManagementPlugin.java delete mode 100644 plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/rest/package-info.java delete mode 100644 plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/Persistable.java delete mode 100644 plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceService.java delete mode 100644 plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/package-info.java rename plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/{action => }/rest/RestGetQueryGroupAction.java (98%) delete mode 100644 plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/QueryGroupTestUtils.java delete mode 100644 plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceServiceTests.java diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/WorkloadManagementPlugin.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/WorkloadManagementPlugin.java index 80807f0d5bc37..6b4496af76dc3 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/WorkloadManagementPlugin.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/WorkloadManagementPlugin.java @@ -18,8 +18,11 @@ import org.opensearch.common.settings.SettingsFilter; import org.opensearch.core.action.ActionResponse; import org.opensearch.plugin.wlm.action.CreateQueryGroupAction; +import org.opensearch.plugin.wlm.action.GetQueryGroupAction; import org.opensearch.plugin.wlm.action.TransportCreateQueryGroupAction; +import org.opensearch.plugin.wlm.action.TransportGetQueryGroupAction; import org.opensearch.plugin.wlm.rest.RestCreateQueryGroupAction; +import org.opensearch.plugin.wlm.rest.RestGetQueryGroupAction; import org.opensearch.plugin.wlm.service.QueryGroupPersistenceService; import org.opensearch.plugins.ActionPlugin; import org.opensearch.plugins.Plugin; @@ -41,7 +44,10 @@ public WorkloadManagementPlugin() {} @Override public List> getActions() { - return List.of(new ActionPlugin.ActionHandler<>(CreateQueryGroupAction.INSTANCE, TransportCreateQueryGroupAction.class)); + return List.of( + new ActionPlugin.ActionHandler<>(CreateQueryGroupAction.INSTANCE, TransportCreateQueryGroupAction.class), + new ActionPlugin.ActionHandler<>(GetQueryGroupAction.INSTANCE, TransportGetQueryGroupAction.class) + ); } @Override @@ -54,7 +60,7 @@ public List getRestHandlers( IndexNameExpressionResolver indexNameExpressionResolver, Supplier nodesInCluster ) { - return List.of(new RestCreateQueryGroupAction()); + return List.of(new RestCreateQueryGroupAction(), new RestGetQueryGroupAction()); } @Override diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/WorkloadManagementPluginModule.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/WorkloadManagementPluginModule.java similarity index 54% rename from plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/WorkloadManagementPluginModule.java rename to plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/WorkloadManagementPluginModule.java index 65f92a59a576b..87109d06259bb 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/WorkloadManagementPluginModule.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/WorkloadManagementPluginModule.java @@ -6,13 +6,9 @@ * compatible open source license. */ -package org.opensearch.plugin.wlm.action; +package org.opensearch.plugin.wlm; -import org.opensearch.cluster.metadata.QueryGroup; import org.opensearch.common.inject.AbstractModule; -import org.opensearch.common.inject.TypeLiteral; -import org.opensearch.plugin.wlm.action.service.Persistable; -import org.opensearch.plugin.wlm.action.service.QueryGroupPersistenceService; /** * Guice Module to manage WorkloadManagement related objects @@ -25,8 +21,5 @@ public class WorkloadManagementPluginModule extends AbstractModule { public WorkloadManagementPluginModule() {} @Override - protected void configure() { - bind(new TypeLiteral>() { - }).to(QueryGroupPersistenceService.class).asEagerSingleton(); - } + protected void configure() {} } diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java index 32cb865e769fb..45d26ef9f97e4 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java @@ -10,10 +10,9 @@ import org.opensearch.action.support.ActionFilters; import org.opensearch.action.support.HandledTransportAction; -import org.opensearch.cluster.metadata.QueryGroup; import org.opensearch.common.inject.Inject; import org.opensearch.core.action.ActionListener; -import org.opensearch.plugin.wlm.action.service.Persistable; +import org.opensearch.plugin.wlm.service.QueryGroupPersistenceService; import org.opensearch.tasks.Task; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.TransportService; @@ -26,7 +25,7 @@ public class TransportGetQueryGroupAction extends HandledTransportAction { private final ThreadPool threadPool; - private final Persistable queryGroupPersistenceService; + private final QueryGroupPersistenceService queryGroupPersistenceService; /** * Constructor for TransportGetQueryGroupAction @@ -35,7 +34,7 @@ public class TransportGetQueryGroupAction extends HandledTransportAction queryGroupPersistenceService + QueryGroupPersistenceService queryGroupPersistenceService ) { super(GetQueryGroupAction.NAME, transportService, actionFilters, GetQueryGroupRequest::new); this.threadPool = threadPool; @@ -53,6 +52,7 @@ public TransportGetQueryGroupAction( @Override protected void doExecute(Task task, GetQueryGroupRequest request, ActionListener listener) { String name = request.getName(); - threadPool.executor(ThreadPool.Names.GENERIC).execute(() -> queryGroupPersistenceService.get(name, listener)); + threadPool.executor(ThreadPool.Names.GENERIC) + .execute(() -> queryGroupPersistenceService.getFromClusterStateMetadata(name, listener)); } } diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/WorkloadManagementPlugin.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/WorkloadManagementPlugin.java deleted file mode 100644 index 8860e9aae91dc..0000000000000 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/WorkloadManagementPlugin.java +++ /dev/null @@ -1,62 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.plugin.wlm.action; - -import org.opensearch.action.ActionRequest; -import org.opensearch.cluster.metadata.IndexNameExpressionResolver; -import org.opensearch.cluster.node.DiscoveryNodes; -import org.opensearch.common.inject.Module; -import org.opensearch.common.settings.ClusterSettings; -import org.opensearch.common.settings.IndexScopedSettings; -import org.opensearch.common.settings.Settings; -import org.opensearch.common.settings.SettingsFilter; -import org.opensearch.core.action.ActionResponse; -import org.opensearch.plugin.wlm.action.rest.RestGetQueryGroupAction; -import org.opensearch.plugins.ActionPlugin; -import org.opensearch.plugins.Plugin; -import org.opensearch.rest.RestController; -import org.opensearch.rest.RestHandler; - -import java.util.Collection; -import java.util.List; -import java.util.function.Supplier; - -/** - * Plugin class for WorkloadManagement - */ -public class WorkloadManagementPlugin extends Plugin implements ActionPlugin { - - /** - * Default constructor - */ - public WorkloadManagementPlugin() {} - - @Override - public List> getActions() { - return List.of(new ActionPlugin.ActionHandler<>(GetQueryGroupAction.INSTANCE, TransportGetQueryGroupAction.class)); - } - - @Override - public List getRestHandlers( - Settings settings, - RestController restController, - ClusterSettings clusterSettings, - IndexScopedSettings indexScopedSettings, - SettingsFilter settingsFilter, - IndexNameExpressionResolver indexNameExpressionResolver, - Supplier nodesInCluster - ) { - return List.of(new RestGetQueryGroupAction()); - } - - @Override - public Collection createGuiceModules() { - return List.of(new WorkloadManagementPluginModule()); - } -} diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/rest/package-info.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/rest/package-info.java deleted file mode 100644 index 6d468b26c5649..0000000000000 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/rest/package-info.java +++ /dev/null @@ -1,12 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -/** - * Base Package of CRUD API of QueryGroup - */ -package org.opensearch.plugin.wlm.action.rest; diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/Persistable.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/Persistable.java deleted file mode 100644 index cb6870599d306..0000000000000 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/Persistable.java +++ /dev/null @@ -1,24 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.plugin.wlm.action.service; - -import org.opensearch.core.action.ActionListener; -import org.opensearch.plugin.wlm.action.GetQueryGroupResponse; - -/** - * This interface defines the key APIs for implementing QueruGroup persistence - */ -public interface Persistable { - /** - * fetch the QueryGroup in a durable storage - * @param name - name of the QueryGroup to get - * @param listener - ActionListener for GetQueryGroupResponse - */ - void get(String name, ActionListener listener); -} diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceService.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceService.java deleted file mode 100644 index 54735044355ac..0000000000000 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceService.java +++ /dev/null @@ -1,112 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.plugin.wlm.action.service; - -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.opensearch.cluster.ClusterState; -import org.opensearch.cluster.metadata.QueryGroup; -import org.opensearch.cluster.service.ClusterManagerTaskThrottler.ThrottlingKey; -import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.inject.Inject; -import org.opensearch.common.settings.ClusterSettings; -import org.opensearch.common.settings.Settings; -import org.opensearch.core.action.ActionListener; -import org.opensearch.core.rest.RestStatus; -import org.opensearch.plugin.wlm.action.GetQueryGroupResponse; - -import java.util.ArrayList; -import java.util.List; -import java.util.Map; - -import static org.opensearch.search.query_group.QueryGroupServiceSettings.MAX_QUERY_GROUP_COUNT; - -/** - * This class defines the functions for QueryGroup persistence - */ -public class QueryGroupPersistenceService implements Persistable { - private static final Logger logger = LogManager.getLogger(QueryGroupPersistenceService.class); - private final ClusterService clusterService; - private static final String SOURCE = "query-group-persistence-service"; - private static final String CREATE_QUERY_GROUP_THROTTLING_KEY = "create-query-group"; - private static final String UPDATE_QUERY_GROUP_THROTTLING_KEY = "update-query-group"; - private static final String DELETE_QUERY_GROUP_THROTTLING_KEY = "delete-query-group"; - private volatile int maxQueryGroupCount; - final ThrottlingKey createQueryGroupThrottlingKey; - final ThrottlingKey updateQueryGroupThrottlingKey; - final ThrottlingKey deleteQueryGroupThrottlingKey; - - /** - * Constructor for QueryGroupPersistenceService - * - * @param clusterService {@link ClusterService} - The cluster service to be used by QueryGroupPersistenceService - * @param settings {@link Settings} - The settings to be used by QueryGroupPersistenceService - * @param clusterSettings {@link ClusterSettings} - The cluster settings to be used by QueryGroupPersistenceService - */ - @Inject - public QueryGroupPersistenceService( - final ClusterService clusterService, - final Settings settings, - final ClusterSettings clusterSettings - ) { - this.clusterService = clusterService; - this.createQueryGroupThrottlingKey = clusterService.registerClusterManagerTask(CREATE_QUERY_GROUP_THROTTLING_KEY, true); - this.deleteQueryGroupThrottlingKey = clusterService.registerClusterManagerTask(DELETE_QUERY_GROUP_THROTTLING_KEY, true); - this.updateQueryGroupThrottlingKey = clusterService.registerClusterManagerTask(UPDATE_QUERY_GROUP_THROTTLING_KEY, true); - maxQueryGroupCount = MAX_QUERY_GROUP_COUNT.get(settings); - clusterSettings.addSettingsUpdateConsumer(MAX_QUERY_GROUP_COUNT, this::setMaxQueryGroupCount); - } - - /** - * Set maxQueryGroupCount to be newMaxQueryGroupCount - * @param newMaxQueryGroupCount - the max number of QueryGroup allowed - */ - public void setMaxQueryGroupCount(int newMaxQueryGroupCount) { - if (newMaxQueryGroupCount < 0) { - throw new IllegalArgumentException("node.query_group.max_count can't be negative"); - } - this.maxQueryGroupCount = newMaxQueryGroupCount; - } - - @Override - public void get(String name, ActionListener listener) { - ClusterState currentState = clusterService.state(); - List resultGroups = getFromClusterStateMetadata(name, currentState); - if (resultGroups.isEmpty() && name != null && !name.isEmpty()) { - logger.warn("No QueryGroup exists with the provided name: {}", name); - Exception e = new RuntimeException("No QueryGroup exists with the provided name: " + name); - listener.onFailure(e); - return; - } - GetQueryGroupResponse response = new GetQueryGroupResponse(resultGroups, RestStatus.OK); - listener.onResponse(response); - } - - List getFromClusterStateMetadata(String name, ClusterState currentState) { - Map currentGroups = currentState.getMetadata().queryGroups(); - if (name == null || name.isEmpty()) { - return new ArrayList<>(currentGroups.values()); - } - List resultGroups = new ArrayList<>(); - for (QueryGroup group : currentGroups.values()) { - if (group.getName().equals(name)) { - resultGroups.add(group); - break; - } - } - return resultGroups; - } - - /** - * maxQueryGroupCount getter - */ - public int getMaxQueryGroupCount() { - return maxQueryGroupCount; - } -} diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/package-info.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/package-info.java deleted file mode 100644 index e70ac3afb81b5..0000000000000 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/service/package-info.java +++ /dev/null @@ -1,12 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -/** - * Package for the service classes for QueryGroup CRUD operations - */ -package org.opensearch.plugin.wlm.action.service; diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/package-info.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/package-info.java index 84c99967b226b..c0e0942da24f1 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/package-info.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/package-info.java @@ -7,6 +7,10 @@ */ /** +<<<<<<< HEAD * Base package for WorkloadManagementPlugin +======= + * Base package for CRUD API of QueryGroup +>>>>>>> bf0bea8a7d1 (addressed comments) */ package org.opensearch.plugin.wlm; diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/rest/RestGetQueryGroupAction.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/rest/RestGetQueryGroupAction.java similarity index 98% rename from plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/rest/RestGetQueryGroupAction.java rename to plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/rest/RestGetQueryGroupAction.java index 209a3cf19be8f..810afaf38131a 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/rest/RestGetQueryGroupAction.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/rest/RestGetQueryGroupAction.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.plugin.wlm.action.rest; +package org.opensearch.plugin.wlm.rest; import org.opensearch.client.node.NodeClient; import org.opensearch.core.rest.RestStatus; diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceService.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceService.java index b2164df561bf9..d97fb4ce57832 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceService.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceService.java @@ -24,10 +24,13 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.core.rest.RestStatus; import org.opensearch.plugin.wlm.action.CreateQueryGroupResponse; +import org.opensearch.plugin.wlm.action.GetQueryGroupResponse; import org.opensearch.search.ResourceType; import java.util.EnumMap; import java.util.Map; +import java.util.List; +import java.util.ArrayList; import java.util.Optional; /** @@ -192,6 +195,39 @@ private Map calculateTotalUsage(Map ex return map; } + /** + * Get the QueryGroup with the specified name from cluster state + * @param name - the QueryGroup name we are getting + * @param listener - ActionListener for GetQueryGroupResponse + */ + public void getFromClusterStateMetadata(String name, ActionListener listener) { + ClusterState currentState = clusterService.state(); + List resultGroups = getQueryGroupsFromClusterState(name, currentState); + if (resultGroups.isEmpty() && name != null && !name.isEmpty()) { + logger.warn("No QueryGroup exists with the provided name: {}", name); + Exception e = new RuntimeException("No QueryGroup exists with the provided name: " + name); + listener.onFailure(e); + return; + } + GetQueryGroupResponse response = new GetQueryGroupResponse(resultGroups, RestStatus.OK); + listener.onResponse(response); + } + + List getQueryGroupsFromClusterState(String name, ClusterState currentState) { + Map currentGroups = currentState.getMetadata().queryGroups(); + if (name == null || name.isEmpty()) { + return new ArrayList<>(currentGroups.values()); + } + List resultGroups = new ArrayList<>(); + for (QueryGroup group : currentGroups.values()) { + if (group.getName().equals(name)) { + resultGroups.add(group); + break; + } + } + return resultGroups; + } + /** * maxQueryGroupCount getter */ diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequestTests.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequestTests.java index 107c4d085ff46..6f768a78d7998 100644 --- a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequestTests.java +++ b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequestTests.java @@ -10,6 +10,7 @@ import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.plugin.wlm.QueryGroupTestUtils; import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponseTests.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponseTests.java index 6de9a6194eba5..ae50e893fdc61 100644 --- a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponseTests.java +++ b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponseTests.java @@ -15,21 +15,20 @@ import org.opensearch.core.rest.RestStatus; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.plugin.wlm.QueryGroupTestUtils; import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; import java.util.ArrayList; import java.util.List; -import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.queryGroupOne; -import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.queryGroupTwo; import static org.mockito.Mockito.mock; public class GetQueryGroupResponseTests extends OpenSearchTestCase { public void testSerializationSingleQueryGroup() throws IOException { List list = new ArrayList<>(); - list.add(queryGroupOne); + list.add(QueryGroupTestUtils.queryGroupOne); GetQueryGroupResponse response = new GetQueryGroupResponse(list, RestStatus.OK); assertEquals(response.getQueryGroups(), list); @@ -39,7 +38,7 @@ public void testSerializationSingleQueryGroup() throws IOException { GetQueryGroupResponse otherResponse = new GetQueryGroupResponse(streamInput); assertEquals(response.getRestStatus(), otherResponse.getRestStatus()); - QueryGroupTestUtils.compareQueryGroups(response.getQueryGroups(), otherResponse.getQueryGroups()); + QueryGroupTestUtils.assertEqualQueryGroups(response.getQueryGroups(), otherResponse.getQueryGroups()); } public void testSerializationMultipleQueryGroup() throws IOException { @@ -53,7 +52,7 @@ public void testSerializationMultipleQueryGroup() throws IOException { GetQueryGroupResponse otherResponse = new GetQueryGroupResponse(streamInput); assertEquals(response.getRestStatus(), otherResponse.getRestStatus()); assertEquals(2, otherResponse.getQueryGroups().size()); - QueryGroupTestUtils.compareQueryGroups(response.getQueryGroups(), otherResponse.getQueryGroups()); + QueryGroupTestUtils.assertEqualQueryGroups(response.getQueryGroups(), otherResponse.getQueryGroups()); } public void testSerializationNull() throws IOException { @@ -72,7 +71,7 @@ public void testSerializationNull() throws IOException { public void testToXContentGetSingleQueryGroup() throws IOException { List queryGroupList = new ArrayList<>(); - queryGroupList.add(queryGroupOne); + queryGroupList.add(QueryGroupTestUtils.queryGroupOne); XContentBuilder builder = JsonXContent.contentBuilder().prettyPrint(); GetQueryGroupResponse response = new GetQueryGroupResponse(queryGroupList, RestStatus.OK); String actual = response.toXContent(builder, mock(ToXContent.Params.class)).toString(); @@ -94,8 +93,8 @@ public void testToXContentGetSingleQueryGroup() throws IOException { public void testToXContentGetMultipleQueryGroup() throws IOException { List queryGroupList = new ArrayList<>(); - queryGroupList.add(queryGroupOne); - queryGroupList.add(queryGroupTwo); + queryGroupList.add(QueryGroupTestUtils.queryGroupOne); + queryGroupList.add(QueryGroupTestUtils.queryGroupTwo); XContentBuilder builder = JsonXContent.contentBuilder().prettyPrint(); GetQueryGroupResponse response = new GetQueryGroupResponse(queryGroupList, RestStatus.OK); String actual = response.toXContent(builder, mock(ToXContent.Params.class)).toString(); diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/QueryGroupTestUtils.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/QueryGroupTestUtils.java deleted file mode 100644 index 96831919d5516..0000000000000 --- a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/QueryGroupTestUtils.java +++ /dev/null @@ -1,129 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.plugin.wlm.action; - -import org.opensearch.cluster.ClusterName; -import org.opensearch.cluster.ClusterState; -import org.opensearch.cluster.metadata.Metadata; -import org.opensearch.cluster.metadata.QueryGroup; -import org.opensearch.cluster.service.ClusterApplierService; -import org.opensearch.cluster.service.ClusterManagerService; -import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.settings.ClusterSettings; -import org.opensearch.common.settings.Settings; -import org.opensearch.plugin.wlm.action.service.QueryGroupPersistenceService; -import org.opensearch.threadpool.ThreadPool; - -import java.util.ArrayList; -import java.util.Comparator; -import java.util.List; -import java.util.Map; - -import static org.opensearch.cluster.metadata.QueryGroup.builder; -import static org.opensearch.search.ResourceType.fromName; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.mock; - -public class QueryGroupTestUtils { - public static final String NAME_ONE = "query_group_one"; - public static final String NAME_TWO = "query_group_two"; - public static final String _ID_ONE = "AgfUO5Ja9yfsYlONlYi3TQ=="; - public static final String _ID_TWO = "G5iIqHy4g7eK1qIAAAAIH53=1"; - public static final String NAME_NONE_EXISTED = "query_group_none_existed"; - public static final String MEMORY_STRING = "memory"; - public static final String MONITOR_STRING = "monitor"; - public static final long TIMESTAMP_ONE = 4513232413L; - public static final long TIMESTAMP_TWO = 4513232415L; - public static final QueryGroup queryGroupOne = builder().name(NAME_ONE) - ._id(_ID_ONE) - .mode(MONITOR_STRING) - .resourceLimits(Map.of(fromName(MEMORY_STRING), 0.3)) - .updatedAt(TIMESTAMP_ONE) - .build(); - - public static final QueryGroup queryGroupTwo = builder().name(NAME_TWO) - ._id(_ID_TWO) - .mode(MONITOR_STRING) - .resourceLimits(Map.of(fromName(MEMORY_STRING), 0.6)) - .updatedAt(TIMESTAMP_TWO) - .build(); - - public static List queryGroupList() { - List list = new ArrayList<>(); - list.add(queryGroupOne); - list.add(queryGroupTwo); - return list; - } - - public static ClusterState clusterState() { - final Metadata metadata = Metadata.builder().queryGroups(Map.of(_ID_ONE, queryGroupOne, _ID_TWO, queryGroupTwo)).build(); - return ClusterState.builder(new ClusterName("_name")).metadata(metadata).build(); - } - - public static Settings settings() { - return Settings.builder().build(); - } - - public static ClusterSettings clusterSettings() { - return new ClusterSettings(settings(), ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - } - - public static QueryGroupPersistenceService queryGroupPersistenceService() { - ClusterApplierService clusterApplierService = new ClusterApplierService( - "name", - settings(), - clusterSettings(), - mock(ThreadPool.class) - ); - clusterApplierService.setInitialState(clusterState()); - ClusterService clusterService = new ClusterService( - settings(), - clusterSettings(), - mock(ClusterManagerService.class), - clusterApplierService - ); - return new QueryGroupPersistenceService(clusterService, settings(), clusterSettings()); - } - - public static List preparePersistenceServiceSetup(Map queryGroups) { - Metadata metadata = Metadata.builder().queryGroups(queryGroups).build(); - Settings settings = Settings.builder().build(); - ClusterState clusterState = ClusterState.builder(new ClusterName("_name")).metadata(metadata).build(); - ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - ClusterApplierService clusterApplierService = new ClusterApplierService( - "name", - settings(), - clusterSettings(), - mock(ThreadPool.class) - ); - clusterApplierService.setInitialState(clusterState); - ClusterService clusterService = new ClusterService( - settings(), - clusterSettings(), - mock(ClusterManagerService.class), - clusterApplierService - ); - QueryGroupPersistenceService queryGroupPersistenceService = new QueryGroupPersistenceService( - clusterService, - settings, - clusterSettings - ); - return List.of(queryGroupPersistenceService, clusterState); - } - - public static void compareQueryGroups(List listOne, List listTwo) { - assertEquals(listOne.size(), listTwo.size()); - listOne.sort(Comparator.comparing(QueryGroup::getName)); - listTwo.sort(Comparator.comparing(QueryGroup::getName)); - for (int i = 0; i < listOne.size(); i++) { - assertTrue(listOne.get(i).equals(listTwo.get(i))); - } - } -} diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceServiceTests.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceServiceTests.java deleted file mode 100644 index 10128afb884ee..0000000000000 --- a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/service/QueryGroupPersistenceServiceTests.java +++ /dev/null @@ -1,89 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.plugin.wlm.action.service; - -import org.opensearch.cluster.metadata.QueryGroup; -import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.settings.ClusterSettings; -import org.opensearch.common.settings.Settings; -import org.opensearch.core.action.ActionListener; -import org.opensearch.plugin.wlm.action.GetQueryGroupResponse; -import org.opensearch.test.OpenSearchTestCase; - -import java.util.ArrayList; -import java.util.List; -import java.util.Set; -import java.util.stream.Collectors; - -import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.NAME_NONE_EXISTED; -import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.NAME_ONE; -import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.NAME_TWO; -import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.clusterState; -import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.compareQueryGroups; -import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.queryGroupList; -import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.queryGroupOne; -import static org.opensearch.plugin.wlm.action.QueryGroupTestUtils.queryGroupPersistenceService; -import static org.mockito.Mockito.mock; - -public class QueryGroupPersistenceServiceTests extends OpenSearchTestCase { - - public void testGetSingleQueryGroup() { - List groups = queryGroupPersistenceService().getFromClusterStateMetadata(NAME_ONE, clusterState()); - assertEquals(1, groups.size()); - QueryGroup queryGroup = groups.get(0); - List listOne = new ArrayList<>(); - List listTwo = new ArrayList<>(); - listOne.add(queryGroupOne); - listTwo.add(queryGroup); - compareQueryGroups(listOne, listTwo); - } - - public void testGetAllQueryGroups() { - assertEquals(2, clusterState().metadata().queryGroups().size()); - List res = queryGroupPersistenceService().getFromClusterStateMetadata(null, clusterState()); - assertEquals(2, res.size()); - Set currentNAME = res.stream().map(QueryGroup::getName).collect(Collectors.toSet()); - assertTrue(currentNAME.contains(NAME_ONE)); - assertTrue(currentNAME.contains(NAME_TWO)); - compareQueryGroups(queryGroupList(), res); - } - - public void testGetZeroQueryGroups() { - Settings settings = Settings.builder().build(); - ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - QueryGroupPersistenceService queryGroupPersistenceService = new QueryGroupPersistenceService( - mock(ClusterService.class), - settings, - clusterSettings - ); - List res = queryGroupPersistenceService.getFromClusterStateMetadata(NAME_NONE_EXISTED, clusterState()); - assertEquals(0, res.size()); - } - - public void testGetNonExistedQueryGroups() { - List groups = queryGroupPersistenceService().getFromClusterStateMetadata(NAME_NONE_EXISTED, clusterState()); - assertEquals(0, groups.size()); - } - - @SuppressWarnings("unchecked") - public void testGet() { - QueryGroupPersistenceService queryGroupPersistenceService = queryGroupPersistenceService(); - ActionListener mockListener = mock(ActionListener.class); - queryGroupPersistenceService.get(NAME_ONE, mockListener); - queryGroupPersistenceService.get(NAME_NONE_EXISTED, mockListener); - } - - public void testMaxQueryGroupCount() { - assertThrows(IllegalArgumentException.class, () -> queryGroupPersistenceService().setMaxQueryGroupCount(-1)); - QueryGroupPersistenceService queryGroupPersistenceService = queryGroupPersistenceService(); - queryGroupPersistenceService.setMaxQueryGroupCount(50); - assertEquals(50, queryGroupPersistenceService.getMaxQueryGroupCount()); - - } -} diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceServiceTests.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceServiceTests.java index 533c98b44685d..f5ddecfde7e28 100644 --- a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceServiceTests.java +++ b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceServiceTests.java @@ -20,6 +20,7 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.plugin.wlm.QueryGroupTestUtils; import org.opensearch.plugin.wlm.action.CreateQueryGroupResponse; +import org.opensearch.plugin.wlm.action.GetQueryGroupResponse; import org.opensearch.search.ResourceType; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.threadpool.ThreadPool; @@ -29,6 +30,9 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; + import org.mockito.ArgumentCaptor; @@ -244,4 +248,63 @@ public void testPersistInClusterStateMetadataFailure() { queryGroupPersistenceService.persistInClusterStateMetadata(queryGroupOne, listener); verify(listener).onFailure(any(RuntimeException.class)); } + + public void testGetSingleQueryGroup() { + List groups = QueryGroupTestUtils.queryGroupPersistenceService() + .getQueryGroupsFromClusterState(QueryGroupTestUtils.NAME_ONE, QueryGroupTestUtils.clusterState()); + assertEquals(1, groups.size()); + QueryGroup queryGroup = groups.get(0); + List listOne = new ArrayList<>(); + List listTwo = new ArrayList<>(); + listOne.add(QueryGroupTestUtils.queryGroupOne); + listTwo.add(queryGroup); + QueryGroupTestUtils.assertEqualQueryGroups(listOne, listTwo); + } + + public void testGetAllQueryGroups() { + assertEquals(2, QueryGroupTestUtils.clusterState().metadata().queryGroups().size()); + List res = QueryGroupTestUtils.queryGroupPersistenceService() + .getQueryGroupsFromClusterState(null, QueryGroupTestUtils.clusterState()); + assertEquals(2, res.size()); + Set currentNAME = res.stream().map(QueryGroup::getName).collect(Collectors.toSet()); + assertTrue(currentNAME.contains(QueryGroupTestUtils.NAME_ONE)); + assertTrue(currentNAME.contains(QueryGroupTestUtils.NAME_TWO)); + QueryGroupTestUtils.assertEqualQueryGroups(QueryGroupTestUtils.queryGroupList(), res); + } + + public void testGetZeroQueryGroups() { + Settings settings = Settings.builder().build(); + ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + QueryGroupPersistenceService queryGroupPersistenceService = new QueryGroupPersistenceService( + mock(ClusterService.class), + settings, + clusterSettings + ); + List res = queryGroupPersistenceService.getQueryGroupsFromClusterState( + QueryGroupTestUtils.NAME_NONE_EXISTED, + QueryGroupTestUtils.clusterState() + ); + assertEquals(0, res.size()); + } + + public void testGetNonExistedQueryGroups() { + List groups = QueryGroupTestUtils.queryGroupPersistenceService() + .getQueryGroupsFromClusterState(QueryGroupTestUtils.NAME_NONE_EXISTED, QueryGroupTestUtils.clusterState()); + assertEquals(0, groups.size()); + } + + @SuppressWarnings("unchecked") + public void testGet() { + QueryGroupPersistenceService queryGroupPersistenceService = QueryGroupTestUtils.queryGroupPersistenceService(); + ActionListener mockListener = mock(ActionListener.class); + queryGroupPersistenceService.getFromClusterStateMetadata(QueryGroupTestUtils.NAME_ONE, mockListener); + queryGroupPersistenceService.getFromClusterStateMetadata(QueryGroupTestUtils.NAME_NONE_EXISTED, mockListener); + } + + public void testMaxQueryGroupCount() { + assertThrows(IllegalArgumentException.class, () -> QueryGroupTestUtils.queryGroupPersistenceService().setMaxQueryGroupCount(-1)); + QueryGroupPersistenceService queryGroupPersistenceService = QueryGroupTestUtils.queryGroupPersistenceService(); + queryGroupPersistenceService.setMaxQueryGroupCount(50); + assertEquals(50, queryGroupPersistenceService.getMaxQueryGroupCount()); + } } diff --git a/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java b/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java index 4da6c68b40733..26f2e9ff34db5 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java @@ -1831,8 +1831,8 @@ static void validateDataStreams(SortedMap indicesLooku for (DataStream ds : dsMetadata.dataStreams().values()) { String prefix = DataStream.BACKING_INDEX_PREFIX + ds.getName() + "-"; Set conflicts = indicesLookup.subMap(prefix, DataStream.BACKING_INDEX_PREFIX + ds.getName() + ".") // '.' is the - // char after - // '-' + // char after + // '-' .keySet() .stream() .filter(s -> NUMBER_PATTERN.matcher(s.substring(prefix.length())).matches()) diff --git a/server/src/main/java/org/opensearch/search/query_group/QueryGroupServiceSettings.java b/server/src/main/java/org/opensearch/search/query_group/QueryGroupServiceSettings.java index 425916ce3e924..00489414b5262 100644 --- a/server/src/main/java/org/opensearch/search/query_group/QueryGroupServiceSettings.java +++ b/server/src/main/java/org/opensearch/search/query_group/QueryGroupServiceSettings.java @@ -18,20 +18,26 @@ */ public class QueryGroupServiceSettings { private static final Long DEFAULT_RUN_INTERVAL_MILLIS = 1000l; - private static final Double DEFAULT_NODE_LEVEL_REJECTION_THRESHOLD = 0.8; - private static final Double DEFAULT_NODE_LEVEL_CANCELLATION_THRESHOLD = 0.9; + private static final Double DEFAULT_NODE_LEVEL_MEMORY_REJECTION_THRESHOLD = 0.8; + private static final Double DEFAULT_NODE_LEVEL_MEMORY_CANCELLATION_THRESHOLD = 0.9; + private static final Double DEFAULT_NODE_LEVEL_CPU_REJECTION_THRESHOLD = 0.8; + private static final Double DEFAULT_NODE_LEVEL_CPU_CANCELLATION_THRESHOLD = 0.9; /** * default max queryGroup count on any node at any given point in time */ public static final int DEFAULT_MAX_QUERY_GROUP_COUNT_VALUE = 100; public static final String QUERY_GROUP_COUNT_SETTING_NAME = "node.query_group.max_count"; - public static final double NODE_LEVEL_CANCELLATION_THRESHOLD_MAX_VALUE = 0.95; - public static final double NODE_LEVEL_REJECTION_THRESHOLD_MAX_VALUE = 0.90; + public static final double NODE_LEVEL_MEMORY_CANCELLATION_THRESHOLD_MAX_VALUE = 0.95; + public static final double NODE_LEVEL_MEMORY_REJECTION_THRESHOLD_MAX_VALUE = 0.90; + public static final double NODE_LEVEL_CPU_CANCELLATION_THRESHOLD_MAX_VALUE = 0.95; + public static final double NODE_LEVEL_CPU_REJECTION_THRESHOLD_MAX_VALUE = 0.90; private TimeValue runIntervalMillis; private Double nodeLevelMemoryCancellationThreshold; private Double nodeLevelMemoryRejectionThreshold; + private Double nodeLevelCpuCancellationThreshold; + private Double nodeLevelCpuRejectionThreshold; private volatile int maxQueryGroupCount; /** * max QueryGroup count setting @@ -64,28 +70,54 @@ public class QueryGroupServiceSettings { ); /** - * Setting name for node level rejection threshold for QSB + * Setting name for node level memory rejection threshold for QSB */ - public static final String NODE_REJECTION_THRESHOLD_SETTING_NAME = "query_group.node.rejection_threshold"; + public static final String NODE_MEMORY_REJECTION_THRESHOLD_SETTING_NAME = "query_group.node.memory_rejection_threshold"; /** - * Setting to control the rejection threshold + * Setting to control the memory rejection threshold */ - public static final Setting NODE_LEVEL_REJECTION_THRESHOLD = Setting.doubleSetting( - NODE_REJECTION_THRESHOLD_SETTING_NAME, - DEFAULT_NODE_LEVEL_REJECTION_THRESHOLD, + public static final Setting NODE_LEVEL_MEMORY_REJECTION_THRESHOLD = Setting.doubleSetting( + NODE_MEMORY_REJECTION_THRESHOLD_SETTING_NAME, + DEFAULT_NODE_LEVEL_MEMORY_REJECTION_THRESHOLD, Setting.Property.Dynamic, Setting.Property.NodeScope ); /** - * Setting name for node level cancellation threshold + * Setting name for node level cpu rejection threshold for QSB */ - public static final String NODE_CANCELLATION_THRESHOLD_SETTING_NAME = "query_group.node.cancellation_threshold"; + public static final String NODE_CPU_REJECTION_THRESHOLD_SETTING_NAME = "query_group.node.cpu_rejection_threshold"; /** - * Setting name for node level cancellation threshold + * Setting to control the cpu rejection threshold */ - public static final Setting NODE_LEVEL_CANCELLATION_THRESHOLD = Setting.doubleSetting( - NODE_CANCELLATION_THRESHOLD_SETTING_NAME, - DEFAULT_NODE_LEVEL_CANCELLATION_THRESHOLD, + public static final Setting NODE_LEVEL_CPU_REJECTION_THRESHOLD = Setting.doubleSetting( + NODE_CPU_REJECTION_THRESHOLD_SETTING_NAME, + DEFAULT_NODE_LEVEL_CPU_REJECTION_THRESHOLD, + Setting.Property.Dynamic, + Setting.Property.NodeScope + ); + /** + * Setting name for node level memory cancellation threshold + */ + public static final String NODE_MEMORY_CANCELLATION_THRESHOLD_SETTING_NAME = "query_group.node.memory_cancellation_threshold"; + /** + * Setting name for node level memory cancellation threshold + */ + public static final Setting NODE_LEVEL_MEMORY_CANCELLATION_THRESHOLD = Setting.doubleSetting( + NODE_MEMORY_CANCELLATION_THRESHOLD_SETTING_NAME, + DEFAULT_NODE_LEVEL_MEMORY_CANCELLATION_THRESHOLD, + Setting.Property.Dynamic, + Setting.Property.NodeScope + ); + /** + * Setting name for node level cpu cancellation threshold + */ + public static final String NODE_CPU_CANCELLATION_THRESHOLD_SETTING_NAME = "query_group.node.cpu_cancellation_threshold"; + /** + * Setting name for node level cpu cancellation threshold + */ + public static final Setting NODE_LEVEL_CPU_CANCELLATION_THRESHOLD = Setting.doubleSetting( + NODE_CPU_CANCELLATION_THRESHOLD_SETTING_NAME, + DEFAULT_NODE_LEVEL_CPU_CANCELLATION_THRESHOLD, Setting.Property.Dynamic, Setting.Property.NodeScope ); @@ -97,15 +129,30 @@ public class QueryGroupServiceSettings { */ public QueryGroupServiceSettings(Settings settings, ClusterSettings clusterSettings) { runIntervalMillis = new TimeValue(QUERY_GROUP_RUN_INTERVAL_SETTING.get(settings)); - nodeLevelMemoryCancellationThreshold = NODE_LEVEL_CANCELLATION_THRESHOLD.get(settings); - nodeLevelMemoryRejectionThreshold = NODE_LEVEL_REJECTION_THRESHOLD.get(settings); + nodeLevelMemoryCancellationThreshold = NODE_LEVEL_MEMORY_CANCELLATION_THRESHOLD.get(settings); + nodeLevelMemoryRejectionThreshold = NODE_LEVEL_MEMORY_REJECTION_THRESHOLD.get(settings); + nodeLevelCpuCancellationThreshold = NODE_LEVEL_CPU_CANCELLATION_THRESHOLD.get(settings); + nodeLevelCpuRejectionThreshold = NODE_LEVEL_CPU_REJECTION_THRESHOLD.get(settings); maxQueryGroupCount = MAX_QUERY_GROUP_COUNT.get(settings); - ensureRejectionThresholdIsLessThanCancellation(nodeLevelMemoryRejectionThreshold, nodeLevelMemoryCancellationThreshold); + ensureRejectionThresholdIsLessThanCancellation( + nodeLevelMemoryRejectionThreshold, + nodeLevelMemoryCancellationThreshold, + NODE_MEMORY_REJECTION_THRESHOLD_SETTING_NAME, + NODE_MEMORY_CANCELLATION_THRESHOLD_SETTING_NAME + ); + ensureRejectionThresholdIsLessThanCancellation( + nodeLevelCpuRejectionThreshold, + nodeLevelCpuCancellationThreshold, + NODE_CPU_REJECTION_THRESHOLD_SETTING_NAME, + NODE_CPU_CANCELLATION_THRESHOLD_SETTING_NAME + ); clusterSettings.addSettingsUpdateConsumer(MAX_QUERY_GROUP_COUNT, this::setMaxQueryGroupCount); - clusterSettings.addSettingsUpdateConsumer(NODE_LEVEL_CANCELLATION_THRESHOLD, this::setNodeLevelMemoryCancellationThreshold); - clusterSettings.addSettingsUpdateConsumer(NODE_LEVEL_REJECTION_THRESHOLD, this::setNodeLevelMemoryRejectionThreshold); + clusterSettings.addSettingsUpdateConsumer(NODE_LEVEL_MEMORY_CANCELLATION_THRESHOLD, this::setNodeLevelMemoryCancellationThreshold); + clusterSettings.addSettingsUpdateConsumer(NODE_LEVEL_MEMORY_REJECTION_THRESHOLD, this::setNodeLevelMemoryRejectionThreshold); + clusterSettings.addSettingsUpdateConsumer(NODE_LEVEL_CPU_CANCELLATION_THRESHOLD, this::setNodeLevelCpuCancellationThreshold); + clusterSettings.addSettingsUpdateConsumer(NODE_LEVEL_CPU_REJECTION_THRESHOLD, this::setNodeLevelCpuRejectionThreshold); } /** @@ -128,62 +175,134 @@ public void setMaxQueryGroupCount(int newMaxQueryGroupCount) { } /** - * Method to get the node level cancellation threshold - * @return current node level cancellation threshold + * Method to get the node level memory cancellation threshold + * @return current node level memory cancellation threshold */ public Double getNodeLevelMemoryCancellationThreshold() { return nodeLevelMemoryCancellationThreshold; } /** - * Method to set the node level cancellation threshold - * @param nodeLevelMemoryCancellationThreshold sets the new node level cancellation threshold + * Method to set the node level memory cancellation threshold + * @param nodeLevelMemoryCancellationThreshold sets the new node level memory cancellation threshold * @throws IllegalArgumentException if the value is > 0.95 and cancellation < rejection threshold */ public void setNodeLevelMemoryCancellationThreshold(Double nodeLevelMemoryCancellationThreshold) { - if (Double.compare(nodeLevelMemoryCancellationThreshold, NODE_LEVEL_CANCELLATION_THRESHOLD_MAX_VALUE) > 0) { + if (Double.compare(nodeLevelMemoryCancellationThreshold, NODE_LEVEL_MEMORY_CANCELLATION_THRESHOLD_MAX_VALUE) > 0) { throw new IllegalArgumentException( - NODE_CANCELLATION_THRESHOLD_SETTING_NAME + " value should not be greater than 0.95 as it pose a threat of node drop" + NODE_MEMORY_CANCELLATION_THRESHOLD_SETTING_NAME + " value should not be greater than 0.95 as it pose a threat of node drop" ); } - ensureRejectionThresholdIsLessThanCancellation(nodeLevelMemoryRejectionThreshold, nodeLevelMemoryCancellationThreshold); + ensureRejectionThresholdIsLessThanCancellation( + nodeLevelMemoryRejectionThreshold, + nodeLevelMemoryCancellationThreshold, + NODE_MEMORY_REJECTION_THRESHOLD_SETTING_NAME, + NODE_MEMORY_CANCELLATION_THRESHOLD_SETTING_NAME + ); this.nodeLevelMemoryCancellationThreshold = nodeLevelMemoryCancellationThreshold; } /** - * Method to get the node level rejection threshold - * @return the current node level rejection threshold + * Method to get the node level cpu cancellation threshold + * @return current node level cpu cancellation threshold + */ + public Double getNodeLevelCpuCancellationThreshold() { + return nodeLevelCpuCancellationThreshold; + } + + /** + * Method to set the node level cpu cancellation threshold + * @param nodeLevelCpuCancellationThreshold sets the new node level cpu cancellation threshold + * @throws IllegalArgumentException if the value is > 0.95 and cancellation < rejection threshold + */ + public void setNodeLevelCpuCancellationThreshold(Double nodeLevelCpuCancellationThreshold) { + if (Double.compare(nodeLevelCpuCancellationThreshold, NODE_LEVEL_CPU_CANCELLATION_THRESHOLD_MAX_VALUE) > 0) { + throw new IllegalArgumentException( + NODE_CPU_CANCELLATION_THRESHOLD_SETTING_NAME + " value should not be greater than 0.95 as it pose a threat of node drop" + ); + } + + ensureRejectionThresholdIsLessThanCancellation( + nodeLevelCpuRejectionThreshold, + nodeLevelCpuCancellationThreshold, + NODE_CPU_REJECTION_THRESHOLD_SETTING_NAME, + NODE_CPU_CANCELLATION_THRESHOLD_SETTING_NAME + ); + + this.nodeLevelCpuCancellationThreshold = nodeLevelCpuCancellationThreshold; + } + + /** + * Method to get the memory node level rejection threshold + * @return the current memory node level rejection threshold */ public Double getNodeLevelMemoryRejectionThreshold() { return nodeLevelMemoryRejectionThreshold; } /** - * Method to set the node level rejection threshold - * @param nodeLevelMemoryRejectionThreshold sets the new rejection threshold + * Method to set the node level memory rejection threshold + * @param nodeLevelMemoryRejectionThreshold sets the new memory rejection threshold * @throws IllegalArgumentException if rejection > 0.90 and rejection < cancellation threshold */ public void setNodeLevelMemoryRejectionThreshold(Double nodeLevelMemoryRejectionThreshold) { - if (Double.compare(nodeLevelMemoryRejectionThreshold, NODE_LEVEL_REJECTION_THRESHOLD_MAX_VALUE) > 0) { + if (Double.compare(nodeLevelMemoryRejectionThreshold, NODE_LEVEL_MEMORY_REJECTION_THRESHOLD_MAX_VALUE) > 0) { throw new IllegalArgumentException( - NODE_REJECTION_THRESHOLD_SETTING_NAME + " value not be greater than 0.90 as it pose a threat of node drop" + NODE_MEMORY_REJECTION_THRESHOLD_SETTING_NAME + " value not be greater than 0.90 as it pose a threat of node drop" ); } - ensureRejectionThresholdIsLessThanCancellation(nodeLevelMemoryRejectionThreshold, nodeLevelMemoryCancellationThreshold); + ensureRejectionThresholdIsLessThanCancellation( + nodeLevelMemoryRejectionThreshold, + nodeLevelMemoryCancellationThreshold, + NODE_MEMORY_REJECTION_THRESHOLD_SETTING_NAME, + NODE_MEMORY_CANCELLATION_THRESHOLD_SETTING_NAME + ); this.nodeLevelMemoryRejectionThreshold = nodeLevelMemoryRejectionThreshold; } + /** + * Method to get the cpu node level rejection threshold + * @return the current cpu node level rejection threshold + */ + public Double getNodeLevelCpuRejectionThreshold() { + return nodeLevelCpuRejectionThreshold; + } + + /** + * Method to set the node level cpu rejection threshold + * @param nodeLevelCpuRejectionThreshold sets the new cpu rejection threshold + * @throws IllegalArgumentException if rejection > 0.90 and rejection < cancellation threshold + */ + public void setNodeLevelCpuRejectionThreshold(Double nodeLevelCpuRejectionThreshold) { + if (Double.compare(nodeLevelCpuRejectionThreshold, NODE_LEVEL_CPU_REJECTION_THRESHOLD_MAX_VALUE) > 0) { + throw new IllegalArgumentException( + NODE_CPU_REJECTION_THRESHOLD_SETTING_NAME + " value not be greater than 0.90 as it pose a threat of node drop" + ); + } + + ensureRejectionThresholdIsLessThanCancellation( + nodeLevelCpuRejectionThreshold, + nodeLevelCpuCancellationThreshold, + NODE_CPU_REJECTION_THRESHOLD_SETTING_NAME, + NODE_CPU_CANCELLATION_THRESHOLD_SETTING_NAME + ); + + this.nodeLevelCpuRejectionThreshold = nodeLevelCpuRejectionThreshold; + } + private void ensureRejectionThresholdIsLessThanCancellation( - Double nodeLevelMemoryRejectionThreshold, - Double nodeLevelMemoryCancellationThreshold + Double nodeLevelRejectionThreshold, + Double nodeLevelCancellationThreshold, + String rejectionThresholdSettingName, + String cancellationThresholdSettingName ) { - if (Double.compare(nodeLevelMemoryCancellationThreshold, nodeLevelMemoryRejectionThreshold) < 0) { + if (Double.compare(nodeLevelCancellationThreshold, nodeLevelRejectionThreshold) < 0) { throw new IllegalArgumentException( - NODE_CANCELLATION_THRESHOLD_SETTING_NAME + " value should not be less than " + NODE_REJECTION_THRESHOLD_SETTING_NAME + cancellationThresholdSettingName + " value should not be less than " + rejectionThresholdSettingName ); } } From b4fbe96a1d19f1b8113d3544beac2eb020515c9c Mon Sep 17 00:00:00 2001 From: Ruirui Zhang Date: Mon, 29 Jul 2024 13:30:54 -0700 Subject: [PATCH 08/14] incorperate comments from create api PR Signed-off-by: Ruirui Zhang --- .../wlm/WorkloadManagementPluginModule.java | 25 -- .../wlm/action/GetQueryGroupAction.java | 1 - .../wlm/action/GetQueryGroupRequest.java | 22 +- .../wlm/action/GetQueryGroupResponse.java | 2 +- .../action/TransportGetQueryGroupAction.java | 5 +- .../wlm/rest/RestGetQueryGroupAction.java | 3 +- .../service/QueryGroupPersistenceService.java | 20 +- .../action/GetQueryGroupResponseTests.java | 12 +- .../QueryGroupPersistenceServiceTests.java | 17 +- .../common/settings/ClusterSettings.java | 1 - .../QueryGroupServiceSettings.java | 317 ------------------ .../search/query_group/package-info.java | 12 - 12 files changed, 39 insertions(+), 398 deletions(-) delete mode 100644 plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/WorkloadManagementPluginModule.java delete mode 100644 server/src/main/java/org/opensearch/search/query_group/QueryGroupServiceSettings.java delete mode 100644 server/src/main/java/org/opensearch/search/query_group/package-info.java diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/WorkloadManagementPluginModule.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/WorkloadManagementPluginModule.java deleted file mode 100644 index 87109d06259bb..0000000000000 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/WorkloadManagementPluginModule.java +++ /dev/null @@ -1,25 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.plugin.wlm; - -import org.opensearch.common.inject.AbstractModule; - -/** - * Guice Module to manage WorkloadManagement related objects - */ -public class WorkloadManagementPluginModule extends AbstractModule { - - /** - * Constructor for WorkloadManagementPluginModule - */ - public WorkloadManagementPluginModule() {} - - @Override - protected void configure() {} -} diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupAction.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupAction.java index a4c199cf1da61..0200185580f7d 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupAction.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupAction.java @@ -18,7 +18,6 @@ public class GetQueryGroupAction extends ActionType { /** - /** * An instance of GetQueryGroupAction */ public static final GetQueryGroupAction INSTANCE = new GetQueryGroupAction(); diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequest.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequest.java index 4633d710dcb52..fd556890f4d69 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequest.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequest.java @@ -12,17 +12,16 @@ import org.opensearch.action.ActionRequestValidationException; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; -import org.opensearch.core.common.io.stream.Writeable; import java.io.IOException; /** - * A request for get QueryGroup + * Request for get QueryGroup * * @opensearch.experimental */ -public class GetQueryGroupRequest extends ActionRequest implements Writeable.Reader { - String name; +public class GetQueryGroupRequest extends ActionRequest { + final String name; /** * Default constructor for GetQueryGroupRequest @@ -42,13 +41,14 @@ public GetQueryGroupRequest(StreamInput in) throws IOException { } @Override - public GetQueryGroupRequest read(StreamInput in) throws IOException { - return new GetQueryGroupRequest(in); + public ActionRequestValidationException validate() { + return null; } @Override - public ActionRequestValidationException validate() { - return null; + public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); + out.writeOptionalString(name); } /** @@ -57,10 +57,4 @@ public ActionRequestValidationException validate() { public String getName() { return name; } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - out.writeOptionalString(name); - } } diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponse.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponse.java index c6b6dbc4a967f..fa87e4e4e7277 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponse.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponse.java @@ -27,7 +27,7 @@ */ public class GetQueryGroupResponse extends ActionResponse implements ToXContent, ToXContentObject { private final List queryGroups; - private RestStatus restStatus; + private final RestStatus restStatus; /** * Constructor for GetQueryGroupResponse diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java index 45d26ef9f97e4..6c662341bae5f 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java @@ -18,7 +18,7 @@ import org.opensearch.transport.TransportService; /** - * Transport action for get QueryGroup + * Transport action to get QueryGroup * * @opensearch.experimental */ @@ -51,8 +51,7 @@ public TransportGetQueryGroupAction( @Override protected void doExecute(Task task, GetQueryGroupRequest request, ActionListener listener) { - String name = request.getName(); threadPool.executor(ThreadPool.Names.GENERIC) - .execute(() -> queryGroupPersistenceService.getFromClusterStateMetadata(name, listener)); + .execute(() -> queryGroupPersistenceService.getFromClusterStateMetadata(request.getName(), listener)); } } diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/rest/RestGetQueryGroupAction.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/rest/RestGetQueryGroupAction.java index 810afaf38131a..9566ddcce3e42 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/rest/RestGetQueryGroupAction.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/rest/RestGetQueryGroupAction.java @@ -53,8 +53,7 @@ public List routes() { @Override protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { - String name = request.param("name"); - GetQueryGroupRequest getQueryGroupRequest = new GetQueryGroupRequest(name); + GetQueryGroupRequest getQueryGroupRequest = new GetQueryGroupRequest(request.param("name")); return channel -> client.execute(GetQueryGroupAction.INSTANCE, getQueryGroupRequest, getQueryGroupResponse(channel)); } diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceService.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceService.java index d97fb4ce57832..4e958db89eab9 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceService.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceService.java @@ -201,30 +201,28 @@ private Map calculateTotalUsage(Map ex * @param listener - ActionListener for GetQueryGroupResponse */ public void getFromClusterStateMetadata(String name, ActionListener listener) { - ClusterState currentState = clusterService.state(); - List resultGroups = getQueryGroupsFromClusterState(name, currentState); + List resultGroups = getQueryGroupsFromClusterState(name, clusterService.state()); if (resultGroups.isEmpty() && name != null && !name.isEmpty()) { logger.warn("No QueryGroup exists with the provided name: {}", name); - Exception e = new RuntimeException("No QueryGroup exists with the provided name: " + name); + Exception e = new IllegalArgumentException("No QueryGroup exists with the provided name: " + name); listener.onFailure(e); return; } - GetQueryGroupResponse response = new GetQueryGroupResponse(resultGroups, RestStatus.OK); - listener.onResponse(response); + listener.onResponse(new GetQueryGroupResponse(resultGroups, RestStatus.OK)); } + /** + * Get the QueryGroups with the specified name from cluster state + * @param name - the QueryGroup name we are getting + * @param currentState - current cluster state + */ List getQueryGroupsFromClusterState(String name, ClusterState currentState) { Map currentGroups = currentState.getMetadata().queryGroups(); if (name == null || name.isEmpty()) { return new ArrayList<>(currentGroups.values()); } List resultGroups = new ArrayList<>(); - for (QueryGroup group : currentGroups.values()) { - if (group.getName().equals(name)) { - resultGroups.add(group); - break; - } - } + currentGroups.values().stream().filter(group -> group.getName().equals(name)).findFirst().ifPresent(resultGroups::add); return resultGroups; } diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponseTests.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponseTests.java index ae50e893fdc61..356ccf6a32d9a 100644 --- a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponseTests.java +++ b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponseTests.java @@ -81,8 +81,8 @@ public void testToXContentGetSingleQueryGroup() throws IOException { + " \"_id\" : \"AgfUO5Ja9yfsYlONlYi3TQ==\",\n" + " \"name\" : \"query_group_one\",\n" + " \"resiliency_mode\" : \"monitor\",\n" - + " \"updatedAt\" : 4513232413,\n" - + " \"resourceLimits\" : {\n" + + " \"updated_at\" : 4513232413,\n" + + " \"resource_limits\" : {\n" + " \"memory\" : 0.3\n" + " }\n" + " }\n" @@ -104,8 +104,8 @@ public void testToXContentGetMultipleQueryGroup() throws IOException { + " \"_id\" : \"AgfUO5Ja9yfsYlONlYi3TQ==\",\n" + " \"name\" : \"query_group_one\",\n" + " \"resiliency_mode\" : \"monitor\",\n" - + " \"updatedAt\" : 4513232413,\n" - + " \"resourceLimits\" : {\n" + + " \"updated_at\" : 4513232413,\n" + + " \"resource_limits\" : {\n" + " \"memory\" : 0.3\n" + " }\n" + " },\n" @@ -113,8 +113,8 @@ public void testToXContentGetMultipleQueryGroup() throws IOException { + " \"_id\" : \"G5iIqHy4g7eK1qIAAAAIH53=1\",\n" + " \"name\" : \"query_group_two\",\n" + " \"resiliency_mode\" : \"monitor\",\n" - + " \"updatedAt\" : 4513232415,\n" - + " \"resourceLimits\" : {\n" + + " \"updated_at\" : 4513232415,\n" + + " \"resource_limits\" : {\n" + " \"memory\" : 0.6\n" + " }\n" + " }\n" diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceServiceTests.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceServiceTests.java index f5ddecfde7e28..31dee4f46e3d5 100644 --- a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceServiceTests.java +++ b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceServiceTests.java @@ -6,6 +6,14 @@ * compatible open source license. */ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + package org.opensearch.plugin.wlm.service; import org.opensearch.cluster.ClusterName; @@ -33,7 +41,6 @@ import java.util.Set; import java.util.stream.Collectors; - import org.mockito.ArgumentCaptor; import static org.opensearch.cluster.metadata.QueryGroup.builder; @@ -273,12 +280,12 @@ public void testGetAllQueryGroups() { } public void testGetZeroQueryGroups() { - Settings settings = Settings.builder().build(); - ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + // Settings settings = Settings.builder().build(); + // ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); QueryGroupPersistenceService queryGroupPersistenceService = new QueryGroupPersistenceService( mock(ClusterService.class), - settings, - clusterSettings + QueryGroupTestUtils.settings(), + clusterSettings() ); List res = queryGroupPersistenceService.getQueryGroupsFromClusterState( QueryGroupTestUtils.NAME_NONE_EXISTED, diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index 03c18caf08506..7baae17dd77cd 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -160,7 +160,6 @@ import org.opensearch.search.backpressure.settings.SearchShardTaskSettings; import org.opensearch.search.backpressure.settings.SearchTaskSettings; import org.opensearch.search.fetch.subphase.highlight.FastVectorHighlighter; -import org.opensearch.search.query_group.QueryGroupServiceSettings; import org.opensearch.snapshots.InternalSnapshotsInfoService; import org.opensearch.snapshots.SnapshotsService; import org.opensearch.tasks.TaskCancellationMonitoringSettings; diff --git a/server/src/main/java/org/opensearch/search/query_group/QueryGroupServiceSettings.java b/server/src/main/java/org/opensearch/search/query_group/QueryGroupServiceSettings.java deleted file mode 100644 index 00489414b5262..0000000000000 --- a/server/src/main/java/org/opensearch/search/query_group/QueryGroupServiceSettings.java +++ /dev/null @@ -1,317 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.search.query_group; - -import org.opensearch.common.settings.ClusterSettings; -import org.opensearch.common.settings.Setting; -import org.opensearch.common.settings.Settings; -import org.opensearch.common.unit.TimeValue; - -/** - * Main class to declare the QueryGroup feature related settings - */ -public class QueryGroupServiceSettings { - private static final Long DEFAULT_RUN_INTERVAL_MILLIS = 1000l; - private static final Double DEFAULT_NODE_LEVEL_MEMORY_REJECTION_THRESHOLD = 0.8; - private static final Double DEFAULT_NODE_LEVEL_MEMORY_CANCELLATION_THRESHOLD = 0.9; - private static final Double DEFAULT_NODE_LEVEL_CPU_REJECTION_THRESHOLD = 0.8; - private static final Double DEFAULT_NODE_LEVEL_CPU_CANCELLATION_THRESHOLD = 0.9; - /** - * default max queryGroup count on any node at any given point in time - */ - public static final int DEFAULT_MAX_QUERY_GROUP_COUNT_VALUE = 100; - - public static final String QUERY_GROUP_COUNT_SETTING_NAME = "node.query_group.max_count"; - public static final double NODE_LEVEL_MEMORY_CANCELLATION_THRESHOLD_MAX_VALUE = 0.95; - public static final double NODE_LEVEL_MEMORY_REJECTION_THRESHOLD_MAX_VALUE = 0.90; - public static final double NODE_LEVEL_CPU_CANCELLATION_THRESHOLD_MAX_VALUE = 0.95; - public static final double NODE_LEVEL_CPU_REJECTION_THRESHOLD_MAX_VALUE = 0.90; - - private TimeValue runIntervalMillis; - private Double nodeLevelMemoryCancellationThreshold; - private Double nodeLevelMemoryRejectionThreshold; - private Double nodeLevelCpuCancellationThreshold; - private Double nodeLevelCpuRejectionThreshold; - private volatile int maxQueryGroupCount; - /** - * max QueryGroup count setting - */ - public static final Setting MAX_QUERY_GROUP_COUNT = Setting.intSetting( - QUERY_GROUP_COUNT_SETTING_NAME, - DEFAULT_MAX_QUERY_GROUP_COUNT_VALUE, - 0, - (newVal) -> { - if (newVal > 100 || newVal < 1) throw new IllegalArgumentException( - QUERY_GROUP_COUNT_SETTING_NAME + " should be in range [1-100]" - ); - }, - Setting.Property.Dynamic, - Setting.Property.NodeScope - ); - /** - * Setting name for default QueryGroup count - */ - public static final String SERVICE_RUN_INTERVAL_MILLIS_SETTING_NAME = "query_group.service.run_interval_millis"; - /** - * Setting to control the run interval of QSB service - */ - private static final Setting QUERY_GROUP_RUN_INTERVAL_SETTING = Setting.longSetting( - SERVICE_RUN_INTERVAL_MILLIS_SETTING_NAME, - DEFAULT_RUN_INTERVAL_MILLIS, - 1, - Setting.Property.Dynamic, - Setting.Property.NodeScope - ); - - /** - * Setting name for node level memory rejection threshold for QSB - */ - public static final String NODE_MEMORY_REJECTION_THRESHOLD_SETTING_NAME = "query_group.node.memory_rejection_threshold"; - /** - * Setting to control the memory rejection threshold - */ - public static final Setting NODE_LEVEL_MEMORY_REJECTION_THRESHOLD = Setting.doubleSetting( - NODE_MEMORY_REJECTION_THRESHOLD_SETTING_NAME, - DEFAULT_NODE_LEVEL_MEMORY_REJECTION_THRESHOLD, - Setting.Property.Dynamic, - Setting.Property.NodeScope - ); - /** - * Setting name for node level cpu rejection threshold for QSB - */ - public static final String NODE_CPU_REJECTION_THRESHOLD_SETTING_NAME = "query_group.node.cpu_rejection_threshold"; - /** - * Setting to control the cpu rejection threshold - */ - public static final Setting NODE_LEVEL_CPU_REJECTION_THRESHOLD = Setting.doubleSetting( - NODE_CPU_REJECTION_THRESHOLD_SETTING_NAME, - DEFAULT_NODE_LEVEL_CPU_REJECTION_THRESHOLD, - Setting.Property.Dynamic, - Setting.Property.NodeScope - ); - /** - * Setting name for node level memory cancellation threshold - */ - public static final String NODE_MEMORY_CANCELLATION_THRESHOLD_SETTING_NAME = "query_group.node.memory_cancellation_threshold"; - /** - * Setting name for node level memory cancellation threshold - */ - public static final Setting NODE_LEVEL_MEMORY_CANCELLATION_THRESHOLD = Setting.doubleSetting( - NODE_MEMORY_CANCELLATION_THRESHOLD_SETTING_NAME, - DEFAULT_NODE_LEVEL_MEMORY_CANCELLATION_THRESHOLD, - Setting.Property.Dynamic, - Setting.Property.NodeScope - ); - /** - * Setting name for node level cpu cancellation threshold - */ - public static final String NODE_CPU_CANCELLATION_THRESHOLD_SETTING_NAME = "query_group.node.cpu_cancellation_threshold"; - /** - * Setting name for node level cpu cancellation threshold - */ - public static final Setting NODE_LEVEL_CPU_CANCELLATION_THRESHOLD = Setting.doubleSetting( - NODE_CPU_CANCELLATION_THRESHOLD_SETTING_NAME, - DEFAULT_NODE_LEVEL_CPU_CANCELLATION_THRESHOLD, - Setting.Property.Dynamic, - Setting.Property.NodeScope - ); - - /** - * QueryGroup service settings constructor - * @param settings - QueryGroup service settings - * @param clusterSettings - QueryGroup cluster settings - */ - public QueryGroupServiceSettings(Settings settings, ClusterSettings clusterSettings) { - runIntervalMillis = new TimeValue(QUERY_GROUP_RUN_INTERVAL_SETTING.get(settings)); - nodeLevelMemoryCancellationThreshold = NODE_LEVEL_MEMORY_CANCELLATION_THRESHOLD.get(settings); - nodeLevelMemoryRejectionThreshold = NODE_LEVEL_MEMORY_REJECTION_THRESHOLD.get(settings); - nodeLevelCpuCancellationThreshold = NODE_LEVEL_CPU_CANCELLATION_THRESHOLD.get(settings); - nodeLevelCpuRejectionThreshold = NODE_LEVEL_CPU_REJECTION_THRESHOLD.get(settings); - maxQueryGroupCount = MAX_QUERY_GROUP_COUNT.get(settings); - - ensureRejectionThresholdIsLessThanCancellation( - nodeLevelMemoryRejectionThreshold, - nodeLevelMemoryCancellationThreshold, - NODE_MEMORY_REJECTION_THRESHOLD_SETTING_NAME, - NODE_MEMORY_CANCELLATION_THRESHOLD_SETTING_NAME - ); - ensureRejectionThresholdIsLessThanCancellation( - nodeLevelCpuRejectionThreshold, - nodeLevelCpuCancellationThreshold, - NODE_CPU_REJECTION_THRESHOLD_SETTING_NAME, - NODE_CPU_CANCELLATION_THRESHOLD_SETTING_NAME - ); - - clusterSettings.addSettingsUpdateConsumer(MAX_QUERY_GROUP_COUNT, this::setMaxQueryGroupCount); - clusterSettings.addSettingsUpdateConsumer(NODE_LEVEL_MEMORY_CANCELLATION_THRESHOLD, this::setNodeLevelMemoryCancellationThreshold); - clusterSettings.addSettingsUpdateConsumer(NODE_LEVEL_MEMORY_REJECTION_THRESHOLD, this::setNodeLevelMemoryRejectionThreshold); - clusterSettings.addSettingsUpdateConsumer(NODE_LEVEL_CPU_CANCELLATION_THRESHOLD, this::setNodeLevelCpuCancellationThreshold); - clusterSettings.addSettingsUpdateConsumer(NODE_LEVEL_CPU_REJECTION_THRESHOLD, this::setNodeLevelCpuRejectionThreshold); - } - - /** - * Method to get runInterval for QSB - * @return runInterval in milliseconds for QSB Service - */ - public TimeValue getRunIntervalMillis() { - return runIntervalMillis; - } - - /** - * Method to set the new QueryGroup count - * @param newMaxQueryGroupCount is the new maxQueryGroupCount per node - */ - public void setMaxQueryGroupCount(int newMaxQueryGroupCount) { - if (newMaxQueryGroupCount < 0) { - throw new IllegalArgumentException("node.node.query_group.max_count can't be negative"); - } - this.maxQueryGroupCount = newMaxQueryGroupCount; - } - - /** - * Method to get the node level memory cancellation threshold - * @return current node level memory cancellation threshold - */ - public Double getNodeLevelMemoryCancellationThreshold() { - return nodeLevelMemoryCancellationThreshold; - } - - /** - * Method to set the node level memory cancellation threshold - * @param nodeLevelMemoryCancellationThreshold sets the new node level memory cancellation threshold - * @throws IllegalArgumentException if the value is > 0.95 and cancellation < rejection threshold - */ - public void setNodeLevelMemoryCancellationThreshold(Double nodeLevelMemoryCancellationThreshold) { - if (Double.compare(nodeLevelMemoryCancellationThreshold, NODE_LEVEL_MEMORY_CANCELLATION_THRESHOLD_MAX_VALUE) > 0) { - throw new IllegalArgumentException( - NODE_MEMORY_CANCELLATION_THRESHOLD_SETTING_NAME + " value should not be greater than 0.95 as it pose a threat of node drop" - ); - } - - ensureRejectionThresholdIsLessThanCancellation( - nodeLevelMemoryRejectionThreshold, - nodeLevelMemoryCancellationThreshold, - NODE_MEMORY_REJECTION_THRESHOLD_SETTING_NAME, - NODE_MEMORY_CANCELLATION_THRESHOLD_SETTING_NAME - ); - - this.nodeLevelMemoryCancellationThreshold = nodeLevelMemoryCancellationThreshold; - } - - /** - * Method to get the node level cpu cancellation threshold - * @return current node level cpu cancellation threshold - */ - public Double getNodeLevelCpuCancellationThreshold() { - return nodeLevelCpuCancellationThreshold; - } - - /** - * Method to set the node level cpu cancellation threshold - * @param nodeLevelCpuCancellationThreshold sets the new node level cpu cancellation threshold - * @throws IllegalArgumentException if the value is > 0.95 and cancellation < rejection threshold - */ - public void setNodeLevelCpuCancellationThreshold(Double nodeLevelCpuCancellationThreshold) { - if (Double.compare(nodeLevelCpuCancellationThreshold, NODE_LEVEL_CPU_CANCELLATION_THRESHOLD_MAX_VALUE) > 0) { - throw new IllegalArgumentException( - NODE_CPU_CANCELLATION_THRESHOLD_SETTING_NAME + " value should not be greater than 0.95 as it pose a threat of node drop" - ); - } - - ensureRejectionThresholdIsLessThanCancellation( - nodeLevelCpuRejectionThreshold, - nodeLevelCpuCancellationThreshold, - NODE_CPU_REJECTION_THRESHOLD_SETTING_NAME, - NODE_CPU_CANCELLATION_THRESHOLD_SETTING_NAME - ); - - this.nodeLevelCpuCancellationThreshold = nodeLevelCpuCancellationThreshold; - } - - /** - * Method to get the memory node level rejection threshold - * @return the current memory node level rejection threshold - */ - public Double getNodeLevelMemoryRejectionThreshold() { - return nodeLevelMemoryRejectionThreshold; - } - - /** - * Method to set the node level memory rejection threshold - * @param nodeLevelMemoryRejectionThreshold sets the new memory rejection threshold - * @throws IllegalArgumentException if rejection > 0.90 and rejection < cancellation threshold - */ - public void setNodeLevelMemoryRejectionThreshold(Double nodeLevelMemoryRejectionThreshold) { - if (Double.compare(nodeLevelMemoryRejectionThreshold, NODE_LEVEL_MEMORY_REJECTION_THRESHOLD_MAX_VALUE) > 0) { - throw new IllegalArgumentException( - NODE_MEMORY_REJECTION_THRESHOLD_SETTING_NAME + " value not be greater than 0.90 as it pose a threat of node drop" - ); - } - - ensureRejectionThresholdIsLessThanCancellation( - nodeLevelMemoryRejectionThreshold, - nodeLevelMemoryCancellationThreshold, - NODE_MEMORY_REJECTION_THRESHOLD_SETTING_NAME, - NODE_MEMORY_CANCELLATION_THRESHOLD_SETTING_NAME - ); - - this.nodeLevelMemoryRejectionThreshold = nodeLevelMemoryRejectionThreshold; - } - - /** - * Method to get the cpu node level rejection threshold - * @return the current cpu node level rejection threshold - */ - public Double getNodeLevelCpuRejectionThreshold() { - return nodeLevelCpuRejectionThreshold; - } - - /** - * Method to set the node level cpu rejection threshold - * @param nodeLevelCpuRejectionThreshold sets the new cpu rejection threshold - * @throws IllegalArgumentException if rejection > 0.90 and rejection < cancellation threshold - */ - public void setNodeLevelCpuRejectionThreshold(Double nodeLevelCpuRejectionThreshold) { - if (Double.compare(nodeLevelCpuRejectionThreshold, NODE_LEVEL_CPU_REJECTION_THRESHOLD_MAX_VALUE) > 0) { - throw new IllegalArgumentException( - NODE_CPU_REJECTION_THRESHOLD_SETTING_NAME + " value not be greater than 0.90 as it pose a threat of node drop" - ); - } - - ensureRejectionThresholdIsLessThanCancellation( - nodeLevelCpuRejectionThreshold, - nodeLevelCpuCancellationThreshold, - NODE_CPU_REJECTION_THRESHOLD_SETTING_NAME, - NODE_CPU_CANCELLATION_THRESHOLD_SETTING_NAME - ); - - this.nodeLevelCpuRejectionThreshold = nodeLevelCpuRejectionThreshold; - } - - private void ensureRejectionThresholdIsLessThanCancellation( - Double nodeLevelRejectionThreshold, - Double nodeLevelCancellationThreshold, - String rejectionThresholdSettingName, - String cancellationThresholdSettingName - ) { - if (Double.compare(nodeLevelCancellationThreshold, nodeLevelRejectionThreshold) < 0) { - throw new IllegalArgumentException( - cancellationThresholdSettingName + " value should not be less than " + rejectionThresholdSettingName - ); - } - } - - /** - * Method to get the current QueryGroup count - * @return the current max QueryGroup count - */ - public int getMaxQueryGroupCount() { - return maxQueryGroupCount; - } -} diff --git a/server/src/main/java/org/opensearch/search/query_group/package-info.java b/server/src/main/java/org/opensearch/search/query_group/package-info.java deleted file mode 100644 index 00b68b0d3306c..0000000000000 --- a/server/src/main/java/org/opensearch/search/query_group/package-info.java +++ /dev/null @@ -1,12 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -/** - * QueryGroup related artifacts - */ -package org.opensearch.search.query_group; From 94fd6840faf960e0cf63317adc5798ee783bf9cf Mon Sep 17 00:00:00 2001 From: Ruirui Zhang Date: Thu, 8 Aug 2024 17:09:49 -0700 Subject: [PATCH 09/14] use clustermanager to get the most recent querygroups Signed-off-by: Ruirui Zhang --- .../wlm/action/GetQueryGroupRequest.java | 4 +- .../action/TransportGetQueryGroupAction.java | 72 +++++++++++++++---- .../service/QueryGroupPersistenceService.java | 18 +---- .../QueryGroupPersistenceServiceTests.java | 33 +++------ 4 files changed, 72 insertions(+), 55 deletions(-) diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequest.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequest.java index fd556890f4d69..884628999f65c 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequest.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequest.java @@ -8,8 +8,8 @@ package org.opensearch.plugin.wlm.action; -import org.opensearch.action.ActionRequest; import org.opensearch.action.ActionRequestValidationException; +import org.opensearch.action.support.clustermanager.ClusterManagerNodeReadRequest; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; @@ -20,7 +20,7 @@ * * @opensearch.experimental */ -public class GetQueryGroupRequest extends ActionRequest { +public class GetQueryGroupRequest extends ClusterManagerNodeReadRequest { final String name; /** diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java index 6c662341bae5f..7ad8cdf33966c 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java @@ -8,50 +8,94 @@ package org.opensearch.plugin.wlm.action; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.opensearch.action.support.ActionFilters; -import org.opensearch.action.support.HandledTransportAction; +import org.opensearch.action.support.clustermanager.TransportClusterManagerNodeReadAction; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.block.ClusterBlockException; +import org.opensearch.cluster.block.ClusterBlockLevel; +import org.opensearch.cluster.metadata.IndexNameExpressionResolver; +import org.opensearch.cluster.metadata.QueryGroup; +import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.inject.Inject; import org.opensearch.core.action.ActionListener; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.rest.RestStatus; import org.opensearch.plugin.wlm.service.QueryGroupPersistenceService; -import org.opensearch.tasks.Task; +import org.opensearch.search.pipeline.SearchPipelineService; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.TransportService; +import java.io.IOException; +import java.util.List; + /** * Transport action to get QueryGroup * * @opensearch.experimental */ -public class TransportGetQueryGroupAction extends HandledTransportAction { - - private final ThreadPool threadPool; - private final QueryGroupPersistenceService queryGroupPersistenceService; +public class TransportGetQueryGroupAction extends TransportClusterManagerNodeReadAction { + private static final Logger logger = LogManager.getLogger(SearchPipelineService.class); /** * Constructor for TransportGetQueryGroupAction * - * @param actionName - action name + * @param clusterService - a {@link ClusterService} object * @param transportService - a {@link TransportService} object * @param actionFilters - a {@link ActionFilters} object * @param threadPool - a {@link ThreadPool} object + * @param indexNameExpressionResolver - a {@link IndexNameExpressionResolver} object * @param queryGroupPersistenceService - a {@link QueryGroupPersistenceService} object */ @Inject public TransportGetQueryGroupAction( - String actionName, + ClusterService clusterService, TransportService transportService, ActionFilters actionFilters, ThreadPool threadPool, + IndexNameExpressionResolver indexNameExpressionResolver, QueryGroupPersistenceService queryGroupPersistenceService ) { - super(GetQueryGroupAction.NAME, transportService, actionFilters, GetQueryGroupRequest::new); - this.threadPool = threadPool; - this.queryGroupPersistenceService = queryGroupPersistenceService; + super( + GetQueryGroupAction.NAME, + transportService, + clusterService, + threadPool, + actionFilters, + GetQueryGroupRequest::new, + indexNameExpressionResolver, + true + ); + } + + @Override + protected String executor() { + return ThreadPool.Names.SAME; + } + + @Override + protected GetQueryGroupResponse read(StreamInput in) throws IOException { + return new GetQueryGroupResponse(in); + } + + @Override + protected ClusterBlockException checkBlock(GetQueryGroupRequest request, ClusterState state) { + return state.blocks().globalBlockedException(ClusterBlockLevel.METADATA_READ); } @Override - protected void doExecute(Task task, GetQueryGroupRequest request, ActionListener listener) { - threadPool.executor(ThreadPool.Names.GENERIC) - .execute(() -> queryGroupPersistenceService.getFromClusterStateMetadata(request.getName(), listener)); + protected void clusterManagerOperation(GetQueryGroupRequest request, ClusterState state, ActionListener listener) + throws Exception { + String name = request.getName(); + List resultGroups = QueryGroupPersistenceService.getFromClusterStateMetadata(name, state); + + if (resultGroups.isEmpty() && name != null && !name.isEmpty()) { + logger.warn("No QueryGroup exists with the provided name: {}", name); + Exception e = new IllegalArgumentException("No QueryGroup exists with the provided name: " + name); + listener.onFailure(e); + return; + } + listener.onResponse(new GetQueryGroupResponse(resultGroups, RestStatus.OK)); } } diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceService.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceService.java index 4e958db89eab9..4dea6b622caab 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceService.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceService.java @@ -195,28 +195,12 @@ private Map calculateTotalUsage(Map ex return map; } - /** - * Get the QueryGroup with the specified name from cluster state - * @param name - the QueryGroup name we are getting - * @param listener - ActionListener for GetQueryGroupResponse - */ - public void getFromClusterStateMetadata(String name, ActionListener listener) { - List resultGroups = getQueryGroupsFromClusterState(name, clusterService.state()); - if (resultGroups.isEmpty() && name != null && !name.isEmpty()) { - logger.warn("No QueryGroup exists with the provided name: {}", name); - Exception e = new IllegalArgumentException("No QueryGroup exists with the provided name: " + name); - listener.onFailure(e); - return; - } - listener.onResponse(new GetQueryGroupResponse(resultGroups, RestStatus.OK)); - } - /** * Get the QueryGroups with the specified name from cluster state * @param name - the QueryGroup name we are getting * @param currentState - current cluster state */ - List getQueryGroupsFromClusterState(String name, ClusterState currentState) { + public static List getFromClusterStateMetadata(String name, ClusterState currentState) { Map currentGroups = currentState.getMetadata().queryGroups(); if (name == null || name.isEmpty()) { return new ArrayList<>(currentGroups.values()); diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceServiceTests.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceServiceTests.java index 31dee4f46e3d5..4a83132b10663 100644 --- a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceServiceTests.java +++ b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceServiceTests.java @@ -30,6 +30,7 @@ import org.opensearch.plugin.wlm.action.CreateQueryGroupResponse; import org.opensearch.plugin.wlm.action.GetQueryGroupResponse; import org.opensearch.search.ResourceType; +import org.opensearch.plugin.wlm.QueryGroupTestUtils; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.threadpool.ThreadPool; @@ -257,8 +258,10 @@ public void testPersistInClusterStateMetadataFailure() { } public void testGetSingleQueryGroup() { - List groups = QueryGroupTestUtils.queryGroupPersistenceService() - .getQueryGroupsFromClusterState(QueryGroupTestUtils.NAME_ONE, QueryGroupTestUtils.clusterState()); + List groups = QueryGroupPersistenceService.getFromClusterStateMetadata( + QueryGroupTestUtils.NAME_ONE, + QueryGroupTestUtils.clusterState() + ); assertEquals(1, groups.size()); QueryGroup queryGroup = groups.get(0); List listOne = new ArrayList<>(); @@ -270,8 +273,7 @@ public void testGetSingleQueryGroup() { public void testGetAllQueryGroups() { assertEquals(2, QueryGroupTestUtils.clusterState().metadata().queryGroups().size()); - List res = QueryGroupTestUtils.queryGroupPersistenceService() - .getQueryGroupsFromClusterState(null, QueryGroupTestUtils.clusterState()); + List res = QueryGroupPersistenceService.getFromClusterStateMetadata(null, QueryGroupTestUtils.clusterState()); assertEquals(2, res.size()); Set currentNAME = res.stream().map(QueryGroup::getName).collect(Collectors.toSet()); assertTrue(currentNAME.contains(QueryGroupTestUtils.NAME_ONE)); @@ -280,14 +282,7 @@ public void testGetAllQueryGroups() { } public void testGetZeroQueryGroups() { - // Settings settings = Settings.builder().build(); - // ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - QueryGroupPersistenceService queryGroupPersistenceService = new QueryGroupPersistenceService( - mock(ClusterService.class), - QueryGroupTestUtils.settings(), - clusterSettings() - ); - List res = queryGroupPersistenceService.getQueryGroupsFromClusterState( + List res = QueryGroupPersistenceService.getFromClusterStateMetadata( QueryGroupTestUtils.NAME_NONE_EXISTED, QueryGroupTestUtils.clusterState() ); @@ -295,19 +290,13 @@ public void testGetZeroQueryGroups() { } public void testGetNonExistedQueryGroups() { - List groups = QueryGroupTestUtils.queryGroupPersistenceService() - .getQueryGroupsFromClusterState(QueryGroupTestUtils.NAME_NONE_EXISTED, QueryGroupTestUtils.clusterState()); + List groups = QueryGroupPersistenceService.getFromClusterStateMetadata( + QueryGroupTestUtils.NAME_NONE_EXISTED, + QueryGroupTestUtils.clusterState() + ); assertEquals(0, groups.size()); } - @SuppressWarnings("unchecked") - public void testGet() { - QueryGroupPersistenceService queryGroupPersistenceService = QueryGroupTestUtils.queryGroupPersistenceService(); - ActionListener mockListener = mock(ActionListener.class); - queryGroupPersistenceService.getFromClusterStateMetadata(QueryGroupTestUtils.NAME_ONE, mockListener); - queryGroupPersistenceService.getFromClusterStateMetadata(QueryGroupTestUtils.NAME_NONE_EXISTED, mockListener); - } - public void testMaxQueryGroupCount() { assertThrows(IllegalArgumentException.class, () -> QueryGroupTestUtils.queryGroupPersistenceService().setMaxQueryGroupCount(-1)); QueryGroupPersistenceService queryGroupPersistenceService = QueryGroupTestUtils.queryGroupPersistenceService(); From 1a96416dc9532acd078ff3e905856ee97539593f Mon Sep 17 00:00:00 2001 From: Ruirui Zhang Date: Mon, 12 Aug 2024 14:35:05 -0700 Subject: [PATCH 10/14] address comments Signed-off-by: Ruirui Zhang --- .../wlm/action/GetQueryGroupRequest.java | 4 ++ .../action/TransportGetQueryGroupAction.java | 8 +--- .../wlm/action/GetQueryGroupRequestTests.java | 5 +++ .../TransportGetQueryGroupActionTests.java | 43 +++++++++++++++++++ 4 files changed, 54 insertions(+), 6 deletions(-) create mode 100644 plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupActionTests.java diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequest.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequest.java index 884628999f65c..0524c615a84e7 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequest.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequest.java @@ -10,6 +10,7 @@ import org.opensearch.action.ActionRequestValidationException; import org.opensearch.action.support.clustermanager.ClusterManagerNodeReadRequest; +import org.opensearch.cluster.metadata.QueryGroup; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; @@ -42,6 +43,9 @@ public GetQueryGroupRequest(StreamInput in) throws IOException { @Override public ActionRequestValidationException validate() { + if (name != null) { + QueryGroup.validateName(name); + } return null; } diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java index 7ad8cdf33966c..cafa580d8743c 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java @@ -46,7 +46,6 @@ public class TransportGetQueryGroupAction extends TransportClusterManagerNodeRea * @param actionFilters - a {@link ActionFilters} object * @param threadPool - a {@link ThreadPool} object * @param indexNameExpressionResolver - a {@link IndexNameExpressionResolver} object - * @param queryGroupPersistenceService - a {@link QueryGroupPersistenceService} object */ @Inject public TransportGetQueryGroupAction( @@ -54,8 +53,7 @@ public TransportGetQueryGroupAction( TransportService transportService, ActionFilters actionFilters, ThreadPool threadPool, - IndexNameExpressionResolver indexNameExpressionResolver, - QueryGroupPersistenceService queryGroupPersistenceService + IndexNameExpressionResolver indexNameExpressionResolver ) { super( GetQueryGroupAction.NAME, @@ -92,9 +90,7 @@ protected void clusterManagerOperation(GetQueryGroupRequest request, ClusterStat if (resultGroups.isEmpty() && name != null && !name.isEmpty()) { logger.warn("No QueryGroup exists with the provided name: {}", name); - Exception e = new IllegalArgumentException("No QueryGroup exists with the provided name: " + name); - listener.onFailure(e); - return; + throw new IllegalArgumentException("No QueryGroup exists with the provided name: " + name); } listener.onResponse(new GetQueryGroupResponse(resultGroups, RestStatus.OK)); } diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequestTests.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequestTests.java index 6f768a78d7998..35efe52ac6fae 100644 --- a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequestTests.java +++ b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequestTests.java @@ -36,4 +36,9 @@ public void testSerializationWithNull() throws IOException { GetQueryGroupRequest otherRequest = new GetQueryGroupRequest(streamInput); assertEquals(request.getName(), otherRequest.getName()); } + + public void testValidation() { + GetQueryGroupRequest request = new GetQueryGroupRequest("a".repeat(51)); + assertThrows(IllegalArgumentException.class, request::validate); + } } diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupActionTests.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupActionTests.java new file mode 100644 index 0000000000000..7e3992bc4dc1e --- /dev/null +++ b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupActionTests.java @@ -0,0 +1,43 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.plugin.wlm.action; + +import org.opensearch.action.support.ActionFilters; +import org.opensearch.cluster.metadata.IndexNameExpressionResolver; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.core.action.ActionListener; +import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.threadpool.ThreadPool; +import org.opensearch.transport.TransportService; + +import static org.opensearch.plugin.wlm.QueryGroupTestUtils.NAME_NONE_EXISTED; +import static org.opensearch.plugin.wlm.QueryGroupTestUtils.NAME_ONE; +import static org.opensearch.plugin.wlm.QueryGroupTestUtils.clusterState; +import static org.mockito.Mockito.mock; + +public class TransportGetQueryGroupActionTests extends OpenSearchTestCase { + + @SuppressWarnings("unchecked") + public void testClusterManagerOperation() throws Exception { + GetQueryGroupRequest getQueryGroupRequest1 = new GetQueryGroupRequest(NAME_NONE_EXISTED); + GetQueryGroupRequest getQueryGroupRequest2 = new GetQueryGroupRequest(NAME_ONE); + TransportGetQueryGroupAction transportGetQueryGroupAction = new TransportGetQueryGroupAction( + mock(ClusterService.class), + mock(TransportService.class), + mock(ActionFilters.class), + mock(ThreadPool.class), + mock(IndexNameExpressionResolver.class) + ); + assertThrows( + IllegalArgumentException.class, + () -> transportGetQueryGroupAction.clusterManagerOperation(getQueryGroupRequest1, clusterState(), mock(ActionListener.class)) + ); + transportGetQueryGroupAction.clusterManagerOperation(getQueryGroupRequest2, clusterState(), mock(ActionListener.class)); + } +} From 384314b41ad4a2796a7ac19bd8354808cbf5dc1e Mon Sep 17 00:00:00 2001 From: Ruirui Zhang Date: Tue, 13 Aug 2024 14:17:48 -0700 Subject: [PATCH 11/14] rebase with main Signed-off-by: Ruirui Zhang --- .../java/org/opensearch/plugin/wlm/package-info.java | 4 ---- .../wlm/service/QueryGroupPersistenceService.java | 5 ++--- .../service/QueryGroupPersistenceServiceTests.java | 2 -- .../org/opensearch/cluster/metadata/QueryGroup.java | 11 +++++++---- 4 files changed, 9 insertions(+), 13 deletions(-) diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/package-info.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/package-info.java index c0e0942da24f1..84c99967b226b 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/package-info.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/package-info.java @@ -7,10 +7,6 @@ */ /** -<<<<<<< HEAD * Base package for WorkloadManagementPlugin -======= - * Base package for CRUD API of QueryGroup ->>>>>>> bf0bea8a7d1 (addressed comments) */ package org.opensearch.plugin.wlm; diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceService.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceService.java index 4dea6b622caab..cf3cc647f50eb 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceService.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceService.java @@ -24,13 +24,12 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.core.rest.RestStatus; import org.opensearch.plugin.wlm.action.CreateQueryGroupResponse; -import org.opensearch.plugin.wlm.action.GetQueryGroupResponse; import org.opensearch.search.ResourceType; +import java.util.ArrayList; import java.util.EnumMap; -import java.util.Map; import java.util.List; -import java.util.ArrayList; +import java.util.Map; import java.util.Optional; /** diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceServiceTests.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceServiceTests.java index 4a83132b10663..617197917942c 100644 --- a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceServiceTests.java +++ b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceServiceTests.java @@ -28,9 +28,7 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.plugin.wlm.QueryGroupTestUtils; import org.opensearch.plugin.wlm.action.CreateQueryGroupResponse; -import org.opensearch.plugin.wlm.action.GetQueryGroupResponse; import org.opensearch.search.ResourceType; -import org.opensearch.plugin.wlm.QueryGroupTestUtils; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.threadpool.ThreadPool; diff --git a/server/src/main/java/org/opensearch/cluster/metadata/QueryGroup.java b/server/src/main/java/org/opensearch/cluster/metadata/QueryGroup.java index 6ab11b1d6f150..5ce226cd8b074 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/QueryGroup.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/QueryGroup.java @@ -62,10 +62,7 @@ public QueryGroup(String name, String _id, ResiliencyMode resiliencyMode, Map MAX_CHARS_ALLOWED_IN_NAME || name.isEmpty()) { - throw new IllegalArgumentException("QueryGroup.name shouldn't be empty or more than 50 chars long"); - } + validateName(name); if (resourceLimits.isEmpty()) { throw new IllegalArgumentException("QueryGroup.resourceLimits should at least have 1 resource limit"); @@ -111,6 +108,12 @@ public void writeTo(StreamOutput out) throws IOException { out.writeLong(updatedAtInMillis); } + public static void validateName(String name) { + if (name.length() > MAX_CHARS_ALLOWED_IN_NAME || name.isEmpty()) { + throw new IllegalArgumentException("QueryGroup.name shouldn't be empty or more than 50 chars long"); + } + } + private void validateResourceLimits(Map resourceLimits) { for (Map.Entry resource : resourceLimits.entrySet()) { Double threshold = resource.getValue(); From cdf1925173b0a879190bed4b1e7fb0666cf3149f Mon Sep 17 00:00:00 2001 From: Ruirui Zhang Date: Tue, 13 Aug 2024 15:07:49 -0700 Subject: [PATCH 12/14] add IT Signed-off-by: Ruirui Zhang --- .../api/create_query_group_context.json | 18 ------ .../api/query_group_context.json | 30 ++++++++++ .../test/wlm/10_create_query_group.yml | 12 ++-- .../test/wlm/15_get_query_group.yml | 57 +++++++++++++++++++ 4 files changed, 93 insertions(+), 24 deletions(-) delete mode 100644 plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/api/create_query_group_context.json create mode 100644 plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/api/query_group_context.json create mode 100644 plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/test/wlm/15_get_query_group.yml diff --git a/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/api/create_query_group_context.json b/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/api/create_query_group_context.json deleted file mode 100644 index bb4620c01f2d6..0000000000000 --- a/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/api/create_query_group_context.json +++ /dev/null @@ -1,18 +0,0 @@ -{ - "create_query_group_context": { - "stability": "experimental", - "url": { - "paths": [ - { - "path": "/_wlm/query_group", - "methods": ["PUT", "POST"], - "parts": {} - } - ] - }, - "params":{}, - "body":{ - "description":"The QueryGroup schema" - } - } -} diff --git a/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/api/query_group_context.json b/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/api/query_group_context.json new file mode 100644 index 0000000000000..b3320306d9270 --- /dev/null +++ b/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/api/query_group_context.json @@ -0,0 +1,30 @@ +{ + "query_group_context": { + "stability": "experimental", + "url": { + "paths": [ + { + "path": "/_wlm/query_group", + "methods": ["PUT", "POST", "GET"], + "parts": {} + }, + { + "path": "/_wlm/query_group/{name}", + "methods": [ + "GET" + ], + "parts": { + "name": { + "type": "string", + "description": "QueryGroup name" + } + } + } + ] + }, + "params":{}, + "body":{ + "description":"The QueryGroup schema" + } + } +} diff --git a/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/test/wlm/10_create_query_group.yml b/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/test/wlm/10_create_query_group.yml index ae82a8146e9cd..caa9342640164 100644 --- a/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/test/wlm/10_create_query_group.yml +++ b/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/test/wlm/10_create_query_group.yml @@ -4,7 +4,7 @@ reason: "QueryGroup WorkloadManagement feature was added in 2.17" - do: - create_query_group_context: + query_group_context: body: { "name": "analytics", @@ -22,7 +22,7 @@ - do: catch: /illegal_argument_exception/ - create_query_group_context: + query_group_context: body: { "name": "analytics", @@ -35,7 +35,7 @@ - do: catch: /illegal_argument_exception/ - create_query_group_context: + query_group_context: body: { "name": "analytics2", @@ -48,7 +48,7 @@ - do: catch: /illegal_argument_exception/ - create_query_group_context: + query_group_context: body: { "name": "analytics2", @@ -61,7 +61,7 @@ - do: catch: /illegal_argument_exception/ - create_query_group_context: + query_group_context: body: { "name": "", @@ -73,7 +73,7 @@ } - do: - create_query_group_context: + query_group_context: body: { "name": "analytics2", diff --git a/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/test/wlm/15_get_query_group.yml b/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/test/wlm/15_get_query_group.yml new file mode 100644 index 0000000000000..74eb454b6912f --- /dev/null +++ b/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/test/wlm/15_get_query_group.yml @@ -0,0 +1,57 @@ +"test get QueryGroup API": + - skip: + version: " - 2.16.99" + reason: "QueryGroup WorkloadManagement feature was added in 2.17" + + - do: + query_group_context: + body: + { + "name": "analytics", + "resiliency_mode": "monitor", + "resource_limits": { + "cpu": 0.4, + "memory": 0.2 + } + } + + - match: { name: "analytics" } + - match: { resiliency_mode: "monitor" } + - match: { resource_limits.cpu: 0.4 } + - match: { resource_limits.memory: 0.2 } + + - do: + query_group_context: + name: "analytics" + + - match: { query_groups.0.name: "analytics" } + - match: { query_groups.0.resiliency_mode: "monitor" } + - match: { query_groups.0.resource_limits.cpu: 0.4 } + - match: { query_groups.0.resource_limits.memory: 0.2 } + + - do: + catch: /illegal_argument_exception/ + query_group_context: + name: "analytics1" + + - do: + query_group_context: + body: + { + "name": "analytics2", + "resiliency_mode": "monitor", + "resource_limits": { + "cpu": 0.1, + "memory": 0.1 + } + } + + - match: { name: "analytics2" } + - match: { resiliency_mode: "monitor" } + - match: { resource_limits.cpu: 0.1 } + - match: { resource_limits.memory: 0.1 } + + - do: + query_group_context: + + - match: { query_groups.0.name: ["analytics", "analytics2"] } From 23bd5b59027bb50269ec957a8526813e0160e0e7 Mon Sep 17 00:00:00 2001 From: Ruirui Zhang Date: Tue, 13 Aug 2024 17:06:42 -0700 Subject: [PATCH 13/14] address comments Signed-off-by: Ruirui Zhang --- .../wlm/action/GetQueryGroupResponse.java | 10 ++-- .../action/TransportGetQueryGroupAction.java | 9 +-- .../wlm/rest/RestGetQueryGroupAction.java | 2 +- .../service/QueryGroupPersistenceService.java | 19 ++++--- .../plugin/wlm/QueryGroupTestUtils.java | 7 ++- .../wlm/action/GetQueryGroupRequestTests.java | 9 +++ .../action/GetQueryGroupResponseTests.java | 18 ++++++ .../TransportGetQueryGroupActionTests.java | 6 +- .../QueryGroupPersistenceServiceTests.java | 37 ++++++------ .../api/create_query_group_context.json | 18 ++++++ ...text.json => get_query_group_context.json} | 6 +- ...ate_query_group.yml => 10_query_group.yml} | 32 ++++++++--- .../test/wlm/15_get_query_group.yml | 57 ------------------- .../opensearch/cluster/metadata/Metadata.java | 4 +- .../cluster/metadata/QueryGroup.java | 4 +- 15 files changed, 129 insertions(+), 109 deletions(-) create mode 100644 plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/api/create_query_group_context.json rename plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/api/{query_group_context.json => get_query_group_context.json} (83%) rename plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/test/wlm/{10_create_query_group.yml => 10_query_group.yml} (70%) delete mode 100644 plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/test/wlm/15_get_query_group.yml diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponse.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponse.java index fa87e4e4e7277..547c501e6a28e 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponse.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponse.java @@ -18,7 +18,7 @@ import org.opensearch.core.xcontent.XContentBuilder; import java.io.IOException; -import java.util.List; +import java.util.Collection; /** * Response for the get API for QueryGroup @@ -26,7 +26,7 @@ * @opensearch.experimental */ public class GetQueryGroupResponse extends ActionResponse implements ToXContent, ToXContentObject { - private final List queryGroups; + private final Collection queryGroups; private final RestStatus restStatus; /** @@ -34,7 +34,7 @@ public class GetQueryGroupResponse extends ActionResponse implements ToXContent, * @param queryGroups - The QueryGroup list to be fetched * @param restStatus - The rest status of the request */ - public GetQueryGroupResponse(final List queryGroups, RestStatus restStatus) { + public GetQueryGroupResponse(final Collection queryGroups, RestStatus restStatus) { this.queryGroups = queryGroups; this.restStatus = restStatus; } @@ -50,7 +50,7 @@ public GetQueryGroupResponse(StreamInput in) throws IOException { @Override public void writeTo(StreamOutput out) throws IOException { - out.writeList(queryGroups); + out.writeCollection(queryGroups); RestStatus.writeTo(out, restStatus); } @@ -69,7 +69,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws /** * queryGroups getter */ - public List getQueryGroups() { + public Collection getQueryGroups() { return queryGroups; } diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java index cafa580d8743c..51bb21b255511 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupAction.java @@ -10,6 +10,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.opensearch.ResourceNotFoundException; import org.opensearch.action.support.ActionFilters; import org.opensearch.action.support.clustermanager.TransportClusterManagerNodeReadAction; import org.opensearch.cluster.ClusterState; @@ -28,7 +29,7 @@ import org.opensearch.transport.TransportService; import java.io.IOException; -import java.util.List; +import java.util.Collection; /** * Transport action to get QueryGroup @@ -85,12 +86,12 @@ protected ClusterBlockException checkBlock(GetQueryGroupRequest request, Cluster @Override protected void clusterManagerOperation(GetQueryGroupRequest request, ClusterState state, ActionListener listener) throws Exception { - String name = request.getName(); - List resultGroups = QueryGroupPersistenceService.getFromClusterStateMetadata(name, state); + final String name = request.getName(); + final Collection resultGroups = QueryGroupPersistenceService.getFromClusterStateMetadata(name, state); if (resultGroups.isEmpty() && name != null && !name.isEmpty()) { logger.warn("No QueryGroup exists with the provided name: {}", name); - throw new IllegalArgumentException("No QueryGroup exists with the provided name: " + name); + throw new ResourceNotFoundException("No QueryGroup exists with the provided name: " + name); } listener.onResponse(new GetQueryGroupResponse(resultGroups, RestStatus.OK)); } diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/rest/RestGetQueryGroupAction.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/rest/RestGetQueryGroupAction.java index 9566ddcce3e42..c250bd2979e98 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/rest/RestGetQueryGroupAction.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/rest/RestGetQueryGroupAction.java @@ -53,7 +53,7 @@ public List routes() { @Override protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { - GetQueryGroupRequest getQueryGroupRequest = new GetQueryGroupRequest(request.param("name")); + final GetQueryGroupRequest getQueryGroupRequest = new GetQueryGroupRequest(request.param("name")); return channel -> client.execute(GetQueryGroupAction.INSTANCE, getQueryGroupRequest, getQueryGroupResponse(channel)); } diff --git a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceService.java b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceService.java index cf3cc647f50eb..fe7080da78bbe 100644 --- a/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceService.java +++ b/plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceService.java @@ -26,11 +26,11 @@ import org.opensearch.plugin.wlm.action.CreateQueryGroupResponse; import org.opensearch.search.ResourceType; -import java.util.ArrayList; +import java.util.Collection; import java.util.EnumMap; -import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.stream.Collectors; /** * This class defines the functions for QueryGroup persistence @@ -199,14 +199,17 @@ private Map calculateTotalUsage(Map ex * @param name - the QueryGroup name we are getting * @param currentState - current cluster state */ - public static List getFromClusterStateMetadata(String name, ClusterState currentState) { - Map currentGroups = currentState.getMetadata().queryGroups(); + public static Collection getFromClusterStateMetadata(String name, ClusterState currentState) { + final Map currentGroups = currentState.getMetadata().queryGroups(); if (name == null || name.isEmpty()) { - return new ArrayList<>(currentGroups.values()); + return currentGroups.values(); } - List resultGroups = new ArrayList<>(); - currentGroups.values().stream().filter(group -> group.getName().equals(name)).findFirst().ifPresent(resultGroups::add); - return resultGroups; + return currentGroups.values() + .stream() + .filter(group -> group.getName().equals(name)) + .findAny() + .stream() + .collect(Collectors.toList()); } /** diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/QueryGroupTestUtils.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/QueryGroupTestUtils.java index fc324853d9b34..5ba1ad5334712 100644 --- a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/QueryGroupTestUtils.java +++ b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/QueryGroupTestUtils.java @@ -23,6 +23,7 @@ import org.opensearch.threadpool.ThreadPool; import java.util.ArrayList; +import java.util.Collection; import java.util.Comparator; import java.util.HashSet; import java.util.List; @@ -130,8 +131,10 @@ public static Tuple preparePersisten return new Tuple(queryGroupPersistenceService, clusterState); } - public static void assertEqualQueryGroups(List listOne, List listTwo) { - assertEquals(listOne.size(), listTwo.size()); + public static void assertEqualQueryGroups(Collection collectionOne, Collection collectionTwo) { + assertEquals(collectionOne.size(), collectionTwo.size()); + List listOne = new ArrayList<>(collectionOne); + List listTwo = new ArrayList<>(collectionTwo); listOne.sort(Comparator.comparing(QueryGroup::getName)); listTwo.sort(Comparator.comparing(QueryGroup::getName)); for (int i = 0; i < listOne.size(); i++) { diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequestTests.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequestTests.java index 35efe52ac6fae..32b5f7ec9e2c3 100644 --- a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequestTests.java +++ b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupRequestTests.java @@ -17,6 +17,9 @@ public class GetQueryGroupRequestTests extends OpenSearchTestCase { + /** + * Test case to verify the serialization and deserialization of GetQueryGroupRequest. + */ public void testSerialization() throws IOException { GetQueryGroupRequest request = new GetQueryGroupRequest(QueryGroupTestUtils.NAME_ONE); assertEquals(QueryGroupTestUtils.NAME_ONE, request.getName()); @@ -27,6 +30,9 @@ public void testSerialization() throws IOException { assertEquals(request.getName(), otherRequest.getName()); } + /** + * Test case to verify the serialization and deserialization of GetQueryGroupRequest when name is null. + */ public void testSerializationWithNull() throws IOException { GetQueryGroupRequest request = new GetQueryGroupRequest((String) null); assertNull(request.getName()); @@ -37,6 +43,9 @@ public void testSerializationWithNull() throws IOException { assertEquals(request.getName(), otherRequest.getName()); } + /** + * Test case the validation function of GetQueryGroupRequest + */ public void testValidation() { GetQueryGroupRequest request = new GetQueryGroupRequest("a".repeat(51)); assertThrows(IllegalArgumentException.class, request::validate); diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponseTests.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponseTests.java index 356ccf6a32d9a..774f4b2d8db52 100644 --- a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponseTests.java +++ b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetQueryGroupResponseTests.java @@ -26,6 +26,9 @@ public class GetQueryGroupResponseTests extends OpenSearchTestCase { + /** + * Test case to verify the serialization and deserialization of GetQueryGroupResponse. + */ public void testSerializationSingleQueryGroup() throws IOException { List list = new ArrayList<>(); list.add(QueryGroupTestUtils.queryGroupOne); @@ -41,6 +44,9 @@ public void testSerializationSingleQueryGroup() throws IOException { QueryGroupTestUtils.assertEqualQueryGroups(response.getQueryGroups(), otherResponse.getQueryGroups()); } + /** + * Test case to verify the serialization and deserialization of GetQueryGroupResponse when the result contains multiple QueryGroups. + */ public void testSerializationMultipleQueryGroup() throws IOException { GetQueryGroupResponse response = new GetQueryGroupResponse(QueryGroupTestUtils.queryGroupList(), RestStatus.OK); assertEquals(response.getQueryGroups(), QueryGroupTestUtils.queryGroupList()); @@ -55,6 +61,9 @@ public void testSerializationMultipleQueryGroup() throws IOException { QueryGroupTestUtils.assertEqualQueryGroups(response.getQueryGroups(), otherResponse.getQueryGroups()); } + /** + * Test case to verify the serialization and deserialization of GetQueryGroupResponse when the result is empty. + */ public void testSerializationNull() throws IOException { List list = new ArrayList<>(); GetQueryGroupResponse response = new GetQueryGroupResponse(list, RestStatus.OK); @@ -69,6 +78,9 @@ public void testSerializationNull() throws IOException { assertEquals(0, otherResponse.getQueryGroups().size()); } + /** + * Test case to verify the toXContent of GetQueryGroupResponse. + */ public void testToXContentGetSingleQueryGroup() throws IOException { List queryGroupList = new ArrayList<>(); queryGroupList.add(QueryGroupTestUtils.queryGroupOne); @@ -91,6 +103,9 @@ public void testToXContentGetSingleQueryGroup() throws IOException { assertEquals(expected, actual); } + /** + * Test case to verify the toXContent of GetQueryGroupResponse when the result contains multiple QueryGroups. + */ public void testToXContentGetMultipleQueryGroup() throws IOException { List queryGroupList = new ArrayList<>(); queryGroupList.add(QueryGroupTestUtils.queryGroupOne); @@ -123,6 +138,9 @@ public void testToXContentGetMultipleQueryGroup() throws IOException { assertEquals(expected, actual); } + /** + * Test case to verify toXContent of GetQueryGroupResponse when the result contains zero QueryGroup. + */ public void testToXContentGetZeroQueryGroup() throws IOException { XContentBuilder builder = JsonXContent.contentBuilder().prettyPrint(); GetQueryGroupResponse otherResponse = new GetQueryGroupResponse(new ArrayList<>(), RestStatus.OK); diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupActionTests.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupActionTests.java index 7e3992bc4dc1e..755b11a5f4b89 100644 --- a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupActionTests.java +++ b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/TransportGetQueryGroupActionTests.java @@ -8,6 +8,7 @@ package org.opensearch.plugin.wlm.action; +import org.opensearch.ResourceNotFoundException; import org.opensearch.action.support.ActionFilters; import org.opensearch.cluster.metadata.IndexNameExpressionResolver; import org.opensearch.cluster.service.ClusterService; @@ -23,6 +24,9 @@ public class TransportGetQueryGroupActionTests extends OpenSearchTestCase { + /** + * Test case for ClusterManagerOperation function + */ @SuppressWarnings("unchecked") public void testClusterManagerOperation() throws Exception { GetQueryGroupRequest getQueryGroupRequest1 = new GetQueryGroupRequest(NAME_NONE_EXISTED); @@ -35,7 +39,7 @@ public void testClusterManagerOperation() throws Exception { mock(IndexNameExpressionResolver.class) ); assertThrows( - IllegalArgumentException.class, + ResourceNotFoundException.class, () -> transportGetQueryGroupAction.clusterManagerOperation(getQueryGroupRequest1, clusterState(), mock(ActionListener.class)) ); transportGetQueryGroupAction.clusterManagerOperation(getQueryGroupRequest2, clusterState(), mock(ActionListener.class)); diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceServiceTests.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceServiceTests.java index 617197917942c..80ecfdf9b7e47 100644 --- a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceServiceTests.java +++ b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceServiceTests.java @@ -52,6 +52,7 @@ import static org.opensearch.plugin.wlm.QueryGroupTestUtils.assertEqualQueryGroups; import static org.opensearch.plugin.wlm.QueryGroupTestUtils.clusterSettings; import static org.opensearch.plugin.wlm.QueryGroupTestUtils.clusterSettingsSet; +import static org.opensearch.plugin.wlm.QueryGroupTestUtils.clusterState; import static org.opensearch.plugin.wlm.QueryGroupTestUtils.preparePersistenceServiceSetup; import static org.opensearch.plugin.wlm.QueryGroupTestUtils.queryGroupList; import static org.opensearch.plugin.wlm.QueryGroupTestUtils.queryGroupOne; @@ -255,11 +256,12 @@ public void testPersistInClusterStateMetadataFailure() { verify(listener).onFailure(any(RuntimeException.class)); } + /** + * Tests getting a single QueryGroup + */ public void testGetSingleQueryGroup() { - List groups = QueryGroupPersistenceService.getFromClusterStateMetadata( - QueryGroupTestUtils.NAME_ONE, - QueryGroupTestUtils.clusterState() - ); + Collection groupsCollections = QueryGroupPersistenceService.getFromClusterStateMetadata(NAME_ONE, clusterState()); + List groups = new ArrayList<>(groupsCollections); assertEquals(1, groups.size()); QueryGroup queryGroup = groups.get(0); List listOne = new ArrayList<>(); @@ -269,9 +271,13 @@ public void testGetSingleQueryGroup() { QueryGroupTestUtils.assertEqualQueryGroups(listOne, listTwo); } + /** + * Tests getting all QueryGroups + */ public void testGetAllQueryGroups() { assertEquals(2, QueryGroupTestUtils.clusterState().metadata().queryGroups().size()); - List res = QueryGroupPersistenceService.getFromClusterStateMetadata(null, QueryGroupTestUtils.clusterState()); + Collection groupsCollections = QueryGroupPersistenceService.getFromClusterStateMetadata(null, clusterState()); + List res = new ArrayList<>(groupsCollections); assertEquals(2, res.size()); Set currentNAME = res.stream().map(QueryGroup::getName).collect(Collectors.toSet()); assertTrue(currentNAME.contains(QueryGroupTestUtils.NAME_ONE)); @@ -279,22 +285,21 @@ public void testGetAllQueryGroups() { QueryGroupTestUtils.assertEqualQueryGroups(QueryGroupTestUtils.queryGroupList(), res); } - public void testGetZeroQueryGroups() { - List res = QueryGroupPersistenceService.getFromClusterStateMetadata( - QueryGroupTestUtils.NAME_NONE_EXISTED, - QueryGroupTestUtils.clusterState() - ); - assertEquals(0, res.size()); - } - + /** + * Tests getting a QueryGroup with invalid name + */ public void testGetNonExistedQueryGroups() { - List groups = QueryGroupPersistenceService.getFromClusterStateMetadata( - QueryGroupTestUtils.NAME_NONE_EXISTED, - QueryGroupTestUtils.clusterState() + Collection groupsCollections = QueryGroupPersistenceService.getFromClusterStateMetadata( + NAME_NONE_EXISTED, + clusterState() ); + List groups = new ArrayList<>(groupsCollections); assertEquals(0, groups.size()); } + /** + * Tests setting maxQueryGroupCount + */ public void testMaxQueryGroupCount() { assertThrows(IllegalArgumentException.class, () -> QueryGroupTestUtils.queryGroupPersistenceService().setMaxQueryGroupCount(-1)); QueryGroupPersistenceService queryGroupPersistenceService = QueryGroupTestUtils.queryGroupPersistenceService(); diff --git a/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/api/create_query_group_context.json b/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/api/create_query_group_context.json new file mode 100644 index 0000000000000..bb4620c01f2d6 --- /dev/null +++ b/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/api/create_query_group_context.json @@ -0,0 +1,18 @@ +{ + "create_query_group_context": { + "stability": "experimental", + "url": { + "paths": [ + { + "path": "/_wlm/query_group", + "methods": ["PUT", "POST"], + "parts": {} + } + ] + }, + "params":{}, + "body":{ + "description":"The QueryGroup schema" + } + } +} diff --git a/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/api/query_group_context.json b/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/api/get_query_group_context.json similarity index 83% rename from plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/api/query_group_context.json rename to plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/api/get_query_group_context.json index b3320306d9270..23c6cfe6d46ce 100644 --- a/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/api/query_group_context.json +++ b/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/api/get_query_group_context.json @@ -5,14 +5,12 @@ "paths": [ { "path": "/_wlm/query_group", - "methods": ["PUT", "POST", "GET"], + "methods": ["GET"], "parts": {} }, { "path": "/_wlm/query_group/{name}", - "methods": [ - "GET" - ], + "methods": ["GET"], "parts": { "name": { "type": "string", diff --git a/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/test/wlm/10_create_query_group.yml b/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/test/wlm/10_query_group.yml similarity index 70% rename from plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/test/wlm/10_create_query_group.yml rename to plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/test/wlm/10_query_group.yml index caa9342640164..a22dfa2f4477e 100644 --- a/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/test/wlm/10_create_query_group.yml +++ b/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/test/wlm/10_query_group.yml @@ -1,10 +1,10 @@ -"test create QueryGroup API": +"test CRUD Operations for QueryGroup API ": - skip: version: " - 2.16.99" reason: "QueryGroup WorkloadManagement feature was added in 2.17" - do: - query_group_context: + create_query_group_context: body: { "name": "analytics", @@ -20,9 +20,18 @@ - match: { resource_limits.cpu: 0.4 } - match: { resource_limits.memory: 0.2 } + - do: + get_query_group_context: + name: "analytics" + + - match: { query_groups.0.name: "analytics" } + - match: { query_groups.0.resiliency_mode: "monitor" } + - match: { query_groups.0.resource_limits.cpu: 0.4 } + - match: { query_groups.0.resource_limits.memory: 0.2 } + - do: catch: /illegal_argument_exception/ - query_group_context: + create_query_group_context: body: { "name": "analytics", @@ -35,7 +44,7 @@ - do: catch: /illegal_argument_exception/ - query_group_context: + create_query_group_context: body: { "name": "analytics2", @@ -48,7 +57,7 @@ - do: catch: /illegal_argument_exception/ - query_group_context: + create_query_group_context: body: { "name": "analytics2", @@ -61,7 +70,7 @@ - do: catch: /illegal_argument_exception/ - query_group_context: + create_query_group_context: body: { "name": "", @@ -73,7 +82,7 @@ } - do: - query_group_context: + create_query_group_context: body: { "name": "analytics2", @@ -88,3 +97,12 @@ - match: { resiliency_mode: "monitor" } - match: { resource_limits.cpu: 0.35 } - match: { resource_limits.memory: 0.25 } + + - do: + get_query_group_context: + name: "analytics2" + + - match: { query_groups.0.name: "analytics2" } + - match: { query_groups.0.resiliency_mode: "monitor" } + - match: { query_groups.0.resource_limits.cpu: 0.35 } + - match: { query_groups.0.resource_limits.memory: 0.25 } diff --git a/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/test/wlm/15_get_query_group.yml b/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/test/wlm/15_get_query_group.yml deleted file mode 100644 index 74eb454b6912f..0000000000000 --- a/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/test/wlm/15_get_query_group.yml +++ /dev/null @@ -1,57 +0,0 @@ -"test get QueryGroup API": - - skip: - version: " - 2.16.99" - reason: "QueryGroup WorkloadManagement feature was added in 2.17" - - - do: - query_group_context: - body: - { - "name": "analytics", - "resiliency_mode": "monitor", - "resource_limits": { - "cpu": 0.4, - "memory": 0.2 - } - } - - - match: { name: "analytics" } - - match: { resiliency_mode: "monitor" } - - match: { resource_limits.cpu: 0.4 } - - match: { resource_limits.memory: 0.2 } - - - do: - query_group_context: - name: "analytics" - - - match: { query_groups.0.name: "analytics" } - - match: { query_groups.0.resiliency_mode: "monitor" } - - match: { query_groups.0.resource_limits.cpu: 0.4 } - - match: { query_groups.0.resource_limits.memory: 0.2 } - - - do: - catch: /illegal_argument_exception/ - query_group_context: - name: "analytics1" - - - do: - query_group_context: - body: - { - "name": "analytics2", - "resiliency_mode": "monitor", - "resource_limits": { - "cpu": 0.1, - "memory": 0.1 - } - } - - - match: { name: "analytics2" } - - match: { resiliency_mode: "monitor" } - - match: { resource_limits.cpu: 0.1 } - - match: { resource_limits.memory: 0.1 } - - - do: - query_group_context: - - - match: { query_groups.0.name: ["analytics", "analytics2"] } diff --git a/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java b/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java index 26f2e9ff34db5..4da6c68b40733 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java @@ -1831,8 +1831,8 @@ static void validateDataStreams(SortedMap indicesLooku for (DataStream ds : dsMetadata.dataStreams().values()) { String prefix = DataStream.BACKING_INDEX_PREFIX + ds.getName() + "-"; Set conflicts = indicesLookup.subMap(prefix, DataStream.BACKING_INDEX_PREFIX + ds.getName() + ".") // '.' is the - // char after - // '-' + // char after + // '-' .keySet() .stream() .filter(s -> NUMBER_PATTERN.matcher(s.substring(prefix.length())).matches()) diff --git a/server/src/main/java/org/opensearch/cluster/metadata/QueryGroup.java b/server/src/main/java/org/opensearch/cluster/metadata/QueryGroup.java index 5ce226cd8b074..9b5c6bc2369a6 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/QueryGroup.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/QueryGroup.java @@ -109,8 +109,8 @@ public void writeTo(StreamOutput out) throws IOException { } public static void validateName(String name) { - if (name.length() > MAX_CHARS_ALLOWED_IN_NAME || name.isEmpty()) { - throw new IllegalArgumentException("QueryGroup.name shouldn't be empty or more than 50 chars long"); + if (name == null || name.isEmpty() || name.length() > MAX_CHARS_ALLOWED_IN_NAME) { + throw new IllegalArgumentException("QueryGroup.name shouldn't be null, empty or more than 50 chars long"); } } From d4615c79a4b958152442a95085481f5a015853b0 Mon Sep 17 00:00:00 2001 From: Ruirui Zhang Date: Thu, 15 Aug 2024 17:29:02 -0700 Subject: [PATCH 14/14] fix IT Signed-off-by: Ruirui Zhang --- .../wlm/service/QueryGroupPersistenceServiceTests.java | 8 -------- .../rest-api-spec/api/get_query_group_context.json | 7 ++----- 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceServiceTests.java b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceServiceTests.java index 80ecfdf9b7e47..2aa3b9e168852 100644 --- a/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceServiceTests.java +++ b/plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceServiceTests.java @@ -6,14 +6,6 @@ * compatible open source license. */ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - package org.opensearch.plugin.wlm.service; import org.opensearch.cluster.ClusterName; diff --git a/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/api/get_query_group_context.json b/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/api/get_query_group_context.json index 23c6cfe6d46ce..e0d552be616b2 100644 --- a/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/api/get_query_group_context.json +++ b/plugins/workload-management/src/yamlRestTest/resources/rest-api-spec/api/get_query_group_context.json @@ -1,5 +1,5 @@ { - "query_group_context": { + "get_query_group_context": { "stability": "experimental", "url": { "paths": [ @@ -20,9 +20,6 @@ } ] }, - "params":{}, - "body":{ - "description":"The QueryGroup schema" - } + "params":{} } }