From ac8b002e85f5786d18299336b21e5b6522c53e48 Mon Sep 17 00:00:00 2001 From: Kaushal Kumar Date: Wed, 10 Apr 2024 10:18:54 -0700 Subject: [PATCH 01/10] add sandbox service Signed-off-by: Kaushal Kumar --- .../common/settings/ClusterSettings.java | 8 +- .../search/sandbox/QuerySandboxService.java | 94 ++++++++ .../sandbox/QuerySandboxServiceSettings.java | 202 ++++++++++++++++++ .../search/sandbox/SandboxPruner.java | 20 ++ .../cancellation/SandboxRequestCanceller.java | 19 ++ .../sandbox/cancellation/package-info.java | 12 ++ .../search/sandbox/module/SandboxModule.java | 33 +++ .../search/sandbox/module/package-info.java | 13 ++ .../search/sandbox/package-info.java | 12 ++ .../tracker/SandboxResourceTracker.java | 19 ++ .../SandboxResourceTrackerService.java | 178 +++++++++++++++ .../search/sandbox/tracker/package-info.java | 12 ++ .../QuerySandboxServiceSettingsTests.java | 129 +++++++++++ 13 files changed, 750 insertions(+), 1 deletion(-) create mode 100644 server/src/main/java/org/opensearch/search/sandbox/QuerySandboxService.java create mode 100644 server/src/main/java/org/opensearch/search/sandbox/QuerySandboxServiceSettings.java create mode 100644 server/src/main/java/org/opensearch/search/sandbox/SandboxPruner.java create mode 100644 server/src/main/java/org/opensearch/search/sandbox/cancellation/SandboxRequestCanceller.java create mode 100644 server/src/main/java/org/opensearch/search/sandbox/cancellation/package-info.java create mode 100644 server/src/main/java/org/opensearch/search/sandbox/module/SandboxModule.java create mode 100644 server/src/main/java/org/opensearch/search/sandbox/module/package-info.java create mode 100644 server/src/main/java/org/opensearch/search/sandbox/package-info.java create mode 100644 server/src/main/java/org/opensearch/search/sandbox/tracker/SandboxResourceTracker.java create mode 100644 server/src/main/java/org/opensearch/search/sandbox/tracker/SandboxResourceTrackerService.java create mode 100644 server/src/main/java/org/opensearch/search/sandbox/tracker/package-info.java create mode 100644 server/src/test/java/org/opensearch/search/sandbox/QuerySandboxServiceSettingsTests.java 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 dab0f6bcf1c85..b1853e932c260 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -154,6 +154,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.sandbox.QuerySandboxServiceSettings; import org.opensearch.snapshots.InternalSnapshotsInfoService; import org.opensearch.snapshots.SnapshotsService; import org.opensearch.tasks.TaskCancellationMonitoringSettings; @@ -732,7 +733,12 @@ public void apply(Settings value, Settings current, Settings previous) { RemoteStoreSettings.CLUSTER_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING, RemoteStoreSettings.CLUSTER_REMOTE_TRANSLOG_TRANSFER_TIMEOUT_SETTING, RemoteStoreSettings.CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING, - RemoteStoreSettings.CLUSTER_REMOTE_STORE_PATH_HASH_ALGORITHM_SETTING + RemoteStoreSettings.CLUSTER_REMOTE_STORE_PATH_HASH_ALGORITHM_SETTING, + + // Sandbox settings + QuerySandboxServiceSettings.MAX_SANDBOX_COUNT, + QuerySandboxServiceSettings.NODE_LEVEL_REJECTION_THRESHOLD, + QuerySandboxServiceSettings.NODE_LEVEL_CANCELLATION_THRESHOLD ) ) ); diff --git a/server/src/main/java/org/opensearch/search/sandbox/QuerySandboxService.java b/server/src/main/java/org/opensearch/search/sandbox/QuerySandboxService.java new file mode 100644 index 0000000000000..001e2e7315aac --- /dev/null +++ b/server/src/main/java/org/opensearch/search/sandbox/QuerySandboxService.java @@ -0,0 +1,94 @@ +/* + * 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.sandbox; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.common.inject.Inject; +import org.opensearch.common.lifecycle.AbstractLifecycleComponent; +import org.opensearch.search.sandbox.tracker.SandboxResourceTracker; +import org.opensearch.search.sandbox.cancellation.SandboxRequestCanceller; +import org.opensearch.threadpool.Scheduler; +import org.opensearch.threadpool.ThreadPool; + +import java.io.IOException; + +/** + * Main service which will run periodically to track and cancel resource constraint violating tasks in sandboxes + */ +public class QuerySandboxService extends AbstractLifecycleComponent { + private static final Logger logger = LogManager.getLogger(QuerySandboxService.class); + + private final SandboxResourceTracker requestTracker; + private final SandboxRequestCanceller requestCanceller; + private final SandboxPruner sandboxPruner; + private volatile Scheduler.Cancellable scheduledFuture; + private final QuerySandboxServiceSettings sandboxServiceSettings; + private final ThreadPool threadPool; + + /** + * Guice managed constructor + * @param requestTrackerService + * @param requestCanceller + * @param sandboxPruner + * @param sandboxServiceSettings + * @param threadPool + */ + @Inject + public QuerySandboxService( + SandboxResourceTracker requestTrackerService, + SandboxRequestCanceller requestCanceller, + SandboxPruner sandboxPruner, + QuerySandboxServiceSettings sandboxServiceSettings, + ThreadPool threadPool + ) { + this.requestTracker = requestTrackerService; + this.requestCanceller = requestCanceller; + this.sandboxPruner = sandboxPruner; + this.sandboxServiceSettings = sandboxServiceSettings; + this.threadPool = threadPool; + } + + /** + * run at regular interval + */ + private void doRun() { + requestTracker.updateSandboxResourceUsages(); + requestCanceller.cancelViolatingTasks(); + sandboxPruner.pruneSandboxes(); + } + + /** + * {@link AbstractLifecycleComponent} lifecycle method + */ + @Override + protected void doStart() { + scheduledFuture = threadPool.scheduleWithFixedDelay(() -> { + try { + doRun(); + } catch (Exception e) { + logger.debug("Exception occurred in Query Sandbox service", e); + } + }, sandboxServiceSettings.getRunIntervalMillis(), ThreadPool.Names.GENERIC); + } + + @Override + protected void doStop() { + if (scheduledFuture != null) { + scheduledFuture.cancel(); + } + } + + @Override + protected void doClose() throws IOException {} + + // public SandboxStatsHolder stats() { + // return requestTrackerService.getSandboxLevelStats(); + // } +} diff --git a/server/src/main/java/org/opensearch/search/sandbox/QuerySandboxServiceSettings.java b/server/src/main/java/org/opensearch/search/sandbox/QuerySandboxServiceSettings.java new file mode 100644 index 0000000000000..70d6a7076d4d2 --- /dev/null +++ b/server/src/main/java/org/opensearch/search/sandbox/QuerySandboxServiceSettings.java @@ -0,0 +1,202 @@ +/* + * 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.sandbox; + +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Setting; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.unit.TimeValue; + +import java.util.List; + +/** + * Main class to declare the query sandboxing feature related settings + */ +public class QuerySandboxServiceSettings { + private static final Long DEFAULT_RUN_INTERVAL_MILLIS = 1000l; + private static final Double DEFAULT_NODE_LEVEL_REJECTION_QUERY_SANDBOX_THRESHOLD = 0.8; + private static final Double DEFAULT_NODE_LEVEL_CANCELLATION_QUERY_SANDBOX_THRESHOLD = 0.9; + /** + * default max sandbox count on any node at any given point in time + */ + public static final int DEFAULT_MAX_SANDBOX_COUNT_VALUE = 100; + + public static final String SANDBOX_COUNT_SETTING_NAME = "node.sandbox.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 nodeLevelJvmCancellationThreshold; + private Double nodeLevelJvmRejectionThreshold; + private volatile int maxSandboxCount; + /** + * max sandbox count setting + */ + public static final Setting MAX_SANDBOX_COUNT = Setting.intSetting( + SANDBOX_COUNT_SETTING_NAME, + DEFAULT_MAX_SANDBOX_COUNT_VALUE, + 0, + (newVal) -> { + if (newVal > 100 || newVal < 1) + throw new IllegalArgumentException(SANDBOX_COUNT_SETTING_NAME + " should be in range [1-100]"); + }, + Setting.Property.Dynamic, + Setting.Property.NodeScope + ); + /** + * Setting name for default sandbox count + */ + public static final String QUERY_SANDBOX_SERVICE_RUN_INTERVAL_MILLIS_SETTING_NAME = "query_sandbox.service.run_interval_millis"; + /** + * Setting to control the run interval of QSB service + */ + private static final Setting QSB_RUN_INTERVAL_SETTING = Setting.longSetting( + QUERY_SANDBOX_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 QUERY_SANDBOX_NODE_REJECTION_THRESHOLD_SETTING_NAME = "query_sandbox.node.rejection_threshold"; + /** + * Setting to control the rejection threshold + */ + public static final Setting NODE_LEVEL_REJECTION_THRESHOLD = Setting.doubleSetting( + QUERY_SANDBOX_NODE_REJECTION_THRESHOLD_SETTING_NAME, + DEFAULT_NODE_LEVEL_REJECTION_QUERY_SANDBOX_THRESHOLD, + Setting.Property.Dynamic, + Setting.Property.NodeScope + ); + /** + * Setting name for node level cancellation threshold + */ + public static final String QUERY_SANDBOX_NODE_CANCELLATION_THRESHOLD_SETTING_NAME = "query_sandbox.node.cancellation_threshold"; + /** + * Setting name for node level cancellation threshold + */ + public static final Setting NODE_LEVEL_CANCELLATION_THRESHOLD = Setting.doubleSetting( + QUERY_SANDBOX_NODE_CANCELLATION_THRESHOLD_SETTING_NAME, + DEFAULT_NODE_LEVEL_CANCELLATION_QUERY_SANDBOX_THRESHOLD, + Setting.Property.Dynamic, + Setting.Property.NodeScope + ); + + /** + * Sandbox service settings constructor + * @param settings + * @param clusterSettings + */ + public QuerySandboxServiceSettings(Settings settings, ClusterSettings clusterSettings) { + runIntervalMillis = new TimeValue(QSB_RUN_INTERVAL_SETTING.get(settings)); + nodeLevelJvmCancellationThreshold = NODE_LEVEL_CANCELLATION_THRESHOLD.get(settings); + nodeLevelJvmRejectionThreshold = NODE_LEVEL_REJECTION_THRESHOLD.get(settings); + maxSandboxCount = MAX_SANDBOX_COUNT.get(settings); + + ensureRejectionThresholdIsLessThanCancellation(nodeLevelJvmRejectionThreshold, nodeLevelJvmCancellationThreshold); + + clusterSettings.addSettingsUpdateConsumer(MAX_SANDBOX_COUNT, this::setMaxSandboxCount); + clusterSettings.addSettingsUpdateConsumer(NODE_LEVEL_CANCELLATION_THRESHOLD, this::setNodeLevelJvmCancellationThreshold); + clusterSettings.addSettingsUpdateConsumer(NODE_LEVEL_REJECTION_THRESHOLD, this::setNodeLevelJvmRejectionThreshold); + } + + /** + * Method to get runInterval for QSB + * @return runInterval in milliseconds for QSB Service + */ + public TimeValue getRunIntervalMillis() { + return runIntervalMillis; + } + + /** + * Method to set the new sandbox count + * @param newMaxSandboxCount is the new maxSandboxCount per node + */ + public void setMaxSandboxCount(int newMaxSandboxCount) { + if (newMaxSandboxCount < 0) { + throw new IllegalArgumentException("node.sandbox.max_count can't be negative"); + } + this.maxSandboxCount = newMaxSandboxCount; + } + + /** + * Method to get the node level cancellation threshold + * @return current node level cancellation threshold + */ + public Double getNodeLevelJvmCancellationThreshold() { + return nodeLevelJvmCancellationThreshold; + } + + /** + * Method to set the node level cancellation threshold + * @param nodeLevelJvmCancellationThreshold sets the new node level cancellation threshold + * @throws IllegalArgumentException if the value is > 0.95 and cancellation < rejection threshold + */ + public void setNodeLevelJvmCancellationThreshold(Double nodeLevelJvmCancellationThreshold) { + if (Double.compare(nodeLevelJvmCancellationThreshold, NODE_LEVEL_CANCELLATION_THRESHOLD_MAX_VALUE) > 0) { + throw new IllegalArgumentException( + QUERY_SANDBOX_NODE_CANCELLATION_THRESHOLD_SETTING_NAME + + " value should not be greater than 0.95 as it pose a threat of node drop" + ); + } + + ensureRejectionThresholdIsLessThanCancellation(nodeLevelJvmRejectionThreshold, nodeLevelJvmCancellationThreshold); + + this.nodeLevelJvmCancellationThreshold = nodeLevelJvmCancellationThreshold; + } + + /** + * Method to get the node level rejection threshold + * @return the current node level rejection threshold + */ + public Double getNodeLevelJvmRejectionThreshold() { + return nodeLevelJvmRejectionThreshold; + } + + /** + * Method to set the node level rejection threshold + * @param nodeLevelJvmRejectionThreshold sets the new rejection threshold + * @throws IllegalArgumentException if rejection > 0.90 and rejection < cancellation threshold + */ + public void setNodeLevelJvmRejectionThreshold(Double nodeLevelJvmRejectionThreshold) { + if (Double.compare(nodeLevelJvmRejectionThreshold, NODE_LEVEL_REJECTION_THRESHOLD_MAX_VALUE) > 0) { + throw new IllegalArgumentException( + QUERY_SANDBOX_NODE_REJECTION_THRESHOLD_SETTING_NAME + " value not be greater than 0.90 as it pose a threat of node drop" + ); + } + + ensureRejectionThresholdIsLessThanCancellation(nodeLevelJvmRejectionThreshold, nodeLevelJvmCancellationThreshold); + + this.nodeLevelJvmRejectionThreshold = nodeLevelJvmRejectionThreshold; + } + + private void ensureRejectionThresholdIsLessThanCancellation( + Double nodeLevelJvmRejectionThreshold, + Double nodeLevelJvmCancellationThreshold + ) { + if (Double.compare(nodeLevelJvmCancellationThreshold , nodeLevelJvmRejectionThreshold) < 0) { + throw new IllegalArgumentException( + QUERY_SANDBOX_NODE_CANCELLATION_THRESHOLD_SETTING_NAME + + " value should not be less than " + + QUERY_SANDBOX_NODE_REJECTION_THRESHOLD_SETTING_NAME + ); + } + } + + /** + * Method to get the current sandbox count + * @return the current max sandbox count + */ + public int getMaxSandboxCount() { + return maxSandboxCount; + } +} diff --git a/server/src/main/java/org/opensearch/search/sandbox/SandboxPruner.java b/server/src/main/java/org/opensearch/search/sandbox/SandboxPruner.java new file mode 100644 index 0000000000000..c47e5c22d4c3a --- /dev/null +++ b/server/src/main/java/org/opensearch/search/sandbox/SandboxPruner.java @@ -0,0 +1,20 @@ +/* + * 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.sandbox; + +/** + * This interface is used to identify and completely remove deleted sandboxes which has been marked as deleted + * previously but had the tasks running at the time of deletion request + */ +public interface SandboxPruner { + /** + * remove the deleted sandboxes from the system once all the tasks in that sandbox are completed/cancelled + */ + void pruneSandboxes(); +} diff --git a/server/src/main/java/org/opensearch/search/sandbox/cancellation/SandboxRequestCanceller.java b/server/src/main/java/org/opensearch/search/sandbox/cancellation/SandboxRequestCanceller.java new file mode 100644 index 0000000000000..713d6314f4bf2 --- /dev/null +++ b/server/src/main/java/org/opensearch/search/sandbox/cancellation/SandboxRequestCanceller.java @@ -0,0 +1,19 @@ +/* + * 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.sandbox.cancellation; + +/** + * This interface is used to identify and cancel the violating tasks in a sandbox + */ +public interface SandboxRequestCanceller { + /** + * Cancels the tasks from conteded sandboxes + */ + void cancelViolatingTasks(); +} diff --git a/server/src/main/java/org/opensearch/search/sandbox/cancellation/package-info.java b/server/src/main/java/org/opensearch/search/sandbox/cancellation/package-info.java new file mode 100644 index 0000000000000..52c31596380f0 --- /dev/null +++ b/server/src/main/java/org/opensearch/search/sandbox/cancellation/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 cancellation related abstracts + */ +package org.opensearch.search.sandbox.cancellation; diff --git a/server/src/main/java/org/opensearch/search/sandbox/module/SandboxModule.java b/server/src/main/java/org/opensearch/search/sandbox/module/SandboxModule.java new file mode 100644 index 0000000000000..b6da7a1d9539b --- /dev/null +++ b/server/src/main/java/org/opensearch/search/sandbox/module/SandboxModule.java @@ -0,0 +1,33 @@ +/* + * 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.sandbox.module; + +import org.opensearch.common.inject.AbstractModule; +import org.opensearch.search.sandbox.SandboxPruner; +import org.opensearch.search.sandbox.tracker.SandboxResourceTracker; +import org.opensearch.search.sandbox.cancellation.SandboxRequestCanceller; +import org.opensearch.search.sandbox.tracker.SandboxResourceTrackerService; + +/** + * Module class for sandboxing related artifacts + */ +public class SandboxModule extends AbstractModule { + + /** + * Default constructor + */ + public SandboxModule() {} + + @Override + protected void configure() { + bind(SandboxResourceTracker.class).to(SandboxResourceTrackerService.class).asEagerSingleton(); + bind(SandboxRequestCanceller.class).to(SandboxResourceTrackerService.class).asEagerSingleton(); + bind(SandboxPruner.class).to(SandboxResourceTrackerService.class).asEagerSingleton(); + } +} diff --git a/server/src/main/java/org/opensearch/search/sandbox/module/package-info.java b/server/src/main/java/org/opensearch/search/sandbox/module/package-info.java new file mode 100644 index 0000000000000..d1bcb8d6e01d3 --- /dev/null +++ b/server/src/main/java/org/opensearch/search/sandbox/module/package-info.java @@ -0,0 +1,13 @@ +/* + * 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. + */ + +/** + * Guice Module + */ + +package org.opensearch.search.sandbox.module; diff --git a/server/src/main/java/org/opensearch/search/sandbox/package-info.java b/server/src/main/java/org/opensearch/search/sandbox/package-info.java new file mode 100644 index 0000000000000..36bfefab5a9aa --- /dev/null +++ b/server/src/main/java/org/opensearch/search/sandbox/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. + */ + +/** + * Query Sandboxing related artifacts + */ +package org.opensearch.search.sandbox; diff --git a/server/src/main/java/org/opensearch/search/sandbox/tracker/SandboxResourceTracker.java b/server/src/main/java/org/opensearch/search/sandbox/tracker/SandboxResourceTracker.java new file mode 100644 index 0000000000000..0ab1702d4c3a1 --- /dev/null +++ b/server/src/main/java/org/opensearch/search/sandbox/tracker/SandboxResourceTracker.java @@ -0,0 +1,19 @@ +/* + * 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.sandbox.tracker; + +/** + * This interface is mainly for tracking the sandbox level resource usages + */ +public interface SandboxResourceTracker { + /** + * updates the current resource usage of sandboxes + */ + public void updateSandboxResourceUsages(); +} diff --git a/server/src/main/java/org/opensearch/search/sandbox/tracker/SandboxResourceTrackerService.java b/server/src/main/java/org/opensearch/search/sandbox/tracker/SandboxResourceTrackerService.java new file mode 100644 index 0000000000000..aa0722ebcc7a5 --- /dev/null +++ b/server/src/main/java/org/opensearch/search/sandbox/tracker/SandboxResourceTrackerService.java @@ -0,0 +1,178 @@ +/* + * 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.sandbox.tracker; + +import org.opensearch.common.inject.Inject; +import org.opensearch.core.tasks.resourcetracker.TaskResourceUsage; +import org.opensearch.search.sandbox.SandboxPruner; +import org.opensearch.search.sandbox.cancellation.SandboxRequestCanceller; +import org.opensearch.tasks.Task; +import org.opensearch.tasks.TaskCancellation; +import org.opensearch.tasks.TaskManager; +import org.opensearch.tasks.TaskResourceTrackingService; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.function.LongSupplier; +import java.util.stream.Collectors; + +/** + * This class tracks requests per sandboxes + */ +public class SandboxResourceTrackerService + implements + TaskManager.TaskEventListeners, + SandboxResourceTracker, + SandboxRequestCanceller, + SandboxPruner { + + private static final String CPU = "CPU"; + private static final String JVM_ALLOCATIONS = "JVM_Allocations"; + private static final int numberOfAvailableProcessors = Runtime.getRuntime().availableProcessors(); + private static final long totalAvailableJvmMemory = Runtime.getRuntime().totalMemory(); + private final LongSupplier timeNanosSupplier; + /** + * Sandbox ids which are marked for deletion in between the @link SandboxService runs + */ + private List toDeleteSandboxes; + private List activeSandboxes; + private final TaskManager taskManager; + private final TaskResourceTrackingService taskResourceTrackingService; + + /** + * SandboxResourceTrackerService constructor + * @param taskManager + * @param taskResourceTrackingService + */ + @Inject + public SandboxResourceTrackerService( + TaskManager taskManager, + TaskResourceTrackingService taskResourceTrackingService + ) { + this.taskManager = taskManager; + this.taskResourceTrackingService = taskResourceTrackingService; + toDeleteSandboxes = Collections.synchronizedList(new ArrayList<>()); + this.timeNanosSupplier = System::nanoTime; + } + + @Override + public void updateSandboxResourceUsages() { + + } + + // @Override + // public SandboxStatsHolder getStats() { + // return null; + // } + + private AbsoluteResourceUsage calculateAbsoluteResourceUsageFor(Task task) { + TaskResourceUsage taskResourceUsage = task.getTotalResourceStats(); + long cpuTimeInNanos = taskResourceUsage.getCpuTimeInNanos(); + long jvmAllocations = taskResourceUsage.getMemoryInBytes(); + long taskElapsedTime = timeNanosSupplier.getAsLong() - task.getStartTimeNanos(); + return new AbsoluteResourceUsage( + (cpuTimeInNanos * 1.0f) / (taskElapsedTime * numberOfAvailableProcessors), + ((jvmAllocations * 1.0f) / totalAvailableJvmMemory) + ); + } + + /** + * Value holder class for resource usage in absolute terms with respect to system/process mem + */ + private static class AbsoluteResourceUsage { + private final double absoluteCpuUsage; + private final double absoluteJvmAllocationsUsage; + + public AbsoluteResourceUsage(double absoluteCpuUsage, double absoluteJvmAllocationsUsage) { + this.absoluteCpuUsage = absoluteCpuUsage; + this.absoluteJvmAllocationsUsage = absoluteJvmAllocationsUsage; + } + + public static AbsoluteResourceUsage merge(AbsoluteResourceUsage a, AbsoluteResourceUsage b) { + return new AbsoluteResourceUsage( + a.absoluteCpuUsage + b.absoluteCpuUsage, + a.absoluteJvmAllocationsUsage + b.absoluteJvmAllocationsUsage + ); + } + + public double getAbsoluteCpuUsageInPercentage() { + return absoluteCpuUsage * 100; + } + + public double getAbsoluteJvmAllocationsUsageInPercent() { + return absoluteJvmAllocationsUsage * 100; + } + } + + /** + * filter out the deleted sandboxes which still has unfi + */ + public void pruneSandboxes() { + toDeleteSandboxes = toDeleteSandboxes.stream().filter(this::hasUnfinishedTasks).collect(Collectors.toList()); + } + + private boolean hasUnfinishedTasks(String sandboxId) { + return false; + } + + /** + * method to handle the completed tasks + * @param task represents completed task on the node + */ + @Override + public void onTaskCompleted(Task task) {} + + /** + * This method will select the sandboxes violating the enforced constraints + * and cancel the tasks from the violating sandboxes + * Cancellation happens in two scenarios + *
    + *
  1. If the sandbox is of enforced type and it is breaching its cancellation limit for the threshold
    2. + *
  2. Node is in duress and sandboxes which are breaching the cancellation thresholds will have cancellations
    3. + *
+ */ + @Override + public void cancelViolatingTasks() { + List cancellableTasks = getCancellableTasks(); + for (TaskCancellation taskCancellation : cancellableTasks) { + taskCancellation.cancel(); + } + } + + private List getCancellableTasks() { + // perform cancellations from enforced type sandboxes + List inViolationSandboxes = getBreachingSandboxIds(); + List cancellableTasks = new ArrayList<>(); + for (String sandboxId : inViolationSandboxes) { + cancellableTasks.addAll(getCancellableTasksFrom(sandboxId)); + } + + // perform cancellations from soft type sandboxes if the node is in duress (hitting node level cancellation + // threshold) + + + return cancellableTasks; + } + + public void deleteSandbox(String sandboxId) { + if (hasUnfinishedTasks(sandboxId)) { + toDeleteSandboxes.add(sandboxId); + } + // remove this sandbox from the active sandboxes + } + + private List getBreachingSandboxIds() { + return Collections.emptyList(); + } + + private List getCancellableTasksFrom(String sandboxId) { + return Collections.emptyList(); + } +} diff --git a/server/src/main/java/org/opensearch/search/sandbox/tracker/package-info.java b/server/src/main/java/org/opensearch/search/sandbox/tracker/package-info.java new file mode 100644 index 0000000000000..e7be9a36f3ce5 --- /dev/null +++ b/server/src/main/java/org/opensearch/search/sandbox/tracker/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. + */ + +/** + * Sandbox resource tracking artifacts + */ +package org.opensearch.search.sandbox.tracker; diff --git a/server/src/test/java/org/opensearch/search/sandbox/QuerySandboxServiceSettingsTests.java b/server/src/test/java/org/opensearch/search/sandbox/QuerySandboxServiceSettingsTests.java new file mode 100644 index 0000000000000..9d6158c8244ab --- /dev/null +++ b/server/src/test/java/org/opensearch/search/sandbox/QuerySandboxServiceSettingsTests.java @@ -0,0 +1,129 @@ +/* + * 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.sandbox; + +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Settings; +import org.opensearch.test.OpenSearchTestCase; + +import static org.opensearch.search.sandbox.QuerySandboxServiceSettings.*; + +public class QuerySandboxServiceSettingsTests extends OpenSearchTestCase { + + /** + * Tests the valid value of {@code node.sandbox.max_count} + */ + public void testValidMaxSandboxCountSetting() { + Settings settings = Settings.builder().put(SANDBOX_COUNT_SETTING_NAME, 100).build(); + ClusterSettings cs = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + QuerySandboxServiceSettings querySandboxServiceSettings = new QuerySandboxServiceSettings(settings, cs); + assertEquals(100, querySandboxServiceSettings.getMaxSandboxCount()); + } + + + /** + * test the invalid value of {@code node.sandbox.max_count} + */ + public void testInValidMaxSandboxCountSetting() { + Settings settings = Settings.builder().put(SANDBOX_COUNT_SETTING_NAME, -100).build(); + ClusterSettings cs = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + assertThrows(IllegalArgumentException.class, + () -> new QuerySandboxServiceSettings(settings, cs)); + } + + /** + * Tests the valid value for {@code query_sandbox.node.rejection_threshold} + */ + public void testValidNodeLevelRejectionThreshold() { + Settings settings = Settings.builder().put(QUERY_SANDBOX_NODE_REJECTION_THRESHOLD_SETTING_NAME, 0.80).build(); + ClusterSettings cs = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + QuerySandboxServiceSettings querySandboxServiceSettings = new QuerySandboxServiceSettings(settings, cs); + assertEquals(0.80, querySandboxServiceSettings.getNodeLevelJvmRejectionThreshold(), 1e-9); + } + + /** + * Tests the invalid value for {@code query_sandbox.node.rejection_threshold} + * When the value is set more than {@literal 0.90} + */ + public void testInValidNodeLevelRejectionThresholdCase1() { + Settings settings = Settings.builder().put(QUERY_SANDBOX_NODE_REJECTION_THRESHOLD_SETTING_NAME, 0.80).build(); + ClusterSettings cs = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + QuerySandboxServiceSettings querySandboxServiceSettings = new QuerySandboxServiceSettings(settings, cs); + assertThrows(IllegalArgumentException.class, + () -> querySandboxServiceSettings.setNodeLevelJvmRejectionThreshold(0.95)); + } + + /** + * Tests the invalid value for {@code query_sandbox.node.rejection_threshold} + * When the value is set more than {@code query_sandbox.node.cancellation_threshold} + */ + public void testInValidNodeLevelRejectionThresholdCase2() { + Settings settings = Settings.builder().put(QUERY_SANDBOX_NODE_REJECTION_THRESHOLD_SETTING_NAME, 0.70) + .put(QUERY_SANDBOX_NODE_CANCELLATION_THRESHOLD_SETTING_NAME, 0.80) + .build(); + ClusterSettings cs = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + QuerySandboxServiceSettings querySandboxServiceSettings = new QuerySandboxServiceSettings(settings, cs); + assertThrows(IllegalArgumentException.class, + () -> querySandboxServiceSettings.setNodeLevelJvmRejectionThreshold(0.85)); + } + + + /** + * Tests the invalid value for {@code query_sandbox.node.rejection_threshold} + * When the value is set more than {@code query_sandbox.node.cancellation_threshold} accidentally during + * new feature development. This test is to ensure that {@link QuerySandboxServiceSettings} holds the + * invariant {@code nodeLevelRejectionThreshold < nodeLevelCancellationThreshold} + */ + public void testInValidInstantiationOfQuerySandboxServiceSettings() { + Settings settings = Settings.builder().put(QUERY_SANDBOX_NODE_REJECTION_THRESHOLD_SETTING_NAME, 0.80) + .put(QUERY_SANDBOX_NODE_CANCELLATION_THRESHOLD_SETTING_NAME, 0.70) + .build(); + ClusterSettings cs = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + + assertThrows(IllegalArgumentException.class, + () -> new QuerySandboxServiceSettings(settings, cs)); + } + + /** + * Tests the valid value for {@code query_sandbox.node.cancellation_threshold} + */ + public void testValidNodeLevelCancellationThreshold() { + Settings settings = Settings.builder().put(QUERY_SANDBOX_NODE_CANCELLATION_THRESHOLD_SETTING_NAME, 0.80).build(); + ClusterSettings cs = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + QuerySandboxServiceSettings querySandboxServiceSettings = new QuerySandboxServiceSettings(settings, cs); + assertEquals(0.80, querySandboxServiceSettings.getNodeLevelJvmRejectionThreshold(), 1e-9); + } + + /** + * Tests the invalid value for {@code query_sandbox.node.cancellation_threshold} + * When the value is set more than {@literal 0.95} + */ + public void testInValidNodeLevelCancellationThresholdCase1() { + Settings settings = Settings.builder().put(QUERY_SANDBOX_NODE_CANCELLATION_THRESHOLD_SETTING_NAME, 0.80).build(); + ClusterSettings cs = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + QuerySandboxServiceSettings querySandboxServiceSettings = new QuerySandboxServiceSettings(settings, cs); + assertThrows(IllegalArgumentException.class, + () -> querySandboxServiceSettings.setNodeLevelJvmRejectionThreshold(0.96)); + } + + /** + * Tests the invalid value for {@code query_sandbox.node.cancellation_threshold} + * When the value is set less than {@code query_sandbox.node.rejection_threshold} + */ + public void testInValidNodeLevelCancellationThresholdCase2() { + Settings settings = Settings.builder().put(QUERY_SANDBOX_NODE_REJECTION_THRESHOLD_SETTING_NAME, 0.70) + .put(QUERY_SANDBOX_NODE_CANCELLATION_THRESHOLD_SETTING_NAME, 0.80) + .build(); + ClusterSettings cs = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + QuerySandboxServiceSettings querySandboxServiceSettings = new QuerySandboxServiceSettings(settings, cs); + assertThrows(IllegalArgumentException.class, + () -> querySandboxServiceSettings.setNodeLevelJvmRejectionThreshold(0.85)); + } + +} From 7f5bac598a30a7d0b0865c366549fdfe088b5bdc Mon Sep 17 00:00:00 2001 From: Kaushal Kumar Date: Thu, 11 Apr 2024 18:47:58 -0700 Subject: [PATCH 02/10] add resourceLimitGroupId propagation logic from coordinator to data nodes Signed-off-by: Kaushal Kumar --- .../search/AbstractSearchAsyncAction.java | 3 + .../action/search/SearchRequest.java | 27 +++++- .../action/search/SearchShardTask.java | 9 ++ .../opensearch/action/search/SearchTask.java | 9 ++ .../action/search/TransportSearchAction.java | 3 + .../common/settings/ClusterSettings.java | 8 +- .../rest/action/search/RestSearchAction.java | 5 ++ .../org/opensearch/search/SearchService.java | 3 + .../search/internal/ShardSearchRequest.java | 16 ++++ .../ResourceLimitGroupPruner.java} | 10 +-- .../ResourceLimitGroupService.java} | 38 ++++---- .../ResourceLimitGroupServiceSettings.java} | 88 +++++++++---------- .../ResourceLimitGroupRequestCanceller.java} | 8 +- .../cancellation/package-info.java | 2 +- .../module/ResourceLimitGroupModule.java | 33 +++++++ .../module/package-info.java | 2 +- .../package-info.java | 2 +- ...esourceLimitGroupResourceUsageTracker.java | 19 ++++ ...mitsGroupResourceUsageTrackerService.java} | 43 +++++---- .../tracker/package-info.java | 4 +- .../search/sandbox/module/SandboxModule.java | 33 ------- .../tracker/SandboxResourceTracker.java | 19 ---- ...sourceLimitGroupServiceSettingsTests.java} | 64 +++++++------- 23 files changed, 261 insertions(+), 187 deletions(-) rename server/src/main/java/org/opensearch/search/{sandbox/SandboxPruner.java => resource_limit_group/ResourceLimitGroupPruner.java} (53%) rename server/src/main/java/org/opensearch/search/{sandbox/QuerySandboxService.java => resource_limit_group/ResourceLimitGroupService.java} (61%) rename server/src/main/java/org/opensearch/search/{sandbox/QuerySandboxServiceSettings.java => resource_limit_group/ResourceLimitGroupServiceSettings.java} (62%) rename server/src/main/java/org/opensearch/search/{sandbox/cancellation/SandboxRequestCanceller.java => resource_limit_group/cancellation/ResourceLimitGroupRequestCanceller.java} (61%) rename server/src/main/java/org/opensearch/search/{sandbox => resource_limit_group}/cancellation/package-info.java (79%) create mode 100644 server/src/main/java/org/opensearch/search/resource_limit_group/module/ResourceLimitGroupModule.java rename server/src/main/java/org/opensearch/search/{sandbox => resource_limit_group}/module/package-info.java (79%) rename server/src/main/java/org/opensearch/search/{sandbox => resource_limit_group}/package-info.java (82%) create mode 100644 server/src/main/java/org/opensearch/search/resource_limit_group/tracker/ResourceLimitGroupResourceUsageTracker.java rename server/src/main/java/org/opensearch/search/{sandbox/tracker/SandboxResourceTrackerService.java => resource_limit_group/tracker/ResourceLimitsGroupResourceUsageTrackerService.java} (78%) rename server/src/main/java/org/opensearch/search/{sandbox => resource_limit_group}/tracker/package-info.java (65%) delete mode 100644 server/src/main/java/org/opensearch/search/sandbox/module/SandboxModule.java delete mode 100644 server/src/main/java/org/opensearch/search/sandbox/tracker/SandboxResourceTracker.java rename server/src/test/java/org/opensearch/search/sandbox/{QuerySandboxServiceSettingsTests.java => ResourceLimitGroupServiceSettingsTests.java} (53%) diff --git a/server/src/main/java/org/opensearch/action/search/AbstractSearchAsyncAction.java b/server/src/main/java/org/opensearch/action/search/AbstractSearchAsyncAction.java index 0520a4a7aecec..c706189f993a2 100644 --- a/server/src/main/java/org/opensearch/action/search/AbstractSearchAsyncAction.java +++ b/server/src/main/java/org/opensearch/action/search/AbstractSearchAsyncAction.java @@ -839,6 +839,9 @@ public final ShardSearchRequest buildShardSearchRequest(SearchShardIterator shar // than creating an empty response in the search thread pool. // Note that, we have to disable this shortcut for queries that create a context (scroll and search context). shardRequest.canReturnNullResponseIfMatchNoDocs(hasShardResponse.get() && shardRequest.scroll() == null); + + // Propagate the resource limit group from co-ordinator to data nodes + shardRequest.setResourceLimitGroupId(getTask().getResourceLimitGroupId()); return shardRequest; } diff --git a/server/src/main/java/org/opensearch/action/search/SearchRequest.java b/server/src/main/java/org/opensearch/action/search/SearchRequest.java index 3b8a6937815aa..2b6c64883a9fb 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchRequest.java +++ b/server/src/main/java/org/opensearch/action/search/SearchRequest.java @@ -122,6 +122,8 @@ public class SearchRequest extends ActionRequest implements IndicesRequest.Repla private Boolean phaseTook = null; + private String resourceLimitGroupId; + public SearchRequest() { this.localClusterAlias = null; this.absoluteStartMillis = DEFAULT_ABSOLUTE_START_MILLIS; @@ -262,6 +264,10 @@ public SearchRequest(StreamInput in) throws IOException { if (in.getVersion().onOrAfter(Version.V_2_12_0)) { phaseTook = in.readOptionalBoolean(); } + + if (in.getVersion().onOrAfter(Version.V_3_0_0)) { + resourceLimitGroupId = in.readOptionalString(); + } } @Override @@ -296,6 +302,9 @@ public void writeTo(StreamOutput out) throws IOException { if (out.getVersion().onOrAfter(Version.V_2_12_0)) { out.writeOptionalBoolean(phaseTook); } + if (out.getVersion().onOrAfter(Version.V_3_0_0)) { + out.writeOptionalString(resourceLimitGroupId); + } } @Override @@ -698,6 +707,15 @@ public String pipeline() { return pipeline; } + public SearchRequest resourceLimitGroupId(String resourceLimitGroupId) { + this.resourceLimitGroupId = resourceLimitGroupId; + return this; + } + + public String resourceLimitGroupId() { + return resourceLimitGroupId; + } + @Override public SearchTask createTask(long id, String type, String action, TaskId parentTaskId, Map headers) { return new SearchTask(id, type, action, this::buildDescription, parentTaskId, headers, cancelAfterTimeInterval); @@ -752,7 +770,8 @@ public boolean equals(Object o) { && ccsMinimizeRoundtrips == that.ccsMinimizeRoundtrips && Objects.equals(cancelAfterTimeInterval, that.cancelAfterTimeInterval) && Objects.equals(pipeline, that.pipeline) - && Objects.equals(phaseTook, that.phaseTook); + && Objects.equals(phaseTook, that.phaseTook) + && Objects.equals(resourceLimitGroupId, that.resourceLimitGroupId); } @Override @@ -774,7 +793,8 @@ public int hashCode() { absoluteStartMillis, ccsMinimizeRoundtrips, cancelAfterTimeInterval, - phaseTook + phaseTook, + resourceLimitGroupId ); } @@ -819,6 +839,7 @@ public String toString() { + pipeline + ", phaseTook=" + phaseTook - + "}"; + + ", resourceLimitGroupId=" + + resourceLimitGroupId + "}"; } } diff --git a/server/src/main/java/org/opensearch/action/search/SearchShardTask.java b/server/src/main/java/org/opensearch/action/search/SearchShardTask.java index dfecf4f462c4d..9e78a06aa7da5 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchShardTask.java +++ b/server/src/main/java/org/opensearch/action/search/SearchShardTask.java @@ -53,6 +53,7 @@ public class SearchShardTask extends CancellableTask implements SearchBackpressureTask { // generating metadata in a lazy way since source can be quite big private final MemoizedSupplier metadataSupplier; + private String resourceLimitGroupId; public SearchShardTask(long id, String type, String action, String description, TaskId parentTaskId, Map headers) { this(id, type, action, description, parentTaskId, headers, () -> ""); @@ -84,4 +85,12 @@ public boolean supportsResourceTracking() { public boolean shouldCancelChildrenOnCancellation() { return false; } + + public String getResourceLimitGroupId() { + return resourceLimitGroupId; + } + + public void setResourceLimitGroupId(String resourceLimitGroupId) { + this.resourceLimitGroupId = resourceLimitGroupId; + } } diff --git a/server/src/main/java/org/opensearch/action/search/SearchTask.java b/server/src/main/java/org/opensearch/action/search/SearchTask.java index d3c1043c50cce..363f3fde65338 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchTask.java +++ b/server/src/main/java/org/opensearch/action/search/SearchTask.java @@ -53,6 +53,7 @@ public class SearchTask extends CancellableTask implements SearchBackpressureTas // generating description in a lazy way since source can be quite big private final Supplier descriptionSupplier; private SearchProgressListener progressListener = SearchProgressListener.NOOP; + private String resourceLimitGroupId; public SearchTask( long id, @@ -106,4 +107,12 @@ public final SearchProgressListener getProgressListener() { public boolean shouldCancelChildrenOnCancellation() { return true; } + + public String getResourceLimitGroupId() { + return resourceLimitGroupId; + } + + public void setResourceLimitGroupId(String resourceLimitGroupId) { + this.resourceLimitGroupId = resourceLimitGroupId; + } } diff --git a/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java b/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java index 65cfd35489033..7ee7292176473 100644 --- a/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java +++ b/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java @@ -1104,6 +1104,9 @@ private void executeSearch( concreteLocalIndices, localShardIterators.size() + remoteShardIterators.size() ); + + task.setResourceLimitGroupId(searchRequest.resourceLimitGroupId()); + searchAsyncActionProvider.asyncSearchAction( task, searchRequest, 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 b1853e932c260..b0e997a0a606d 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -154,7 +154,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.sandbox.QuerySandboxServiceSettings; +import org.opensearch.search.sandbox.ResourceLimitGroupServiceSettings; import org.opensearch.snapshots.InternalSnapshotsInfoService; import org.opensearch.snapshots.SnapshotsService; import org.opensearch.tasks.TaskCancellationMonitoringSettings; @@ -736,9 +736,9 @@ public void apply(Settings value, Settings current, Settings previous) { RemoteStoreSettings.CLUSTER_REMOTE_STORE_PATH_HASH_ALGORITHM_SETTING, // Sandbox settings - QuerySandboxServiceSettings.MAX_SANDBOX_COUNT, - QuerySandboxServiceSettings.NODE_LEVEL_REJECTION_THRESHOLD, - QuerySandboxServiceSettings.NODE_LEVEL_CANCELLATION_THRESHOLD + ResourceLimitGroupServiceSettings.MAX_RESOURCE_LIMIT_GROUP_COUNT, + ResourceLimitGroupServiceSettings.NODE_LEVEL_REJECTION_THRESHOLD, + ResourceLimitGroupServiceSettings.NODE_LEVEL_CANCELLATION_THRESHOLD ) ) ); diff --git a/server/src/main/java/org/opensearch/rest/action/search/RestSearchAction.java b/server/src/main/java/org/opensearch/rest/action/search/RestSearchAction.java index 80dc34c4d5d68..7fa1b38ba95bf 100644 --- a/server/src/main/java/org/opensearch/rest/action/search/RestSearchAction.java +++ b/server/src/main/java/org/opensearch/rest/action/search/RestSearchAction.java @@ -222,6 +222,11 @@ public static void parseSearchRequest( ); } + // As part of first phase in enforcing the resource limits on the groups of queries + if (request.hasParam("resource_limit_group_id")) { + searchRequest.resourceLimitGroupId(request.param("resource_limit_group_id")); + } + searchRequest.setCancelAfterTimeInterval(request.paramAsTime("cancel_after_time_interval", null)); } diff --git a/server/src/main/java/org/opensearch/search/SearchService.java b/server/src/main/java/org/opensearch/search/SearchService.java index 6b3620e65a271..1167dd43784aa 100644 --- a/server/src/main/java/org/opensearch/search/SearchService.java +++ b/server/src/main/java/org/opensearch/search/SearchService.java @@ -568,6 +568,7 @@ public void executeQueryPhase( assert request.canReturnNullResponseIfMatchNoDocs() == false || request.numberOfShards() > 1 : "empty responses require more than one shard"; final IndexShard shard = getShard(request); + task.setResourceLimitGroupId(request.resourceLimitGroupId()); rewriteAndFetchShardRequest(shard, request, new ActionListener() { @Override public void onResponse(ShardSearchRequest orig) { @@ -676,6 +677,7 @@ public void executeQueryPhase( } runAsync(getExecutor(readerContext.indexShard()), () -> { final ShardSearchRequest shardSearchRequest = readerContext.getShardSearchRequest(null); + task.setResourceLimitGroupId(shardSearchRequest.resourceLimitGroupId()); try ( SearchContext searchContext = createContext(readerContext, shardSearchRequest, task, false); SearchOperationListenerExecutor executor = new SearchOperationListenerExecutor(searchContext) @@ -778,6 +780,7 @@ public void executeFetchPhase( public void executeFetchPhase(ShardFetchRequest request, SearchShardTask task, ActionListener listener) { final ReaderContext readerContext = findReaderContext(request.contextId(), request); final ShardSearchRequest shardSearchRequest = readerContext.getShardSearchRequest(request.getShardSearchRequest()); + task.setResourceLimitGroupId(shardSearchRequest.resourceLimitGroupId()); final Releasable markAsUsed = readerContext.markAsUsed(getKeepAlive(shardSearchRequest)); runAsync(getExecutor(readerContext.indexShard()), () -> { try (SearchContext searchContext = createContext(readerContext, shardSearchRequest, task, false)) { diff --git a/server/src/main/java/org/opensearch/search/internal/ShardSearchRequest.java b/server/src/main/java/org/opensearch/search/internal/ShardSearchRequest.java index de1d5fb8b4098..cf661c7243bb8 100644 --- a/server/src/main/java/org/opensearch/search/internal/ShardSearchRequest.java +++ b/server/src/main/java/org/opensearch/search/internal/ShardSearchRequest.java @@ -112,6 +112,7 @@ public class ShardSearchRequest extends TransportRequest implements IndicesReque private SearchSourceBuilder source; private final ShardSearchContextId readerId; private final TimeValue keepAlive; + private String resourceLimitGroupId; public ShardSearchRequest( OriginalIndices originalIndices, @@ -266,6 +267,9 @@ public ShardSearchRequest(StreamInput in) throws IOException { readerId = in.readOptionalWriteable(ShardSearchContextId::new); keepAlive = in.readOptionalTimeValue(); originalIndices = OriginalIndices.readOriginalIndices(in); + if (in.getVersion().onOrAfter(Version.V_3_0_0)) { + resourceLimitGroupId = in.readOptionalString(); + } assert keepAlive == null || readerId != null : "readerId: " + readerId + " keepAlive: " + keepAlive; } @@ -290,6 +294,7 @@ public ShardSearchRequest(ShardSearchRequest clone) { this.originalIndices = clone.originalIndices; this.readerId = clone.readerId; this.keepAlive = clone.keepAlive; + this.resourceLimitGroupId = clone.resourceLimitGroupId; } @Override @@ -297,6 +302,9 @@ public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); innerWriteTo(out, false); OriginalIndices.writeOriginalIndices(originalIndices, out); + if (out.getVersion().onOrAfter(Version.V_3_0_0)) { + out.writeOptionalString(resourceLimitGroupId); + } } protected final void innerWriteTo(StreamOutput out, boolean asKey) throws IOException { @@ -425,6 +433,14 @@ public String preference() { return preference; } + public String resourceLimitGroupId() { + return resourceLimitGroupId; + } + + public void setResourceLimitGroupId(String resourceLimitGroupId) { + this.resourceLimitGroupId = resourceLimitGroupId; + } + /** * Sets the bottom sort values that can be used by the searcher to filter documents * that are after it. This value is computed by coordinating nodes that throttles the diff --git a/server/src/main/java/org/opensearch/search/sandbox/SandboxPruner.java b/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupPruner.java similarity index 53% rename from server/src/main/java/org/opensearch/search/sandbox/SandboxPruner.java rename to server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupPruner.java index c47e5c22d4c3a..50b532eeadeb8 100644 --- a/server/src/main/java/org/opensearch/search/sandbox/SandboxPruner.java +++ b/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupPruner.java @@ -6,15 +6,15 @@ * compatible open source license. */ -package org.opensearch.search.sandbox; +package org.opensearch.search.resource_limit_group; /** - * This interface is used to identify and completely remove deleted sandboxes which has been marked as deleted + * This interface is used to identify and completely remove deleted resourceLimitGroups which has been marked as deleted * previously but had the tasks running at the time of deletion request */ -public interface SandboxPruner { +public interface ResourceLimitGroupPruner { /** - * remove the deleted sandboxes from the system once all the tasks in that sandbox are completed/cancelled + * remove the deleted resourceLimitGroups from the system once all the tasks in those resourceLimitGroups are completed/cancelled */ - void pruneSandboxes(); + void pruneResourceLimitGroup(); } diff --git a/server/src/main/java/org/opensearch/search/sandbox/QuerySandboxService.java b/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupService.java similarity index 61% rename from server/src/main/java/org/opensearch/search/sandbox/QuerySandboxService.java rename to server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupService.java index 001e2e7315aac..02954aa140f06 100644 --- a/server/src/main/java/org/opensearch/search/sandbox/QuerySandboxService.java +++ b/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupService.java @@ -6,51 +6,51 @@ * compatible open source license. */ -package org.opensearch.search.sandbox; +package org.opensearch.search.resource_limit_group; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.common.inject.Inject; import org.opensearch.common.lifecycle.AbstractLifecycleComponent; -import org.opensearch.search.sandbox.tracker.SandboxResourceTracker; -import org.opensearch.search.sandbox.cancellation.SandboxRequestCanceller; +import org.opensearch.search.resource_limit_group.tracker.ResourceLimitGroupResourceUsageTracker; +import org.opensearch.search.resource_limit_group.cancellation.ResourceLimitGroupRequestCanceller; import org.opensearch.threadpool.Scheduler; import org.opensearch.threadpool.ThreadPool; import java.io.IOException; /** - * Main service which will run periodically to track and cancel resource constraint violating tasks in sandboxes + * Main service which will run periodically to track and cancel resource constraint violating tasks in resourceLimitGroups */ -public class QuerySandboxService extends AbstractLifecycleComponent { - private static final Logger logger = LogManager.getLogger(QuerySandboxService.class); +public class ResourceLimitGroupService extends AbstractLifecycleComponent { + private static final Logger logger = LogManager.getLogger(ResourceLimitGroupService.class); - private final SandboxResourceTracker requestTracker; - private final SandboxRequestCanceller requestCanceller; - private final SandboxPruner sandboxPruner; + private final ResourceLimitGroupResourceUsageTracker requestTracker; + private final ResourceLimitGroupRequestCanceller requestCanceller; + private final ResourceLimitGroupPruner resourceLimitGroupPruner; private volatile Scheduler.Cancellable scheduledFuture; - private final QuerySandboxServiceSettings sandboxServiceSettings; + private final ResourceLimitGroupServiceSettings sandboxServiceSettings; private final ThreadPool threadPool; /** * Guice managed constructor * @param requestTrackerService * @param requestCanceller - * @param sandboxPruner + * @param resourceLimitGroupPruner * @param sandboxServiceSettings * @param threadPool */ @Inject - public QuerySandboxService( - SandboxResourceTracker requestTrackerService, - SandboxRequestCanceller requestCanceller, - SandboxPruner sandboxPruner, - QuerySandboxServiceSettings sandboxServiceSettings, + public ResourceLimitGroupService( + ResourceLimitGroupResourceUsageTracker requestTrackerService, + ResourceLimitGroupRequestCanceller requestCanceller, + ResourceLimitGroupPruner resourceLimitGroupPruner, + ResourceLimitGroupServiceSettings sandboxServiceSettings, ThreadPool threadPool ) { this.requestTracker = requestTrackerService; this.requestCanceller = requestCanceller; - this.sandboxPruner = sandboxPruner; + this.resourceLimitGroupPruner = resourceLimitGroupPruner; this.sandboxServiceSettings = sandboxServiceSettings; this.threadPool = threadPool; } @@ -59,9 +59,9 @@ public QuerySandboxService( * run at regular interval */ private void doRun() { - requestTracker.updateSandboxResourceUsages(); + requestTracker.updateResourceLimitGroupsResourceUsage(); requestCanceller.cancelViolatingTasks(); - sandboxPruner.pruneSandboxes(); + resourceLimitGroupPruner.pruneResourceLimitGroup(); } /** diff --git a/server/src/main/java/org/opensearch/search/sandbox/QuerySandboxServiceSettings.java b/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupServiceSettings.java similarity index 62% rename from server/src/main/java/org/opensearch/search/sandbox/QuerySandboxServiceSettings.java rename to server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupServiceSettings.java index 70d6a7076d4d2..56bdfbae87168 100644 --- a/server/src/main/java/org/opensearch/search/sandbox/QuerySandboxServiceSettings.java +++ b/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupServiceSettings.java @@ -6,58 +6,56 @@ * compatible open source license. */ -package org.opensearch.search.sandbox; +package org.opensearch.search.resource_limit_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; -import java.util.List; - /** - * Main class to declare the query sandboxing feature related settings + * Main class to declare the query ResourceLimitGrouping feature related settings */ -public class QuerySandboxServiceSettings { +public class ResourceLimitGroupServiceSettings { private static final Long DEFAULT_RUN_INTERVAL_MILLIS = 1000l; - private static final Double DEFAULT_NODE_LEVEL_REJECTION_QUERY_SANDBOX_THRESHOLD = 0.8; - private static final Double DEFAULT_NODE_LEVEL_CANCELLATION_QUERY_SANDBOX_THRESHOLD = 0.9; + private static final Double DEFAULT_NODE_LEVEL_REJECTION_THRESHOLD = 0.8; + private static final Double DEFAULT_NODE_LEVEL_CANCELLATION_THRESHOLD = 0.9; /** - * default max sandbox count on any node at any given point in time + * default max ResourceLimitGroup count on any node at any given point in time */ - public static final int DEFAULT_MAX_SANDBOX_COUNT_VALUE = 100; + public static final int DEFAULT_MAX_RESOURCE_LIMIT_GROUP_COUNT_VALUE = 100; - public static final String SANDBOX_COUNT_SETTING_NAME = "node.sandbox.max_count"; + public static final String RESOURCE_LIMIT_GROUP_COUNT_SETTING_NAME = "node.resource_limit_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 nodeLevelJvmCancellationThreshold; private Double nodeLevelJvmRejectionThreshold; - private volatile int maxSandboxCount; + private volatile int maxResourceLimitGroupCount; /** - * max sandbox count setting + * max ResourceLimitGroup count setting */ - public static final Setting MAX_SANDBOX_COUNT = Setting.intSetting( - SANDBOX_COUNT_SETTING_NAME, - DEFAULT_MAX_SANDBOX_COUNT_VALUE, + public static final Setting MAX_RESOURCE_LIMIT_GROUP_COUNT = Setting.intSetting( + RESOURCE_LIMIT_GROUP_COUNT_SETTING_NAME, + DEFAULT_MAX_RESOURCE_LIMIT_GROUP_COUNT_VALUE, 0, (newVal) -> { if (newVal > 100 || newVal < 1) - throw new IllegalArgumentException(SANDBOX_COUNT_SETTING_NAME + " should be in range [1-100]"); + throw new IllegalArgumentException(RESOURCE_LIMIT_GROUP_COUNT_SETTING_NAME + " should be in range [1-100]"); }, Setting.Property.Dynamic, Setting.Property.NodeScope ); /** - * Setting name for default sandbox count + * Setting name for default ResourceLimitGroup count */ - public static final String QUERY_SANDBOX_SERVICE_RUN_INTERVAL_MILLIS_SETTING_NAME = "query_sandbox.service.run_interval_millis"; + public static final String SERVICE_RUN_INTERVAL_MILLIS_SETTING_NAME = "resource_limit_group.service.run_interval_millis"; /** * Setting to control the run interval of QSB service */ - private static final Setting QSB_RUN_INTERVAL_SETTING = Setting.longSetting( - QUERY_SANDBOX_SERVICE_RUN_INTERVAL_MILLIS_SETTING_NAME, + private static final Setting RESOURCE_LIMIT_GROUP_RUN_INTERVAL_SETTING = Setting.longSetting( + SERVICE_RUN_INTERVAL_MILLIS_SETTING_NAME, DEFAULT_RUN_INTERVAL_MILLIS, 1, Setting.Property.Dynamic, @@ -67,44 +65,44 @@ public class QuerySandboxServiceSettings { /** * Setting name for node level rejection threshold for QSB */ - public static final String QUERY_SANDBOX_NODE_REJECTION_THRESHOLD_SETTING_NAME = "query_sandbox.node.rejection_threshold"; + public static final String NODE_REJECTION_THRESHOLD_SETTING_NAME = "resource_limit_group.node.rejection_threshold"; /** * Setting to control the rejection threshold */ public static final Setting NODE_LEVEL_REJECTION_THRESHOLD = Setting.doubleSetting( - QUERY_SANDBOX_NODE_REJECTION_THRESHOLD_SETTING_NAME, - DEFAULT_NODE_LEVEL_REJECTION_QUERY_SANDBOX_THRESHOLD, + 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 QUERY_SANDBOX_NODE_CANCELLATION_THRESHOLD_SETTING_NAME = "query_sandbox.node.cancellation_threshold"; + public static final String NODE_CANCELLATION_THRESHOLD_SETTING_NAME = "resource_limit_group.node.cancellation_threshold"; /** * Setting name for node level cancellation threshold */ public static final Setting NODE_LEVEL_CANCELLATION_THRESHOLD = Setting.doubleSetting( - QUERY_SANDBOX_NODE_CANCELLATION_THRESHOLD_SETTING_NAME, - DEFAULT_NODE_LEVEL_CANCELLATION_QUERY_SANDBOX_THRESHOLD, + NODE_CANCELLATION_THRESHOLD_SETTING_NAME, + DEFAULT_NODE_LEVEL_CANCELLATION_THRESHOLD, Setting.Property.Dynamic, Setting.Property.NodeScope ); /** - * Sandbox service settings constructor + * ResourceLimitGroup service settings constructor * @param settings * @param clusterSettings */ - public QuerySandboxServiceSettings(Settings settings, ClusterSettings clusterSettings) { - runIntervalMillis = new TimeValue(QSB_RUN_INTERVAL_SETTING.get(settings)); + public ResourceLimitGroupServiceSettings(Settings settings, ClusterSettings clusterSettings) { + runIntervalMillis = new TimeValue(RESOURCE_LIMIT_GROUP_RUN_INTERVAL_SETTING.get(settings)); nodeLevelJvmCancellationThreshold = NODE_LEVEL_CANCELLATION_THRESHOLD.get(settings); nodeLevelJvmRejectionThreshold = NODE_LEVEL_REJECTION_THRESHOLD.get(settings); - maxSandboxCount = MAX_SANDBOX_COUNT.get(settings); + maxResourceLimitGroupCount = MAX_RESOURCE_LIMIT_GROUP_COUNT.get(settings); ensureRejectionThresholdIsLessThanCancellation(nodeLevelJvmRejectionThreshold, nodeLevelJvmCancellationThreshold); - clusterSettings.addSettingsUpdateConsumer(MAX_SANDBOX_COUNT, this::setMaxSandboxCount); + clusterSettings.addSettingsUpdateConsumer(MAX_RESOURCE_LIMIT_GROUP_COUNT, this::setMaxResourceLimitGroupCount); clusterSettings.addSettingsUpdateConsumer(NODE_LEVEL_CANCELLATION_THRESHOLD, this::setNodeLevelJvmCancellationThreshold); clusterSettings.addSettingsUpdateConsumer(NODE_LEVEL_REJECTION_THRESHOLD, this::setNodeLevelJvmRejectionThreshold); } @@ -118,14 +116,14 @@ public TimeValue getRunIntervalMillis() { } /** - * Method to set the new sandbox count - * @param newMaxSandboxCount is the new maxSandboxCount per node + * Method to set the new ResourceLimitGroup count + * @param newMaxResourceLimitGroupCount is the new maxResourceLimitGroupCount per node */ - public void setMaxSandboxCount(int newMaxSandboxCount) { - if (newMaxSandboxCount < 0) { - throw new IllegalArgumentException("node.sandbox.max_count can't be negative"); + public void setMaxResourceLimitGroupCount(int newMaxResourceLimitGroupCount) { + if (newMaxResourceLimitGroupCount < 0) { + throw new IllegalArgumentException("node.ResourceLimitGroup.max_count can't be negative"); } - this.maxSandboxCount = newMaxSandboxCount; + this.maxResourceLimitGroupCount = newMaxResourceLimitGroupCount; } /** @@ -144,7 +142,7 @@ public Double getNodeLevelJvmCancellationThreshold() { public void setNodeLevelJvmCancellationThreshold(Double nodeLevelJvmCancellationThreshold) { if (Double.compare(nodeLevelJvmCancellationThreshold, NODE_LEVEL_CANCELLATION_THRESHOLD_MAX_VALUE) > 0) { throw new IllegalArgumentException( - QUERY_SANDBOX_NODE_CANCELLATION_THRESHOLD_SETTING_NAME + NODE_CANCELLATION_THRESHOLD_SETTING_NAME + " value should not be greater than 0.95 as it pose a threat of node drop" ); } @@ -170,7 +168,7 @@ public Double getNodeLevelJvmRejectionThreshold() { public void setNodeLevelJvmRejectionThreshold(Double nodeLevelJvmRejectionThreshold) { if (Double.compare(nodeLevelJvmRejectionThreshold, NODE_LEVEL_REJECTION_THRESHOLD_MAX_VALUE) > 0) { throw new IllegalArgumentException( - QUERY_SANDBOX_NODE_REJECTION_THRESHOLD_SETTING_NAME + " value not be greater than 0.90 as it pose a threat of node drop" + NODE_REJECTION_THRESHOLD_SETTING_NAME + " value not be greater than 0.90 as it pose a threat of node drop" ); } @@ -185,18 +183,18 @@ private void ensureRejectionThresholdIsLessThanCancellation( ) { if (Double.compare(nodeLevelJvmCancellationThreshold , nodeLevelJvmRejectionThreshold) < 0) { throw new IllegalArgumentException( - QUERY_SANDBOX_NODE_CANCELLATION_THRESHOLD_SETTING_NAME + NODE_CANCELLATION_THRESHOLD_SETTING_NAME + " value should not be less than " - + QUERY_SANDBOX_NODE_REJECTION_THRESHOLD_SETTING_NAME + + NODE_REJECTION_THRESHOLD_SETTING_NAME ); } } /** - * Method to get the current sandbox count - * @return the current max sandbox count + * Method to get the current ResourceLimitGroup count + * @return the current max ResourceLimitGroup count */ - public int getMaxSandboxCount() { - return maxSandboxCount; + public int getMaxResourceLimitGroupCount() { + return maxResourceLimitGroupCount; } } diff --git a/server/src/main/java/org/opensearch/search/sandbox/cancellation/SandboxRequestCanceller.java b/server/src/main/java/org/opensearch/search/resource_limit_group/cancellation/ResourceLimitGroupRequestCanceller.java similarity index 61% rename from server/src/main/java/org/opensearch/search/sandbox/cancellation/SandboxRequestCanceller.java rename to server/src/main/java/org/opensearch/search/resource_limit_group/cancellation/ResourceLimitGroupRequestCanceller.java index 713d6314f4bf2..77be6104c9600 100644 --- a/server/src/main/java/org/opensearch/search/sandbox/cancellation/SandboxRequestCanceller.java +++ b/server/src/main/java/org/opensearch/search/resource_limit_group/cancellation/ResourceLimitGroupRequestCanceller.java @@ -6,14 +6,14 @@ * compatible open source license. */ -package org.opensearch.search.sandbox.cancellation; +package org.opensearch.search.resource_limit_group.cancellation; /** - * This interface is used to identify and cancel the violating tasks in a sandbox + * This interface is used to identify and cancel the violating tasks in a resourceLimitGroup */ -public interface SandboxRequestCanceller { +public interface ResourceLimitGroupRequestCanceller { /** - * Cancels the tasks from conteded sandboxes + * Cancels the tasks from conteded resourceLimitGroups */ void cancelViolatingTasks(); } diff --git a/server/src/main/java/org/opensearch/search/sandbox/cancellation/package-info.java b/server/src/main/java/org/opensearch/search/resource_limit_group/cancellation/package-info.java similarity index 79% rename from server/src/main/java/org/opensearch/search/sandbox/cancellation/package-info.java rename to server/src/main/java/org/opensearch/search/resource_limit_group/cancellation/package-info.java index 52c31596380f0..01502e42e9dfc 100644 --- a/server/src/main/java/org/opensearch/search/sandbox/cancellation/package-info.java +++ b/server/src/main/java/org/opensearch/search/resource_limit_group/cancellation/package-info.java @@ -9,4 +9,4 @@ /** * Package for cancellation related abstracts */ -package org.opensearch.search.sandbox.cancellation; +package org.opensearch.search.resource_limit_group.cancellation; diff --git a/server/src/main/java/org/opensearch/search/resource_limit_group/module/ResourceLimitGroupModule.java b/server/src/main/java/org/opensearch/search/resource_limit_group/module/ResourceLimitGroupModule.java new file mode 100644 index 0000000000000..9700612a71edc --- /dev/null +++ b/server/src/main/java/org/opensearch/search/resource_limit_group/module/ResourceLimitGroupModule.java @@ -0,0 +1,33 @@ +/* + * 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.resource_limit_group.module; + +import org.opensearch.common.inject.AbstractModule; +import org.opensearch.search.resource_limit_group.ResourceLimitGroupPruner; +import org.opensearch.search.resource_limit_group.tracker.ResourceLimitGroupResourceUsageTracker; +import org.opensearch.search.resource_limit_group.cancellation.ResourceLimitGroupRequestCanceller; +import org.opensearch.search.resource_limit_group.tracker.ResourceLimitsGroupResourceUsageTrackerService; + +/** + * Module class for resource usage limiting related artifacts + */ +public class ResourceLimitGroupModule extends AbstractModule { + + /** + * Default constructor + */ + public ResourceLimitGroupModule() {} + + @Override + protected void configure() { + bind(ResourceLimitGroupResourceUsageTracker.class).to(ResourceLimitsGroupResourceUsageTrackerService.class).asEagerSingleton(); + bind(ResourceLimitGroupRequestCanceller.class).to(ResourceLimitsGroupResourceUsageTrackerService.class).asEagerSingleton(); + bind(ResourceLimitGroupPruner.class).to(ResourceLimitsGroupResourceUsageTrackerService.class).asEagerSingleton(); + } +} diff --git a/server/src/main/java/org/opensearch/search/sandbox/module/package-info.java b/server/src/main/java/org/opensearch/search/resource_limit_group/module/package-info.java similarity index 79% rename from server/src/main/java/org/opensearch/search/sandbox/module/package-info.java rename to server/src/main/java/org/opensearch/search/resource_limit_group/module/package-info.java index d1bcb8d6e01d3..b547a83cd5a42 100644 --- a/server/src/main/java/org/opensearch/search/sandbox/module/package-info.java +++ b/server/src/main/java/org/opensearch/search/resource_limit_group/module/package-info.java @@ -10,4 +10,4 @@ * Guice Module */ -package org.opensearch.search.sandbox.module; +package org.opensearch.search.resource_limit_group.module; diff --git a/server/src/main/java/org/opensearch/search/sandbox/package-info.java b/server/src/main/java/org/opensearch/search/resource_limit_group/package-info.java similarity index 82% rename from server/src/main/java/org/opensearch/search/sandbox/package-info.java rename to server/src/main/java/org/opensearch/search/resource_limit_group/package-info.java index 36bfefab5a9aa..04e807374c1ff 100644 --- a/server/src/main/java/org/opensearch/search/sandbox/package-info.java +++ b/server/src/main/java/org/opensearch/search/resource_limit_group/package-info.java @@ -9,4 +9,4 @@ /** * Query Sandboxing related artifacts */ -package org.opensearch.search.sandbox; +package org.opensearch.search.resource_limit_group; diff --git a/server/src/main/java/org/opensearch/search/resource_limit_group/tracker/ResourceLimitGroupResourceUsageTracker.java b/server/src/main/java/org/opensearch/search/resource_limit_group/tracker/ResourceLimitGroupResourceUsageTracker.java new file mode 100644 index 0000000000000..dccf5d4552bca --- /dev/null +++ b/server/src/main/java/org/opensearch/search/resource_limit_group/tracker/ResourceLimitGroupResourceUsageTracker.java @@ -0,0 +1,19 @@ +/* + * 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.resource_limit_group.tracker; + +/** + * This interface is mainly for tracking the resourceLimitGroup level resource usages + */ +public interface ResourceLimitGroupResourceUsageTracker { + /** + * updates the current resource usage of resourceLimitGroups + */ + public void updateResourceLimitGroupsResourceUsage(); +} diff --git a/server/src/main/java/org/opensearch/search/sandbox/tracker/SandboxResourceTrackerService.java b/server/src/main/java/org/opensearch/search/resource_limit_group/tracker/ResourceLimitsGroupResourceUsageTrackerService.java similarity index 78% rename from server/src/main/java/org/opensearch/search/sandbox/tracker/SandboxResourceTrackerService.java rename to server/src/main/java/org/opensearch/search/resource_limit_group/tracker/ResourceLimitsGroupResourceUsageTrackerService.java index aa0722ebcc7a5..cb2994b85b829 100644 --- a/server/src/main/java/org/opensearch/search/sandbox/tracker/SandboxResourceTrackerService.java +++ b/server/src/main/java/org/opensearch/search/resource_limit_group/tracker/ResourceLimitsGroupResourceUsageTrackerService.java @@ -6,12 +6,12 @@ * compatible open source license. */ -package org.opensearch.search.sandbox.tracker; +package org.opensearch.search.resource_limit_group.tracker; import org.opensearch.common.inject.Inject; import org.opensearch.core.tasks.resourcetracker.TaskResourceUsage; -import org.opensearch.search.sandbox.SandboxPruner; -import org.opensearch.search.sandbox.cancellation.SandboxRequestCanceller; +import org.opensearch.search.resource_limit_group.ResourceLimitGroupPruner; +import org.opensearch.search.resource_limit_group.cancellation.ResourceLimitGroupRequestCanceller; import org.opensearch.tasks.Task; import org.opensearch.tasks.TaskCancellation; import org.opensearch.tasks.TaskManager; @@ -24,14 +24,14 @@ import java.util.stream.Collectors; /** - * This class tracks requests per sandboxes + * This class tracks requests per resourceLimitGroups */ -public class SandboxResourceTrackerService +public class ResourceLimitsGroupResourceUsageTrackerService implements TaskManager.TaskEventListeners, - SandboxResourceTracker, - SandboxRequestCanceller, - SandboxPruner { + ResourceLimitGroupResourceUsageTracker, + ResourceLimitGroupRequestCanceller, + ResourceLimitGroupPruner { private static final String CPU = "CPU"; private static final String JVM_ALLOCATIONS = "JVM_Allocations"; @@ -39,10 +39,11 @@ public class SandboxResourceTrackerService private static final long totalAvailableJvmMemory = Runtime.getRuntime().totalMemory(); private final LongSupplier timeNanosSupplier; /** - * Sandbox ids which are marked for deletion in between the @link SandboxService runs + * ResourceLimitGroup ids which are marked for deletion in between the + * {@link org.opensearch.search.resource_limit_group.ResourceLimitGroupService} runs */ - private List toDeleteSandboxes; - private List activeSandboxes; + private List toDeleteResourceLimitGroups; + private List activeResourceLimitGroups; private final TaskManager taskManager; private final TaskResourceTrackingService taskResourceTrackingService; @@ -52,18 +53,18 @@ public class SandboxResourceTrackerService * @param taskResourceTrackingService */ @Inject - public SandboxResourceTrackerService( + public ResourceLimitsGroupResourceUsageTrackerService( TaskManager taskManager, TaskResourceTrackingService taskResourceTrackingService ) { this.taskManager = taskManager; this.taskResourceTrackingService = taskResourceTrackingService; - toDeleteSandboxes = Collections.synchronizedList(new ArrayList<>()); + toDeleteResourceLimitGroups = Collections.synchronizedList(new ArrayList<>()); this.timeNanosSupplier = System::nanoTime; } @Override - public void updateSandboxResourceUsages() { + public void updateResourceLimitGroupsResourceUsage() { } @@ -114,8 +115,8 @@ public double getAbsoluteJvmAllocationsUsageInPercent() { /** * filter out the deleted sandboxes which still has unfi */ - public void pruneSandboxes() { - toDeleteSandboxes = toDeleteSandboxes.stream().filter(this::hasUnfinishedTasks).collect(Collectors.toList()); + public void pruneResourceLimitGroup() { + toDeleteResourceLimitGroups = toDeleteResourceLimitGroups.stream().filter(this::hasUnfinishedTasks).collect(Collectors.toList()); } private boolean hasUnfinishedTasks(String sandboxId) { @@ -146,15 +147,19 @@ public void cancelViolatingTasks() { } } + /** + * + * @return list of cancellable tasks + */ private List getCancellableTasks() { - // perform cancellations from enforced type sandboxes + // get cancellations from enforced type sandboxes List inViolationSandboxes = getBreachingSandboxIds(); List cancellableTasks = new ArrayList<>(); for (String sandboxId : inViolationSandboxes) { cancellableTasks.addAll(getCancellableTasksFrom(sandboxId)); } - // perform cancellations from soft type sandboxes if the node is in duress (hitting node level cancellation + // get cancellations from soft type sandboxes if the node is in duress (hitting node level cancellation // threshold) @@ -163,7 +168,7 @@ private List getCancellableTasks() { public void deleteSandbox(String sandboxId) { if (hasUnfinishedTasks(sandboxId)) { - toDeleteSandboxes.add(sandboxId); + toDeleteResourceLimitGroups.add(sandboxId); } // remove this sandbox from the active sandboxes } diff --git a/server/src/main/java/org/opensearch/search/sandbox/tracker/package-info.java b/server/src/main/java/org/opensearch/search/resource_limit_group/tracker/package-info.java similarity index 65% rename from server/src/main/java/org/opensearch/search/sandbox/tracker/package-info.java rename to server/src/main/java/org/opensearch/search/resource_limit_group/tracker/package-info.java index e7be9a36f3ce5..64d8911f8a1a7 100644 --- a/server/src/main/java/org/opensearch/search/sandbox/tracker/package-info.java +++ b/server/src/main/java/org/opensearch/search/resource_limit_group/tracker/package-info.java @@ -7,6 +7,6 @@ */ /** - * Sandbox resource tracking artifacts + * ResourceLimitGroup resource tracking artifacts */ -package org.opensearch.search.sandbox.tracker; +package org.opensearch.search.resource_limit_group.tracker; diff --git a/server/src/main/java/org/opensearch/search/sandbox/module/SandboxModule.java b/server/src/main/java/org/opensearch/search/sandbox/module/SandboxModule.java deleted file mode 100644 index b6da7a1d9539b..0000000000000 --- a/server/src/main/java/org/opensearch/search/sandbox/module/SandboxModule.java +++ /dev/null @@ -1,33 +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.sandbox.module; - -import org.opensearch.common.inject.AbstractModule; -import org.opensearch.search.sandbox.SandboxPruner; -import org.opensearch.search.sandbox.tracker.SandboxResourceTracker; -import org.opensearch.search.sandbox.cancellation.SandboxRequestCanceller; -import org.opensearch.search.sandbox.tracker.SandboxResourceTrackerService; - -/** - * Module class for sandboxing related artifacts - */ -public class SandboxModule extends AbstractModule { - - /** - * Default constructor - */ - public SandboxModule() {} - - @Override - protected void configure() { - bind(SandboxResourceTracker.class).to(SandboxResourceTrackerService.class).asEagerSingleton(); - bind(SandboxRequestCanceller.class).to(SandboxResourceTrackerService.class).asEagerSingleton(); - bind(SandboxPruner.class).to(SandboxResourceTrackerService.class).asEagerSingleton(); - } -} diff --git a/server/src/main/java/org/opensearch/search/sandbox/tracker/SandboxResourceTracker.java b/server/src/main/java/org/opensearch/search/sandbox/tracker/SandboxResourceTracker.java deleted file mode 100644 index 0ab1702d4c3a1..0000000000000 --- a/server/src/main/java/org/opensearch/search/sandbox/tracker/SandboxResourceTracker.java +++ /dev/null @@ -1,19 +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.sandbox.tracker; - -/** - * This interface is mainly for tracking the sandbox level resource usages - */ -public interface SandboxResourceTracker { - /** - * updates the current resource usage of sandboxes - */ - public void updateSandboxResourceUsages(); -} diff --git a/server/src/test/java/org/opensearch/search/sandbox/QuerySandboxServiceSettingsTests.java b/server/src/test/java/org/opensearch/search/sandbox/ResourceLimitGroupServiceSettingsTests.java similarity index 53% rename from server/src/test/java/org/opensearch/search/sandbox/QuerySandboxServiceSettingsTests.java rename to server/src/test/java/org/opensearch/search/sandbox/ResourceLimitGroupServiceSettingsTests.java index 9d6158c8244ab..0e500434ea488 100644 --- a/server/src/test/java/org/opensearch/search/sandbox/QuerySandboxServiceSettingsTests.java +++ b/server/src/test/java/org/opensearch/search/sandbox/ResourceLimitGroupServiceSettingsTests.java @@ -12,18 +12,20 @@ import org.opensearch.common.settings.Settings; import org.opensearch.test.OpenSearchTestCase; -import static org.opensearch.search.sandbox.QuerySandboxServiceSettings.*; +import static org.opensearch.search.sandbox.ResourceLimitGroupServiceSettings.NODE_CANCELLATION_THRESHOLD_SETTING_NAME; +import static org.opensearch.search.sandbox.ResourceLimitGroupServiceSettings.NODE_REJECTION_THRESHOLD_SETTING_NAME; +import static org.opensearch.search.sandbox.ResourceLimitGroupServiceSettings.RESOURCE_LIMIT_GROUP_COUNT_SETTING_NAME; -public class QuerySandboxServiceSettingsTests extends OpenSearchTestCase { +public class ResourceLimitGroupServiceSettingsTests extends OpenSearchTestCase { /** * Tests the valid value of {@code node.sandbox.max_count} */ public void testValidMaxSandboxCountSetting() { - Settings settings = Settings.builder().put(SANDBOX_COUNT_SETTING_NAME, 100).build(); + Settings settings = Settings.builder().put(RESOURCE_LIMIT_GROUP_COUNT_SETTING_NAME, 100).build(); ClusterSettings cs = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - QuerySandboxServiceSettings querySandboxServiceSettings = new QuerySandboxServiceSettings(settings, cs); - assertEquals(100, querySandboxServiceSettings.getMaxSandboxCount()); + ResourceLimitGroupServiceSettings resourceLimitGroupServiceSettings = new ResourceLimitGroupServiceSettings(settings, cs); + assertEquals(100, resourceLimitGroupServiceSettings.getMaxResourceLimitGroupCount()); } @@ -31,20 +33,20 @@ public void testValidMaxSandboxCountSetting() { * test the invalid value of {@code node.sandbox.max_count} */ public void testInValidMaxSandboxCountSetting() { - Settings settings = Settings.builder().put(SANDBOX_COUNT_SETTING_NAME, -100).build(); + Settings settings = Settings.builder().put(RESOURCE_LIMIT_GROUP_COUNT_SETTING_NAME, -100).build(); ClusterSettings cs = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); assertThrows(IllegalArgumentException.class, - () -> new QuerySandboxServiceSettings(settings, cs)); + () -> new ResourceLimitGroupServiceSettings(settings, cs)); } /** * Tests the valid value for {@code query_sandbox.node.rejection_threshold} */ public void testValidNodeLevelRejectionThreshold() { - Settings settings = Settings.builder().put(QUERY_SANDBOX_NODE_REJECTION_THRESHOLD_SETTING_NAME, 0.80).build(); + Settings settings = Settings.builder().put(NODE_REJECTION_THRESHOLD_SETTING_NAME, 0.80).build(); ClusterSettings cs = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - QuerySandboxServiceSettings querySandboxServiceSettings = new QuerySandboxServiceSettings(settings, cs); - assertEquals(0.80, querySandboxServiceSettings.getNodeLevelJvmRejectionThreshold(), 1e-9); + ResourceLimitGroupServiceSettings resourceLimitGroupServiceSettings = new ResourceLimitGroupServiceSettings(settings, cs); + assertEquals(0.80, resourceLimitGroupServiceSettings.getNodeLevelJvmRejectionThreshold(), 1e-9); } /** @@ -52,11 +54,11 @@ public void testValidNodeLevelRejectionThreshold() { * When the value is set more than {@literal 0.90} */ public void testInValidNodeLevelRejectionThresholdCase1() { - Settings settings = Settings.builder().put(QUERY_SANDBOX_NODE_REJECTION_THRESHOLD_SETTING_NAME, 0.80).build(); + Settings settings = Settings.builder().put(NODE_REJECTION_THRESHOLD_SETTING_NAME, 0.80).build(); ClusterSettings cs = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - QuerySandboxServiceSettings querySandboxServiceSettings = new QuerySandboxServiceSettings(settings, cs); + ResourceLimitGroupServiceSettings resourceLimitGroupServiceSettings = new ResourceLimitGroupServiceSettings(settings, cs); assertThrows(IllegalArgumentException.class, - () -> querySandboxServiceSettings.setNodeLevelJvmRejectionThreshold(0.95)); + () -> resourceLimitGroupServiceSettings.setNodeLevelJvmRejectionThreshold(0.95)); } /** @@ -64,40 +66,40 @@ public void testInValidNodeLevelRejectionThresholdCase1() { * When the value is set more than {@code query_sandbox.node.cancellation_threshold} */ public void testInValidNodeLevelRejectionThresholdCase2() { - Settings settings = Settings.builder().put(QUERY_SANDBOX_NODE_REJECTION_THRESHOLD_SETTING_NAME, 0.70) - .put(QUERY_SANDBOX_NODE_CANCELLATION_THRESHOLD_SETTING_NAME, 0.80) + Settings settings = Settings.builder().put(NODE_REJECTION_THRESHOLD_SETTING_NAME, 0.70) + .put(NODE_CANCELLATION_THRESHOLD_SETTING_NAME, 0.80) .build(); ClusterSettings cs = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - QuerySandboxServiceSettings querySandboxServiceSettings = new QuerySandboxServiceSettings(settings, cs); + ResourceLimitGroupServiceSettings resourceLimitGroupServiceSettings = new ResourceLimitGroupServiceSettings(settings, cs); assertThrows(IllegalArgumentException.class, - () -> querySandboxServiceSettings.setNodeLevelJvmRejectionThreshold(0.85)); + () -> resourceLimitGroupServiceSettings.setNodeLevelJvmRejectionThreshold(0.85)); } /** * Tests the invalid value for {@code query_sandbox.node.rejection_threshold} * When the value is set more than {@code query_sandbox.node.cancellation_threshold} accidentally during - * new feature development. This test is to ensure that {@link QuerySandboxServiceSettings} holds the + * new feature development. This test is to ensure that {@link ResourceLimitGroupServiceSettings} holds the * invariant {@code nodeLevelRejectionThreshold < nodeLevelCancellationThreshold} */ public void testInValidInstantiationOfQuerySandboxServiceSettings() { - Settings settings = Settings.builder().put(QUERY_SANDBOX_NODE_REJECTION_THRESHOLD_SETTING_NAME, 0.80) - .put(QUERY_SANDBOX_NODE_CANCELLATION_THRESHOLD_SETTING_NAME, 0.70) + Settings settings = Settings.builder().put(NODE_REJECTION_THRESHOLD_SETTING_NAME, 0.80) + .put(NODE_CANCELLATION_THRESHOLD_SETTING_NAME, 0.70) .build(); ClusterSettings cs = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); assertThrows(IllegalArgumentException.class, - () -> new QuerySandboxServiceSettings(settings, cs)); + () -> new ResourceLimitGroupServiceSettings(settings, cs)); } /** * Tests the valid value for {@code query_sandbox.node.cancellation_threshold} */ public void testValidNodeLevelCancellationThreshold() { - Settings settings = Settings.builder().put(QUERY_SANDBOX_NODE_CANCELLATION_THRESHOLD_SETTING_NAME, 0.80).build(); + Settings settings = Settings.builder().put(NODE_CANCELLATION_THRESHOLD_SETTING_NAME, 0.80).build(); ClusterSettings cs = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - QuerySandboxServiceSettings querySandboxServiceSettings = new QuerySandboxServiceSettings(settings, cs); - assertEquals(0.80, querySandboxServiceSettings.getNodeLevelJvmRejectionThreshold(), 1e-9); + ResourceLimitGroupServiceSettings resourceLimitGroupServiceSettings = new ResourceLimitGroupServiceSettings(settings, cs); + assertEquals(0.80, resourceLimitGroupServiceSettings.getNodeLevelJvmRejectionThreshold(), 1e-9); } /** @@ -105,11 +107,11 @@ public void testValidNodeLevelCancellationThreshold() { * When the value is set more than {@literal 0.95} */ public void testInValidNodeLevelCancellationThresholdCase1() { - Settings settings = Settings.builder().put(QUERY_SANDBOX_NODE_CANCELLATION_THRESHOLD_SETTING_NAME, 0.80).build(); + Settings settings = Settings.builder().put(NODE_CANCELLATION_THRESHOLD_SETTING_NAME, 0.80).build(); ClusterSettings cs = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - QuerySandboxServiceSettings querySandboxServiceSettings = new QuerySandboxServiceSettings(settings, cs); + ResourceLimitGroupServiceSettings resourceLimitGroupServiceSettings = new ResourceLimitGroupServiceSettings(settings, cs); assertThrows(IllegalArgumentException.class, - () -> querySandboxServiceSettings.setNodeLevelJvmRejectionThreshold(0.96)); + () -> resourceLimitGroupServiceSettings.setNodeLevelJvmRejectionThreshold(0.96)); } /** @@ -117,13 +119,13 @@ public void testInValidNodeLevelCancellationThresholdCase1() { * When the value is set less than {@code query_sandbox.node.rejection_threshold} */ public void testInValidNodeLevelCancellationThresholdCase2() { - Settings settings = Settings.builder().put(QUERY_SANDBOX_NODE_REJECTION_THRESHOLD_SETTING_NAME, 0.70) - .put(QUERY_SANDBOX_NODE_CANCELLATION_THRESHOLD_SETTING_NAME, 0.80) + Settings settings = Settings.builder().put(NODE_REJECTION_THRESHOLD_SETTING_NAME, 0.70) + .put(NODE_CANCELLATION_THRESHOLD_SETTING_NAME, 0.80) .build(); ClusterSettings cs = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - QuerySandboxServiceSettings querySandboxServiceSettings = new QuerySandboxServiceSettings(settings, cs); + ResourceLimitGroupServiceSettings resourceLimitGroupServiceSettings = new ResourceLimitGroupServiceSettings(settings, cs); assertThrows(IllegalArgumentException.class, - () -> querySandboxServiceSettings.setNodeLevelJvmRejectionThreshold(0.85)); + () -> resourceLimitGroupServiceSettings.setNodeLevelJvmRejectionThreshold(0.85)); } } From eba0060073fbf15789744e3071388636b99e7f93 Mon Sep 17 00:00:00 2001 From: Kaushal Kumar Date: Mon, 15 Apr 2024 12:30:09 -0700 Subject: [PATCH 03/10] add sandbox schema Signed-off-by: Kaushal Kumar --- .../opensearch/cluster/metadata/Metadata.java | 39 ++++ .../cluster/metadata/ResourceLimitGroup.java | 211 ++++++++++++++++++ .../metadata/ResourceLimitGroupMetadata.java | 200 +++++++++++++++++ 3 files changed, 450 insertions(+) create mode 100644 server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java create mode 100644 server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadata.java 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 59dc86ea28ed6..f6bb5f4c4a814 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java @@ -835,6 +835,15 @@ public Map views() { return Optional.ofNullable((ViewMetadata) this.custom(ViewMetadata.TYPE)).map(ViewMetadata::views).orElse(Collections.emptyMap()); } + public Map resourceLimitGroups() { + return + Optional.ofNullable( + (ResourceLimitGroupMetadata) this.custom(ResourceLimitGroupMetadata.TYPE)). + map(ResourceLimitGroupMetadata::resourceLimitGroups) + .orElse(Collections.emptyMap()); + + } + public DecommissionAttributeMetadata decommissionAttributeMetadata() { return custom(DecommissionAttributeMetadata.TYPE); } @@ -1329,6 +1338,36 @@ public Builder removeDataStream(String name) { return this; } + public Builder resourceLimitGroups(final Map resourceLimitGroups) { + this.customs.put(ResourceLimitGroupMetadata.TYPE, new ResourceLimitGroupMetadata(resourceLimitGroups)); + return this; + } + + public ResourceLimitGroup getResourceLimitGroup(final String resourceLimitGroupName) { + return getResourceLimitGroups().get(resourceLimitGroupName); + } + + public Builder put(final ResourceLimitGroup resourceLimitGroup) { + Objects.requireNonNull(resourceLimitGroup, "resourceLimitGroup should not be null"); + Map existing = new HashMap<>(getResourceLimitGroups()); + existing.put(resourceLimitGroup.getName(), resourceLimitGroup); + return resourceLimitGroups(existing); + } + + public Builder removeResourceLimitGroup(final String resourceLimitGroupName) { + Objects.requireNonNull(resourceLimitGroupName, "resourceLimitGroup should not be null"); + Map existing = new HashMap<>(getResourceLimitGroups()); + existing.remove(resourceLimitGroupName); + return resourceLimitGroups(existing); + } + + private Map getResourceLimitGroups() { + return Optional.ofNullable(this.customs.get(ResourceLimitGroupMetadata.TYPE)) + .map(o -> (ResourceLimitGroupMetadata) o) + .map(ResourceLimitGroupMetadata::resourceLimitGroups) + .orElse(Collections.emptyMap()); + } + private Map getViews() { return Optional.ofNullable(customs.get(ViewMetadata.TYPE)) .map(o -> (ViewMetadata) o) diff --git a/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java b/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java new file mode 100644 index 0000000000000..84bdfccd570d7 --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java @@ -0,0 +1,211 @@ +/* + * 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.cluster.metadata; + +import org.opensearch.cluster.AbstractDiffable; +import org.opensearch.cluster.Diff; +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.core.ParseField; +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 org.opensearch.core.xcontent.ConstructingObjectParser; +import org.opensearch.core.xcontent.ToXContentObject; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.core.xcontent.XContentParser; + +import java.io.IOException; +import java.util.List; +import java.util.Objects; + +/** + * Class to define the ResourceLimitGroup schema + * { + * "name": "analytics", + * "resourceLimits": [ + * { + * "resourceName": "jvm", + * "value": 0.4 + * } + * ] + * } + */ +@ExperimentalApi +public class ResourceLimitGroup extends AbstractDiffable implements ToXContentObject { + + private final String name; + private final List resourceLimits; + + private static final List ALLOWED_RESOURCES = List.of("jvm"); + + public static final ParseField NAME_FIELD = new ParseField("name"); + public static final ParseField RESOURCE_LIMITS_FIELD = new ParseField("resourceLimits"); + + + @SuppressWarnings("unchecked") + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( + "ResourceLimitGroupParser", + args -> new ResourceLimitGroup((String) args[0], (List) args[1]) + ); + + static { + PARSER.declareString(ConstructingObjectParser.constructorArg(), NAME_FIELD); + PARSER.declareObjectArray(ConstructingObjectParser.constructorArg(), (p, c) -> ResourceLimit.fromXContent(p), RESOURCE_LIMITS_FIELD); + } + + public ResourceLimitGroup(String name, List resourceLimits) { + this.name = name; + this.resourceLimits = resourceLimits; + } + + public ResourceLimitGroup(StreamInput in) throws IOException { + this(in.readString(), in.readList(ResourceLimit::new)); + } + + /** + * Class to hold the system resource limits; + * sample Schema + * + */ + @ExperimentalApi + public static class ResourceLimit implements Writeable, ToXContentObject { + private final String resourceName; + private final Double value; + + static final ParseField RESOURCE_NAME_FIELD = new ParseField("resourceName"); + static final ParseField RESOURCE_VALUE_FIELD = new ParseField("value"); + + public static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( + "ResourceLimitParser", + args -> new ResourceLimit((String) args[0], (Double) args[1]) + ); + + static { + PARSER.declareString(ConstructingObjectParser.constructorArg(), RESOURCE_NAME_FIELD); + PARSER.declareDouble(ConstructingObjectParser.constructorArg(), RESOURCE_VALUE_FIELD); + } + + public ResourceLimit(String resourceName, Double value) { + Objects.requireNonNull(resourceName, "resourceName can't be null"); + Objects.requireNonNull(value, "resource value can't be null"); + + if (Double.compare(value, 1.0) > 0) { + throw new IllegalArgumentException("resource value should be less than 1.0"); + } + + if (!ALLOWED_RESOURCES.contains(resourceName.toLowerCase())) { + throw new IllegalArgumentException("resource has to be valid, valid resources " + + ALLOWED_RESOURCES.stream().reduce((x, e) -> x + ", " + e).get()); + } + this.resourceName = resourceName; + this.value = value; + } + + public ResourceLimit(StreamInput in) throws IOException { + this(in.readString(), in.readDouble()); + } + + /** + * Write this into the {@linkplain StreamOutput}. + * + * @param out + */ + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeString(resourceName); + out.writeDouble(value); + } + + /** + * @param builder + * @param params + * @return + * @throws IOException + */ + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(); + builder.field(RESOURCE_NAME_FIELD.getPreferredName(), resourceName); + builder.field(RESOURCE_VALUE_FIELD.getPreferredName(), value); + builder.endObject(); + return builder; + } + + public static ResourceLimit fromXContent(final XContentParser parser) throws IOException { + return PARSER.parse(parser, null); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ResourceLimit that = (ResourceLimit) o; + return Objects.equals(resourceName, that.resourceName) && Objects.equals(value, that.value); + } + + @Override + public int hashCode() { + return Objects.hash(resourceName, value); + } + } + + + /** + * Write this into the {@linkplain StreamOutput}. + * + * @param out + */ + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeString(name); + out.writeList(resourceLimits); + } + + /** + * @param builder + * @param params + * @return + * @throws IOException + */ + @Override + public XContentBuilder toXContent(final XContentBuilder builder, final Params params) throws IOException { + builder.startObject(); + builder.field(NAME_FIELD.getPreferredName(),name); + builder.field(RESOURCE_LIMITS_FIELD.getPreferredName(), resourceLimits); + builder.endObject(); + return builder; + } + + + public static ResourceLimitGroup fromXContent(final XContentParser parser) throws IOException { + return PARSER.parse(parser, null); + } + + + public static Diff readDiff(final StreamInput in) throws IOException { + return readDiffFrom(ResourceLimitGroup::new, in); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ResourceLimitGroup that = (ResourceLimitGroup) o; + return Objects.equals(name, that.name) && Objects.equals(resourceLimits, that.resourceLimits); + } + + @Override + public int hashCode() { + return Objects.hash(name, resourceLimits); + } + + public String getName() { + return name; + } +} diff --git a/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadata.java new file mode 100644 index 0000000000000..144b4c9e06227 --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadata.java @@ -0,0 +1,200 @@ +/* + * 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.cluster.metadata; + +import org.opensearch.Version; +import org.opensearch.cluster.Diff; +import org.opensearch.cluster.DiffableUtils; +import org.opensearch.cluster.NamedDiff; +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.core.ParseField; +import org.opensearch.core.common.Strings; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.core.xcontent.ConstructingObjectParser; +import org.opensearch.core.xcontent.MediaTypeRegistry; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.core.xcontent.XContentParser; + +import java.io.IOException; +import java.util.EnumSet; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; + +import static org.opensearch.cluster.metadata.Metadata.ALL_CONTEXTS; + +@ExperimentalApi +public class ResourceLimitGroupMetadata implements Metadata.Custom { + public static final String TYPE = "resourceLimitGroup"; + private static final ParseField RESOURCE_LIMIT_GROUP_FIELD = new ParseField("resourceLimitGroups"); + + @SuppressWarnings("unchecked") + static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( + "resourceLimitGroupParser", + args -> new ResourceLimitGroupMetadata((Map) args [0]) + ); + + static { + PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> { + Map resourceLimitGroupMap = new HashMap<>(); + while (p.nextToken() != XContentParser.Token.END_OBJECT) { + resourceLimitGroupMap.put(p.currentName(), ResourceLimitGroup.fromXContent(p)); + } + return resourceLimitGroupMap; + }, RESOURCE_LIMIT_GROUP_FIELD); + } + + + private final Map resourceLimitGroups; + + public ResourceLimitGroupMetadata(Map resourceLimitGroups) { + this.resourceLimitGroups = resourceLimitGroups; + } + + + public Map resourceLimitGroups() { + return this.resourceLimitGroups; + } + + /** + * Returns the name of the writeable object + */ + @Override + public String getWriteableName() { + return TYPE; + } + + /** + * The minimal version of the recipient this object can be sent to + */ + @Override + public Version getMinimalSupportedVersion() { + return Version.V_3_0_0; + } + + /** + * Write this into the {@linkplain StreamOutput}. + * + * @param out + */ + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeMap(resourceLimitGroups, StreamOutput::writeString, (stream, val) -> val.writeTo(stream)); + } + + /** + * @param builder + * @param params + * @return + * @throws IOException + */ + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(RESOURCE_LIMIT_GROUP_FIELD.getPreferredName()); + for (Map.Entry entry: resourceLimitGroups.entrySet()) { + builder.field(entry.getKey(), entry.getValue()); + } + builder.endObject(); + return builder; + } + + public static ResourceLimitGroupMetadata fromXContent(XContentParser parser) throws IOException { + return PARSER.parse(parser, null); + } + + /** + * Returns serializable object representing differences between this and previousState + * + * @param previousState + */ + @Override + public Diff diff(final Metadata.Custom previousState) { + return new ResourceLimitGroupMetadataDiff((ResourceLimitGroupMetadata) previousState, this); + } + + /** + * @return + */ + @Override + public EnumSet context() { + return ALL_CONTEXTS; + } + + /** + * ResourceLimitGroupMetadataDiff + */ + static class ResourceLimitGroupMetadataDiff implements NamedDiff { + final Diff> dataStreanDiff; + + + ResourceLimitGroupMetadataDiff(final ResourceLimitGroupMetadata before, + final ResourceLimitGroupMetadata after) { + dataStreanDiff = DiffableUtils.diff(before.resourceLimitGroups, + after.resourceLimitGroups, + DiffableUtils.getStringKeySerializer()); + } + + ResourceLimitGroupMetadataDiff(final StreamInput in) throws IOException { + this.dataStreanDiff = DiffableUtils.readJdkMapDiff(in, + DiffableUtils.getStringKeySerializer(), + ResourceLimitGroup::new, + ResourceLimitGroup::readDiff); + } + + /** + * Returns the name of the writeable object + */ + @Override + public String getWriteableName() { + return TYPE; + } + + /** + * Write this into the {@linkplain StreamOutput}. + * + * @param out + */ + @Override + public void writeTo(StreamOutput out) throws IOException { + dataStreanDiff.writeTo(out); + } + + /** + * Applies difference to the specified part and returns the resulted part + * + * @param part + */ + @Override + public Metadata.Custom apply(Metadata.Custom part) { + return new ResourceLimitGroupMetadata(dataStreanDiff.apply( + ((ResourceLimitGroupMetadata) part).resourceLimitGroups) + ); + } + } + + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ResourceLimitGroupMetadata that = (ResourceLimitGroupMetadata) o; + return Objects.equals(resourceLimitGroups, that.resourceLimitGroups); + } + + @Override + public int hashCode() { + return Objects.hash(resourceLimitGroups); + } + + @Override + public String toString() { + return Strings.toString(MediaTypeRegistry.JSON, this); + } +} From ba237c14d3f0a7ed4c59e2e83acda8ce718e4ce0 Mon Sep 17 00:00:00 2001 From: Kaushal Kumar Date: Mon, 15 Apr 2024 21:02:46 -0700 Subject: [PATCH 04/10] add resourceLimitGroupTests Signed-off-by: Kaushal Kumar --- .../cluster/metadata/ResourceLimitGroup.java | 18 ++- .../common/settings/ClusterSettings.java | 2 +- .../metadata/ResourceLimitGroupTests.java | 103 ++++++++++++++++++ ...esourceLimitGroupServiceSettingsTests.java | 7 +- 4 files changed, 125 insertions(+), 5 deletions(-) create mode 100644 server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupTests.java diff --git a/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java b/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java index 84bdfccd570d7..197dad9ddd738 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java @@ -39,6 +39,7 @@ @ExperimentalApi public class ResourceLimitGroup extends AbstractDiffable implements ToXContentObject { + public static final int MAX_CHARS_ALLOWED_IN_NAME = 50; private final String name; private final List resourceLimits; @@ -59,7 +60,18 @@ public class ResourceLimitGroup extends AbstractDiffable imp PARSER.declareObjectArray(ConstructingObjectParser.constructorArg(), (p, c) -> ResourceLimit.fromXContent(p), RESOURCE_LIMITS_FIELD); } - public ResourceLimitGroup(String name, List resourceLimits) { + public ResourceLimitGroup(final String name, final List resourceLimits) { + Objects.requireNonNull(name, "ResourceLimitGroup.name can't be null"); + Objects.requireNonNull(resourceLimits, "ResourceLimitGroup.resourceLimits can't be null"); + + if (name.length() > MAX_CHARS_ALLOWED_IN_NAME) { + throw new IllegalArgumentException("ResourceLimitGroup.name shouldn't be more than 50 chars long"); + } + + if (resourceLimits.isEmpty()) { + throw new IllegalArgumentException("ResourceLimitGroup.resourceLimits should at least have 1 resource limit"); + } + this.name = name; this.resourceLimits = resourceLimits; } @@ -208,4 +220,8 @@ public int hashCode() { public String getName() { return name; } + + public List getResourceLimits() { + return resourceLimits; + } } 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 b0e997a0a606d..2406696ecaa0d 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -154,7 +154,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.sandbox.ResourceLimitGroupServiceSettings; +import org.opensearch.search.resource_limit_group.ResourceLimitGroupServiceSettings; import org.opensearch.snapshots.InternalSnapshotsInfoService; import org.opensearch.snapshots.SnapshotsService; import org.opensearch.tasks.TaskCancellationMonitoringSettings; diff --git a/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupTests.java b/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupTests.java new file mode 100644 index 0000000000000..7d24bf1d48048 --- /dev/null +++ b/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupTests.java @@ -0,0 +1,103 @@ +/* + * 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.cluster.metadata; + +import org.opensearch.core.common.io.stream.Writeable; +import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.test.AbstractSerializingTestCase; + +import java.io.IOException; +import java.util.List; + +public class ResourceLimitGroupTests extends AbstractSerializingTestCase { + + + private ResourceLimitGroup createRandomResourceLimitGroup() { + String name = randomAlphaOfLength(10); + ResourceLimitGroup.ResourceLimit resourceLimit = new ResourceLimitGroup.ResourceLimit("jvm", + randomDoubleBetween(0.0, 0.80, false)); + return new ResourceLimitGroup(name, List.of(resourceLimit)); + } + + /** + * Parses to a new instance using the provided {@link XContentParser} + * + * @param parser + */ + @Override + protected ResourceLimitGroup doParseInstance(XContentParser parser) throws IOException { + return ResourceLimitGroup.fromXContent(parser); + } + + /** + * Returns a {@link Writeable.Reader} that can be used to de-serialize the instance + */ + @Override + protected Writeable.Reader instanceReader() { + return ResourceLimitGroup::new; + } + + /** + * Creates a random test instance to use in the tests. This method will be + * called multiple times during test execution and should return a different + * random instance each time it is called. + */ + @Override + protected ResourceLimitGroup createTestInstance() { + return createRandomResourceLimitGroup(); + } + + public void testNullName() { + assertThrows(NullPointerException.class, + () -> new ResourceLimitGroup(null, List.of())); + } + + public void testNullResourceLimits() { + assertThrows(NullPointerException.class, + () -> new ResourceLimitGroup("analytics", null)); + } + + public void testEmptyResourceLimits() { + assertThrows(IllegalArgumentException.class, + () -> new ResourceLimitGroup("analytics", List.of())); + } + + public void testInvalidResourceLimitWhenInvalidSystemResourceNameIsGiven() { + assertThrows(IllegalArgumentException.class, () -> new ResourceLimitGroup( + "analytics", + List.of( + new ResourceLimitGroup.ResourceLimit("RequestRate", randomDoubleBetween(0.01, 0.8, false)) + ) + )); + } + + public void testInvalidResourceLimitWhenInvalidSystemResourceValueIsGiven() { + assertThrows(IllegalArgumentException.class, () -> new ResourceLimitGroup( + "analytics", + List.of( + new ResourceLimitGroup.ResourceLimit("RequestRate", randomDoubleBetween(1.1, 1.8, false)) + ) + )); + } + + public void testValidResourceLimitGroup() { + ResourceLimitGroup resourceLimitGroup = new ResourceLimitGroup( + "analytics", + List.of( + new ResourceLimitGroup.ResourceLimit("jvm", randomDoubleBetween(0.01, 0.8, false)) + ) + ); + + assertNotNull(resourceLimitGroup.getName()); + assertEquals("analytics", resourceLimitGroup.getName()); + assertNotNull(resourceLimitGroup.getResourceLimits()); + assertFalse(resourceLimitGroup.getResourceLimits().isEmpty()); + assertEquals(1, resourceLimitGroup.getResourceLimits().size()); + } +} diff --git a/server/src/test/java/org/opensearch/search/sandbox/ResourceLimitGroupServiceSettingsTests.java b/server/src/test/java/org/opensearch/search/sandbox/ResourceLimitGroupServiceSettingsTests.java index 0e500434ea488..3bf8d22a47aa8 100644 --- a/server/src/test/java/org/opensearch/search/sandbox/ResourceLimitGroupServiceSettingsTests.java +++ b/server/src/test/java/org/opensearch/search/sandbox/ResourceLimitGroupServiceSettingsTests.java @@ -10,11 +10,12 @@ import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; +import org.opensearch.search.resource_limit_group.ResourceLimitGroupServiceSettings; import org.opensearch.test.OpenSearchTestCase; -import static org.opensearch.search.sandbox.ResourceLimitGroupServiceSettings.NODE_CANCELLATION_THRESHOLD_SETTING_NAME; -import static org.opensearch.search.sandbox.ResourceLimitGroupServiceSettings.NODE_REJECTION_THRESHOLD_SETTING_NAME; -import static org.opensearch.search.sandbox.ResourceLimitGroupServiceSettings.RESOURCE_LIMIT_GROUP_COUNT_SETTING_NAME; +import static org.opensearch.search.resource_limit_group.ResourceLimitGroupServiceSettings.NODE_CANCELLATION_THRESHOLD_SETTING_NAME; +import static org.opensearch.search.resource_limit_group.ResourceLimitGroupServiceSettings.NODE_REJECTION_THRESHOLD_SETTING_NAME; +import static org.opensearch.search.resource_limit_group.ResourceLimitGroupServiceSettings.RESOURCE_LIMIT_GROUP_COUNT_SETTING_NAME; public class ResourceLimitGroupServiceSettingsTests extends OpenSearchTestCase { From 62f501d0e816691009cdbe0444c90561978bf3ba Mon Sep 17 00:00:00 2001 From: Kaushal Kumar Date: Thu, 18 Apr 2024 10:43:28 -0700 Subject: [PATCH 05/10] add resourceLimitGroupMetadata tests Signed-off-by: Kaushal Kumar --- .../metadata/ResourceLimitGroupMetadata.java | 16 ++++++ .../ResourceLimitGroupMetadataTests.java | 50 +++++++++++++++++++ .../metadata/ResourceLimitGroupTests.java | 2 +- 3 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadataTests.java diff --git a/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadata.java index 144b4c9e06227..ab40f874fec32 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadata.java @@ -30,6 +30,18 @@ import static org.opensearch.cluster.metadata.Metadata.ALL_CONTEXTS; +/** + * This class holds the resourceLimitGroupMetadata + * sample schema + * { + * "resourceLimitGroups": { + * "name": { + * {@link ResourceLimitGroup} + * }, + * ... + * } + * } + */ @ExperimentalApi public class ResourceLimitGroupMetadata implements Metadata.Custom { public static final String TYPE = "resourceLimitGroup"; @@ -58,6 +70,10 @@ public ResourceLimitGroupMetadata(Map resourceLimitG this.resourceLimitGroups = resourceLimitGroups; } + public ResourceLimitGroupMetadata(StreamInput in) throws IOException { + this.resourceLimitGroups = in.readMap(StreamInput::readString, ResourceLimitGroup::new); + } + public Map resourceLimitGroups() { return this.resourceLimitGroups; diff --git a/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadataTests.java b/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadataTests.java new file mode 100644 index 0000000000000..43a723d27ef32 --- /dev/null +++ b/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadataTests.java @@ -0,0 +1,50 @@ +/* + * 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.cluster.metadata; + + +import org.opensearch.core.common.io.stream.NamedWriteableRegistry; +import org.opensearch.test.AbstractNamedWriteableTestCase; + +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +import static org.opensearch.cluster.metadata.ResourceLimitGroupTests.createRandomResourceLimitGroup; + +public class ResourceLimitGroupMetadataTests extends AbstractNamedWriteableTestCase { + + @Override + protected NamedWriteableRegistry getNamedWriteableRegistry() { + return new NamedWriteableRegistry( + Collections.singletonList( + new NamedWriteableRegistry.Entry( + ResourceLimitGroupMetadata.class, ResourceLimitGroupMetadata.TYPE, ResourceLimitGroupMetadata::new)) + ); + } + + + @Override + protected Class categoryClass() { + return ResourceLimitGroupMetadata.class; + } + + + @Override + protected ResourceLimitGroupMetadata createTestInstance() { + Map resourceLimitGroupMap = getRandomResourceLimitGroups() + .stream().collect(Collectors.toMap(ResourceLimitGroup::getName, resourceLimitGroup -> resourceLimitGroup)); + return new ResourceLimitGroupMetadata(resourceLimitGroupMap); + } + + private List getRandomResourceLimitGroups() { + return List.of(createRandomResourceLimitGroup(), createRandomResourceLimitGroup()); + } +} diff --git a/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupTests.java b/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupTests.java index 7d24bf1d48048..105a0aa5daf1e 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupTests.java @@ -18,7 +18,7 @@ public class ResourceLimitGroupTests extends AbstractSerializingTestCase { - private ResourceLimitGroup createRandomResourceLimitGroup() { + static ResourceLimitGroup createRandomResourceLimitGroup() { String name = randomAlphaOfLength(10); ResourceLimitGroup.ResourceLimit resourceLimit = new ResourceLimitGroup.ResourceLimit("jvm", randomDoubleBetween(0.0, 0.80, false)); From d886fe9ff8e9a57f92dab424b9f2fd51fc8a2112 Mon Sep 17 00:00:00 2001 From: Kaushal Kumar Date: Thu, 18 Apr 2024 10:48:09 -0700 Subject: [PATCH 06/10] run spotlessApply Signed-off-by: Kaushal Kumar --- .../action/search/SearchRequest.java | 3 +- .../opensearch/cluster/metadata/Metadata.java | 8 ++-- .../cluster/metadata/ResourceLimitGroup.java | 17 ++++---- .../metadata/ResourceLimitGroupMetadata.java | 29 +++++++------- .../ResourceLimitGroupService.java | 8 ++-- .../ResourceLimitGroupServiceSettings.java | 14 +++---- .../module/ResourceLimitGroupModule.java | 2 +- ...imitsGroupResourceUsageTrackerService.java | 7 ++-- .../ResourceLimitGroupMetadataTests.java | 13 ++++--- .../metadata/ResourceLimitGroupTests.java | 39 ++++++++----------- ...esourceLimitGroupServiceSettingsTests.java | 29 ++++++-------- 11 files changed, 77 insertions(+), 92 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/search/SearchRequest.java b/server/src/main/java/org/opensearch/action/search/SearchRequest.java index 2b6c64883a9fb..24a349c3964a9 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchRequest.java +++ b/server/src/main/java/org/opensearch/action/search/SearchRequest.java @@ -840,6 +840,7 @@ public String toString() { + ", phaseTook=" + phaseTook + ", resourceLimitGroupId=" - + resourceLimitGroupId + "}"; + + resourceLimitGroupId + + "}"; } } 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 f6bb5f4c4a814..7b08c2b0f8cc1 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java @@ -836,11 +836,9 @@ public Map views() { } public Map resourceLimitGroups() { - return - Optional.ofNullable( - (ResourceLimitGroupMetadata) this.custom(ResourceLimitGroupMetadata.TYPE)). - map(ResourceLimitGroupMetadata::resourceLimitGroups) - .orElse(Collections.emptyMap()); + return Optional.ofNullable((ResourceLimitGroupMetadata) this.custom(ResourceLimitGroupMetadata.TYPE)) + .map(ResourceLimitGroupMetadata::resourceLimitGroups) + .orElse(Collections.emptyMap()); } diff --git a/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java b/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java index 197dad9ddd738..5d4284fab3390 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java @@ -48,7 +48,6 @@ public class ResourceLimitGroup extends AbstractDiffable imp public static final ParseField NAME_FIELD = new ParseField("name"); public static final ParseField RESOURCE_LIMITS_FIELD = new ParseField("resourceLimits"); - @SuppressWarnings("unchecked") private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( "ResourceLimitGroupParser", @@ -57,7 +56,11 @@ public class ResourceLimitGroup extends AbstractDiffable imp static { PARSER.declareString(ConstructingObjectParser.constructorArg(), NAME_FIELD); - PARSER.declareObjectArray(ConstructingObjectParser.constructorArg(), (p, c) -> ResourceLimit.fromXContent(p), RESOURCE_LIMITS_FIELD); + PARSER.declareObjectArray( + ConstructingObjectParser.constructorArg(), + (p, c) -> ResourceLimit.fromXContent(p), + RESOURCE_LIMITS_FIELD + ); } public ResourceLimitGroup(final String name, final List resourceLimits) { @@ -112,8 +115,9 @@ public ResourceLimit(String resourceName, Double value) { } if (!ALLOWED_RESOURCES.contains(resourceName.toLowerCase())) { - throw new IllegalArgumentException("resource has to be valid, valid resources " - + ALLOWED_RESOURCES.stream().reduce((x, e) -> x + ", " + e).get()); + throw new IllegalArgumentException( + "resource has to be valid, valid resources " + ALLOWED_RESOURCES.stream().reduce((x, e) -> x + ", " + e).get() + ); } this.resourceName = resourceName; this.value = value; @@ -167,7 +171,6 @@ public int hashCode() { } } - /** * Write this into the {@linkplain StreamOutput}. * @@ -188,18 +191,16 @@ public void writeTo(StreamOutput out) throws IOException { @Override public XContentBuilder toXContent(final XContentBuilder builder, final Params params) throws IOException { builder.startObject(); - builder.field(NAME_FIELD.getPreferredName(),name); + builder.field(NAME_FIELD.getPreferredName(), name); builder.field(RESOURCE_LIMITS_FIELD.getPreferredName(), resourceLimits); builder.endObject(); return builder; } - public static ResourceLimitGroup fromXContent(final XContentParser parser) throws IOException { return PARSER.parse(parser, null); } - public static Diff readDiff(final StreamInput in) throws IOException { return readDiffFrom(ResourceLimitGroup::new, in); } diff --git a/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadata.java index ab40f874fec32..4f2fb0981a549 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadata.java @@ -50,7 +50,7 @@ public class ResourceLimitGroupMetadata implements Metadata.Custom { @SuppressWarnings("unchecked") static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( "resourceLimitGroupParser", - args -> new ResourceLimitGroupMetadata((Map) args [0]) + args -> new ResourceLimitGroupMetadata((Map) args[0]) ); static { @@ -63,7 +63,6 @@ public class ResourceLimitGroupMetadata implements Metadata.Custom { }, RESOURCE_LIMIT_GROUP_FIELD); } - private final Map resourceLimitGroups; public ResourceLimitGroupMetadata(Map resourceLimitGroups) { @@ -74,7 +73,6 @@ public ResourceLimitGroupMetadata(StreamInput in) throws IOException { this.resourceLimitGroups = in.readMap(StreamInput::readString, ResourceLimitGroup::new); } - public Map resourceLimitGroups() { return this.resourceLimitGroups; } @@ -114,7 +112,7 @@ public void writeTo(StreamOutput out) throws IOException { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(RESOURCE_LIMIT_GROUP_FIELD.getPreferredName()); - for (Map.Entry entry: resourceLimitGroups.entrySet()) { + for (Map.Entry entry : resourceLimitGroups.entrySet()) { builder.field(entry.getKey(), entry.getValue()); } builder.endObject(); @@ -149,19 +147,21 @@ public EnumSet context() { static class ResourceLimitGroupMetadataDiff implements NamedDiff { final Diff> dataStreanDiff; - - ResourceLimitGroupMetadataDiff(final ResourceLimitGroupMetadata before, - final ResourceLimitGroupMetadata after) { - dataStreanDiff = DiffableUtils.diff(before.resourceLimitGroups, - after.resourceLimitGroups, - DiffableUtils.getStringKeySerializer()); + ResourceLimitGroupMetadataDiff(final ResourceLimitGroupMetadata before, final ResourceLimitGroupMetadata after) { + dataStreanDiff = DiffableUtils.diff( + before.resourceLimitGroups, + after.resourceLimitGroups, + DiffableUtils.getStringKeySerializer() + ); } ResourceLimitGroupMetadataDiff(final StreamInput in) throws IOException { - this.dataStreanDiff = DiffableUtils.readJdkMapDiff(in, + this.dataStreanDiff = DiffableUtils.readJdkMapDiff( + in, DiffableUtils.getStringKeySerializer(), ResourceLimitGroup::new, - ResourceLimitGroup::readDiff); + ResourceLimitGroup::readDiff + ); } /** @@ -189,13 +189,10 @@ public void writeTo(StreamOutput out) throws IOException { */ @Override public Metadata.Custom apply(Metadata.Custom part) { - return new ResourceLimitGroupMetadata(dataStreanDiff.apply( - ((ResourceLimitGroupMetadata) part).resourceLimitGroups) - ); + return new ResourceLimitGroupMetadata(dataStreanDiff.apply(((ResourceLimitGroupMetadata) part).resourceLimitGroups)); } } - @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupService.java b/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupService.java index 02954aa140f06..1dc3dba1c1a0d 100644 --- a/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupService.java +++ b/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupService.java @@ -12,8 +12,8 @@ import org.apache.logging.log4j.Logger; import org.opensearch.common.inject.Inject; import org.opensearch.common.lifecycle.AbstractLifecycleComponent; -import org.opensearch.search.resource_limit_group.tracker.ResourceLimitGroupResourceUsageTracker; import org.opensearch.search.resource_limit_group.cancellation.ResourceLimitGroupRequestCanceller; +import org.opensearch.search.resource_limit_group.tracker.ResourceLimitGroupResourceUsageTracker; import org.opensearch.threadpool.Scheduler; import org.opensearch.threadpool.ThreadPool; @@ -59,9 +59,9 @@ public ResourceLimitGroupService( * run at regular interval */ private void doRun() { - requestTracker.updateResourceLimitGroupsResourceUsage(); - requestCanceller.cancelViolatingTasks(); - resourceLimitGroupPruner.pruneResourceLimitGroup(); + requestTracker.updateResourceLimitGroupsResourceUsage(); + requestCanceller.cancelViolatingTasks(); + resourceLimitGroupPruner.pruneResourceLimitGroup(); } /** diff --git a/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupServiceSettings.java b/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupServiceSettings.java index 56bdfbae87168..9f792390fd8ff 100644 --- a/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupServiceSettings.java +++ b/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupServiceSettings.java @@ -41,8 +41,9 @@ public class ResourceLimitGroupServiceSettings { DEFAULT_MAX_RESOURCE_LIMIT_GROUP_COUNT_VALUE, 0, (newVal) -> { - if (newVal > 100 || newVal < 1) - throw new IllegalArgumentException(RESOURCE_LIMIT_GROUP_COUNT_SETTING_NAME + " should be in range [1-100]"); + if (newVal > 100 || newVal < 1) throw new IllegalArgumentException( + RESOURCE_LIMIT_GROUP_COUNT_SETTING_NAME + " should be in range [1-100]" + ); }, Setting.Property.Dynamic, Setting.Property.NodeScope @@ -142,8 +143,7 @@ public Double getNodeLevelJvmCancellationThreshold() { public void setNodeLevelJvmCancellationThreshold(Double nodeLevelJvmCancellationThreshold) { if (Double.compare(nodeLevelJvmCancellationThreshold, 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" + NODE_CANCELLATION_THRESHOLD_SETTING_NAME + " value should not be greater than 0.95 as it pose a threat of node drop" ); } @@ -181,11 +181,9 @@ private void ensureRejectionThresholdIsLessThanCancellation( Double nodeLevelJvmRejectionThreshold, Double nodeLevelJvmCancellationThreshold ) { - if (Double.compare(nodeLevelJvmCancellationThreshold , nodeLevelJvmRejectionThreshold) < 0) { + if (Double.compare(nodeLevelJvmCancellationThreshold, nodeLevelJvmRejectionThreshold) < 0) { throw new IllegalArgumentException( - NODE_CANCELLATION_THRESHOLD_SETTING_NAME - + " value should not be less than " - + NODE_REJECTION_THRESHOLD_SETTING_NAME + NODE_CANCELLATION_THRESHOLD_SETTING_NAME + " value should not be less than " + NODE_REJECTION_THRESHOLD_SETTING_NAME ); } } diff --git a/server/src/main/java/org/opensearch/search/resource_limit_group/module/ResourceLimitGroupModule.java b/server/src/main/java/org/opensearch/search/resource_limit_group/module/ResourceLimitGroupModule.java index 9700612a71edc..fb2d607e4f774 100644 --- a/server/src/main/java/org/opensearch/search/resource_limit_group/module/ResourceLimitGroupModule.java +++ b/server/src/main/java/org/opensearch/search/resource_limit_group/module/ResourceLimitGroupModule.java @@ -10,8 +10,8 @@ import org.opensearch.common.inject.AbstractModule; import org.opensearch.search.resource_limit_group.ResourceLimitGroupPruner; -import org.opensearch.search.resource_limit_group.tracker.ResourceLimitGroupResourceUsageTracker; import org.opensearch.search.resource_limit_group.cancellation.ResourceLimitGroupRequestCanceller; +import org.opensearch.search.resource_limit_group.tracker.ResourceLimitGroupResourceUsageTracker; import org.opensearch.search.resource_limit_group.tracker.ResourceLimitsGroupResourceUsageTrackerService; /** diff --git a/server/src/main/java/org/opensearch/search/resource_limit_group/tracker/ResourceLimitsGroupResourceUsageTrackerService.java b/server/src/main/java/org/opensearch/search/resource_limit_group/tracker/ResourceLimitsGroupResourceUsageTrackerService.java index cb2994b85b829..4f11b6e2a678f 100644 --- a/server/src/main/java/org/opensearch/search/resource_limit_group/tracker/ResourceLimitsGroupResourceUsageTrackerService.java +++ b/server/src/main/java/org/opensearch/search/resource_limit_group/tracker/ResourceLimitsGroupResourceUsageTrackerService.java @@ -29,9 +29,9 @@ public class ResourceLimitsGroupResourceUsageTrackerService implements TaskManager.TaskEventListeners, - ResourceLimitGroupResourceUsageTracker, - ResourceLimitGroupRequestCanceller, - ResourceLimitGroupPruner { + ResourceLimitGroupResourceUsageTracker, + ResourceLimitGroupRequestCanceller, + ResourceLimitGroupPruner { private static final String CPU = "CPU"; private static final String JVM_ALLOCATIONS = "JVM_Allocations"; @@ -162,7 +162,6 @@ private List getCancellableTasks() { // get cancellations from soft type sandboxes if the node is in duress (hitting node level cancellation // threshold) - return cancellableTasks; } diff --git a/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadataTests.java b/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadataTests.java index 43a723d27ef32..77ad49255183f 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadataTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadataTests.java @@ -8,7 +8,6 @@ package org.opensearch.cluster.metadata; - import org.opensearch.core.common.io.stream.NamedWriteableRegistry; import org.opensearch.test.AbstractNamedWriteableTestCase; @@ -26,21 +25,23 @@ protected NamedWriteableRegistry getNamedWriteableRegistry() { return new NamedWriteableRegistry( Collections.singletonList( new NamedWriteableRegistry.Entry( - ResourceLimitGroupMetadata.class, ResourceLimitGroupMetadata.TYPE, ResourceLimitGroupMetadata::new)) + ResourceLimitGroupMetadata.class, + ResourceLimitGroupMetadata.TYPE, + ResourceLimitGroupMetadata::new + ) + ) ); } - @Override protected Class categoryClass() { return ResourceLimitGroupMetadata.class; } - @Override protected ResourceLimitGroupMetadata createTestInstance() { - Map resourceLimitGroupMap = getRandomResourceLimitGroups() - .stream().collect(Collectors.toMap(ResourceLimitGroup::getName, resourceLimitGroup -> resourceLimitGroup)); + Map resourceLimitGroupMap = getRandomResourceLimitGroups().stream() + .collect(Collectors.toMap(ResourceLimitGroup::getName, resourceLimitGroup -> resourceLimitGroup)); return new ResourceLimitGroupMetadata(resourceLimitGroupMap); } diff --git a/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupTests.java b/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupTests.java index 105a0aa5daf1e..0c429c2eb0972 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupTests.java @@ -17,11 +17,9 @@ public class ResourceLimitGroupTests extends AbstractSerializingTestCase { - static ResourceLimitGroup createRandomResourceLimitGroup() { String name = randomAlphaOfLength(10); - ResourceLimitGroup.ResourceLimit resourceLimit = new ResourceLimitGroup.ResourceLimit("jvm", - randomDoubleBetween(0.0, 0.80, false)); + ResourceLimitGroup.ResourceLimit resourceLimit = new ResourceLimitGroup.ResourceLimit("jvm", randomDoubleBetween(0.0, 0.80, false)); return new ResourceLimitGroup(name, List.of(resourceLimit)); } @@ -54,44 +52,41 @@ protected ResourceLimitGroup createTestInstance() { } public void testNullName() { - assertThrows(NullPointerException.class, - () -> new ResourceLimitGroup(null, List.of())); + assertThrows(NullPointerException.class, () -> new ResourceLimitGroup(null, List.of())); } public void testNullResourceLimits() { - assertThrows(NullPointerException.class, - () -> new ResourceLimitGroup("analytics", null)); + assertThrows(NullPointerException.class, () -> new ResourceLimitGroup("analytics", null)); } public void testEmptyResourceLimits() { - assertThrows(IllegalArgumentException.class, - () -> new ResourceLimitGroup("analytics", List.of())); + assertThrows(IllegalArgumentException.class, () -> new ResourceLimitGroup("analytics", List.of())); } public void testInvalidResourceLimitWhenInvalidSystemResourceNameIsGiven() { - assertThrows(IllegalArgumentException.class, () -> new ResourceLimitGroup( - "analytics", - List.of( - new ResourceLimitGroup.ResourceLimit("RequestRate", randomDoubleBetween(0.01, 0.8, false)) + assertThrows( + IllegalArgumentException.class, + () -> new ResourceLimitGroup( + "analytics", + List.of(new ResourceLimitGroup.ResourceLimit("RequestRate", randomDoubleBetween(0.01, 0.8, false))) ) - )); + ); } public void testInvalidResourceLimitWhenInvalidSystemResourceValueIsGiven() { - assertThrows(IllegalArgumentException.class, () -> new ResourceLimitGroup( - "analytics", - List.of( - new ResourceLimitGroup.ResourceLimit("RequestRate", randomDoubleBetween(1.1, 1.8, false)) + assertThrows( + IllegalArgumentException.class, + () -> new ResourceLimitGroup( + "analytics", + List.of(new ResourceLimitGroup.ResourceLimit("RequestRate", randomDoubleBetween(1.1, 1.8, false))) ) - )); + ); } public void testValidResourceLimitGroup() { ResourceLimitGroup resourceLimitGroup = new ResourceLimitGroup( "analytics", - List.of( - new ResourceLimitGroup.ResourceLimit("jvm", randomDoubleBetween(0.01, 0.8, false)) - ) + List.of(new ResourceLimitGroup.ResourceLimit("jvm", randomDoubleBetween(0.01, 0.8, false))) ); assertNotNull(resourceLimitGroup.getName()); diff --git a/server/src/test/java/org/opensearch/search/sandbox/ResourceLimitGroupServiceSettingsTests.java b/server/src/test/java/org/opensearch/search/sandbox/ResourceLimitGroupServiceSettingsTests.java index 3bf8d22a47aa8..fad9424f7bae6 100644 --- a/server/src/test/java/org/opensearch/search/sandbox/ResourceLimitGroupServiceSettingsTests.java +++ b/server/src/test/java/org/opensearch/search/sandbox/ResourceLimitGroupServiceSettingsTests.java @@ -29,15 +29,13 @@ public void testValidMaxSandboxCountSetting() { assertEquals(100, resourceLimitGroupServiceSettings.getMaxResourceLimitGroupCount()); } - /** * test the invalid value of {@code node.sandbox.max_count} */ public void testInValidMaxSandboxCountSetting() { Settings settings = Settings.builder().put(RESOURCE_LIMIT_GROUP_COUNT_SETTING_NAME, -100).build(); ClusterSettings cs = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - assertThrows(IllegalArgumentException.class, - () -> new ResourceLimitGroupServiceSettings(settings, cs)); + assertThrows(IllegalArgumentException.class, () -> new ResourceLimitGroupServiceSettings(settings, cs)); } /** @@ -58,8 +56,7 @@ public void testInValidNodeLevelRejectionThresholdCase1() { Settings settings = Settings.builder().put(NODE_REJECTION_THRESHOLD_SETTING_NAME, 0.80).build(); ClusterSettings cs = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); ResourceLimitGroupServiceSettings resourceLimitGroupServiceSettings = new ResourceLimitGroupServiceSettings(settings, cs); - assertThrows(IllegalArgumentException.class, - () -> resourceLimitGroupServiceSettings.setNodeLevelJvmRejectionThreshold(0.95)); + assertThrows(IllegalArgumentException.class, () -> resourceLimitGroupServiceSettings.setNodeLevelJvmRejectionThreshold(0.95)); } /** @@ -67,16 +64,15 @@ public void testInValidNodeLevelRejectionThresholdCase1() { * When the value is set more than {@code query_sandbox.node.cancellation_threshold} */ public void testInValidNodeLevelRejectionThresholdCase2() { - Settings settings = Settings.builder().put(NODE_REJECTION_THRESHOLD_SETTING_NAME, 0.70) + Settings settings = Settings.builder() + .put(NODE_REJECTION_THRESHOLD_SETTING_NAME, 0.70) .put(NODE_CANCELLATION_THRESHOLD_SETTING_NAME, 0.80) .build(); ClusterSettings cs = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); ResourceLimitGroupServiceSettings resourceLimitGroupServiceSettings = new ResourceLimitGroupServiceSettings(settings, cs); - assertThrows(IllegalArgumentException.class, - () -> resourceLimitGroupServiceSettings.setNodeLevelJvmRejectionThreshold(0.85)); + assertThrows(IllegalArgumentException.class, () -> resourceLimitGroupServiceSettings.setNodeLevelJvmRejectionThreshold(0.85)); } - /** * Tests the invalid value for {@code query_sandbox.node.rejection_threshold} * When the value is set more than {@code query_sandbox.node.cancellation_threshold} accidentally during @@ -84,13 +80,13 @@ public void testInValidNodeLevelRejectionThresholdCase2() { * invariant {@code nodeLevelRejectionThreshold < nodeLevelCancellationThreshold} */ public void testInValidInstantiationOfQuerySandboxServiceSettings() { - Settings settings = Settings.builder().put(NODE_REJECTION_THRESHOLD_SETTING_NAME, 0.80) + Settings settings = Settings.builder() + .put(NODE_REJECTION_THRESHOLD_SETTING_NAME, 0.80) .put(NODE_CANCELLATION_THRESHOLD_SETTING_NAME, 0.70) .build(); ClusterSettings cs = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - assertThrows(IllegalArgumentException.class, - () -> new ResourceLimitGroupServiceSettings(settings, cs)); + assertThrows(IllegalArgumentException.class, () -> new ResourceLimitGroupServiceSettings(settings, cs)); } /** @@ -111,8 +107,7 @@ public void testInValidNodeLevelCancellationThresholdCase1() { Settings settings = Settings.builder().put(NODE_CANCELLATION_THRESHOLD_SETTING_NAME, 0.80).build(); ClusterSettings cs = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); ResourceLimitGroupServiceSettings resourceLimitGroupServiceSettings = new ResourceLimitGroupServiceSettings(settings, cs); - assertThrows(IllegalArgumentException.class, - () -> resourceLimitGroupServiceSettings.setNodeLevelJvmRejectionThreshold(0.96)); + assertThrows(IllegalArgumentException.class, () -> resourceLimitGroupServiceSettings.setNodeLevelJvmRejectionThreshold(0.96)); } /** @@ -120,13 +115,13 @@ public void testInValidNodeLevelCancellationThresholdCase1() { * When the value is set less than {@code query_sandbox.node.rejection_threshold} */ public void testInValidNodeLevelCancellationThresholdCase2() { - Settings settings = Settings.builder().put(NODE_REJECTION_THRESHOLD_SETTING_NAME, 0.70) + Settings settings = Settings.builder() + .put(NODE_REJECTION_THRESHOLD_SETTING_NAME, 0.70) .put(NODE_CANCELLATION_THRESHOLD_SETTING_NAME, 0.80) .build(); ClusterSettings cs = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); ResourceLimitGroupServiceSettings resourceLimitGroupServiceSettings = new ResourceLimitGroupServiceSettings(settings, cs); - assertThrows(IllegalArgumentException.class, - () -> resourceLimitGroupServiceSettings.setNodeLevelJvmRejectionThreshold(0.85)); + assertThrows(IllegalArgumentException.class, () -> resourceLimitGroupServiceSettings.setNodeLevelJvmRejectionThreshold(0.85)); } } From 44e9d11109f3839af1e2c3d11b310f20f80f5441 Mon Sep 17 00:00:00 2001 From: Kaushal Kumar Date: Thu, 18 Apr 2024 12:04:31 -0700 Subject: [PATCH 07/10] add mode field in ResourceLimitGroup schema Signed-off-by: Kaushal Kumar --- .../cluster/metadata/ResourceLimitGroup.java | 47 +++++++++++++++++-- .../metadata/ResourceLimitGroupTests.java | 37 ++++++++++++--- 2 files changed, 74 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java b/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java index 5d4284fab3390..6c8f150ebbbd3 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java @@ -42,16 +42,18 @@ public class ResourceLimitGroup extends AbstractDiffable imp public static final int MAX_CHARS_ALLOWED_IN_NAME = 50; private final String name; private final List resourceLimits; + private final ResourceLimitGroupMode mode; private static final List ALLOWED_RESOURCES = List.of("jvm"); public static final ParseField NAME_FIELD = new ParseField("name"); public static final ParseField RESOURCE_LIMITS_FIELD = new ParseField("resourceLimits"); + public static final ParseField MODE_FIELD = new ParseField("mode"); @SuppressWarnings("unchecked") private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( "ResourceLimitGroupParser", - args -> new ResourceLimitGroup((String) args[0], (List) args[1]) + args -> new ResourceLimitGroup((String) args[0], (List) args[1], (String) args[2]) ); static { @@ -61,9 +63,10 @@ public class ResourceLimitGroup extends AbstractDiffable imp (p, c) -> ResourceLimit.fromXContent(p), RESOURCE_LIMITS_FIELD ); + PARSER.declareString(ConstructingObjectParser.constructorArg(), MODE_FIELD); } - public ResourceLimitGroup(final String name, final List resourceLimits) { + public ResourceLimitGroup(final String name, final List resourceLimits, final String modeName) { Objects.requireNonNull(name, "ResourceLimitGroup.name can't be null"); Objects.requireNonNull(resourceLimits, "ResourceLimitGroup.resourceLimits can't be null"); @@ -77,10 +80,11 @@ public ResourceLimitGroup(final String name, final List resourceL this.name = name; this.resourceLimits = resourceLimits; + this.mode = ResourceLimitGroupMode.fromName(modeName); } public ResourceLimitGroup(StreamInput in) throws IOException { - this(in.readString(), in.readList(ResourceLimit::new)); + this(in.readString(), in.readList(ResourceLimit::new), in.readString()); } /** @@ -171,6 +175,37 @@ public int hashCode() { } } + @ExperimentalApi + public static enum ResourceLimitGroupMode { + SOFT("soft"), + ENFORCED("enforced"), + MONITOR("monitor"); + + private final String name; + + ResourceLimitGroupMode(String mode) { + this.name = mode; + } + + public String getName() { + return name; + } + + public static ResourceLimitGroupMode fromName(String s) { + switch (s) { + case "soft": + return SOFT; + case "enforced": + return ENFORCED; + case "monitor": + return MONITOR; + default: + throw new IllegalArgumentException("Invalid value for ResourceLimitGroupMode: " + s); + } + } + + } + /** * Write this into the {@linkplain StreamOutput}. * @@ -180,6 +215,7 @@ public int hashCode() { public void writeTo(StreamOutput out) throws IOException { out.writeString(name); out.writeList(resourceLimits); + out.writeString(mode.getName()); } /** @@ -193,6 +229,7 @@ public XContentBuilder toXContent(final XContentBuilder builder, final Params pa builder.startObject(); builder.field(NAME_FIELD.getPreferredName(), name); builder.field(RESOURCE_LIMITS_FIELD.getPreferredName(), resourceLimits); + builder.field(MODE_FIELD.getPreferredName(), mode.getName()); builder.endObject(); return builder; } @@ -222,6 +259,10 @@ public String getName() { return name; } + public ResourceLimitGroupMode getMode() { + return mode; + } + public List getResourceLimits() { return resourceLimits; } diff --git a/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupTests.java b/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupTests.java index 0c429c2eb0972..3de6d6c8d3067 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupTests.java @@ -14,13 +14,25 @@ import java.io.IOException; import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.Stream; public class ResourceLimitGroupTests extends AbstractSerializingTestCase { + private static final List allowedModes = Stream.of( + ResourceLimitGroup.ResourceLimitGroupMode.SOFT, + ResourceLimitGroup.ResourceLimitGroupMode.ENFORCED, + ResourceLimitGroup.ResourceLimitGroupMode.MONITOR + ).map(ResourceLimitGroup.ResourceLimitGroupMode::getName).collect(Collectors.toList()); + static ResourceLimitGroup createRandomResourceLimitGroup() { String name = randomAlphaOfLength(10); ResourceLimitGroup.ResourceLimit resourceLimit = new ResourceLimitGroup.ResourceLimit("jvm", randomDoubleBetween(0.0, 0.80, false)); - return new ResourceLimitGroup(name, List.of(resourceLimit)); + return new ResourceLimitGroup(name, List.of(resourceLimit), randomMode()); + } + + private static String randomMode() { + return allowedModes.get((int) (Math.random() * allowedModes.size())); } /** @@ -52,15 +64,22 @@ protected ResourceLimitGroup createTestInstance() { } public void testNullName() { - assertThrows(NullPointerException.class, () -> new ResourceLimitGroup(null, List.of())); + assertThrows(NullPointerException.class, () -> new ResourceLimitGroup(null, List.of(), randomMode())); } public void testNullResourceLimits() { - assertThrows(NullPointerException.class, () -> new ResourceLimitGroup("analytics", null)); + assertThrows(NullPointerException.class, () -> new ResourceLimitGroup("analytics", null, randomMode())); } public void testEmptyResourceLimits() { - assertThrows(IllegalArgumentException.class, () -> new ResourceLimitGroup("analytics", List.of())); + assertThrows(IllegalArgumentException.class, () -> new ResourceLimitGroup("analytics", List.of(), randomMode())); + } + + public void testIllegalResourceLimitGroupMode() { + assertThrows( + IllegalArgumentException.class, + () -> new ResourceLimitGroup("analytics", List.of(new ResourceLimitGroup.ResourceLimit("jvm", 0.4)), "buggy") + ); } public void testInvalidResourceLimitWhenInvalidSystemResourceNameIsGiven() { @@ -68,7 +87,8 @@ public void testInvalidResourceLimitWhenInvalidSystemResourceNameIsGiven() { IllegalArgumentException.class, () -> new ResourceLimitGroup( "analytics", - List.of(new ResourceLimitGroup.ResourceLimit("RequestRate", randomDoubleBetween(0.01, 0.8, false))) + List.of(new ResourceLimitGroup.ResourceLimit("RequestRate", randomDoubleBetween(0.01, 0.8, false))), + randomMode() ) ); } @@ -78,7 +98,8 @@ public void testInvalidResourceLimitWhenInvalidSystemResourceValueIsGiven() { IllegalArgumentException.class, () -> new ResourceLimitGroup( "analytics", - List.of(new ResourceLimitGroup.ResourceLimit("RequestRate", randomDoubleBetween(1.1, 1.8, false))) + List.of(new ResourceLimitGroup.ResourceLimit("RequestRate", randomDoubleBetween(1.1, 1.8, false))), + randomMode() ) ); } @@ -86,7 +107,8 @@ public void testInvalidResourceLimitWhenInvalidSystemResourceValueIsGiven() { public void testValidResourceLimitGroup() { ResourceLimitGroup resourceLimitGroup = new ResourceLimitGroup( "analytics", - List.of(new ResourceLimitGroup.ResourceLimit("jvm", randomDoubleBetween(0.01, 0.8, false))) + List.of(new ResourceLimitGroup.ResourceLimit("jvm", randomDoubleBetween(0.01, 0.8, false))), + randomMode() ); assertNotNull(resourceLimitGroup.getName()); @@ -94,5 +116,6 @@ public void testValidResourceLimitGroup() { assertNotNull(resourceLimitGroup.getResourceLimits()); assertFalse(resourceLimitGroup.getResourceLimits().isEmpty()); assertEquals(1, resourceLimitGroup.getResourceLimits().size()); + assertTrue(allowedModes.contains(resourceLimitGroup.getMode().getName())); } } From 5ba753570a2c039fd749fcfe799b920ee8500ff0 Mon Sep 17 00:00:00 2001 From: Kaushal Kumar Date: Fri, 19 Apr 2024 09:37:18 -0700 Subject: [PATCH 08/10] fix breaking testcases Signed-off-by: Kaushal Kumar --- .../action/search/SearchShardTask.java | 3 ++- .../opensearch/action/search/SearchTask.java | 3 ++- .../cluster/metadata/ResourceLimitGroup.java | 6 +++++- .../ResourceLimitGroupTask.java | 18 ++++++++++++++++++ .../search/AbstractSearchAsyncActionTests.java | 2 +- .../CanMatchPreFilterSearchPhaseTests.java | 12 ++++++------ .../metadata/ResourceLimitGroupTests.java | 2 +- 7 files changed, 35 insertions(+), 11 deletions(-) create mode 100644 server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupTask.java diff --git a/server/src/main/java/org/opensearch/action/search/SearchShardTask.java b/server/src/main/java/org/opensearch/action/search/SearchShardTask.java index 9e78a06aa7da5..17c32d09d15d6 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchShardTask.java +++ b/server/src/main/java/org/opensearch/action/search/SearchShardTask.java @@ -37,6 +37,7 @@ import org.opensearch.core.tasks.TaskId; import org.opensearch.search.fetch.ShardFetchSearchRequest; import org.opensearch.search.internal.ShardSearchRequest; +import org.opensearch.search.resource_limit_group.ResourceLimitGroupTask; import org.opensearch.tasks.CancellableTask; import org.opensearch.tasks.SearchBackpressureTask; @@ -50,7 +51,7 @@ * @opensearch.api */ @PublicApi(since = "1.0.0") -public class SearchShardTask extends CancellableTask implements SearchBackpressureTask { +public class SearchShardTask extends CancellableTask implements SearchBackpressureTask, ResourceLimitGroupTask { // generating metadata in a lazy way since source can be quite big private final MemoizedSupplier metadataSupplier; private String resourceLimitGroupId; diff --git a/server/src/main/java/org/opensearch/action/search/SearchTask.java b/server/src/main/java/org/opensearch/action/search/SearchTask.java index 363f3fde65338..775eadd38ef53 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchTask.java +++ b/server/src/main/java/org/opensearch/action/search/SearchTask.java @@ -35,6 +35,7 @@ import org.opensearch.common.annotation.PublicApi; import org.opensearch.common.unit.TimeValue; import org.opensearch.core.tasks.TaskId; +import org.opensearch.search.resource_limit_group.ResourceLimitGroupTask; import org.opensearch.tasks.CancellableTask; import org.opensearch.tasks.SearchBackpressureTask; @@ -49,7 +50,7 @@ * @opensearch.api */ @PublicApi(since = "1.0.0") -public class SearchTask extends CancellableTask implements SearchBackpressureTask { +public class SearchTask extends CancellableTask implements SearchBackpressureTask, ResourceLimitGroupTask { // generating description in a lazy way since source can be quite big private final Supplier descriptionSupplier; private SearchProgressListener progressListener = SearchProgressListener.NOOP; diff --git a/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java b/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java index 6c8f150ebbbd3..fbe76175e7826 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.util.List; +import java.util.Locale; import java.util.Objects; /** @@ -118,7 +119,7 @@ public ResourceLimit(String resourceName, Double value) { throw new IllegalArgumentException("resource value should be less than 1.0"); } - if (!ALLOWED_RESOURCES.contains(resourceName.toLowerCase())) { + if (!ALLOWED_RESOURCES.contains(resourceName.toLowerCase(Locale.ROOT))) { throw new IllegalArgumentException( "resource has to be valid, valid resources " + ALLOWED_RESOURCES.stream().reduce((x, e) -> x + ", " + e).get() ); @@ -175,6 +176,9 @@ public int hashCode() { } } + /** + * This enum models the different sandbox modes + */ @ExperimentalApi public static enum ResourceLimitGroupMode { SOFT("soft"), diff --git a/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupTask.java b/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupTask.java new file mode 100644 index 0000000000000..0925ad6a3950f --- /dev/null +++ b/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupTask.java @@ -0,0 +1,18 @@ +/* + * 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.resource_limit_group; + +/** + * This interface can be implemented by tasks which will be tracked and monitored using {@link org.opensearch.cluster.metadata.ResourceLimitGroup} + */ +public interface ResourceLimitGroupTask { + public void setResourceLimitGroupId(String sandboxResourceLimitGroup); + + public String getResourceLimitGroupId(); +} diff --git a/server/src/test/java/org/opensearch/action/search/AbstractSearchAsyncActionTests.java b/server/src/test/java/org/opensearch/action/search/AbstractSearchAsyncActionTests.java index 420289d3ff2e5..6ae92799aee54 100644 --- a/server/src/test/java/org/opensearch/action/search/AbstractSearchAsyncActionTests.java +++ b/server/src/test/java/org/opensearch/action/search/AbstractSearchAsyncActionTests.java @@ -179,7 +179,7 @@ private AbstractSearchAsyncAction createAction( new GroupShardsIterator<>(Arrays.asList(shards)), timeProvider, ClusterState.EMPTY_STATE, - null, + new SearchTask(123, null, null, null, null, null), results, request.getMaxConcurrentShardRequests(), SearchResponse.Clusters.EMPTY, diff --git a/server/src/test/java/org/opensearch/action/search/CanMatchPreFilterSearchPhaseTests.java b/server/src/test/java/org/opensearch/action/search/CanMatchPreFilterSearchPhaseTests.java index 1881c705fe6b3..6307691fcd5c6 100644 --- a/server/src/test/java/org/opensearch/action/search/CanMatchPreFilterSearchPhaseTests.java +++ b/server/src/test/java/org/opensearch/action/search/CanMatchPreFilterSearchPhaseTests.java @@ -160,7 +160,7 @@ public void sendCanMatch( shardsIter, timeProvider, ClusterState.EMPTY_STATE, - null, + new SearchTask(1, null, null, null, null, null), (iter) -> new SearchPhase("test") { @Override public void run() throws IOException { @@ -258,7 +258,7 @@ public void sendCanMatch( shardsIter, timeProvider, ClusterState.EMPTY_STATE, - null, + new SearchTask(1, null, null, null, null, null), (iter) -> new SearchPhase("test") { @Override public void run() throws IOException { @@ -346,7 +346,7 @@ public void sendCanMatch( shardsIter, timeProvider, ClusterState.EMPTY_STATE, - null, + new SearchTask(1, null, null, null, null, null), (iter) -> { return new WrappingSearchAsyncActionPhase( new AbstractSearchAsyncAction("test", logger, transportService, (cluster, node) -> { @@ -478,7 +478,7 @@ public void sendCanMatch( shardsIter, timeProvider, ClusterState.EMPTY_STATE, - null, + new SearchTask(1, null, null, null, null, null), (iter) -> new SearchPhase("test") { @Override public void run() { @@ -585,7 +585,7 @@ public void sendCanMatch( shardsIter, timeProvider, ClusterState.EMPTY_STATE, - null, + new SearchTask(1, null, null, null, null, null), (iter) -> new SearchPhase("test") { @Override public void run() { @@ -690,7 +690,7 @@ public void sendCanMatch( shardsIter, timeProvider, ClusterState.EMPTY_STATE, - null, + new SearchTask(1, null, null, null, null, null), (iter) -> { AbstractSearchAsyncAction action = new SearchDfsQueryAsyncAction( logger, diff --git a/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupTests.java b/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupTests.java index 3de6d6c8d3067..273b082b227c2 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupTests.java @@ -32,7 +32,7 @@ static ResourceLimitGroup createRandomResourceLimitGroup() { } private static String randomMode() { - return allowedModes.get((int) (Math.random() * allowedModes.size())); + return allowedModes.get(randomIntBetween(0, allowedModes.size() - 1)); } /** From d2a19ee6898d14769534662ee1d1340ae703c8fc Mon Sep 17 00:00:00 2001 From: Kaushal Kumar Date: Mon, 22 Apr 2024 13:28:26 -0700 Subject: [PATCH 09/10] add task cancellation skeleton Signed-off-by: Kaushal Kumar --- .../search/AbstractSearchAsyncAction.java | 2 +- .../action/search/SearchShardTask.java | 4 +- .../opensearch/action/search/SearchTask.java | 4 +- .../action/search/TransportSearchAction.java | 2 +- .../cluster/metadata/ResourceLimitGroup.java | 15 ++ .../org/opensearch/search/SearchService.java | 6 +- .../ResourceLimitGroupService.java | 13 +- .../ResourceLimitGroupTask.java | 4 +- .../cancellation/CancellableTaskSelector.java | 26 +++ .../ResourceLimitGroupRequestCanceller.java | 19 -- .../ResourceLimitGroupTaskCanceller.java | 22 +++ .../module/ResourceLimitGroupModule.java | 4 +- ...imitsGroupResourceUsageTrackerService.java | 162 +++++++++++++++--- 13 files changed, 224 insertions(+), 59 deletions(-) create mode 100644 server/src/main/java/org/opensearch/search/resource_limit_group/cancellation/CancellableTaskSelector.java delete mode 100644 server/src/main/java/org/opensearch/search/resource_limit_group/cancellation/ResourceLimitGroupRequestCanceller.java create mode 100644 server/src/main/java/org/opensearch/search/resource_limit_group/cancellation/ResourceLimitGroupTaskCanceller.java diff --git a/server/src/main/java/org/opensearch/action/search/AbstractSearchAsyncAction.java b/server/src/main/java/org/opensearch/action/search/AbstractSearchAsyncAction.java index c706189f993a2..338ddaa46d067 100644 --- a/server/src/main/java/org/opensearch/action/search/AbstractSearchAsyncAction.java +++ b/server/src/main/java/org/opensearch/action/search/AbstractSearchAsyncAction.java @@ -841,7 +841,7 @@ public final ShardSearchRequest buildShardSearchRequest(SearchShardIterator shar shardRequest.canReturnNullResponseIfMatchNoDocs(hasShardResponse.get() && shardRequest.scroll() == null); // Propagate the resource limit group from co-ordinator to data nodes - shardRequest.setResourceLimitGroupId(getTask().getResourceLimitGroupId()); + shardRequest.setResourceLimitGroupId(getTask().getResourceLimitGroupName()); return shardRequest; } diff --git a/server/src/main/java/org/opensearch/action/search/SearchShardTask.java b/server/src/main/java/org/opensearch/action/search/SearchShardTask.java index 17c32d09d15d6..5ea916f46c05b 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchShardTask.java +++ b/server/src/main/java/org/opensearch/action/search/SearchShardTask.java @@ -87,11 +87,11 @@ public boolean shouldCancelChildrenOnCancellation() { return false; } - public String getResourceLimitGroupId() { + public String getResourceLimitGroupName() { return resourceLimitGroupId; } - public void setResourceLimitGroupId(String resourceLimitGroupId) { + public void setResourceLimitGroupName(String resourceLimitGroupId) { this.resourceLimitGroupId = resourceLimitGroupId; } } diff --git a/server/src/main/java/org/opensearch/action/search/SearchTask.java b/server/src/main/java/org/opensearch/action/search/SearchTask.java index 775eadd38ef53..c580a82d728c2 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchTask.java +++ b/server/src/main/java/org/opensearch/action/search/SearchTask.java @@ -109,11 +109,11 @@ public boolean shouldCancelChildrenOnCancellation() { return true; } - public String getResourceLimitGroupId() { + public String getResourceLimitGroupName() { return resourceLimitGroupId; } - public void setResourceLimitGroupId(String resourceLimitGroupId) { + public void setResourceLimitGroupName(String resourceLimitGroupId) { this.resourceLimitGroupId = resourceLimitGroupId; } } diff --git a/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java b/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java index 7ee7292176473..b8e11a339ee50 100644 --- a/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java +++ b/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java @@ -1105,7 +1105,7 @@ private void executeSearch( localShardIterators.size() + remoteShardIterators.size() ); - task.setResourceLimitGroupId(searchRequest.resourceLimitGroupId()); + task.setResourceLimitGroupName(searchRequest.resourceLimitGroupId()); searchAsyncActionProvider.asyncSearchAction( task, diff --git a/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java b/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java index fbe76175e7826..b6215ce49961d 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java @@ -174,6 +174,14 @@ public boolean equals(Object o) { public int hashCode() { return Objects.hash(resourceName, value); } + + public String getResourceName() { + return resourceName; + } + + public Double getValue() { + return value; + } } /** @@ -270,4 +278,11 @@ public ResourceLimitGroupMode getMode() { public List getResourceLimits() { return resourceLimits; } + + public ResourceLimit getResourceLimitFor(String resourceName) { + return resourceLimits.stream() + .filter(resourceLimit -> resourceLimit.getResourceName().equals(resourceName)) + .findFirst() + .orElseGet(() -> new ResourceLimit(resourceName, 100.0)); + } } diff --git a/server/src/main/java/org/opensearch/search/SearchService.java b/server/src/main/java/org/opensearch/search/SearchService.java index 1167dd43784aa..214673bae36f6 100644 --- a/server/src/main/java/org/opensearch/search/SearchService.java +++ b/server/src/main/java/org/opensearch/search/SearchService.java @@ -568,7 +568,7 @@ public void executeQueryPhase( assert request.canReturnNullResponseIfMatchNoDocs() == false || request.numberOfShards() > 1 : "empty responses require more than one shard"; final IndexShard shard = getShard(request); - task.setResourceLimitGroupId(request.resourceLimitGroupId()); + task.setResourceLimitGroupName(request.resourceLimitGroupId()); rewriteAndFetchShardRequest(shard, request, new ActionListener() { @Override public void onResponse(ShardSearchRequest orig) { @@ -677,7 +677,7 @@ public void executeQueryPhase( } runAsync(getExecutor(readerContext.indexShard()), () -> { final ShardSearchRequest shardSearchRequest = readerContext.getShardSearchRequest(null); - task.setResourceLimitGroupId(shardSearchRequest.resourceLimitGroupId()); + task.setResourceLimitGroupName(shardSearchRequest.resourceLimitGroupId()); try ( SearchContext searchContext = createContext(readerContext, shardSearchRequest, task, false); SearchOperationListenerExecutor executor = new SearchOperationListenerExecutor(searchContext) @@ -780,7 +780,7 @@ public void executeFetchPhase( public void executeFetchPhase(ShardFetchRequest request, SearchShardTask task, ActionListener listener) { final ReaderContext readerContext = findReaderContext(request.contextId(), request); final ShardSearchRequest shardSearchRequest = readerContext.getShardSearchRequest(request.getShardSearchRequest()); - task.setResourceLimitGroupId(shardSearchRequest.resourceLimitGroupId()); + task.setResourceLimitGroupName(shardSearchRequest.resourceLimitGroupId()); final Releasable markAsUsed = readerContext.markAsUsed(getKeepAlive(shardSearchRequest)); runAsync(getExecutor(readerContext.indexShard()), () -> { try (SearchContext searchContext = createContext(readerContext, shardSearchRequest, task, false)) { diff --git a/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupService.java b/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupService.java index 1dc3dba1c1a0d..98103e9a599e2 100644 --- a/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupService.java +++ b/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupService.java @@ -12,7 +12,7 @@ import org.apache.logging.log4j.Logger; import org.opensearch.common.inject.Inject; import org.opensearch.common.lifecycle.AbstractLifecycleComponent; -import org.opensearch.search.resource_limit_group.cancellation.ResourceLimitGroupRequestCanceller; +import org.opensearch.search.resource_limit_group.cancellation.ResourceLimitGroupTaskCanceller; import org.opensearch.search.resource_limit_group.tracker.ResourceLimitGroupResourceUsageTracker; import org.opensearch.threadpool.Scheduler; import org.opensearch.threadpool.ThreadPool; @@ -26,11 +26,12 @@ public class ResourceLimitGroupService extends AbstractLifecycleComponent { private static final Logger logger = LogManager.getLogger(ResourceLimitGroupService.class); private final ResourceLimitGroupResourceUsageTracker requestTracker; - private final ResourceLimitGroupRequestCanceller requestCanceller; + private final ResourceLimitGroupTaskCanceller requestCanceller; private final ResourceLimitGroupPruner resourceLimitGroupPruner; private volatile Scheduler.Cancellable scheduledFuture; private final ResourceLimitGroupServiceSettings sandboxServiceSettings; private final ThreadPool threadPool; + private final ResourceLimitGroupTaskCanceller taskCanceller; /** * Guice managed constructor @@ -43,16 +44,18 @@ public class ResourceLimitGroupService extends AbstractLifecycleComponent { @Inject public ResourceLimitGroupService( ResourceLimitGroupResourceUsageTracker requestTrackerService, - ResourceLimitGroupRequestCanceller requestCanceller, + ResourceLimitGroupTaskCanceller requestCanceller, ResourceLimitGroupPruner resourceLimitGroupPruner, ResourceLimitGroupServiceSettings sandboxServiceSettings, - ThreadPool threadPool + ThreadPool threadPool, + ResourceLimitGroupTaskCanceller taskCanceller ) { this.requestTracker = requestTrackerService; this.requestCanceller = requestCanceller; this.resourceLimitGroupPruner = resourceLimitGroupPruner; this.sandboxServiceSettings = sandboxServiceSettings; this.threadPool = threadPool; + this.taskCanceller = taskCanceller; } /** @@ -60,7 +63,7 @@ public ResourceLimitGroupService( */ private void doRun() { requestTracker.updateResourceLimitGroupsResourceUsage(); - requestCanceller.cancelViolatingTasks(); + taskCanceller.cancelTasks(); resourceLimitGroupPruner.pruneResourceLimitGroup(); } diff --git a/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupTask.java b/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupTask.java index 0925ad6a3950f..66b034ecd8e82 100644 --- a/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupTask.java +++ b/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupTask.java @@ -12,7 +12,7 @@ * This interface can be implemented by tasks which will be tracked and monitored using {@link org.opensearch.cluster.metadata.ResourceLimitGroup} */ public interface ResourceLimitGroupTask { - public void setResourceLimitGroupId(String sandboxResourceLimitGroup); + public void setResourceLimitGroupName(String sandboxResourceLimitGroup); - public String getResourceLimitGroupId(); + public String getResourceLimitGroupName(); } diff --git a/server/src/main/java/org/opensearch/search/resource_limit_group/cancellation/CancellableTaskSelector.java b/server/src/main/java/org/opensearch/search/resource_limit_group/cancellation/CancellableTaskSelector.java new file mode 100644 index 0000000000000..b40ad3dc91771 --- /dev/null +++ b/server/src/main/java/org/opensearch/search/resource_limit_group/cancellation/CancellableTaskSelector.java @@ -0,0 +1,26 @@ +/* + * 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.resource_limit_group.cancellation; + +import org.opensearch.tasks.Task; +import org.opensearch.tasks.TaskCancellation; + +import java.util.List; + +public interface CancellableTaskSelector { + /** + * This method selects tasks which can be cancelled + * @param tasks is list of available tasks to select from + * @param reduceBy is meant to select enough number of tasks consuming {@param reduceBy} resource + * @param resource it is a system resource e,g; "jvm" or "cpu" + * @return + */ + public List selectTasks(List tasks, Double reduceBy, String resource); + +} diff --git a/server/src/main/java/org/opensearch/search/resource_limit_group/cancellation/ResourceLimitGroupRequestCanceller.java b/server/src/main/java/org/opensearch/search/resource_limit_group/cancellation/ResourceLimitGroupRequestCanceller.java deleted file mode 100644 index 77be6104c9600..0000000000000 --- a/server/src/main/java/org/opensearch/search/resource_limit_group/cancellation/ResourceLimitGroupRequestCanceller.java +++ /dev/null @@ -1,19 +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.resource_limit_group.cancellation; - -/** - * This interface is used to identify and cancel the violating tasks in a resourceLimitGroup - */ -public interface ResourceLimitGroupRequestCanceller { - /** - * Cancels the tasks from conteded resourceLimitGroups - */ - void cancelViolatingTasks(); -} diff --git a/server/src/main/java/org/opensearch/search/resource_limit_group/cancellation/ResourceLimitGroupTaskCanceller.java b/server/src/main/java/org/opensearch/search/resource_limit_group/cancellation/ResourceLimitGroupTaskCanceller.java new file mode 100644 index 0000000000000..6fd3a4c822a95 --- /dev/null +++ b/server/src/main/java/org/opensearch/search/resource_limit_group/cancellation/ResourceLimitGroupTaskCanceller.java @@ -0,0 +1,22 @@ +/* + * 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.resource_limit_group.cancellation; + +/** + * This class is used to identify and cancel the violating tasks in a resourceLimitGroup + */ +public abstract class ResourceLimitGroupTaskCanceller { + private CancellableTaskSelector taskSelector; + + public ResourceLimitGroupTaskCanceller(CancellableTaskSelector taskSelector) { + this.taskSelector = taskSelector; + } + + public abstract void cancelTasks(); +} diff --git a/server/src/main/java/org/opensearch/search/resource_limit_group/module/ResourceLimitGroupModule.java b/server/src/main/java/org/opensearch/search/resource_limit_group/module/ResourceLimitGroupModule.java index fb2d607e4f774..0a59df4565ef2 100644 --- a/server/src/main/java/org/opensearch/search/resource_limit_group/module/ResourceLimitGroupModule.java +++ b/server/src/main/java/org/opensearch/search/resource_limit_group/module/ResourceLimitGroupModule.java @@ -10,7 +10,7 @@ import org.opensearch.common.inject.AbstractModule; import org.opensearch.search.resource_limit_group.ResourceLimitGroupPruner; -import org.opensearch.search.resource_limit_group.cancellation.ResourceLimitGroupRequestCanceller; +import org.opensearch.search.resource_limit_group.cancellation.ResourceLimitGroupTaskCanceller; import org.opensearch.search.resource_limit_group.tracker.ResourceLimitGroupResourceUsageTracker; import org.opensearch.search.resource_limit_group.tracker.ResourceLimitsGroupResourceUsageTrackerService; @@ -27,7 +27,7 @@ public ResourceLimitGroupModule() {} @Override protected void configure() { bind(ResourceLimitGroupResourceUsageTracker.class).to(ResourceLimitsGroupResourceUsageTrackerService.class).asEagerSingleton(); - bind(ResourceLimitGroupRequestCanceller.class).to(ResourceLimitsGroupResourceUsageTrackerService.class).asEagerSingleton(); + bind(ResourceLimitGroupTaskCanceller.class).to(ResourceLimitsGroupResourceUsageTrackerService.class).asEagerSingleton(); bind(ResourceLimitGroupPruner.class).to(ResourceLimitsGroupResourceUsageTrackerService.class).asEagerSingleton(); } } diff --git a/server/src/main/java/org/opensearch/search/resource_limit_group/tracker/ResourceLimitsGroupResourceUsageTrackerService.java b/server/src/main/java/org/opensearch/search/resource_limit_group/tracker/ResourceLimitsGroupResourceUsageTrackerService.java index 4f11b6e2a678f..d91d08b39c903 100644 --- a/server/src/main/java/org/opensearch/search/resource_limit_group/tracker/ResourceLimitsGroupResourceUsageTrackerService.java +++ b/server/src/main/java/org/opensearch/search/resource_limit_group/tracker/ResourceLimitsGroupResourceUsageTrackerService.java @@ -8,33 +8,38 @@ package org.opensearch.search.resource_limit_group.tracker; +import org.opensearch.cluster.metadata.ResourceLimitGroup; +import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.inject.Inject; import org.opensearch.core.tasks.resourcetracker.TaskResourceUsage; import org.opensearch.search.resource_limit_group.ResourceLimitGroupPruner; -import org.opensearch.search.resource_limit_group.cancellation.ResourceLimitGroupRequestCanceller; +import org.opensearch.search.resource_limit_group.ResourceLimitGroupTask; +import org.opensearch.search.resource_limit_group.cancellation.CancellableTaskSelector; +import org.opensearch.search.resource_limit_group.cancellation.ResourceLimitGroupTaskCanceller; import org.opensearch.tasks.Task; import org.opensearch.tasks.TaskCancellation; import org.opensearch.tasks.TaskManager; import org.opensearch.tasks.TaskResourceTrackingService; import java.util.ArrayList; -import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.function.LongSupplier; import java.util.stream.Collectors; /** * This class tracks requests per resourceLimitGroups */ -public class ResourceLimitsGroupResourceUsageTrackerService +public class ResourceLimitsGroupResourceUsageTrackerService extends ResourceLimitGroupTaskCanceller implements TaskManager.TaskEventListeners, ResourceLimitGroupResourceUsageTracker, - ResourceLimitGroupRequestCanceller, ResourceLimitGroupPruner { - private static final String CPU = "CPU"; - private static final String JVM_ALLOCATIONS = "JVM_Allocations"; + private static final String CPU = "cpu"; + private static final String JVM = "jvm"; + private static final List TRACKED_RESOURCES = List.of(JVM); private static final int numberOfAvailableProcessors = Runtime.getRuntime().availableProcessors(); private static final long totalAvailableJvmMemory = Runtime.getRuntime().totalMemory(); private final LongSupplier timeNanosSupplier; @@ -43,29 +48,94 @@ public class ResourceLimitsGroupResourceUsageTrackerService * {@link org.opensearch.search.resource_limit_group.ResourceLimitGroupService} runs */ private List toDeleteResourceLimitGroups; - private List activeResourceLimitGroups; + private List activeResourceLimitGroups; + /** + * This var will hold the sandbox level resource usage as per last run of + * {@link org.opensearch.search.resource_limit_group.ResourceLimitGroupService} + */ + private Map> resourceUsage; + /** + * We are keeping this as instance member as we will need this to identify the contended resource limit groups + * resourceLimitGroupName -> List<ResourceLimitGroupTask> + */ + private Map> resourceLimitGroupTasks; private final TaskManager taskManager; private final TaskResourceTrackingService taskResourceTrackingService; + private final ClusterService clusterService; + private final CancellableTaskSelector taskSelector; /** * SandboxResourceTrackerService constructor * @param taskManager * @param taskResourceTrackingService + * @param clusterService */ @Inject public ResourceLimitsGroupResourceUsageTrackerService( - TaskManager taskManager, - TaskResourceTrackingService taskResourceTrackingService + final TaskManager taskManager, + final TaskResourceTrackingService taskResourceTrackingService, + final ClusterService clusterService, + final LongSupplier timeNanosSupplier, + final CancellableTaskSelector taskSelector ) { + super(taskSelector); this.taskManager = taskManager; this.taskResourceTrackingService = taskResourceTrackingService; - toDeleteResourceLimitGroups = Collections.synchronizedList(new ArrayList<>()); - this.timeNanosSupplier = System::nanoTime; + toDeleteResourceLimitGroups = new ArrayList<>(); + this.clusterService = clusterService; + this.timeNanosSupplier = timeNanosSupplier; + this.taskSelector = taskSelector; } @Override public void updateResourceLimitGroupsResourceUsage() { + activeResourceLimitGroups = new ArrayList<>(clusterService.state().metadata().resourceLimitGroups().values()); + + updateResourceLimitGroupTasks(); + + refreshResourceLimitGroupsUsage(resourceLimitGroupTasks); + } + private void updateResourceLimitGroupTasks() { + List activeTasks = taskResourceTrackingService.getResourceAwareTasks() + .values() + .stream() + .filter(task -> task instanceof ResourceLimitGroupTask) + .map(task -> (ResourceLimitGroupTask) task) + .collect(Collectors.toList()); + + Map> newResourceLimitGroupTasks = new HashMap<>(); + for (Map.Entry> entry : activeTasks.stream() + .collect(Collectors.groupingBy(ResourceLimitGroupTask::getResourceLimitGroupName)) + .entrySet()) { + newResourceLimitGroupTasks.put(entry.getKey(), entry.getValue().stream().map(task -> (Task) task).collect(Collectors.toList())); + } + + resourceLimitGroupTasks = newResourceLimitGroupTasks; + } + + private void refreshResourceLimitGroupsUsage(Map> resourceLimitGroupTasks) { + /** + * remove the deleted resource limit groups + */ + final List nonExistingResourceLimitGroups = new ArrayList<>(resourceUsage.keySet()); + for (String activeResourceLimitGroup : resourceLimitGroupTasks.keySet()) { + nonExistingResourceLimitGroups.remove(activeResourceLimitGroup); + } + nonExistingResourceLimitGroups.forEach(resourceUsage::remove); + + for (Map.Entry> resourceLimitGroup : resourceLimitGroupTasks.entrySet()) { + final String resourceLimitGroupName = resourceLimitGroup.getKey(); + + Map resourceLimitGroupUsage = resourceLimitGroup.getValue() + .stream() + .map(this::calculateAbsoluteResourceUsageFor) + .reduce(new AbsoluteResourceUsage(0, 0), AbsoluteResourceUsage::merge) + .toMap(); + + resourceUsage.put(resourceLimitGroupName, resourceLimitGroupUsage); + + } } // @Override @@ -110,6 +180,18 @@ public double getAbsoluteCpuUsageInPercentage() { public double getAbsoluteJvmAllocationsUsageInPercent() { return absoluteJvmAllocationsUsage * 100; } + + /** + * + * @return {@code Map} + * which captures key as resource and value as the percentage value + */ + public Map toMap() { + Map map = new HashMap<>(); + // We can put the additional resources into this map in the future + map.put(JVM, getAbsoluteJvmAllocationsUsageInPercent()); + return map; + } } /** @@ -140,7 +222,7 @@ public void onTaskCompleted(Task task) {} * */ @Override - public void cancelViolatingTasks() { + public void cancelTasks() { List cancellableTasks = getCancellableTasks(); for (TaskCancellation taskCancellation : cancellableTasks) { taskCancellation.cancel(); @@ -153,10 +235,14 @@ public void cancelViolatingTasks() { */ private List getCancellableTasks() { // get cancellations from enforced type sandboxes - List inViolationSandboxes = getBreachingSandboxIds(); + final List inViolationResourceLimitGroups = getBreachingResourceLimitGroups(); + final List enforcedResourceLimitGroups = inViolationResourceLimitGroups.stream() + .filter(resourceLimitGroup -> resourceLimitGroup.getMode().equals(ResourceLimitGroup.ResourceLimitGroupMode.ENFORCED)) + .collect(Collectors.toList()); List cancellableTasks = new ArrayList<>(); - for (String sandboxId : inViolationSandboxes) { - cancellableTasks.addAll(getCancellableTasksFrom(sandboxId)); + + for (ResourceLimitGroup resourceLimitGroup : enforcedResourceLimitGroups) { + cancellableTasks.addAll(getCancellableTasksFrom(resourceLimitGroup)); } // get cancellations from soft type sandboxes if the node is in duress (hitting node level cancellation @@ -165,18 +251,50 @@ private List getCancellableTasks() { return cancellableTasks; } - public void deleteSandbox(String sandboxId) { - if (hasUnfinishedTasks(sandboxId)) { - toDeleteResourceLimitGroups.add(sandboxId); + public void deleteSandbox(String sandboxName) { + if (hasUnfinishedTasks(sandboxName)) { + toDeleteResourceLimitGroups.add(sandboxName); } // remove this sandbox from the active sandboxes + activeResourceLimitGroups = activeResourceLimitGroups.stream() + .filter(resourceLimitGroup -> !resourceLimitGroup.getName().equals(sandboxName)) + .collect(Collectors.toList()); } - private List getBreachingSandboxIds() { - return Collections.emptyList(); + private List getBreachingResourceLimitGroups() { + final List breachingResourceLimitGroupNames = new ArrayList<>(); + + for (ResourceLimitGroup resourceLimitGroup : activeResourceLimitGroups) { + Map currentResourceUsage = resourceUsage.get(resourceLimitGroup.getName()); + boolean isBreaching = false; + + for (ResourceLimitGroup.ResourceLimit resourceLimit : resourceLimitGroup.getResourceLimits()) { + if (currentResourceUsage.get(resourceLimit.getResourceName()) > resourceLimit.getValue()) { + isBreaching = true; + break; + } + } + + if (isBreaching) breachingResourceLimitGroupNames.add(resourceLimitGroup); + } + + return breachingResourceLimitGroupNames; } - private List getCancellableTasksFrom(String sandboxId) { - return Collections.emptyList(); + List getCancellableTasksFrom(ResourceLimitGroup resourceLimitGroup) { + List cancellations = new ArrayList<>(); + for (String resource : TRACKED_RESOURCES) { + final double reduceBy = resourceUsage.get(resourceLimitGroup.getName()).get(resource) - resourceLimitGroup.getResourceLimitFor( + resource + ).getValue(); + /** + * if the resource is not defined for this sandbox then ignore cancellations from it + */ + if (reduceBy < 0.0) { + continue; + } + cancellations.addAll(taskSelector.selectTasks(resourceLimitGroupTasks.get(resourceLimitGroup.getName()), reduceBy, resource)); + } + return cancellations; } } From 3afd9db86ea3be0dc8d92d0ac397deef2d0755f2 Mon Sep 17 00:00:00 2001 From: Kaushal Kumar Date: Wed, 8 May 2024 11:42:39 -0700 Subject: [PATCH 10/10] add multitenant labels in searchSource builder Signed-off-by: Kaushal Kumar --- .../search/AbstractSearchAsyncAction.java | 2 - .../action/search/SearchRequest.java | 27 +---- .../action/search/TransportSearchAction.java | 13 ++- .../opensearch/cluster/metadata/Metadata.java | 25 ++--- .../cluster/metadata/ResourceLimitGroup.java | 21 ++-- .../metadata/ResourceLimitGroupMetadata.java | 101 +++++++++--------- .../rest/action/search/RestSearchAction.java | 4 - .../opensearch/search/MultiTenantLabel.java | 53 +++++++++ .../org/opensearch/search/SearchService.java | 15 ++- .../search/builder/SearchSourceBuilder.java | 26 ++++- .../search/internal/ShardSearchRequest.java | 16 --- .../ResourceLimitGroupService.java | 13 +-- ...imitsGroupResourceUsageTrackerService.java | 13 +-- .../ResourceLimitGroupMetadataTests.java | 12 +-- 14 files changed, 184 insertions(+), 157 deletions(-) create mode 100644 server/src/main/java/org/opensearch/search/MultiTenantLabel.java diff --git a/server/src/main/java/org/opensearch/action/search/AbstractSearchAsyncAction.java b/server/src/main/java/org/opensearch/action/search/AbstractSearchAsyncAction.java index 338ddaa46d067..1359c7608707d 100644 --- a/server/src/main/java/org/opensearch/action/search/AbstractSearchAsyncAction.java +++ b/server/src/main/java/org/opensearch/action/search/AbstractSearchAsyncAction.java @@ -840,8 +840,6 @@ public final ShardSearchRequest buildShardSearchRequest(SearchShardIterator shar // Note that, we have to disable this shortcut for queries that create a context (scroll and search context). shardRequest.canReturnNullResponseIfMatchNoDocs(hasShardResponse.get() && shardRequest.scroll() == null); - // Propagate the resource limit group from co-ordinator to data nodes - shardRequest.setResourceLimitGroupId(getTask().getResourceLimitGroupName()); return shardRequest; } diff --git a/server/src/main/java/org/opensearch/action/search/SearchRequest.java b/server/src/main/java/org/opensearch/action/search/SearchRequest.java index 24a349c3964a9..4372624965dac 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchRequest.java +++ b/server/src/main/java/org/opensearch/action/search/SearchRequest.java @@ -122,8 +122,6 @@ public class SearchRequest extends ActionRequest implements IndicesRequest.Repla private Boolean phaseTook = null; - private String resourceLimitGroupId; - public SearchRequest() { this.localClusterAlias = null; this.absoluteStartMillis = DEFAULT_ABSOLUTE_START_MILLIS; @@ -264,10 +262,6 @@ public SearchRequest(StreamInput in) throws IOException { if (in.getVersion().onOrAfter(Version.V_2_12_0)) { phaseTook = in.readOptionalBoolean(); } - - if (in.getVersion().onOrAfter(Version.V_3_0_0)) { - resourceLimitGroupId = in.readOptionalString(); - } } @Override @@ -302,9 +296,6 @@ public void writeTo(StreamOutput out) throws IOException { if (out.getVersion().onOrAfter(Version.V_2_12_0)) { out.writeOptionalBoolean(phaseTook); } - if (out.getVersion().onOrAfter(Version.V_3_0_0)) { - out.writeOptionalString(resourceLimitGroupId); - } } @Override @@ -706,16 +697,6 @@ public SearchRequest pipeline(String pipeline) { public String pipeline() { return pipeline; } - - public SearchRequest resourceLimitGroupId(String resourceLimitGroupId) { - this.resourceLimitGroupId = resourceLimitGroupId; - return this; - } - - public String resourceLimitGroupId() { - return resourceLimitGroupId; - } - @Override public SearchTask createTask(long id, String type, String action, TaskId parentTaskId, Map headers) { return new SearchTask(id, type, action, this::buildDescription, parentTaskId, headers, cancelAfterTimeInterval); @@ -770,8 +751,7 @@ public boolean equals(Object o) { && ccsMinimizeRoundtrips == that.ccsMinimizeRoundtrips && Objects.equals(cancelAfterTimeInterval, that.cancelAfterTimeInterval) && Objects.equals(pipeline, that.pipeline) - && Objects.equals(phaseTook, that.phaseTook) - && Objects.equals(resourceLimitGroupId, that.resourceLimitGroupId); + && Objects.equals(phaseTook, that.phaseTook); } @Override @@ -793,8 +773,7 @@ public int hashCode() { absoluteStartMillis, ccsMinimizeRoundtrips, cancelAfterTimeInterval, - phaseTook, - resourceLimitGroupId + phaseTook ); } @@ -839,8 +818,6 @@ public String toString() { + pipeline + ", phaseTook=" + phaseTook - + ", resourceLimitGroupId=" - + resourceLimitGroupId + "}"; } } diff --git a/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java b/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java index b8e11a339ee50..fd0656cc0a2e8 100644 --- a/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java +++ b/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java @@ -72,6 +72,7 @@ import org.opensearch.core.indices.breaker.CircuitBreakerService; import org.opensearch.core.tasks.TaskId; import org.opensearch.index.query.Rewriteable; +import org.opensearch.search.MultiTenantLabel; import org.opensearch.search.SearchPhaseResult; import org.opensearch.search.SearchService; import org.opensearch.search.SearchShardTarget; @@ -122,8 +123,7 @@ import java.util.stream.StreamSupport; import static org.opensearch.action.admin.cluster.node.tasks.get.GetTaskAction.TASKS_ORIGIN; -import static org.opensearch.action.search.SearchType.DFS_QUERY_THEN_FETCH; -import static org.opensearch.action.search.SearchType.QUERY_THEN_FETCH; +import static org.opensearch.action.search.SearchType.*; import static org.opensearch.search.sort.FieldSortBuilder.hasPrimaryFieldSort; /** @@ -166,6 +166,7 @@ public class TransportSearchAction extends HandledTransportAction multiTenantLabels = searchRequest.source().multiTenantLabels(); + String tenant = NOT_PROVIDED; + if (multiTenantLabels != null) { + tenant = (String) multiTenantLabels.get(MultiTenantLabel.TENANT_LABEL.name()); + } + task.setResourceLimitGroupName(tenant); searchAsyncActionProvider.asyncSearchAction( task, 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 7b08c2b0f8cc1..22e9799ab608f 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java @@ -835,10 +835,10 @@ public Map views() { return Optional.ofNullable((ViewMetadata) this.custom(ViewMetadata.TYPE)).map(ViewMetadata::views).orElse(Collections.emptyMap()); } - public Map resourceLimitGroups() { + public Set resourceLimitGroups() { return Optional.ofNullable((ResourceLimitGroupMetadata) this.custom(ResourceLimitGroupMetadata.TYPE)) .map(ResourceLimitGroupMetadata::resourceLimitGroups) - .orElse(Collections.emptyMap()); + .orElse(Collections.emptySet()); } @@ -1336,34 +1336,23 @@ public Builder removeDataStream(String name) { return this; } - public Builder resourceLimitGroups(final Map resourceLimitGroups) { + public Builder resourceLimitGroups(final Set resourceLimitGroups) { this.customs.put(ResourceLimitGroupMetadata.TYPE, new ResourceLimitGroupMetadata(resourceLimitGroups)); return this; } - public ResourceLimitGroup getResourceLimitGroup(final String resourceLimitGroupName) { - return getResourceLimitGroups().get(resourceLimitGroupName); - } - public Builder put(final ResourceLimitGroup resourceLimitGroup) { Objects.requireNonNull(resourceLimitGroup, "resourceLimitGroup should not be null"); - Map existing = new HashMap<>(getResourceLimitGroups()); - existing.put(resourceLimitGroup.getName(), resourceLimitGroup); - return resourceLimitGroups(existing); - } - - public Builder removeResourceLimitGroup(final String resourceLimitGroupName) { - Objects.requireNonNull(resourceLimitGroupName, "resourceLimitGroup should not be null"); - Map existing = new HashMap<>(getResourceLimitGroups()); - existing.remove(resourceLimitGroupName); + Set existing = getResourceLimitGroups(); + existing.add(resourceLimitGroup); return resourceLimitGroups(existing); } - private Map getResourceLimitGroups() { + private Set getResourceLimitGroups() { return Optional.ofNullable(this.customs.get(ResourceLimitGroupMetadata.TYPE)) .map(o -> (ResourceLimitGroupMetadata) o) .map(ResourceLimitGroupMetadata::resourceLimitGroups) - .orElse(Collections.emptyMap()); + .orElse(Collections.emptySet()); } private Map getViews() { diff --git a/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java b/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java index b6215ce49961d..88031514f42df 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroup.java @@ -45,7 +45,8 @@ public class ResourceLimitGroup extends AbstractDiffable imp private final List resourceLimits; private final ResourceLimitGroupMode mode; - private static final List ALLOWED_RESOURCES = List.of("jvm"); + // list of resources that are allowed to be present in the ResourceLimitGroupSchema + public static final List ALLOWED_RESOURCES = List.of("jvm"); public static final ParseField NAME_FIELD = new ParseField("name"); public static final ParseField RESOURCE_LIMITS_FIELD = new ParseField("resourceLimits"); @@ -96,10 +97,10 @@ public ResourceLimitGroup(StreamInput in) throws IOException { @ExperimentalApi public static class ResourceLimit implements Writeable, ToXContentObject { private final String resourceName; - private final Double value; + private final Double threshold; static final ParseField RESOURCE_NAME_FIELD = new ParseField("resourceName"); - static final ParseField RESOURCE_VALUE_FIELD = new ParseField("value"); + static final ParseField RESOURCE_VALUE_FIELD = new ParseField("threshold"); public static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( "ResourceLimitParser", @@ -125,7 +126,7 @@ public ResourceLimit(String resourceName, Double value) { ); } this.resourceName = resourceName; - this.value = value; + this.threshold = value; } public ResourceLimit(StreamInput in) throws IOException { @@ -140,7 +141,7 @@ public ResourceLimit(StreamInput in) throws IOException { @Override public void writeTo(StreamOutput out) throws IOException { out.writeString(resourceName); - out.writeDouble(value); + out.writeDouble(threshold); } /** @@ -153,7 +154,7 @@ public void writeTo(StreamOutput out) throws IOException { public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); builder.field(RESOURCE_NAME_FIELD.getPreferredName(), resourceName); - builder.field(RESOURCE_VALUE_FIELD.getPreferredName(), value); + builder.field(RESOURCE_VALUE_FIELD.getPreferredName(), threshold); builder.endObject(); return builder; } @@ -167,20 +168,20 @@ public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; ResourceLimit that = (ResourceLimit) o; - return Objects.equals(resourceName, that.resourceName) && Objects.equals(value, that.value); + return Objects.equals(resourceName, that.resourceName) && Objects.equals(threshold, that.threshold); } @Override public int hashCode() { - return Objects.hash(resourceName, value); + return Objects.hash(resourceName, threshold); } public String getResourceName() { return resourceName; } - public Double getValue() { - return value; + public Double getThreshold() { + return threshold; } } diff --git a/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadata.java index 4f2fb0981a549..ab93a073b50f7 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadata.java @@ -23,10 +23,7 @@ import org.opensearch.core.xcontent.XContentParser; import java.io.IOException; -import java.util.EnumSet; -import java.util.HashMap; -import java.util.Map; -import java.util.Objects; +import java.util.*; import static org.opensearch.cluster.metadata.Metadata.ALL_CONTEXTS; @@ -50,30 +47,27 @@ public class ResourceLimitGroupMetadata implements Metadata.Custom { @SuppressWarnings("unchecked") static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( "resourceLimitGroupParser", - args -> new ResourceLimitGroupMetadata((Map) args[0]) + args -> new ResourceLimitGroupMetadata((Set) args[0]) ); static { - PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> { - Map resourceLimitGroupMap = new HashMap<>(); - while (p.nextToken() != XContentParser.Token.END_OBJECT) { - resourceLimitGroupMap.put(p.currentName(), ResourceLimitGroup.fromXContent(p)); - } - return resourceLimitGroupMap; - }, RESOURCE_LIMIT_GROUP_FIELD); + PARSER.declareObjectArray( + ConstructingObjectParser.constructorArg(), + (p, c) -> ResourceLimitGroup.fromXContent(p), + RESOURCE_LIMIT_GROUP_FIELD); } - private final Map resourceLimitGroups; + private final Set resourceLimitGroups; - public ResourceLimitGroupMetadata(Map resourceLimitGroups) { + public ResourceLimitGroupMetadata(Set resourceLimitGroups) { this.resourceLimitGroups = resourceLimitGroups; } public ResourceLimitGroupMetadata(StreamInput in) throws IOException { - this.resourceLimitGroups = in.readMap(StreamInput::readString, ResourceLimitGroup::new); + this.resourceLimitGroups = in.readSet(ResourceLimitGroup::new); } - public Map resourceLimitGroups() { + public Set resourceLimitGroups() { return this.resourceLimitGroups; } @@ -100,7 +94,7 @@ public Version getMinimalSupportedVersion() { */ @Override public void writeTo(StreamOutput out) throws IOException { - out.writeMap(resourceLimitGroups, StreamOutput::writeString, (stream, val) -> val.writeTo(stream)); + out.writeCollection(resourceLimitGroups); } /** @@ -111,10 +105,8 @@ public void writeTo(StreamOutput out) throws IOException { */ @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.startObject(RESOURCE_LIMIT_GROUP_FIELD.getPreferredName()); - for (Map.Entry entry : resourceLimitGroups.entrySet()) { - builder.field(entry.getKey(), entry.getValue()); - } + builder.startObject(); + builder.field(RESOURCE_LIMIT_GROUP_FIELD.getPreferredName(), resourceLimitGroups); builder.endObject(); return builder; } @@ -133,30 +125,45 @@ public Diff diff(final Metadata.Custom previousState) { return new ResourceLimitGroupMetadataDiff((ResourceLimitGroupMetadata) previousState, this); } - /** - * @return - */ @Override public EnumSet context() { return ALL_CONTEXTS; } + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ResourceLimitGroupMetadata that = (ResourceLimitGroupMetadata) o; + return Objects.equals(resourceLimitGroups, that.resourceLimitGroups); + } + + @Override + public int hashCode() { + return Objects.hash(resourceLimitGroups); + } + + @Override + public String toString() { + return Strings.toString(MediaTypeRegistry.JSON, this); + } + /** * ResourceLimitGroupMetadataDiff */ static class ResourceLimitGroupMetadataDiff implements NamedDiff { - final Diff> dataStreanDiff; + final Diff> dataStreamDiff; ResourceLimitGroupMetadataDiff(final ResourceLimitGroupMetadata before, final ResourceLimitGroupMetadata after) { - dataStreanDiff = DiffableUtils.diff( - before.resourceLimitGroups, - after.resourceLimitGroups, + dataStreamDiff = DiffableUtils.diff( + toMap(before.resourceLimitGroups), + toMap(after.resourceLimitGroups), DiffableUtils.getStringKeySerializer() ); } ResourceLimitGroupMetadataDiff(final StreamInput in) throws IOException { - this.dataStreanDiff = DiffableUtils.readJdkMapDiff( + this.dataStreamDiff = DiffableUtils.readJdkMapDiff( in, DiffableUtils.getStringKeySerializer(), ResourceLimitGroup::new, @@ -164,6 +171,15 @@ static class ResourceLimitGroupMetadataDiff implements NamedDiff toMap(Set resourceLimitGroups) { + final Map resourceLimitGroupMap = new HashMap<>(); + + resourceLimitGroups.forEach(resourceLimitGroup -> + resourceLimitGroupMap.put(resourceLimitGroup.getName(), resourceLimitGroup) + ); + return resourceLimitGroupMap; + } + /** * Returns the name of the writeable object */ @@ -179,7 +195,7 @@ public String getWriteableName() { */ @Override public void writeTo(StreamOutput out) throws IOException { - dataStreanDiff.writeTo(out); + dataStreamDiff.writeTo(out); } /** @@ -189,25 +205,12 @@ public void writeTo(StreamOutput out) throws IOException { */ @Override public Metadata.Custom apply(Metadata.Custom part) { - return new ResourceLimitGroupMetadata(dataStreanDiff.apply(((ResourceLimitGroupMetadata) part).resourceLimitGroups)); + return new ResourceLimitGroupMetadata( + new HashSet<>( + dataStreamDiff.apply( + toMap( ( (ResourceLimitGroupMetadata) part).resourceLimitGroups) + ).values()) + ); } } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - ResourceLimitGroupMetadata that = (ResourceLimitGroupMetadata) o; - return Objects.equals(resourceLimitGroups, that.resourceLimitGroups); - } - - @Override - public int hashCode() { - return Objects.hash(resourceLimitGroups); - } - - @Override - public String toString() { - return Strings.toString(MediaTypeRegistry.JSON, this); - } } diff --git a/server/src/main/java/org/opensearch/rest/action/search/RestSearchAction.java b/server/src/main/java/org/opensearch/rest/action/search/RestSearchAction.java index 7fa1b38ba95bf..285251abe05aa 100644 --- a/server/src/main/java/org/opensearch/rest/action/search/RestSearchAction.java +++ b/server/src/main/java/org/opensearch/rest/action/search/RestSearchAction.java @@ -222,10 +222,6 @@ public static void parseSearchRequest( ); } - // As part of first phase in enforcing the resource limits on the groups of queries - if (request.hasParam("resource_limit_group_id")) { - searchRequest.resourceLimitGroupId(request.param("resource_limit_group_id")); - } searchRequest.setCancelAfterTimeInterval(request.paramAsTime("cancel_after_time_interval", null)); } diff --git a/server/src/main/java/org/opensearch/search/MultiTenantLabel.java b/server/src/main/java/org/opensearch/search/MultiTenantLabel.java new file mode 100644 index 0000000000000..13234a7a3e662 --- /dev/null +++ b/server/src/main/java/org/opensearch/search/MultiTenantLabel.java @@ -0,0 +1,53 @@ +/* + * 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; + +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; +import java.util.Locale; + +/** + * Enum to hold all multitenant labels in workloads + */ +public enum MultiTenantLabel implements Writeable { + // This label is basically used to define tenancy for multiple features e,g; Query Sandboxing, Query Insights + TENANT_LABEL("tenant_label"); + + private final String value; + MultiTenantLabel(String name) { + this.value = name; + } + + public static MultiTenantLabel fromName(String name) { + switch (name.toLowerCase(Locale.ROOT)) { + // Other cases can be added for other keys in the ENUM + case "tenant_label": + return TENANT_LABEL; + } + throw new IllegalArgumentException("Illegal name + " + name); + } + + public static MultiTenantLabel fromName(StreamInput in) throws IOException { + return fromName(in.readString()); + } + + + /** + * Write this into the {@linkplain StreamOutput}. + * + * @param out + */ + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeString(value); + } +} diff --git a/server/src/main/java/org/opensearch/search/SearchService.java b/server/src/main/java/org/opensearch/search/SearchService.java index 214673bae36f6..4b67f1cdb2e2b 100644 --- a/server/src/main/java/org/opensearch/search/SearchService.java +++ b/server/src/main/java/org/opensearch/search/SearchService.java @@ -158,6 +158,7 @@ import java.util.concurrent.atomic.AtomicLong; import java.util.function.LongSupplier; +import static org.opensearch.action.search.TransportSearchAction.NOT_PROVIDED; import static org.opensearch.common.unit.TimeValue.timeValueHours; import static org.opensearch.common.unit.TimeValue.timeValueMillis; import static org.opensearch.common.unit.TimeValue.timeValueMinutes; @@ -568,7 +569,7 @@ public void executeQueryPhase( assert request.canReturnNullResponseIfMatchNoDocs() == false || request.numberOfShards() > 1 : "empty responses require more than one shard"; final IndexShard shard = getShard(request); - task.setResourceLimitGroupName(request.resourceLimitGroupId()); + setTenantInTask(task, request); rewriteAndFetchShardRequest(shard, request, new ActionListener() { @Override public void onResponse(ShardSearchRequest orig) { @@ -599,6 +600,14 @@ public void onFailure(Exception exc) { }); } + private void setTenantInTask(SearchShardTask task, ShardSearchRequest request) { + String tenant = NOT_PROVIDED; + if (request.source().multiTenantLabels() != null) { + tenant = (String) request.source().multiTenantLabels().get(MultiTenantLabel.TENANT_LABEL.name()); + } + task.setResourceLimitGroupName(tenant); + } + private IndexShard getShard(ShardSearchRequest request) { if (request.readerId() != null) { return findReaderContext(request.readerId(), request).indexShard(); @@ -677,7 +686,7 @@ public void executeQueryPhase( } runAsync(getExecutor(readerContext.indexShard()), () -> { final ShardSearchRequest shardSearchRequest = readerContext.getShardSearchRequest(null); - task.setResourceLimitGroupName(shardSearchRequest.resourceLimitGroupId()); + setTenantInTask(task, shardSearchRequest); try ( SearchContext searchContext = createContext(readerContext, shardSearchRequest, task, false); SearchOperationListenerExecutor executor = new SearchOperationListenerExecutor(searchContext) @@ -780,7 +789,7 @@ public void executeFetchPhase( public void executeFetchPhase(ShardFetchRequest request, SearchShardTask task, ActionListener listener) { final ReaderContext readerContext = findReaderContext(request.contextId(), request); final ShardSearchRequest shardSearchRequest = readerContext.getShardSearchRequest(request.getShardSearchRequest()); - task.setResourceLimitGroupName(shardSearchRequest.resourceLimitGroupId()); + setTenantInTask(task, shardSearchRequest); final Releasable markAsUsed = readerContext.markAsUsed(getKeepAlive(shardSearchRequest)); runAsync(getExecutor(readerContext.indexShard()), () -> { try (SearchContext searchContext = createContext(readerContext, shardSearchRequest, task, false)) { diff --git a/server/src/main/java/org/opensearch/search/builder/SearchSourceBuilder.java b/server/src/main/java/org/opensearch/search/builder/SearchSourceBuilder.java index 182350c22f697..aab5d3687eea0 100644 --- a/server/src/main/java/org/opensearch/search/builder/SearchSourceBuilder.java +++ b/server/src/main/java/org/opensearch/search/builder/SearchSourceBuilder.java @@ -77,11 +77,7 @@ import org.opensearch.search.suggest.SuggestBuilder; import java.io.IOException; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.Objects; +import java.util.*; import static org.opensearch.index.query.AbstractQueryBuilder.parseInnerQueryBuilder; import static org.opensearch.search.internal.SearchContext.TRACK_TOTAL_HITS_ACCURATE; @@ -136,6 +132,7 @@ public final class SearchSourceBuilder implements Writeable, ToXContentObject, R public static final ParseField SLICE = new ParseField("slice"); public static final ParseField POINT_IN_TIME = new ParseField("pit"); public static final ParseField SEARCH_PIPELINE = new ParseField("search_pipeline"); + public static final ParseField MULTI_TENANT_LABELS = new ParseField("multitenant_attrs"); public static SearchSourceBuilder fromXContent(XContentParser parser) throws IOException { return fromXContent(parser, true); @@ -223,6 +220,7 @@ public static HighlightBuilder highlight() { private PointInTimeBuilder pointInTimeBuilder = null; private Map searchPipelineSource = null; + private Map multiTenantLabels = new HashMap<>(); /** * Constructs a new search source builder. @@ -297,6 +295,10 @@ public SearchSourceBuilder(StreamInput in) throws IOException { derivedFields = in.readList(DerivedField::new); } } + + if (in.getVersion().onOrAfter(Version.V_2_14_0)) { + multiTenantLabels = in.readMap(); + } } @Override @@ -377,6 +379,10 @@ public void writeTo(StreamOutput out) throws IOException { out.writeList(derivedFields); } } + + if (out.getVersion().onOrAfter(Version.V_2_14_0)) { + out.writeMap(multiTenantLabels); + } } /** @@ -1088,6 +1094,14 @@ public SearchSourceBuilder searchPipelineSource(Map searchPipeli return this; } + /** + * + * @return {@code } pairs + */ + public Map multiTenantLabels() { + return multiTenantLabels; + } + /** * Rewrites this search source builder into its primitive form. e.g. by * rewriting the QueryBuilder. If the builder did not change the identity @@ -1334,6 +1348,8 @@ public void parseXContent(XContentParser parser, boolean checkTrailingTokens) th searchPipelineSource = parser.mapOrdered(); } else if (DERIVED_FIELDS_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { derivedFieldsObject = parser.map(); + } else if (MULTI_TENANT_LABELS.match(currentFieldName, parser.getDeprecationHandler())) { + multiTenantLabels = parser.map(); } else { throw new ParsingException( parser.getTokenLocation(), diff --git a/server/src/main/java/org/opensearch/search/internal/ShardSearchRequest.java b/server/src/main/java/org/opensearch/search/internal/ShardSearchRequest.java index cf661c7243bb8..de1d5fb8b4098 100644 --- a/server/src/main/java/org/opensearch/search/internal/ShardSearchRequest.java +++ b/server/src/main/java/org/opensearch/search/internal/ShardSearchRequest.java @@ -112,7 +112,6 @@ public class ShardSearchRequest extends TransportRequest implements IndicesReque private SearchSourceBuilder source; private final ShardSearchContextId readerId; private final TimeValue keepAlive; - private String resourceLimitGroupId; public ShardSearchRequest( OriginalIndices originalIndices, @@ -267,9 +266,6 @@ public ShardSearchRequest(StreamInput in) throws IOException { readerId = in.readOptionalWriteable(ShardSearchContextId::new); keepAlive = in.readOptionalTimeValue(); originalIndices = OriginalIndices.readOriginalIndices(in); - if (in.getVersion().onOrAfter(Version.V_3_0_0)) { - resourceLimitGroupId = in.readOptionalString(); - } assert keepAlive == null || readerId != null : "readerId: " + readerId + " keepAlive: " + keepAlive; } @@ -294,7 +290,6 @@ public ShardSearchRequest(ShardSearchRequest clone) { this.originalIndices = clone.originalIndices; this.readerId = clone.readerId; this.keepAlive = clone.keepAlive; - this.resourceLimitGroupId = clone.resourceLimitGroupId; } @Override @@ -302,9 +297,6 @@ public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); innerWriteTo(out, false); OriginalIndices.writeOriginalIndices(originalIndices, out); - if (out.getVersion().onOrAfter(Version.V_3_0_0)) { - out.writeOptionalString(resourceLimitGroupId); - } } protected final void innerWriteTo(StreamOutput out, boolean asKey) throws IOException { @@ -433,14 +425,6 @@ public String preference() { return preference; } - public String resourceLimitGroupId() { - return resourceLimitGroupId; - } - - public void setResourceLimitGroupId(String resourceLimitGroupId) { - this.resourceLimitGroupId = resourceLimitGroupId; - } - /** * Sets the bottom sort values that can be used by the searcher to filter documents * that are after it. This value is computed by coordinating nodes that throttles the diff --git a/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupService.java b/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupService.java index 98103e9a599e2..2684f49e12b0c 100644 --- a/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupService.java +++ b/server/src/main/java/org/opensearch/search/resource_limit_group/ResourceLimitGroupService.java @@ -26,17 +26,16 @@ public class ResourceLimitGroupService extends AbstractLifecycleComponent { private static final Logger logger = LogManager.getLogger(ResourceLimitGroupService.class); private final ResourceLimitGroupResourceUsageTracker requestTracker; - private final ResourceLimitGroupTaskCanceller requestCanceller; + private final ResourceLimitGroupTaskCanceller taskCanceller; private final ResourceLimitGroupPruner resourceLimitGroupPruner; private volatile Scheduler.Cancellable scheduledFuture; private final ResourceLimitGroupServiceSettings sandboxServiceSettings; private final ThreadPool threadPool; - private final ResourceLimitGroupTaskCanceller taskCanceller; /** * Guice managed constructor * @param requestTrackerService - * @param requestCanceller + * @param taskCanceller * @param resourceLimitGroupPruner * @param sandboxServiceSettings * @param threadPool @@ -44,18 +43,16 @@ public class ResourceLimitGroupService extends AbstractLifecycleComponent { @Inject public ResourceLimitGroupService( ResourceLimitGroupResourceUsageTracker requestTrackerService, - ResourceLimitGroupTaskCanceller requestCanceller, + ResourceLimitGroupTaskCanceller taskCanceller, ResourceLimitGroupPruner resourceLimitGroupPruner, ResourceLimitGroupServiceSettings sandboxServiceSettings, - ThreadPool threadPool, - ResourceLimitGroupTaskCanceller taskCanceller + ThreadPool threadPool ) { this.requestTracker = requestTrackerService; - this.requestCanceller = requestCanceller; + this.taskCanceller = taskCanceller; this.resourceLimitGroupPruner = resourceLimitGroupPruner; this.sandboxServiceSettings = sandboxServiceSettings; this.threadPool = threadPool; - this.taskCanceller = taskCanceller; } /** diff --git a/server/src/main/java/org/opensearch/search/resource_limit_group/tracker/ResourceLimitsGroupResourceUsageTrackerService.java b/server/src/main/java/org/opensearch/search/resource_limit_group/tracker/ResourceLimitsGroupResourceUsageTrackerService.java index d91d08b39c903..53440b6871db4 100644 --- a/server/src/main/java/org/opensearch/search/resource_limit_group/tracker/ResourceLimitsGroupResourceUsageTrackerService.java +++ b/server/src/main/java/org/opensearch/search/resource_limit_group/tracker/ResourceLimitsGroupResourceUsageTrackerService.java @@ -28,6 +28,8 @@ import java.util.function.LongSupplier; import java.util.stream.Collectors; +import static org.opensearch.cluster.metadata.ResourceLimitGroup.ALLOWED_RESOURCES; + /** * This class tracks requests per resourceLimitGroups */ @@ -39,7 +41,6 @@ public class ResourceLimitsGroupResourceUsageTrackerService extends ResourceLimi private static final String CPU = "cpu"; private static final String JVM = "jvm"; - private static final List TRACKED_RESOURCES = List.of(JVM); private static final int numberOfAvailableProcessors = Runtime.getRuntime().availableProcessors(); private static final long totalAvailableJvmMemory = Runtime.getRuntime().totalMemory(); private final LongSupplier timeNanosSupplier; @@ -89,7 +90,7 @@ public ResourceLimitsGroupResourceUsageTrackerService( @Override public void updateResourceLimitGroupsResourceUsage() { - activeResourceLimitGroups = new ArrayList<>(clusterService.state().metadata().resourceLimitGroups().values()); + activeResourceLimitGroups = new ArrayList<>(clusterService.state().metadata().resourceLimitGroups()); updateResourceLimitGroupTasks(); @@ -195,7 +196,7 @@ public Map toMap() { } /** - * filter out the deleted sandboxes which still has unfi + * filter out the deleted sandboxes which still has unfinished tasks */ public void pruneResourceLimitGroup() { toDeleteResourceLimitGroups = toDeleteResourceLimitGroups.stream().filter(this::hasUnfinishedTasks).collect(Collectors.toList()); @@ -269,7 +270,7 @@ private List getBreachingResourceLimitGroups() { boolean isBreaching = false; for (ResourceLimitGroup.ResourceLimit resourceLimit : resourceLimitGroup.getResourceLimits()) { - if (currentResourceUsage.get(resourceLimit.getResourceName()) > resourceLimit.getValue()) { + if (currentResourceUsage.get(resourceLimit.getResourceName()) > resourceLimit.getThreshold()) { isBreaching = true; break; } @@ -283,10 +284,10 @@ private List getBreachingResourceLimitGroups() { List getCancellableTasksFrom(ResourceLimitGroup resourceLimitGroup) { List cancellations = new ArrayList<>(); - for (String resource : TRACKED_RESOURCES) { + for (String resource : ALLOWED_RESOURCES) { final double reduceBy = resourceUsage.get(resourceLimitGroup.getName()).get(resource) - resourceLimitGroup.getResourceLimitFor( resource - ).getValue(); + ).getThreshold(); /** * if the resource is not defined for this sandbox then ignore cancellations from it */ diff --git a/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadataTests.java b/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadataTests.java index 77ad49255183f..bccfc5ed8f386 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadataTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/ResourceLimitGroupMetadataTests.java @@ -12,9 +12,7 @@ import org.opensearch.test.AbstractNamedWriteableTestCase; import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.stream.Collectors; +import java.util.Set; import static org.opensearch.cluster.metadata.ResourceLimitGroupTests.createRandomResourceLimitGroup; @@ -40,12 +38,10 @@ protected Class categoryClass() { @Override protected ResourceLimitGroupMetadata createTestInstance() { - Map resourceLimitGroupMap = getRandomResourceLimitGroups().stream() - .collect(Collectors.toMap(ResourceLimitGroup::getName, resourceLimitGroup -> resourceLimitGroup)); - return new ResourceLimitGroupMetadata(resourceLimitGroupMap); + return new ResourceLimitGroupMetadata(getRandomResourceLimitGroups()); } - private List getRandomResourceLimitGroups() { - return List.of(createRandomResourceLimitGroup(), createRandomResourceLimitGroup()); + private Set getRandomResourceLimitGroups() { + return Set.of(createRandomResourceLimitGroup(), createRandomResourceLimitGroup()); } }