From 13dd322c6bcbc54abeffc9cd51588275dbb322d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Vl=C4=8Dek?= Date: Mon, 10 Jun 2024 19:44:48 +0200 Subject: [PATCH 1/7] Small fix in jvmStats test (#14075) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removing second use of the `frequentyl()` function. This function is alraedy used in the beginning of the testing block immediately after the declaration of `jvmStats` variable. The second use of the `frequently()` could cause all the other variables that were carefully prepared for the constructor to not be used at all. Signed-off-by: Lukáš Vlček --- .../cluster/node/stats/NodeStatsTests.java | 30 +++++++++---------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java index 14bfc85abac16..f7bc96bdfe769 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java @@ -682,24 +682,22 @@ public static NodeStats createNodeStats(boolean remoteStoreStats) throws IOExcep ); } JvmStats.Classes classes = new JvmStats.Classes(randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong()); - jvmStats = frequently() - ? new JvmStats( + jvmStats = new JvmStats( + randomNonNegativeLong(), + randomNonNegativeLong(), + new JvmStats.Mem( randomNonNegativeLong(), randomNonNegativeLong(), - new JvmStats.Mem( - randomNonNegativeLong(), - randomNonNegativeLong(), - randomNonNegativeLong(), - randomNonNegativeLong(), - randomNonNegativeLong(), - memoryPools - ), - threads, - garbageCollectors, - randomBoolean() ? Collections.emptyList() : bufferPoolList, - classes - ) - : null; + randomNonNegativeLong(), + randomNonNegativeLong(), + randomNonNegativeLong(), + memoryPools + ), + threads, + garbageCollectors, + randomBoolean() ? Collections.emptyList() : bufferPoolList, + classes + ); } ThreadPoolStats threadPoolStats = null; if (frequently()) { From 8b1a4b3cbf84b07579746d241ae2e0d7ca1dfd16 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 10 Jun 2024 12:45:59 -0500 Subject: [PATCH 2/7] Bump tim-actions/get-pr-commits from 1.1.0 to 1.3.1 (#14126) * Bump tim-actions/get-pr-commits from 1.1.0 to 1.3.1 Bumps [tim-actions/get-pr-commits](https://github.com/tim-actions/get-pr-commits) from 1.1.0 to 1.3.1. - [Release notes](https://github.com/tim-actions/get-pr-commits/releases) - [Commits](https://github.com/tim-actions/get-pr-commits/compare/v1.1.0...v1.3.1) --- updated-dependencies: - dependency-name: tim-actions/get-pr-commits dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] * Update changelog Signed-off-by: dependabot[bot] --------- Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] --- .github/workflows/dco.yml | 2 +- CHANGELOG.md | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/dco.yml b/.github/workflows/dco.yml index 9580d510fd108..ef842bb405d60 100644 --- a/.github/workflows/dco.yml +++ b/.github/workflows/dco.yml @@ -9,7 +9,7 @@ jobs: steps: - name: Get PR Commits id: 'get-pr-commits' - uses: tim-actions/get-pr-commits@v1.1.0 + uses: tim-actions/get-pr-commits@v1.3.1 with: token: ${{ secrets.GITHUB_TOKEN }} - name: DCO Check diff --git a/CHANGELOG.md b/CHANGELOG.md index 801ae522b7c3a..6b53ac305935b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Bump `com.netflix.nebula.ospackage-base` from 11.9.0 to 11.9.1 ([#13933](https://github.com/opensearch-project/OpenSearch/pull/13933)) - Update to Apache Lucene 9.11.0 ([#14042](https://github.com/opensearch-project/OpenSearch/pull/14042)) - Bump `com.azure:azure-core-http-netty` from 1.12.8 to 1.15.1 ([#14128](https://github.com/opensearch-project/OpenSearch/pull/14128)) +- Bump `tim-actions/get-pr-commits` from 1.1.0 to 1.3.1 ([#14126](https://github.com/opensearch-project/OpenSearch/pull/14126)) ### Changed - Add ability for Boolean and date field queries to run when only doc_values are enabled ([#11650](https://github.com/opensearch-project/OpenSearch/pull/11650)) From 04a417a7328ef7d4c501508703898ff2b477f302 Mon Sep 17 00:00:00 2001 From: Chenyang Ji Date: Mon, 10 Jun 2024 11:47:39 -0700 Subject: [PATCH 3/7] consume query level cpu and memory usage in query insights (#13739) * consume query level cpu and memory usage in query insights Signed-off-by: Chenyang Ji * handle failed requests metrics in query insights Signed-off-by: Chenyang Ji * refactor the code to make it more maintainable Signed-off-by: Chenyang Ji --------- Signed-off-by: Chenyang Ji --- CHANGELOG.md | 1 + .../plugin/insights/QueryInsightsPlugin.java | 10 +- .../QueryInsightsExporterFactory.java | 4 +- .../core/listener/QueryInsightsListener.java | 89 +++++++--- .../core/service/QueryInsightsService.java | 88 +++++++++- .../core/service/TopQueriesService.java | 9 +- .../insights/rules/model/Attribute.java | 4 + .../insights/rules/model/MetricType.java | 12 +- .../TransportTopQueriesAction.java | 37 ++-- .../settings/QueryInsightsSettings.java | 159 +++++++++++++++++- .../insights/QueryInsightsPluginTests.java | 16 +- .../insights/QueryInsightsTestUtils.java | 24 ++- .../listener/QueryInsightsListenerTests.java | 48 ++++-- .../service/QueryInsightsServiceTests.java | 4 +- .../rules/model/SearchQueryRecordTests.java | 2 +- 15 files changed, 415 insertions(+), 92 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b53ac305935b..6df084d8f2572 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [Query Insights] Add X-Opaque-Id to search request metadata for top n queries ([#13374](https://github.com/opensearch-project/OpenSearch/pull/13374)) - Add support for query level resource usage tracking ([#13172](https://github.com/opensearch-project/OpenSearch/pull/13172)) - Move Remote Store Migration from DocRep to GA and modify remote migration settings name ([#14100](https://github.com/opensearch-project/OpenSearch/pull/14100)) +- [Query Insights] Add cpu and memory metrics to top n queries ([#13739](https://github.com/opensearch-project/OpenSearch/pull/13739)) ### Dependencies - Bump `com.github.spullara.mustache.java:compiler` from 0.9.10 to 0.9.13 ([#13329](https://github.com/opensearch-project/OpenSearch/pull/13329), [#13559](https://github.com/opensearch-project/OpenSearch/pull/13559)) diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/QueryInsightsPlugin.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/QueryInsightsPlugin.java index 22831c3e0f8ba..bba676436c39a 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/QueryInsightsPlugin.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/QueryInsightsPlugin.java @@ -111,7 +111,15 @@ public List> getSettings() { QueryInsightsSettings.TOP_N_LATENCY_QUERIES_ENABLED, QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE, QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE, - QueryInsightsSettings.TOP_N_LATENCY_EXPORTER_SETTINGS + QueryInsightsSettings.TOP_N_LATENCY_EXPORTER_SETTINGS, + QueryInsightsSettings.TOP_N_CPU_QUERIES_ENABLED, + QueryInsightsSettings.TOP_N_CPU_QUERIES_SIZE, + QueryInsightsSettings.TOP_N_CPU_QUERIES_WINDOW_SIZE, + QueryInsightsSettings.TOP_N_CPU_EXPORTER_SETTINGS, + QueryInsightsSettings.TOP_N_MEMORY_QUERIES_ENABLED, + QueryInsightsSettings.TOP_N_MEMORY_QUERIES_SIZE, + QueryInsightsSettings.TOP_N_MEMORY_QUERIES_WINDOW_SIZE, + QueryInsightsSettings.TOP_N_MEMORY_EXPORTER_SETTINGS ); } } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterFactory.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterFactory.java index 7324590c9f582..016911761a3d0 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterFactory.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterFactory.java @@ -19,7 +19,7 @@ import java.util.Locale; import java.util.Set; -import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.DEFAULT_TOP_N_LATENCY_QUERIES_INDEX_PATTERN; +import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.DEFAULT_TOP_N_QUERIES_INDEX_PATTERN; import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.DEFAULT_TOP_QUERIES_EXPORTER_TYPE; import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.EXPORTER_TYPE; import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.EXPORT_INDEX; @@ -71,7 +71,7 @@ public void validateExporterConfig(final Settings settings) throws IllegalArgume } switch (type) { case LOCAL_INDEX: - final String indexPattern = settings.get(EXPORT_INDEX, DEFAULT_TOP_N_LATENCY_QUERIES_INDEX_PATTERN); + final String indexPattern = settings.get(EXPORT_INDEX, DEFAULT_TOP_N_QUERIES_INDEX_PATTERN); if (indexPattern.length() == 0) { throw new IllegalArgumentException("Empty index pattern configured for the exporter"); } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java index cad2fe374f1b6..a1f810ad5987c 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java @@ -14,8 +14,10 @@ import org.opensearch.action.search.SearchRequest; import org.opensearch.action.search.SearchRequestContext; import org.opensearch.action.search.SearchRequestOperationsListener; +import org.opensearch.action.search.SearchTask; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.inject.Inject; +import org.opensearch.core.tasks.resourcetracker.TaskResourceInfo; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.plugin.insights.core.service.QueryInsightsService; import org.opensearch.plugin.insights.rules.model.Attribute; @@ -25,13 +27,14 @@ import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Locale; import java.util.Map; import java.util.concurrent.TimeUnit; -import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_ENABLED; -import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE; -import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE; +import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.getTopNEnabledSetting; +import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.getTopNSizeSetting; +import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.getTopNWindowSizeSetting; /** * The listener for query insights services. @@ -46,6 +49,7 @@ public final class QueryInsightsListener extends SearchRequestOperationsListener private static final Logger log = LogManager.getLogger(QueryInsightsListener.class); private final QueryInsightsService queryInsightsService; + private final ClusterService clusterService; /** * Constructor for QueryInsightsListener @@ -55,26 +59,32 @@ public final class QueryInsightsListener extends SearchRequestOperationsListener */ @Inject public QueryInsightsListener(final ClusterService clusterService, final QueryInsightsService queryInsightsService) { + this.clusterService = clusterService; this.queryInsightsService = queryInsightsService; - clusterService.getClusterSettings() - .addSettingsUpdateConsumer(TOP_N_LATENCY_QUERIES_ENABLED, v -> this.setEnableTopQueries(MetricType.LATENCY, v)); - clusterService.getClusterSettings() - .addSettingsUpdateConsumer( - TOP_N_LATENCY_QUERIES_SIZE, - v -> this.queryInsightsService.getTopQueriesService(MetricType.LATENCY).setTopNSize(v), - v -> this.queryInsightsService.getTopQueriesService(MetricType.LATENCY).validateTopNSize(v) - ); - clusterService.getClusterSettings() - .addSettingsUpdateConsumer( - TOP_N_LATENCY_QUERIES_WINDOW_SIZE, - v -> this.queryInsightsService.getTopQueriesService(MetricType.LATENCY).setWindowSize(v), - v -> this.queryInsightsService.getTopQueriesService(MetricType.LATENCY).validateWindowSize(v) - ); - this.setEnableTopQueries(MetricType.LATENCY, clusterService.getClusterSettings().get(TOP_N_LATENCY_QUERIES_ENABLED)); - this.queryInsightsService.getTopQueriesService(MetricType.LATENCY) - .setTopNSize(clusterService.getClusterSettings().get(TOP_N_LATENCY_QUERIES_SIZE)); - this.queryInsightsService.getTopQueriesService(MetricType.LATENCY) - .setWindowSize(clusterService.getClusterSettings().get(TOP_N_LATENCY_QUERIES_WINDOW_SIZE)); + // Setting endpoints set up for top n queries, including enabling top n queries, window size and top n size + // Expected metricTypes are Latency, CPU and Memory. + for (MetricType type : MetricType.allMetricTypes()) { + clusterService.getClusterSettings() + .addSettingsUpdateConsumer(getTopNEnabledSetting(type), v -> this.setEnableTopQueries(type, v)); + clusterService.getClusterSettings() + .addSettingsUpdateConsumer( + getTopNSizeSetting(type), + v -> this.queryInsightsService.setTopNSize(type, v), + v -> this.queryInsightsService.validateTopNSize(type, v) + ); + clusterService.getClusterSettings() + .addSettingsUpdateConsumer( + getTopNWindowSizeSetting(type), + v -> this.queryInsightsService.setWindowSize(type, v), + v -> this.queryInsightsService.validateWindowSize(type, v) + ); + + this.setEnableTopQueries(type, clusterService.getClusterSettings().get(getTopNEnabledSetting(type))); + this.queryInsightsService.validateTopNSize(type, clusterService.getClusterSettings().get(getTopNSizeSetting(type))); + this.queryInsightsService.setTopNSize(type, clusterService.getClusterSettings().get(getTopNSizeSetting(type))); + this.queryInsightsService.validateWindowSize(type, clusterService.getClusterSettings().get(getTopNWindowSizeSetting(type))); + this.queryInsightsService.setWindowSize(type, clusterService.getClusterSettings().get(getTopNWindowSizeSetting(type))); + } } /** @@ -124,6 +134,27 @@ public void onRequestStart(SearchRequestContext searchRequestContext) {} @Override public void onRequestEnd(final SearchPhaseContext context, final SearchRequestContext searchRequestContext) { + constructSearchQueryRecord(context, searchRequestContext); + } + + @Override + public void onRequestFailure(final SearchPhaseContext context, final SearchRequestContext searchRequestContext) { + constructSearchQueryRecord(context, searchRequestContext); + } + + private void constructSearchQueryRecord(final SearchPhaseContext context, final SearchRequestContext searchRequestContext) { + SearchTask searchTask = context.getTask(); + List tasksResourceUsages = searchRequestContext.getPhaseResourceUsage(); + tasksResourceUsages.add( + new TaskResourceInfo( + searchTask.getAction(), + searchTask.getId(), + searchTask.getParentTaskId().getId(), + clusterService.localNode().getId(), + searchTask.getTotalResourceStats() + ) + ); + final SearchRequest request = context.getRequest(); try { Map measurements = new HashMap<>(); @@ -133,12 +164,25 @@ public void onRequestEnd(final SearchPhaseContext context, final SearchRequestCo TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - searchRequestContext.getAbsoluteStartNanos()) ); } + if (queryInsightsService.isCollectionEnabled(MetricType.CPU)) { + measurements.put( + MetricType.CPU, + tasksResourceUsages.stream().map(a -> a.getTaskResourceUsage().getCpuTimeInNanos()).mapToLong(Long::longValue).sum() + ); + } + if (queryInsightsService.isCollectionEnabled(MetricType.MEMORY)) { + measurements.put( + MetricType.MEMORY, + tasksResourceUsages.stream().map(a -> a.getTaskResourceUsage().getMemoryInBytes()).mapToLong(Long::longValue).sum() + ); + } Map attributes = new HashMap<>(); attributes.put(Attribute.SEARCH_TYPE, request.searchType().toString().toLowerCase(Locale.ROOT)); attributes.put(Attribute.SOURCE, request.source().toString(FORMAT_PARAMS)); attributes.put(Attribute.TOTAL_SHARDS, context.getNumShards()); attributes.put(Attribute.INDICES, request.indices()); attributes.put(Attribute.PHASE_LATENCY_MAP, searchRequestContext.phaseTookMap()); + attributes.put(Attribute.TASK_RESOURCE_USAGES, tasksResourceUsages); Map labels = new HashMap<>(); // Retrieve user provided label if exists @@ -154,4 +198,5 @@ public void onRequestEnd(final SearchPhaseContext context, final SearchRequestCo log.error(String.format(Locale.ROOT, "fail to ingest query insight data, error: %s", e)); } } + } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java index a83bb2094f165..c63430a1a726c 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java @@ -12,6 +12,8 @@ import org.opensearch.common.inject.Inject; import org.opensearch.common.lifecycle.AbstractLifecycleComponent; import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.unit.TimeValue; import org.opensearch.plugin.insights.core.exporter.QueryInsightsExporterFactory; import org.opensearch.plugin.insights.rules.model.MetricType; import org.opensearch.plugin.insights.rules.model.SearchQueryRecord; @@ -27,7 +29,7 @@ import java.util.Map; import java.util.concurrent.LinkedBlockingQueue; -import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_N_LATENCY_EXPORTER_SETTINGS; +import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.getExporterSettings; /** * Service responsible for gathering, analyzing, storing and exporting @@ -86,11 +88,13 @@ public QueryInsightsService(final ClusterSettings clusterSettings, final ThreadP enableCollect.put(metricType, false); topQueriesServices.put(metricType, new TopQueriesService(metricType, threadPool, queryInsightsExporterFactory)); } - clusterSettings.addSettingsUpdateConsumer( - TOP_N_LATENCY_EXPORTER_SETTINGS, - (settings -> getTopQueriesService(MetricType.LATENCY).setExporter(settings)), - (settings -> getTopQueriesService(MetricType.LATENCY).validateExporterConfig(settings)) - ); + for (MetricType type : MetricType.allMetricTypes()) { + clusterSettings.addSettingsUpdateConsumer( + getExporterSettings(type), + (settings -> setExporter(type, settings)), + (settings -> validateExporterConfig(type, settings)) + ); + } } /** @@ -177,6 +181,78 @@ public boolean isEnabled() { return false; } + /** + * Validate the window size config for a metricType + * + * @param type {@link MetricType} + * @param windowSize {@link TimeValue} + */ + public void validateWindowSize(final MetricType type, final TimeValue windowSize) { + if (topQueriesServices.containsKey(type)) { + topQueriesServices.get(type).validateWindowSize(windowSize); + } + } + + /** + * Set window size for a metricType + * + * @param type {@link MetricType} + * @param windowSize {@link TimeValue} + */ + public void setWindowSize(final MetricType type, final TimeValue windowSize) { + if (topQueriesServices.containsKey(type)) { + topQueriesServices.get(type).setWindowSize(windowSize); + } + } + + /** + * Validate the top n size config for a metricType + * + * @param type {@link MetricType} + * @param topNSize top n size + */ + public void validateTopNSize(final MetricType type, final int topNSize) { + if (topQueriesServices.containsKey(type)) { + topQueriesServices.get(type).validateTopNSize(topNSize); + } + } + + /** + * Set the top n size config for a metricType + * + * @param type {@link MetricType} + * @param topNSize top n size + */ + public void setTopNSize(final MetricType type, final int topNSize) { + if (topQueriesServices.containsKey(type)) { + topQueriesServices.get(type).setTopNSize(topNSize); + } + } + + /** + * Set the exporter config for a metricType + * + * @param type {@link MetricType} + * @param settings exporter settings + */ + public void setExporter(final MetricType type, final Settings settings) { + if (topQueriesServices.containsKey(type)) { + topQueriesServices.get(type).setExporter(settings); + } + } + + /** + * Validate the exporter config for a metricType + * + * @param type {@link MetricType} + * @param settings exporter settings + */ + public void validateExporterConfig(final MetricType type, final Settings settings) { + if (topQueriesServices.containsKey(type)) { + topQueriesServices.get(type).validateExporterConfig(settings); + } + } + @Override protected void doStart() { if (isEnabled()) { diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesService.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesService.java index ff90edf1ec33d..c21b89be4dcca 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesService.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesService.java @@ -35,7 +35,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.DEFAULT_TOP_N_LATENCY_QUERIES_INDEX_PATTERN; +import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.DEFAULT_TOP_N_QUERIES_INDEX_PATTERN; import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.DEFAULT_TOP_QUERIES_EXPORTER_TYPE; import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.EXPORTER_TYPE; import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.EXPORT_INDEX; @@ -218,10 +218,7 @@ public void setExporter(final Settings settings) { if (settings.get(EXPORTER_TYPE) != null) { SinkType expectedType = SinkType.parse(settings.get(EXPORTER_TYPE, DEFAULT_TOP_QUERIES_EXPORTER_TYPE)); if (exporter != null && expectedType == SinkType.getSinkTypeFromExporter(exporter)) { - queryInsightsExporterFactory.updateExporter( - exporter, - settings.get(EXPORT_INDEX, DEFAULT_TOP_N_LATENCY_QUERIES_INDEX_PATTERN) - ); + queryInsightsExporterFactory.updateExporter(exporter, settings.get(EXPORT_INDEX, DEFAULT_TOP_N_QUERIES_INDEX_PATTERN)); } else { try { queryInsightsExporterFactory.closeExporter(this.exporter); @@ -230,7 +227,7 @@ public void setExporter(final Settings settings) { } this.exporter = queryInsightsExporterFactory.createExporter( SinkType.parse(settings.get(EXPORTER_TYPE, DEFAULT_TOP_QUERIES_EXPORTER_TYPE)), - settings.get(EXPORT_INDEX, DEFAULT_TOP_N_LATENCY_QUERIES_INDEX_PATTERN) + settings.get(EXPORT_INDEX, DEFAULT_TOP_N_QUERIES_INDEX_PATTERN) ); } } else { diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/Attribute.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/Attribute.java index 7ee4883c54023..dcdb085fdc6fa 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/Attribute.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/Attribute.java @@ -44,6 +44,10 @@ public enum Attribute { * The node id for this request */ NODE_ID, + /** + * Tasks level resource usages in this request + */ + TASK_RESOURCE_USAGES, /** * Custom search request labels */ diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/MetricType.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/MetricType.java index cdd090fbf4804..4694c757f4ef2 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/MetricType.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/model/MetricType.java @@ -35,7 +35,7 @@ public enum MetricType implements Comparator { /** * JVM heap usage metric type */ - JVM; + MEMORY; /** * Read a MetricType from a StreamInput @@ -93,10 +93,9 @@ public static Set allMetricTypes() { public int compare(final Number a, final Number b) { switch (this) { case LATENCY: - return Long.compare(a.longValue(), b.longValue()); - case JVM: case CPU: - return Double.compare(a.doubleValue(), b.doubleValue()); + case MEMORY: + return Long.compare(a.longValue(), b.longValue()); } return -1; } @@ -110,10 +109,9 @@ public int compare(final Number a, final Number b) { Number parseValue(final Object o) { switch (this) { case LATENCY: - return (Long) o; - case JVM: case CPU: - return (Double) o; + case MEMORY: + return (Long) o; default: return (Number) o; } diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java index ddf614211bc41..7949b70a16db6 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java @@ -8,7 +8,6 @@ package org.opensearch.plugin.insights.rules.transport.top_queries; -import org.opensearch.OpenSearchException; import org.opensearch.action.FailedNodeException; import org.opensearch.action.support.ActionFilters; import org.opensearch.action.support.nodes.TransportNodesAction; @@ -21,7 +20,6 @@ import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesAction; import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesRequest; import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesResponse; -import org.opensearch.plugin.insights.rules.model.MetricType; import org.opensearch.plugin.insights.settings.QueryInsightsSettings; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.TransportRequest; @@ -29,7 +27,6 @@ import java.io.IOException; import java.util.List; -import java.util.Locale; /** * Transport action for cluster/node level top queries information. @@ -81,17 +78,18 @@ protected TopQueriesResponse newResponse( final List responses, final List failures ) { - if (topQueriesRequest.getMetricType() == MetricType.LATENCY) { - return new TopQueriesResponse( - clusterService.getClusterName(), - responses, - failures, - clusterService.getClusterSettings().get(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE), - MetricType.LATENCY - ); - } else { - throw new OpenSearchException(String.format(Locale.ROOT, "invalid metric type %s", topQueriesRequest.getMetricType())); + int size; + switch (topQueriesRequest.getMetricType()) { + case CPU: + size = clusterService.getClusterSettings().get(QueryInsightsSettings.TOP_N_CPU_QUERIES_SIZE); + break; + case MEMORY: + size = clusterService.getClusterSettings().get(QueryInsightsSettings.TOP_N_MEMORY_QUERIES_SIZE); + break; + default: + size = clusterService.getClusterSettings().get(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE); } + return new TopQueriesResponse(clusterService.getClusterName(), responses, failures, size, topQueriesRequest.getMetricType()); } @Override @@ -107,15 +105,10 @@ protected TopQueries newNodeResponse(final StreamInput in) throws IOException { @Override protected TopQueries nodeOperation(final NodeRequest nodeRequest) { final TopQueriesRequest request = nodeRequest.request; - if (request.getMetricType() == MetricType.LATENCY) { - return new TopQueries( - clusterService.localNode(), - queryInsightsService.getTopQueriesService(MetricType.LATENCY).getTopQueriesRecords(true) - ); - } else { - throw new OpenSearchException(String.format(Locale.ROOT, "invalid metric type %s", request.getMetricType())); - } - + return new TopQueries( + clusterService.localNode(), + queryInsightsService.getTopQueriesService(request.getMetricType()).getTopQueriesRecords(true) + ); } /** diff --git a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/QueryInsightsSettings.java b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/QueryInsightsSettings.java index b2e01062e334c..25309b5721792 100644 --- a/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/QueryInsightsSettings.java +++ b/plugins/query-insights/src/main/java/org/opensearch/plugin/insights/settings/QueryInsightsSettings.java @@ -12,6 +12,7 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; import org.opensearch.plugin.insights.core.exporter.SinkType; +import org.opensearch.plugin.insights.rules.model.MetricType; import java.util.Arrays; import java.util.HashSet; @@ -81,6 +82,10 @@ public class QueryInsightsSettings { public static final String TOP_N_QUERIES_SETTING_PREFIX = "search.insights.top_queries"; /** Default prefix for top N queries by latency feature */ public static final String TOP_N_LATENCY_QUERIES_PREFIX = TOP_N_QUERIES_SETTING_PREFIX + ".latency"; + /** Default prefix for top N queries by cpu feature */ + public static final String TOP_N_CPU_QUERIES_PREFIX = TOP_N_QUERIES_SETTING_PREFIX + ".cpu"; + /** Default prefix for top N queries by memory feature */ + public static final String TOP_N_MEMORY_QUERIES_PREFIX = TOP_N_QUERIES_SETTING_PREFIX + ".memory"; /** * Boolean setting for enabling top queries by latency. */ @@ -111,6 +116,66 @@ public class QueryInsightsSettings { Setting.Property.Dynamic ); + /** + * Boolean setting for enabling top queries by cpu. + */ + public static final Setting TOP_N_CPU_QUERIES_ENABLED = Setting.boolSetting( + TOP_N_CPU_QUERIES_PREFIX + ".enabled", + false, + Setting.Property.Dynamic, + Setting.Property.NodeScope + ); + + /** + * Int setting to define the top n size for top queries by cpu. + */ + public static final Setting TOP_N_CPU_QUERIES_SIZE = Setting.intSetting( + TOP_N_CPU_QUERIES_PREFIX + ".top_n_size", + DEFAULT_TOP_N_SIZE, + Setting.Property.Dynamic, + Setting.Property.NodeScope + ); + + /** + * Time setting to define the window size in seconds for top queries by cpu. + */ + public static final Setting TOP_N_CPU_QUERIES_WINDOW_SIZE = Setting.positiveTimeSetting( + TOP_N_CPU_QUERIES_PREFIX + ".window_size", + DEFAULT_WINDOW_SIZE, + Setting.Property.NodeScope, + Setting.Property.Dynamic + ); + + /** + * Boolean setting for enabling top queries by memory. + */ + public static final Setting TOP_N_MEMORY_QUERIES_ENABLED = Setting.boolSetting( + TOP_N_MEMORY_QUERIES_PREFIX + ".enabled", + false, + Setting.Property.Dynamic, + Setting.Property.NodeScope + ); + + /** + * Int setting to define the top n size for top queries by memory. + */ + public static final Setting TOP_N_MEMORY_QUERIES_SIZE = Setting.intSetting( + TOP_N_MEMORY_QUERIES_PREFIX + ".top_n_size", + DEFAULT_TOP_N_SIZE, + Setting.Property.Dynamic, + Setting.Property.NodeScope + ); + + /** + * Time setting to define the window size in seconds for top queries by memory. + */ + public static final Setting TOP_N_MEMORY_QUERIES_WINDOW_SIZE = Setting.positiveTimeSetting( + TOP_N_MEMORY_QUERIES_PREFIX + ".window_size", + DEFAULT_WINDOW_SIZE, + Setting.Property.NodeScope, + Setting.Property.Dynamic + ); + /** * Config key for exporter type */ @@ -125,9 +190,17 @@ public class QueryInsightsSettings { */ private static final String TOP_N_LATENCY_QUERIES_EXPORTER_PREFIX = TOP_N_LATENCY_QUERIES_PREFIX + ".exporter."; /** - * Default index pattern of top n queries by latency + * Prefix for top n queries by cpu exporters + */ + private static final String TOP_N_CPU_QUERIES_EXPORTER_PREFIX = TOP_N_CPU_QUERIES_PREFIX + ".exporter."; + /** + * Prefix for top n queries by memory exporters */ - public static final String DEFAULT_TOP_N_LATENCY_QUERIES_INDEX_PATTERN = "'top_queries_by_latency-'YYYY.MM.dd"; + private static final String TOP_N_MEMORY_QUERIES_EXPORTER_PREFIX = TOP_N_MEMORY_QUERIES_PREFIX + ".exporter."; + /** + * Default index pattern of top n queries + */ + public static final String DEFAULT_TOP_N_QUERIES_INDEX_PATTERN = "'top_queries-'YYYY.MM.dd"; /** * Default exporter type of top queries */ @@ -142,6 +215,88 @@ public class QueryInsightsSettings { Setting.Property.NodeScope ); + /** + * Settings for the exporter of top cpu queries + */ + public static final Setting TOP_N_CPU_EXPORTER_SETTINGS = Setting.groupSetting( + TOP_N_CPU_QUERIES_EXPORTER_PREFIX, + Setting.Property.Dynamic, + Setting.Property.NodeScope + ); + + /** + * Settings for the exporter of top cpu queries + */ + public static final Setting TOP_N_MEMORY_EXPORTER_SETTINGS = Setting.groupSetting( + TOP_N_MEMORY_QUERIES_EXPORTER_PREFIX, + Setting.Property.Dynamic, + Setting.Property.NodeScope + ); + + /** + * Get the enabled setting based on type + * @param type MetricType + * @return enabled setting + */ + public static Setting getTopNEnabledSetting(MetricType type) { + switch (type) { + case CPU: + return TOP_N_CPU_QUERIES_ENABLED; + case MEMORY: + return TOP_N_MEMORY_QUERIES_ENABLED; + default: + return TOP_N_LATENCY_QUERIES_ENABLED; + } + } + + /** + * Get the top n size setting based on type + * @param type MetricType + * @return top n size setting + */ + public static Setting getTopNSizeSetting(MetricType type) { + switch (type) { + case CPU: + return TOP_N_CPU_QUERIES_SIZE; + case MEMORY: + return TOP_N_MEMORY_QUERIES_SIZE; + default: + return TOP_N_LATENCY_QUERIES_SIZE; + } + } + + /** + * Get the window size setting based on type + * @param type MetricType + * @return top n queries window size setting + */ + public static Setting getTopNWindowSizeSetting(MetricType type) { + switch (type) { + case CPU: + return TOP_N_CPU_QUERIES_WINDOW_SIZE; + case MEMORY: + return TOP_N_MEMORY_QUERIES_WINDOW_SIZE; + default: + return TOP_N_LATENCY_QUERIES_WINDOW_SIZE; + } + } + + /** + * Get the exporter settings based on type + * @param type MetricType + * @return exporter setting + */ + public static Setting getExporterSettings(MetricType type) { + switch (type) { + case CPU: + return TOP_N_CPU_EXPORTER_SETTINGS; + case MEMORY: + return TOP_N_MEMORY_EXPORTER_SETTINGS; + default: + return TOP_N_LATENCY_EXPORTER_SETTINGS; + } + } + /** * Default constructor */ diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/QueryInsightsPluginTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/QueryInsightsPluginTests.java index 8b8856e3e305c..2efe9085a39ee 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/QueryInsightsPluginTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/QueryInsightsPluginTests.java @@ -47,11 +47,7 @@ public void setup() { Settings.Builder settingsBuilder = Settings.builder(); Settings settings = settingsBuilder.build(); ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_ENABLED); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_EXPORTER_SETTINGS); - + QueryInsightsTestUtils.registerAllQueryInsightsSettings(clusterSettings); clusterService = ClusterServiceUtils.createClusterService(settings, clusterSettings, threadPool); } @@ -61,7 +57,15 @@ public void testGetSettings() { QueryInsightsSettings.TOP_N_LATENCY_QUERIES_ENABLED, QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE, QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE, - QueryInsightsSettings.TOP_N_LATENCY_EXPORTER_SETTINGS + QueryInsightsSettings.TOP_N_LATENCY_EXPORTER_SETTINGS, + QueryInsightsSettings.TOP_N_CPU_QUERIES_ENABLED, + QueryInsightsSettings.TOP_N_CPU_QUERIES_SIZE, + QueryInsightsSettings.TOP_N_CPU_QUERIES_WINDOW_SIZE, + QueryInsightsSettings.TOP_N_CPU_EXPORTER_SETTINGS, + QueryInsightsSettings.TOP_N_MEMORY_QUERIES_ENABLED, + QueryInsightsSettings.TOP_N_MEMORY_QUERIES_SIZE, + QueryInsightsSettings.TOP_N_MEMORY_QUERIES_WINDOW_SIZE, + QueryInsightsSettings.TOP_N_MEMORY_EXPORTER_SETTINGS ), queryInsightsPlugin.getSettings() ); diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/QueryInsightsTestUtils.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/QueryInsightsTestUtils.java index 870ef5b9c8be9..7fa4e9841c20e 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/QueryInsightsTestUtils.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/QueryInsightsTestUtils.java @@ -10,6 +10,7 @@ import org.opensearch.action.search.SearchType; import org.opensearch.cluster.node.DiscoveryNode; +import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.util.Maps; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; @@ -17,6 +18,7 @@ import org.opensearch.plugin.insights.rules.model.Attribute; import org.opensearch.plugin.insights.rules.model.MetricType; import org.opensearch.plugin.insights.rules.model.SearchQueryRecord; +import org.opensearch.plugin.insights.settings.QueryInsightsSettings; import org.opensearch.test.VersionUtils; import java.io.IOException; @@ -36,7 +38,6 @@ import static org.opensearch.test.OpenSearchTestCase.random; import static org.opensearch.test.OpenSearchTestCase.randomAlphaOfLengthBetween; import static org.opensearch.test.OpenSearchTestCase.randomArray; -import static org.opensearch.test.OpenSearchTestCase.randomDouble; import static org.opensearch.test.OpenSearchTestCase.randomIntBetween; import static org.opensearch.test.OpenSearchTestCase.randomLong; import static org.opensearch.test.OpenSearchTestCase.randomLongBetween; @@ -63,9 +64,9 @@ public static List generateQueryInsightRecords(int lower, int MetricType.LATENCY, randomLongBetween(1000, 10000), MetricType.CPU, - randomDouble(), - MetricType.JVM, - randomDouble() + randomLongBetween(1000, 10000), + MetricType.MEMORY, + randomLongBetween(1000, 10000) ); Map phaseLatencyMap = new HashMap<>(); @@ -186,4 +187,19 @@ public static boolean checkRecordsEqualsWithoutOrder( } return true; } + + public static void registerAllQueryInsightsSettings(ClusterSettings clusterSettings) { + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_ENABLED); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_EXPORTER_SETTINGS); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_CPU_QUERIES_ENABLED); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_CPU_QUERIES_SIZE); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_CPU_QUERIES_WINDOW_SIZE); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_CPU_EXPORTER_SETTINGS); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_MEMORY_QUERIES_ENABLED); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_MEMORY_QUERIES_SIZE); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_MEMORY_QUERIES_WINDOW_SIZE); + clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_MEMORY_EXPORTER_SETTINGS); + } } diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListenerTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListenerTests.java index b794a2e4b8608..86de44c680188 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListenerTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListenerTests.java @@ -13,23 +13,28 @@ import org.opensearch.action.search.SearchRequestContext; import org.opensearch.action.search.SearchTask; import org.opensearch.action.search.SearchType; +import org.opensearch.action.support.replication.ClusterStateCreationUtils; +import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.collect.Tuple; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.common.util.io.IOUtils; +import org.opensearch.core.tasks.TaskId; +import org.opensearch.plugin.insights.QueryInsightsTestUtils; import org.opensearch.plugin.insights.core.service.QueryInsightsService; import org.opensearch.plugin.insights.core.service.TopQueriesService; import org.opensearch.plugin.insights.rules.model.Attribute; import org.opensearch.plugin.insights.rules.model.MetricType; import org.opensearch.plugin.insights.rules.model.SearchQueryRecord; -import org.opensearch.plugin.insights.settings.QueryInsightsSettings; import org.opensearch.search.aggregations.bucket.terms.TermsAggregationBuilder; import org.opensearch.search.aggregations.support.ValueType; import org.opensearch.search.builder.SearchSourceBuilder; import org.opensearch.tasks.Task; import org.opensearch.test.ClusterServiceUtils; import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.threadpool.TestThreadPool; import org.opensearch.threadpool.ThreadPool; import org.junit.Before; @@ -41,6 +46,7 @@ import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Phaser; +import java.util.concurrent.TimeUnit; import org.mockito.ArgumentCaptor; @@ -59,7 +65,7 @@ public class QueryInsightsListenerTests extends OpenSearchTestCase { private final SearchRequest searchRequest = mock(SearchRequest.class); private final QueryInsightsService queryInsightsService = mock(QueryInsightsService.class); private final TopQueriesService topQueriesService = mock(TopQueriesService.class); - private final ThreadPool threadPool = mock(ThreadPool.class); + private final ThreadPool threadPool = new TestThreadPool("QueryInsightsThreadPool"); private ClusterService clusterService; @Before @@ -67,16 +73,22 @@ public void setup() { Settings.Builder settingsBuilder = Settings.builder(); Settings settings = settingsBuilder.build(); ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_ENABLED); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_SIZE); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_QUERIES_WINDOW_SIZE); - clusterService = ClusterServiceUtils.createClusterService(settings, clusterSettings, null); + QueryInsightsTestUtils.registerAllQueryInsightsSettings(clusterSettings); + ClusterState state = ClusterStateCreationUtils.stateWithActivePrimary("test", true, 1 + randomInt(3), randomInt(2)); + clusterService = ClusterServiceUtils.createClusterService(threadPool, state.getNodes().getLocalNode(), clusterSettings); + ClusterServiceUtils.setState(clusterService, state); when(queryInsightsService.isCollectionEnabled(MetricType.LATENCY)).thenReturn(true); when(queryInsightsService.getTopQueriesService(MetricType.LATENCY)).thenReturn(topQueriesService); ThreadContext threadContext = new ThreadContext(Settings.EMPTY); - threadContext.setHeaders(new Tuple<>(Collections.singletonMap(Task.X_OPAQUE_ID, "userLabel"), new HashMap<>())); - when(threadPool.getThreadContext()).thenReturn(threadContext); + threadPool.getThreadContext().setHeaders(new Tuple<>(Collections.singletonMap(Task.X_OPAQUE_ID, "userLabel"), new HashMap<>())); + } + + @Override + public void tearDown() throws Exception { + super.tearDown(); + IOUtils.close(clusterService); + ThreadPool.terminate(threadPool, 10, TimeUnit.SECONDS); } @SuppressWarnings("unchecked") @@ -87,7 +99,14 @@ public void testOnRequestEnd() throws InterruptedException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); searchSourceBuilder.aggregation(new TermsAggregationBuilder("agg1").userValueTypeHint(ValueType.STRING).field("type.keyword")); searchSourceBuilder.size(0); - SearchTask task = new SearchTask(0, "n/a", "n/a", () -> "test", null, Collections.singletonMap(Task.X_OPAQUE_ID, "userLabel")); + SearchTask task = new SearchTask( + 0, + "n/a", + "n/a", + () -> "test", + TaskId.EMPTY_TASK_ID, + Collections.singletonMap(Task.X_OPAQUE_ID, "userLabel") + ); String[] indices = new String[] { "index-1", "index-2" }; @@ -129,7 +148,14 @@ public void testConcurrentOnRequestEnd() throws InterruptedException { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); searchSourceBuilder.aggregation(new TermsAggregationBuilder("agg1").userValueTypeHint(ValueType.STRING).field("type.keyword")); searchSourceBuilder.size(0); - SearchTask task = new SearchTask(0, "n/a", "n/a", () -> "test", null, Collections.singletonMap(Task.X_OPAQUE_ID, "userLabel")); + SearchTask task = new SearchTask( + 0, + "n/a", + "n/a", + () -> "test", + TaskId.EMPTY_TASK_ID, + Collections.singletonMap(Task.X_OPAQUE_ID, "userLabel") + ); String[] indices = new String[] { "index-1", "index-2" }; @@ -184,7 +210,7 @@ public void testSetEnabled() { when(queryInsightsService.isCollectionEnabled(MetricType.LATENCY)).thenReturn(false); when(queryInsightsService.isCollectionEnabled(MetricType.CPU)).thenReturn(false); - when(queryInsightsService.isCollectionEnabled(MetricType.JVM)).thenReturn(false); + when(queryInsightsService.isCollectionEnabled(MetricType.MEMORY)).thenReturn(false); queryInsightsListener.setEnableTopQueries(MetricType.LATENCY, false); assertFalse(queryInsightsListener.isEnabled()); } diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/QueryInsightsServiceTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/QueryInsightsServiceTests.java index 428f615ce2f90..75a5768f50681 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/QueryInsightsServiceTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/core/service/QueryInsightsServiceTests.java @@ -34,11 +34,11 @@ public void setup() { Settings.Builder settingsBuilder = Settings.builder(); Settings settings = settingsBuilder.build(); ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - clusterSettings.registerSetting(QueryInsightsSettings.TOP_N_LATENCY_EXPORTER_SETTINGS); + QueryInsightsTestUtils.registerAllQueryInsightsSettings(clusterSettings); queryInsightsService = new QueryInsightsService(clusterSettings, threadPool, client); queryInsightsService.enableCollection(MetricType.LATENCY, true); queryInsightsService.enableCollection(MetricType.CPU, true); - queryInsightsService.enableCollection(MetricType.JVM, true); + queryInsightsService.enableCollection(MetricType.MEMORY, true); } public void testAddRecordToLimitAndDrain() { diff --git a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecordTests.java b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecordTests.java index 793d5878e2300..ad45b53ec5363 100644 --- a/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecordTests.java +++ b/plugins/query-insights/src/test/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecordTests.java @@ -39,7 +39,7 @@ public void testSerializationAndEquals() throws Exception { public void testAllMetricTypes() { Set allMetrics = MetricType.allMetricTypes(); - Set expected = new HashSet<>(Arrays.asList(MetricType.LATENCY, MetricType.CPU, MetricType.JVM)); + Set expected = new HashSet<>(Arrays.asList(MetricType.LATENCY, MetricType.CPU, MetricType.MEMORY)); assertEquals(expected, allMetrics); } From 1ac9d6f65a5d15cf0a520c99e9e155958e5e2c62 Mon Sep 17 00:00:00 2001 From: Chenyang Ji Date: Mon, 10 Jun 2024 14:00:03 -0700 Subject: [PATCH 4/7] fix japicmp check for threadContext (#14147) Signed-off-by: Chenyang Ji --- .../common/util/concurrent/ThreadContext.java | 30 +++++++++++-------- .../util/concurrent/ThreadContextTests.java | 9 ++---- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContext.java b/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContext.java index b9c7177da828b..906a27e9f398c 100644 --- a/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContext.java +++ b/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContext.java @@ -480,7 +480,7 @@ public T getTransient(String key) { * @param value the header value */ public void addResponseHeader(final String key, final String value) { - updateResponseHeader(key, value, v -> v, false); + addResponseHeader(key, value, v -> v); } /** @@ -490,7 +490,19 @@ public void addResponseHeader(final String key, final String value) { * @param value the header value */ public void updateResponseHeader(final String key, final String value) { - updateResponseHeader(key, value, v -> v, true); + updateResponseHeader(key, value, v -> v); + } + + /** + * Add the {@code value} for the specified {@code key} with the specified {@code uniqueValue} used for de-duplication. Any duplicate + * {@code value} after applying {@code uniqueValue} is ignored. + * + * @param key the header name + * @param value the header value + * @param uniqueValue the function that produces de-duplication values + */ + public void addResponseHeader(final String key, final String value, final Function uniqueValue) { + threadLocal.set(threadLocal.get().putResponse(key, value, uniqueValue, maxWarningHeaderCount, maxWarningHeaderSize, false)); } /** @@ -500,17 +512,9 @@ public void updateResponseHeader(final String key, final String value) { * @param key the header name * @param value the header value * @param uniqueValue the function that produces de-duplication values - * @param replaceExistingKey whether to replace the existing header if it already exists - */ - public void updateResponseHeader( - final String key, - final String value, - final Function uniqueValue, - final boolean replaceExistingKey - ) { - threadLocal.set( - threadLocal.get().putResponse(key, value, uniqueValue, maxWarningHeaderCount, maxWarningHeaderSize, replaceExistingKey) - ); + */ + public void updateResponseHeader(final String key, final String value, final Function uniqueValue) { + threadLocal.set(threadLocal.get().putResponse(key, value, uniqueValue, maxWarningHeaderCount, maxWarningHeaderSize, true)); } /** diff --git a/server/src/test/java/org/opensearch/common/util/concurrent/ThreadContextTests.java b/server/src/test/java/org/opensearch/common/util/concurrent/ThreadContextTests.java index e6d07c5630541..4e66575711046 100644 --- a/server/src/test/java/org/opensearch/common/util/concurrent/ThreadContextTests.java +++ b/server/src/test/java/org/opensearch/common/util/concurrent/ThreadContextTests.java @@ -344,16 +344,11 @@ public void testResponseHeaders() { } final String value = HeaderWarning.formatWarning("qux"); - threadContext.updateResponseHeader("baz", value, s -> HeaderWarning.extractWarningValueFromWarningHeader(s, false), false); + threadContext.updateResponseHeader("baz", value, s -> HeaderWarning.extractWarningValueFromWarningHeader(s, false)); // pretend that another thread created the same response at a different time if (randomBoolean()) { final String duplicateValue = HeaderWarning.formatWarning("qux"); - threadContext.updateResponseHeader( - "baz", - duplicateValue, - s -> HeaderWarning.extractWarningValueFromWarningHeader(s, false), - false - ); + threadContext.updateResponseHeader("baz", duplicateValue, s -> HeaderWarning.extractWarningValueFromWarningHeader(s, false)); } threadContext.addResponseHeader("Warning", "One is the loneliest number"); From c49eca4061d3af9af77a3eacd28043200343ba98 Mon Sep 17 00:00:00 2001 From: Rishabh Maurya Date: Mon, 10 Jun 2024 15:05:46 -0700 Subject: [PATCH 5/7] [Derived Field] Integration tests for derived fields (#13721) * Integration test for derived fields * tests derived fields when defined in index mappings * tests derived fields when defined in query * tests mappings and settings updates for derived fields * tests index and search analyzer applicability on derived fields * tests query string Signed-off-by: Rishabh Maurya * rename source_indexed_field to prefilter_field Signed-off-by: Rishabh Maurya * Add entry to CHANGELOG Signed-off-by: Rishabh Maurya --------- Signed-off-by: Rishabh Maurya --- CHANGELOG.md | 1 + ...derived_field_index_mapping_definition.yml | 421 +++++++++++++++ .../20_derived_field_put_mapping.yml | 123 +++++ .../30_derived_field_search_definition.yml | 489 ++++++++++++++++++ .../40_derived_field_fetch_and_highlight.yml | 279 ++++++++++ .../50_derived_field_default_analyzer.yml | 105 ++++ 6 files changed, 1418 insertions(+) create mode 100644 modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/derived_fields/10_derived_field_index_mapping_definition.yml create mode 100644 modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/derived_fields/20_derived_field_put_mapping.yml create mode 100644 modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/derived_fields/30_derived_field_search_definition.yml create mode 100644 modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/derived_fields/40_derived_field_fetch_and_highlight.yml create mode 100644 modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/derived_fields/50_derived_field_default_analyzer.yml diff --git a/CHANGELOG.md b/CHANGELOG.md index 6df084d8f2572..a05aa09b0f13d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [Query Insights] Add X-Opaque-Id to search request metadata for top n queries ([#13374](https://github.com/opensearch-project/OpenSearch/pull/13374)) - Add support for query level resource usage tracking ([#13172](https://github.com/opensearch-project/OpenSearch/pull/13172)) - Move Remote Store Migration from DocRep to GA and modify remote migration settings name ([#14100](https://github.com/opensearch-project/OpenSearch/pull/14100)) +- Derived field object type support ([#13720](https://github.com/opensearch-project/OpenSearch/pull/13720)) - [Query Insights] Add cpu and memory metrics to top n queries ([#13739](https://github.com/opensearch-project/OpenSearch/pull/13739)) ### Dependencies diff --git a/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/derived_fields/10_derived_field_index_mapping_definition.yml b/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/derived_fields/10_derived_field_index_mapping_definition.yml new file mode 100644 index 0000000000000..4f700c3b83e8f --- /dev/null +++ b/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/derived_fields/10_derived_field_index_mapping_definition.yml @@ -0,0 +1,421 @@ +"Test derived_field supported type using index mapping definition": + - skip: + version: " - 2.14.99" + reason: "derived_field feature was added in 2.15" + + - do: + indices.create: + index: test + body: + mappings: + properties: + text: + type: text + keyword: + type: keyword + long: + type: long + float: + type: float + double: + type: double + date: + type: date + geo: + type: geo_point + ip: + type: ip + boolean: + type: boolean + array_of_long: + type: long + json_field: + type: text + derived: + derived_text: + type: text + script: "emit(params._source[\"text\"])" + derived_text_prefilter_field: + type: text + script: "emit(params._source[\"text\"])" + prefilter_field: "text" + derived_keyword: + type: keyword + script: "emit(params._source[\"keyword\"])" + derived_long: + type: long + script: "emit(params._source[\"long\"])" + derived_float: + type: float + script: "emit(params._source[\"float\"])" + derived_double: + type: double + script: "emit(params._source[\"double\"])" + derived_date: + type: date + script: "emit(ZonedDateTime.parse(params._source[\"date\"]).toInstant().toEpochMilli())" + derived_geo: + type: geo_point + script: "emit(params._source[\"geo\"][0], params._source[\"geo\"][1])" + derived_ip: + type: ip + script: "emit(params._source[\"ip\"])" + derived_boolean: + type: boolean + script: "emit(params._source[\"boolean\"])" + derived_array_of_long: + type: long + script: "emit(params._source[\"array_of_long\"][0]);emit(params._source[\"array_of_long\"][1]);" + derived_object: + type: object + properties: + keyword: keyword + script: "emit(params._source[\"json_field\"])" + prefilter_field: "json_field" + + - do: + index: + index: test + id: 1 + body: { + text: "peter piper", + keyword: "foo", + long: 1, + float: 1.0, + double: 1.0, + date: "2017-01-01T00:00:00Z", + geo: [0.0, 20.0], + ip: "192.168.0.1", + boolean: true, + array_of_long: [1, 2], + json_field: "{\"keyword\":\"json_keyword1\",\"long\":10,\"float\":10.0,\"double\":10.0,\"date\":\"2021-01-01T00:00:00Z\",\"ip\":\"10.0.0.1\",\"boolean\":true, \"array_of_long\": [1, 2]}}" + } + + - do: + index: + index: test + id: 2 + body: { + text: "piper picked a peck", + keyword: "bar", + long: 2, + float: 2.0, + double: 2.0, + date: "2017-01-02T00:00:00Z", + geo: [10.0, 30.0], + ip: "192.168.0.2", + boolean: false, + array_of_long: [2, 3], + json_field: "{\"keyword\":\"json_keyword2\",\"long\":20,\"float\":20.0,\"double\":20.0,\"date\":\"2021-02-01T00:00:00Z\",\"ip\":\"10.0.0.2\",\"boolean\":false, \"array_of_long\": [2, 3]}}" + } + + - do: + index: + index: test + id: 3 + body: { + text: "peck of pickled peppers", + keyword: "baz", + long: -3, + float: -3.0, + double: -3.0, + date: "2017-01-03T00:00:00Z", + geo: [20.0, 40.0], + ip: "192.168.0.3", + boolean: true, + array_of_long: [3, 4], + json_field: "{\"keyword\":\"json_keyword3\",\"long\":30,\"float\":30.0,\"double\":30.0,\"date\":\"2021-03-01T00:00:00Z\",\"ip\":\"10.0.0.3\",\"boolean\":true, \"array_of_long\": [3, 4]}" + } + + - do: + index: + index: test + id: 4 + body: { + text: "pickled peppers", + keyword: "qux", + long: 4, + float: 4.0, + double: 4.0, + date: "2017-01-04T00:00:00Z", + geo: [30.0, 50.0], + ip: "192.168.0.4", + boolean: false, + array_of_long: [4, 5], + json_field: "{\"keyword\":\"json_keyword4\",\"long\":40,\"float\":40.0,\"double\":40.0,\"date\":\"2021-04-01T00:00:00Z\",\"ip\":\"10.0.0.4\",\"boolean\":false, \"array_of_long\": [4, 5]}" + } + + - do: + index: + index: test + id: 5 + body: { + text: "peppers", + keyword: "quux", + long: 5, + float: 5.0, + double: 5.0, + date: "2017-01-05T00:00:00Z", + geo: [40.0, 60.0], + ip: "192.168.0.5", + boolean: true, + array_of_long: [5, 6], + json_field: "{\"keyword\":\"json_keyword5\",\"long\":50,\"float\":50.0,\"double\":50.0,\"date\":\"2021-05-01T00:00:00Z\",\"ip\":\"10.0.0.5\",\"boolean\":true, \"array_of_long\": [5, 6]}" + } + + - do: + indices.refresh: + index: [test] + + # Tests for derived_text + - do: + search: + rest_total_hits_as_int: true + index: test + body: + query: + match_phrase: + derived_text: + query: "peter piper" + + - match: { hits.total: 1 } + + # Tests for derived_keyword + - do: + search: + rest_total_hits_as_int: true + index: test + body: + query: + term: + derived_keyword: + value: "foo" + + - match: { hits.total: 1 } + + # Tests for derived_long + - do: + search: + rest_total_hits_as_int: true + index: test + body: + query: + range: + derived_long: + gte: 1 + + - match: { hits.total: 4 } + + # Tests for derived_float + - do: + search: + rest_total_hits_as_int: true + index: test + body: + query: + range: + derived_float: + gte: 1.0 + + - match: { hits.total: 4 } + + # Tests for derived_double + - do: + search: + rest_total_hits_as_int: true + index: test + body: + query: + range: + derived_double: + gte: 1.0 + + - match: { hits.total: 4 } + + # Tests for derived_date + - do: + search: + rest_total_hits_as_int: true + index: test + body: + query: + range: + derived_date: + gte: "2017-01-02" + + - match: { hits.total: 4 } + + # Tests for derived_geo + - do: + search: + rest_total_hits_as_int: true + index: test + body: + query: + geo_distance: + distance: "20km" + derived_geo: + lat: 0.0 + lon: 20.0 + + - match: { hits.total: 1 } + + # Tests for derived_ip + - do: + search: + rest_total_hits_as_int: true + index: test + body: + query: + term: + derived_ip: + value: "192.168.0.1" + + - match: { hits.total: 1 } + + # Tests for derived_boolean + - do: + search: + rest_total_hits_as_int: true + index: test + body: + query: + term: + derived_boolean: + value: true + + - match: { hits.total: 3 } + + # Tests for derived_array_of_long + - do: + search: + rest_total_hits_as_int: true + index: test + body: + query: + range: + derived_array_of_long: + gte: 3 + + - match: { hits.total: 4 } + + # Tests for derived_object.keyword + - do: + search: + rest_total_hits_as_int: true + index: test + body: + query: + term: + derived_object.keyword: + value: "json_keyword1" + + - match: { hits.total: 1 } + + # Tests for derived_object.long + - do: + search: + rest_total_hits_as_int: true + index: test + body: + query: + range: + derived_object.long: + gte: 11 + + - match: { hits.total: 4 } + + # Tests for derived_object.float + - do: + search: + rest_total_hits_as_int: true + index: test + body: + query: + range: + derived_object.float: + gte: 10.1 + + - match: { hits.total: 4 } + + # Tests for derived_object.double + - do: + search: + rest_total_hits_as_int: true + index: test + body: + query: + range: + derived_object.double: + gte: 10.1 + + - match: { hits.total: 4 } + + # Tests for derived_object.date + - do: + search: + rest_total_hits_as_int: true + index: test + body: + query: + range: + derived_object.date: + gte: "2021-03-01" + + - match: { hits.total: 3 } + + # Tests for derived_object.ip + - do: + search: + rest_total_hits_as_int: true + index: test + body: + query: + term: + derived_object.ip: + value: "10.0.0.1" + + - match: { hits.total: 1 } + + # Tests for derived_object.boolean + - do: + search: + rest_total_hits_as_int: true + index: test + body: + query: + term: + derived_object.boolean: + value: true + + - match: { hits.total: 3 } + + # Tests for derived_object.array_of_long + - do: + search: + rest_total_hits_as_int: true + index: test + body: + query: + range: + derived_object.array_of_long: + gte: 3 + + - match: { hits.total: 4 } + + # Tests for query string + - do: + search: + rest_total_hits_as_int: true + index: test + q: "derived_keyword:foo" + + - match: { hits.total: 1 } + + - do: + search: + rest_total_hits_as_int: true + index: test + q: derived_object.keyword:json_keyword1 + + - match: { hits.total: 1 } diff --git a/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/derived_fields/20_derived_field_put_mapping.yml b/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/derived_fields/20_derived_field_put_mapping.yml new file mode 100644 index 0000000000000..0370fd94e8548 --- /dev/null +++ b/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/derived_fields/20_derived_field_put_mapping.yml @@ -0,0 +1,123 @@ +--- +"Test create and update mapping for derived fields": + - skip: + version: " - 2.14.99" + reason: "derived_field feature was added in 2.15" + - do: + indices.create: + index: test_index + + - do: + indices.put_mapping: + index: test_index + body: + properties: + text: + type: text + json_field: + type: text + derived: + derived_text: + type: text + script: "emit(params._source[\"text\"])" + derived_text_prefilter_field: + type: keyword + script: "emit(params._source[\"text\"])" + prefilter_field: "text" + derived_date: + type: date + script: "emit(params._source[\"keyword\"])" + derived_object: + type: object + properties: + keyword: keyword + script: "emit(params._source[\"json_field\"])" + prefilter_field: "json_field" + + - do: + indices.get_mapping: + index: test_index + + - match: {test_index.mappings.derived.derived_text.type: text} + - match: {test_index.mappings.derived.derived_text_prefilter_field.type: keyword} + - match: {test_index.mappings.derived.derived_text_prefilter_field.prefilter_field: text} + - match: {test_index.mappings.derived.derived_date.type: date} + - match: {test_index.mappings.derived.derived_object.type: object} + - match: {test_index.mappings.derived.derived_object.properties.keyword: keyword} + - match: {test_index.mappings.derived.derived_object.prefilter_field: json_field} + + + - do: + indices.put_mapping: + index: test_index + body: + properties: + text: + type: text + json_field: + type: text + derived: + derived_text: + type: keyword + script: "emit(params._source[\"text\"])" + derived_text_prefilter_field: + type: text + script: "emit(params._source[\"text\"])" + prefilter_field: "text" + derived_date: + type: keyword + script: "emit(params._source[\"keyword\"])" + derived_object: + type: object + properties: + keyword: text + script: "emit(params._source[\"text\"])" + prefilter_field: "text" + format: "dd-MM-yyyy" + ignore_malformed: true + + - do: + indices.get_mapping: + index: test_index + + - match: {test_index.mappings.derived.derived_text.type: keyword} + - match: {test_index.mappings.derived.derived_text_prefilter_field.type: text} + - match: {test_index.mappings.derived.derived_text_prefilter_field.prefilter_field: text} + - match: {test_index.mappings.derived.derived_date.type: keyword} + - match: {test_index.mappings.derived.derived_object.type: object} + - match: {test_index.mappings.derived.derived_object.properties.keyword: text} + - match: {test_index.mappings.derived.derived_object.prefilter_field: text} + - match: {test_index.mappings.derived.derived_object.format: "dd-MM-yyyy"} + - match: {test_index.mappings.derived.derived_object.ignore_malformed: true} + + + - do: + indices.put_mapping: + index: test_index + body: + properties: + text: + type: text + json_field: + type: text + derived: + derived_object: + type: object + properties: + keyword: keyword + script: "emit(params._source[\"json_field\"])" + prefilter_field: "json_field" + ignore_malformed: false + + - do: + indices.get_mapping: + index: test_index + + - match: {test_index.mappings.derived.derived_text.type: keyword} + - match: {test_index.mappings.derived.derived_text_prefilter_field.type: text} + - match: {test_index.mappings.derived.derived_text_prefilter_field.prefilter_field: text} + - match: {test_index.mappings.derived.derived_date.type: keyword} + - match: {test_index.mappings.derived.derived_object.type: object} + - match: {test_index.mappings.derived.derived_object.properties.keyword: keyword} + - match: {test_index.mappings.derived.derived_object.prefilter_field: json_field} + - is_false: test_index.mappings.derived.derived_object.ignore_malformed diff --git a/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/derived_fields/30_derived_field_search_definition.yml b/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/derived_fields/30_derived_field_search_definition.yml new file mode 100644 index 0000000000000..bb619dce63010 --- /dev/null +++ b/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/derived_fields/30_derived_field_search_definition.yml @@ -0,0 +1,489 @@ +"Test derived_field supported type using search definition": + - skip: + version: " - 2.14.99" + reason: "derived_field feature was added in 2.15" + + - do: + indices.create: + index: test + body: + mappings: + properties: + text: + type: text + keyword: + type: keyword + long: + type: long + float: + type: float + double: + type: double + date: + type: date + geo: + type: geo_point + ip: + type: ip + boolean: + type: boolean + array_of_long: + type: long + json_field: + type: text + + - do: + index: + index: test + id: 1 + body: { + text: "peter piper", + keyword: "foo", + long: 1, + float: 1.0, + double: 1.0, + date: "2017-01-01T00:00:00Z", + geo: [0.0, 20.0], + ip: "192.168.0.1", + boolean: true, + array_of_long: [1, 2], + json_field: "{\"keyword\":\"json_keyword1\",\"long\":10,\"float\":10.0,\"double\":10.0,\"date\":\"2021-01-01T00:00:00Z\",\"ip\":\"10.0.0.1\",\"boolean\":true, \"array_of_long\": [1, 2]}}" + } + + - do: + index: + index: test + id: 2 + body: { + text: "piper picked a peck", + keyword: "bar", + long: 2, + float: 2.0, + double: 2.0, + date: "2017-01-02T00:00:00Z", + geo: [10.0, 30.0], + ip: "192.168.0.2", + boolean: false, + array_of_long: [2, 3], + json_field: "{\"keyword\":\"json_keyword2\",\"long\":20,\"float\":20.0,\"double\":20.0,\"date\":\"2021-02-01T00:00:00Z\",\"ip\":\"10.0.0.2\",\"boolean\":false, \"array_of_long\": [2, 3]}}" + } + + - do: + index: + index: test + id: 3 + body: { + text: "peck of pickled peppers", + keyword: "baz", + long: -3, + float: -3.0, + double: -3.0, + date: "2017-01-03T00:00:00Z", + geo: [20.0, 40.0], + ip: "192.168.0.3", + boolean: true, + array_of_long: [3, 4], + json_field: "{\"keyword\":\"json_keyword3\",\"long\":30,\"float\":30.0,\"double\":30.0,\"date\":\"2021-03-01T00:00:00Z\",\"ip\":\"10.0.0.3\",\"boolean\":true, \"array_of_long\": [3, 4]}" + } + + - do: + index: + index: test + id: 4 + body: { + text: "pickled peppers", + keyword: "qux", + long: 4, + float: 4.0, + double: 4.0, + date: "2017-01-04T00:00:00Z", + geo: [30.0, 50.0], + ip: "192.168.0.4", + boolean: false, + array_of_long: [4, 5], + json_field: "{\"keyword\":\"json_keyword4\",\"long\":40,\"float\":40.0,\"double\":40.0,\"date\":\"2021-04-01T00:00:00Z\",\"ip\":\"10.0.0.4\",\"boolean\":false, \"array_of_long\": [4, 5]}" + } + + - do: + index: + index: test + id: 5 + body: { + text: "peppers", + keyword: "quux", + long: 5, + float: 5.0, + double: 5.0, + date: "2017-01-05T00:00:00Z", + geo: [40.0, 60.0], + ip: "192.168.0.5", + boolean: true, + array_of_long: [5, 6], + json_field: "{\"keyword\":\"json_keyword5\",\"long\":50,\"float\":50.0,\"double\":50.0,\"date\":\"2021-05-01T00:00:00Z\",\"ip\":\"10.0.0.5\",\"boolean\":true, \"array_of_long\": [5, 6]}" + } + + - do: + indices.refresh: + index: [test] + + # Tests for derived_text + - do: + search: + rest_total_hits_as_int: true + index: test + body: + derived: + derived_text: + type: text + script: "emit(params._source[\"text\"])" + query: + match_phrase: + derived_text: + query: "peter piper" + + - match: { hits.total: 1 } + + # Tests for derived_keyword + - do: + search: + rest_total_hits_as_int: true + index: test + body: + derived: + derived_keyword: + type: keyword + script: "emit(params._source[\"keyword\"])" + query: + term: + derived_keyword: + value: "foo" + + - match: { hits.total: 1 } + + # Tests for derived_long + - do: + search: + rest_total_hits_as_int: true + index: test + body: + derived: + derived_long: + type: long + script: "emit(params._source[\"long\"])" + query: + range: + derived_long: + gte: 1 + + - match: { hits.total: 4 } + + # Tests for derived_float + - do: + search: + rest_total_hits_as_int: true + index: test + body: + derived: + derived_float: + type: float + script: "emit(params._source[\"float\"])" + query: + range: + derived_float: + gte: 1.0 + + - match: { hits.total: 4 } + + # Tests for derived_double + - do: + search: + rest_total_hits_as_int: true + index: test + body: + derived: + derived_double: + type: double + script: "emit(params._source[\"double\"])" + query: + range: + derived_double: + gte: 1.0 + + - match: { hits.total: 4 } + + # Tests for derived_date + - do: + search: + rest_total_hits_as_int: true + index: test + body: + derived: + derived_date: + type: date + script: "emit(ZonedDateTime.parse(params._source[\"date\"]).toInstant().toEpochMilli())" + query: + range: + derived_date: + gte: "2017-01-02" + + - match: { hits.total: 4 } + + # Tests for derived_geo + - do: + search: + rest_total_hits_as_int: true + index: test + body: + derived: + derived_geo: + type: geo_point + script: "emit(params._source[\"geo\"][0], params._source[\"geo\"][1])" + query: + geo_distance: + distance: "20km" + derived_geo: + lat: 0.0 + lon: 20.0 + + - match: { hits.total: 1 } + + # Tests for derived_ip + - do: + search: + rest_total_hits_as_int: true + index: test + body: + derived: + derived_ip: + type: ip + script: "emit(params._source[\"ip\"])" + query: + term: + derived_ip: + value: "192.168.0.1" + + - match: { hits.total: 1 } + + # Tests for derived_boolean + - do: + search: + rest_total_hits_as_int: true + index: test + body: + derived: + derived_boolean: + type: boolean + script: "emit(params._source[\"boolean\"])" + query: + term: + derived_boolean: + value: true + + - match: { hits.total: 3 } + + # Tests for derived_array_of_long + - do: + search: + rest_total_hits_as_int: true + index: test + body: + derived: + derived_array_of_long: + type: long + script: "emit(params._source[\"array_of_long\"][0]);emit(params._source[\"array_of_long\"][1]);" + query: + range: + derived_array_of_long: + gte: 3 + + - match: { hits.total: 4 } + + # Tests for derived_object.keyword + - do: + search: + rest_total_hits_as_int: true + index: test + body: + derived: + derived_object: + type: object + properties: + keyword: keyword + script: "emit(params._source[\"json_field\"])" + prefilter_field: "json_field" + query: + term: + derived_object.keyword: + value: "json_keyword1" + + - match: { hits.total: 1 } + + # Tests for derived_object.long + - do: + search: + rest_total_hits_as_int: true + index: test + body: + derived: + derived_object: + type: object + properties: + keyword: keyword + script: "emit(params._source[\"json_field\"])" + prefilter_field: "json_field" + query: + range: + derived_object.long: + gte: 11 + + - match: { hits.total: 4 } + + # Tests for derived_object.float + - do: + search: + rest_total_hits_as_int: true + index: test + body: + derived: + derived_object: + type: object + properties: + keyword: keyword + script: "emit(params._source[\"json_field\"])" + prefilter_field: "json_field" + query: + range: + derived_object.float: + gte: 10.1 + + - match: { hits.total: 4 } + + # Tests for derived_object.double + - do: + search: + rest_total_hits_as_int: true + index: test + body: + derived: + derived_object: + type: object + properties: + keyword: keyword + script: "emit(params._source[\"json_field\"])" + prefilter_field: "json_field" + query: + range: + derived_object.double: + gte: 10.1 + + - match: { hits.total: 4 } + + # Tests for derived_object.date + - do: + search: + rest_total_hits_as_int: true + index: test + body: + derived: + derived_object: + type: object + properties: + keyword: keyword + script: "emit(params._source[\"json_field\"])" + prefilter_field: "json_field" + query: + range: + derived_object.date: + gte: "2021-03-01" + + - match: { hits.total: 3 } + + # Tests for derived_object.ip + - do: + search: + rest_total_hits_as_int: true + index: test + body: + derived: + derived_object: + type: object + properties: + keyword: keyword + script: "emit(params._source[\"json_field\"])" + prefilter_field: "json_field" + query: + term: + derived_object.ip: + value: "10.0.0.1" + + - match: { hits.total: 1 } + + # Tests for derived_object.boolean + - do: + search: + rest_total_hits_as_int: true + index: test + body: + derived: + derived_object: + type: object + properties: + keyword: keyword + script: "emit(params._source[\"json_field\"])" + prefilter_field: "json_field" + query: + term: + derived_object.boolean: + value: true + + - match: { hits.total: 3 } + + # Tests for derived_object.array_of_long + - do: + search: + rest_total_hits_as_int: true + index: test + body: + derived: + derived_object: + type: object + properties: + keyword: keyword + script: "emit(params._source[\"json_field\"])" + prefilter_field: "json_field" + query: + range: + derived_object.array_of_long: + gte: 3 + + - match: { hits.total: 4 } + + # Tests for query string + - do: + search: + body: + derived: + derived_keyword: + type: keyword + script: "emit(params._source[\"keyword\"])" + rest_total_hits_as_int: true + index: test + q: "derived_keyword:foo" + + - match: { hits.total: 1 } + + - do: + search: + body: + derived: + derived_object: + type: object + properties: + keyword: keyword + script: "emit(params._source[\"json_field\"])" + prefilter_field: "json_field" + rest_total_hits_as_int: true + index: test + q: derived_object.keyword:json_keyword1 + + - match: { hits.total: 1 } diff --git a/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/derived_fields/40_derived_field_fetch_and_highlight.yml b/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/derived_fields/40_derived_field_fetch_and_highlight.yml new file mode 100644 index 0000000000000..52a897c341419 --- /dev/null +++ b/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/derived_fields/40_derived_field_fetch_and_highlight.yml @@ -0,0 +1,279 @@ +setup: + - skip: + version: " - 2.14.99" + reason: "derived_field feature was added in 2.15" + +--- +"Test basic field retrieval": + - do: + indices.create: + index: test + body: + mappings: + properties: + text: + type: text + keyword: + type: keyword + long: + type: long + float: + type: float + double: + type: double + date: + type: date + geo: + type: geo_point + ip: + type: ip + boolean: + type: boolean + array_of_long: + type: long + json_field: + type: text + derived: + derived_text: + type: text + script: "emit(params._source[\"text\"])" + derived_text_prefilter_field: + type: text + script: "emit(params._source[\"text\"])" + prefilter_field: "text" + derived_keyword: + type: keyword + script: "emit(params._source[\"keyword\"])" + derived_long: + type: long + script: "emit(params._source[\"long\"])" + derived_float: + type: float + script: "emit(params._source[\"float\"])" + derived_double: + type: double + script: "emit(params._source[\"double\"])" + derived_date: + type: date + script: "emit(ZonedDateTime.parse(params._source[\"date\"]).toInstant().toEpochMilli())" + derived_geo: + type: geo_point + script: "emit(params._source[\"geo\"][0], params._source[\"geo\"][1])" + derived_ip: + type: ip + script: "emit(params._source[\"ip\"])" + derived_boolean: + type: boolean + script: "emit(params._source[\"boolean\"])" + derived_array_of_long: + type: long + script: "emit(params._source[\"array_of_long\"][0]);emit(params._source[\"array_of_long\"][1]);" + derived_object: + type: object + properties: + keyword: keyword + script: "emit(params._source[\"json_field\"])" + prefilter_field: "json_field" + format: "yyyy-MM-dd" + + - do: + index: + index: test + id: 1 + body: { + text: "peter piper", + keyword: "foo", + long: 1, + float: 1.0, + double: 1.0, + date: "2017-01-01T00:00:00Z", + geo: [0.0, 20.0], + ip: "192.168.0.1", + boolean: true, + array_of_long: [1, 2], + json_field: "{\"keyword\":\"json_keyword1\",\"long\":10,\"float\":10.0,\"double\":10.0,\"date\":\"2021-01-01T00:00:00Z\",\"ip\":\"10.0.0.1\",\"boolean\":true, \"array_of_long\": [1, 2]}}" + } + + - do: + index: + index: test + id: 2 + body: { + text: "piper picked a peck", + keyword: "bar", + long: 2, + float: 2.0, + double: 2.0, + date: "2017-01-02T00:00:00Z", + geo: [10.0, 30.0], + ip: "192.168.0.2", + boolean: false, + array_of_long: [2, 3], + json_field: "{\"keyword\":\"json_keyword2\",\"long\":20,\"float\":20.0,\"double\":20.0,\"date\":\"2021-02-01T00:00:00Z\",\"ip\":\"10.0.0.2\",\"boolean\":false, \"array_of_long\": [2, 3]}}" + } + + - do: + indices.refresh: + index: [test] + + - do: + search: + index: test + body: + fields: [derived_text, derived_keyword, derived_long, derived_float, derived_double, derived_date, derived_geo, derived_ip, derived_boolean, derived_array_of_long, + derived_object, derived_object.keyword, derived_object.long, derived_object.float, derived_object.double, derived_object.date, derived_object.ip, derived_object.boolean, derived_object.array_of_long] + + - is_true: hits.hits.0._id + - is_true: hits.hits.0._source + + - match: { hits.hits.0.fields.derived_text.0: "peter piper" } + - match: { hits.hits.0.fields.derived_keyword.0: foo } + - match: { hits.hits.0.fields.derived_long.0: 1 } + - match: { hits.hits.0.fields.derived_float.0: 1.0 } + - match: { hits.hits.0.fields.derived_double.0: 1 } + - match: { hits.hits.0.fields.derived_date.0: 2017-01-01T00:00:00.000Z } + - match: { hits.hits.0.fields.derived_geo.0.lat: 0.0 } + - match: { hits.hits.0.fields.derived_geo.0.lon: 20.0 } + - match: { hits.hits.0.fields.derived_ip.0: 192.168.0.1 } + - match: { hits.hits.0.fields.derived_array_of_long.0: 1 } + - match: { hits.hits.0.fields.derived_array_of_long.1: 2 } + - match: { hits.hits.0.fields.derived_object.0: "{\"keyword\":\"json_keyword1\",\"long\":10,\"float\":10.0,\"double\":10.0,\"date\":\"2021-01-01T00:00:00Z\",\"ip\":\"10.0.0.1\",\"boolean\":true, \"array_of_long\": [1, 2]}}" } + - match: { hits.hits.0.fields.derived_object\.keyword.0: json_keyword1 } + - match: { hits.hits.0.fields.derived_object\.long.0: 10 } + - match: { hits.hits.0.fields.derived_object\.float.0: 10.0 } + - match: { hits.hits.0.fields.derived_object\.double.0: 10.0 } + - match: { hits.hits.0.fields.derived_object\.date.0: 2021-01-01 } + - match: { hits.hits.0.fields.derived_object\.ip.0: 10.0.0.1 } + - match: { hits.hits.0.fields.derived_object\.boolean.0: true } + - match: { hits.hits.0.fields.derived_object\.array_of_long.0: 1 } + - match: { hits.hits.0.fields.derived_object\.array_of_long.1: 2 } + + - match: { hits.hits.1.fields.derived_text.0: "piper picked a peck" } + - match: { hits.hits.1.fields.derived_keyword.0: bar } + - match: { hits.hits.1.fields.derived_long.0: 2 } + - match: { hits.hits.1.fields.derived_float.0: 2.0 } + - match: { hits.hits.1.fields.derived_double.0: 2 } + - match: { hits.hits.1.fields.derived_date.0: 2017-01-02T00:00:00.000Z } + - match: { hits.hits.1.fields.derived_geo.0.lat: 10.0 } + - match: { hits.hits.1.fields.derived_geo.0.lon: 30.0 } + - match: { hits.hits.1.fields.derived_ip.0: 192.168.0.2 } + - match: { hits.hits.1.fields.derived_array_of_long.0: 2 } + - match: { hits.hits.1.fields.derived_array_of_long.1: 3 } + - match: { hits.hits.1.fields.derived_object.0: "{\"keyword\":\"json_keyword2\",\"long\":20,\"float\":20.0,\"double\":20.0,\"date\":\"2021-02-01T00:00:00Z\",\"ip\":\"10.0.0.2\",\"boolean\":false, \"array_of_long\": [2, 3]}}" } + - match: { hits.hits.1.fields.derived_object\.keyword.0: json_keyword2 } + - match: { hits.hits.1.fields.derived_object\.long.0: 20 } + - match: { hits.hits.1.fields.derived_object\.float.0: 20.0 } + - match: { hits.hits.1.fields.derived_object\.double.0: 20.0 } + - match: { hits.hits.1.fields.derived_object\.date.0: 2021-02-01 } + - match: { hits.hits.1.fields.derived_object\.ip.0: 10.0.0.2 } + - match: { hits.hits.1.fields.derived_object\.boolean.0: false } + - match: { hits.hits.1.fields.derived_object\.array_of_long.0: 2 } + - match: { hits.hits.1.fields.derived_object\.array_of_long.1: 3 } + + +--- +"Test highlight": + - do: + indices.create: + index: test + body: + mappings: + properties: + text: + type: text + array_of_text: + type: text + json_field: + type: text + derived: + derived_text: + type: text + script: "emit(params._source[\"text\"])" + derived_keyword: + type: keyword + script: "emit(params._source[\"keyword\"])" + derived_array_of_text: + type: text + script: "emit(params._source[\"array_of_text\"][0]);emit(params._source[\"array_of_text\"][1]);" + derived_object: + type: object + properties: + array_of_text: text + script: "emit(params._source[\"json_field\"])" + prefilter_field: "json_field" + + - do: + index: + index: test + id: 1 + body: { + text: "peter piper", + keyword: "foo", + long: 1, + float: 1.0, + double: 1.0, + date: "2017-01-01T00:00:00Z", + geo: [0.0, 20.0], + ip: "192.168.0.1", + boolean: true, + array_of_text: ["The quick brown fox is brown", "The quick brown fox is black"], + json_field: "{\"keyword\":\"json_keyword1\",\"long\":10,\"float\":10.0,\"double\":10.0,\"date\":\"2021-01-01T00:00:00Z\",\"ip\":\"10.0.0.1\",\"boolean\":true, \"array_of_text\": [\"The quick brown fox is brown\", \"The quick brown fox is black\"]}}" + } + + - do: + index: + index: test + id: 2 + body: { + text: "piper picked a peck", + keyword: "bar", + long: 2, + float: 2.0, + double: 2.0, + date: "2017-01-02T00:00:00Z", + geo: [10.0, 30.0], + ip: "192.168.0.2", + boolean: false, + array_of_text: ["The quick brown fox is brown", "The quick brown fox is black"], + json_field: "{\"keyword\":\"json_keyword2\",\"long\":20,\"float\":20.0,\"double\":20.0,\"date\":\"2021-02-01T00:00:00Z\",\"ip\":\"10.0.0.2\",\"boolean\":false, \"array_of_text\": [\"The quick brown fox is brown\", \"The quick brown fox is black\"]}}" + } + + - do: + indices.refresh: + index: [test] + - do: + search: + rest_total_hits_as_int: true + body: { "query" : {"multi_match" : { "query" : "piper", "fields" : [ "derived_text"] } }, + "fields": [derived_text], + "highlight" : { "type" : "unified", "fields" : { "derived_text" : {} } } + } + + - match: {hits.hits.0.highlight.derived_text.0: "peter piper"} + + + - do: + search: + rest_total_hits_as_int: true + body: { "query" : {"multi_match" : { "query" : "quick brown", "fields" : [ "derived_array_of_text"] } }, + "fields": [derived_array_of_text], + "highlight" : { "type" : "unified", "fields" : { "derived_array_of_text" : {} } } + } + + - match: {hits.hits.0.highlight.derived_array_of_text.0: "The quick brown fox is brown"} + + - do: + search: + rest_total_hits_as_int: true + index: test + body: + query: + match_phrase: + derived_object.array_of_text: + query: "quick brown" + highlight: + type: unified + fields: + derived_object.array_of_text: {} + + - match: {hits.hits.0.highlight.derived_object\.array_of_text.0: "The quick brown fox is brown"} diff --git a/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/derived_fields/50_derived_field_default_analyzer.yml b/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/derived_fields/50_derived_field_default_analyzer.yml new file mode 100644 index 0000000000000..e10c9cb3c133f --- /dev/null +++ b/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/derived_fields/50_derived_field_default_analyzer.yml @@ -0,0 +1,105 @@ +--- +"Test default index analyzer simple is applied on derived fields": + - do: + indices.create: + index: test + body: + settings: + index.analysis.analyzer.default.type: simple + mappings: + properties: + text: + type: text + json_field: + type: text + derived: + derived_text: + type: text + script: "emit(params._source[\"text\"])" + derived_object: + type: object + properties: + array_of_text: text + script: "emit(params._source[\"json_field\"])" + prefilter_field: "json_field" + + - do: + index: + index: test + id: 1 + body: { + text: "Email: example@example.com, Visit https://example.com for more info.", + json_field: "{\"array_of_text\": [\"Email: example@example.com, Visit https://example.com for more info.\", \"Email: example@example.com, Visit https://example.com for more info.\"]}}" + } + + - do: + indices.refresh: + index: [test] + - do: + search: + index: test + q: "derived_text:example.com" + analyzer: standard + + - match: { hits.total.value: 0 } + + - do: + search: + index: test + q: "derived_text:example.com" + analyzer: simple + + - match: { hits.total.value: 1 } + +--- +"Test default index analyzer standard is applied on derived fields": + - do: + indices.create: + index: test + body: + settings: + index.analysis.analyzer.default.type: standard + mappings: + properties: + text: + type: text + json_field: + type: text + derived: + derived_text: + type: text + script: "emit(params._source[\"text\"])" + derived_object: + type: object + properties: + array_of_text: text + script: "emit(params._source[\"json_field\"])" + prefilter_field: "json_field" + + - do: + index: + index: test + id: 1 + body: { + text: "Email: example@example.com, Visit https://example.com for more info.", + json_field: "{\"array_of_text\": [\"Email: example@example.com, Visit https://example.com for more info.\", \"Email: example@example.com, Visit https://example.com for more info.\"]}}" + } + + - do: + indices.refresh: + index: [test] + - do: + search: + index: test + q: "derived_object.array_of_text:example.com" + analyzer: standard + + - match: { hits.total.value: 1 } + + - do: + search: + index: test + q: "derived_object.array_of_text:example.com" + analyzer: simple + + - match: { hits.total.value: 1 } From c71060e29f56918b87a78237ff117d9018456428 Mon Sep 17 00:00:00 2001 From: Andriy Redko Date: Mon, 10 Jun 2024 18:34:25 -0400 Subject: [PATCH 6/7] [Streaming Indexing] Enhance RestAction with request / response streaming support (#13772) Signed-off-by: Andriy Redko --- CHANGELOG.md | 1 + .../client/RestHighLevelClient.java | 6 +- .../OpenSearchDashboardsModulePlugin.java | 2 + .../netty4/ReactorNetty4HttpChunk.java | 49 ++++ .../netty4/ReactorNetty4HttpRequest.java | 4 + .../ReactorNetty4HttpServerTransport.java | 56 +++-- ...ReactorNetty4NonStreamingHttpChannel.java} | 4 +- ...torNetty4NonStreamingRequestConsumer.java} | 8 +- .../ReactorNetty4StreamingHttpChannel.java | 132 +++++++++++ ...ReactorNetty4StreamingRequestConsumer.java | 53 +++++ ...eactorNetty4StreamingResponseProducer.java | 54 +++++ .../netty4/StreamingHttpContentSender.java | 32 +++ .../reactor/netty4/ReactorHttpClient.java | 52 +++++ ...tty4HttpServerTransportStreamingTests.java | 211 ++++++++++++++++++ .../org/opensearch/action/ActionModule.java | 2 + .../xcontent/support/XContentHttpChunk.java | 65 ++++++ .../http/AbstractHttpServerTransport.java | 77 +++++-- .../opensearch/http/DefaultRestChannel.java | 4 +- .../http/DefaultStreamingRestChannel.java | 107 +++++++++ .../java/org/opensearch/http/HttpChunk.java | 33 +++ .../opensearch/http/HttpServerTransport.java | 15 ++ .../java/org/opensearch/http/HttpTracer.java | 30 +++ .../opensearch/http/StreamingHttpChannel.java | 56 +++++ .../org/opensearch/rest/BaseRestHandler.java | 16 ++ .../org/opensearch/rest/RestController.java | 146 +++++++++++- .../java/org/opensearch/rest/RestHandler.java | 8 + .../opensearch/rest/StreamingRestChannel.java | 51 +++++ .../document/RestBulkStreamingAction.java | 199 +++++++++++++++++ .../RestBulkStreamingActionTests.java | 89 ++++++++ 29 files changed, 1513 insertions(+), 49 deletions(-) create mode 100644 plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4HttpChunk.java rename plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/{NonStreamingHttpChannel.java => ReactorNetty4NonStreamingHttpChannel.java} (92%) rename plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/{NonStreamingRequestConsumer.java => ReactorNetty4NonStreamingRequestConsumer.java} (89%) create mode 100644 plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4StreamingHttpChannel.java create mode 100644 plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4StreamingRequestConsumer.java create mode 100644 plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4StreamingResponseProducer.java create mode 100644 plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/StreamingHttpContentSender.java create mode 100644 plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ReactorNetty4HttpServerTransportStreamingTests.java create mode 100644 server/src/main/java/org/opensearch/common/xcontent/support/XContentHttpChunk.java create mode 100644 server/src/main/java/org/opensearch/http/DefaultStreamingRestChannel.java create mode 100644 server/src/main/java/org/opensearch/http/HttpChunk.java create mode 100644 server/src/main/java/org/opensearch/http/StreamingHttpChannel.java create mode 100644 server/src/main/java/org/opensearch/rest/StreamingRestChannel.java create mode 100644 server/src/main/java/org/opensearch/rest/action/document/RestBulkStreamingAction.java create mode 100644 server/src/test/java/org/opensearch/rest/action/document/RestBulkStreamingActionTests.java diff --git a/CHANGELOG.md b/CHANGELOG.md index a05aa09b0f13d..681550cedcc0f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add recovery chunk size setting ([#13997](https://github.com/opensearch-project/OpenSearch/pull/13997)) - [Query Insights] Add exporter support for top n queries ([#12982](https://github.com/opensearch-project/OpenSearch/pull/12982)) - [Query Insights] Add X-Opaque-Id to search request metadata for top n queries ([#13374](https://github.com/opensearch-project/OpenSearch/pull/13374)) +- [Streaming Indexing] Enhance RestAction with request / response streaming support ([#13772](https://github.com/opensearch-project/OpenSearch/pull/13772)) - Add support for query level resource usage tracking ([#13172](https://github.com/opensearch-project/OpenSearch/pull/13172)) - Move Remote Store Migration from DocRep to GA and modify remote migration settings name ([#14100](https://github.com/opensearch-project/OpenSearch/pull/14100)) - Derived field object type support ([#13720](https://github.com/opensearch-project/OpenSearch/pull/13720)) diff --git a/client/rest-high-level/src/main/java/org/opensearch/client/RestHighLevelClient.java b/client/rest-high-level/src/main/java/org/opensearch/client/RestHighLevelClient.java index 9d8d771f1eaed..83c3ba8164c4b 100644 --- a/client/rest-high-level/src/main/java/org/opensearch/client/RestHighLevelClient.java +++ b/client/rest-high-level/src/main/java/org/opensearch/client/RestHighLevelClient.java @@ -2227,11 +2227,11 @@ protected final Resp parseEntity(final HttpEntity entity, final CheckedFu if (entity.getContentType() == null) { throw new IllegalStateException("OpenSearch didn't return the [Content-Type] header, unable to parse response body"); } - MediaType medaiType = MediaType.fromMediaType(entity.getContentType()); - if (medaiType == null) { + MediaType mediaType = MediaType.fromMediaType(entity.getContentType()); + if (mediaType == null) { throw new IllegalStateException("Unsupported Content-Type: " + entity.getContentType()); } - try (XContentParser parser = medaiType.xContent().createParser(registry, DEPRECATION_HANDLER, entity.getContent())) { + try (XContentParser parser = mediaType.xContent().createParser(registry, DEPRECATION_HANDLER, entity.getContent())) { return entityParser.apply(parser); } } diff --git a/modules/opensearch-dashboards/src/main/java/org/opensearch/dashboards/OpenSearchDashboardsModulePlugin.java b/modules/opensearch-dashboards/src/main/java/org/opensearch/dashboards/OpenSearchDashboardsModulePlugin.java index 09fd52ff65c66..6d5020336eb0b 100644 --- a/modules/opensearch-dashboards/src/main/java/org/opensearch/dashboards/OpenSearchDashboardsModulePlugin.java +++ b/modules/opensearch-dashboards/src/main/java/org/opensearch/dashboards/OpenSearchDashboardsModulePlugin.java @@ -54,6 +54,7 @@ import org.opensearch.rest.action.admin.indices.RestRefreshAction; import org.opensearch.rest.action.admin.indices.RestUpdateSettingsAction; import org.opensearch.rest.action.document.RestBulkAction; +import org.opensearch.rest.action.document.RestBulkStreamingAction; import org.opensearch.rest.action.document.RestDeleteAction; import org.opensearch.rest.action.document.RestGetAction; import org.opensearch.rest.action.document.RestIndexAction; @@ -127,6 +128,7 @@ public List getRestHandlers( new OpenSearchDashboardsWrappedRestHandler(new RestMultiGetAction(settings)), new OpenSearchDashboardsWrappedRestHandler(new RestSearchAction()), new OpenSearchDashboardsWrappedRestHandler(new RestBulkAction(settings)), + new OpenSearchDashboardsWrappedRestHandler(new RestBulkStreamingAction(settings)), new OpenSearchDashboardsWrappedRestHandler(new RestDeleteAction()), new OpenSearchDashboardsWrappedRestHandler(new RestDeleteByQueryAction()), diff --git a/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4HttpChunk.java b/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4HttpChunk.java new file mode 100644 index 0000000000000..3b4a308691e7b --- /dev/null +++ b/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4HttpChunk.java @@ -0,0 +1,49 @@ +/* + * 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.http.reactor.netty4; + +import org.opensearch.core.common.bytes.BytesReference; +import org.opensearch.http.HttpChunk; +import org.opensearch.transport.reactor.netty4.Netty4Utils; + +import java.util.concurrent.atomic.AtomicBoolean; + +import io.netty.buffer.ByteBuf; + +class ReactorNetty4HttpChunk implements HttpChunk { + private final AtomicBoolean released; + private final boolean pooled; + private final ByteBuf content; + private final boolean last; + + ReactorNetty4HttpChunk(ByteBuf content, boolean last) { + this.content = content; + this.pooled = true; + this.released = new AtomicBoolean(false); + this.last = last; + } + + @Override + public BytesReference content() { + assert released.get() == false; + return Netty4Utils.toBytesReference(content); + } + + @Override + public void close() { + if (pooled && released.compareAndSet(false, true)) { + content.release(); + } + } + + @Override + public boolean isLast() { + return last; + } +} diff --git a/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4HttpRequest.java b/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4HttpRequest.java index 4406c555a5b04..491c7aa885103 100644 --- a/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4HttpRequest.java +++ b/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4HttpRequest.java @@ -44,6 +44,10 @@ class ReactorNetty4HttpRequest implements HttpRequest { private final Exception inboundException; private final boolean pooled; + ReactorNetty4HttpRequest(HttpServerRequest request) { + this(request, new HttpHeadersMap(request.requestHeaders()), new AtomicBoolean(false), false, Unpooled.EMPTY_BUFFER); + } + ReactorNetty4HttpRequest(HttpServerRequest request, ByteBuf content) { this(request, new HttpHeadersMap(request.requestHeaders()), new AtomicBoolean(false), true, content); } diff --git a/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4HttpServerTransport.java b/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4HttpServerTransport.java index bd1646d753016..906bbfd072da8 100644 --- a/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4HttpServerTransport.java +++ b/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4HttpServerTransport.java @@ -26,6 +26,8 @@ import org.opensearch.http.HttpServerChannel; import org.opensearch.http.reactor.netty4.ssl.SslUtils; import org.opensearch.plugins.SecureHttpTransportSettingsProvider; +import org.opensearch.rest.RestHandler; +import org.opensearch.rest.RestRequest.Method; import org.opensearch.telemetry.tracing.Tracer; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.reactor.SharedGroupFactory; @@ -40,6 +42,7 @@ import java.time.Duration; import java.util.Arrays; import java.util.List; +import java.util.Optional; import io.netty.buffer.ByteBufAllocator; import io.netty.channel.ChannelOption; @@ -351,24 +354,45 @@ public List protocols() { * @return response publisher */ protected Publisher incomingRequest(HttpServerRequest request, HttpServerResponse response) { - final NonStreamingRequestConsumer consumer = new NonStreamingRequestConsumer<>( - this, - request, - response, - maxCompositeBufferComponents + final Method method = HttpConversionUtil.convertMethod(request.method()); + final Optional dispatchHandlerOpt = dispatcher.dispatchHandler( + request.uri(), + request.fullPath(), + method, + request.params() ); + if (dispatchHandlerOpt.map(RestHandler::supportsStreaming).orElse(false)) { + final ReactorNetty4StreamingRequestConsumer consumer = new ReactorNetty4StreamingRequestConsumer<>( + request, + response + ); + + request.receiveContent() + .switchIfEmpty(Mono.just(DefaultLastHttpContent.EMPTY_LAST_CONTENT)) + .subscribe(consumer, error -> {}, () -> consumer.accept(DefaultLastHttpContent.EMPTY_LAST_CONTENT)); + + incomingStream(new ReactorNetty4HttpRequest(request), consumer.httpChannel()); + return response.sendObject(consumer); + } else { + final ReactorNetty4NonStreamingRequestConsumer consumer = new ReactorNetty4NonStreamingRequestConsumer<>( + this, + request, + response, + maxCompositeBufferComponents + ); - request.receiveContent().switchIfEmpty(Mono.just(DefaultLastHttpContent.EMPTY_LAST_CONTENT)).subscribe(consumer); - - return Mono.from(consumer).flatMap(hc -> { - final FullHttpResponse r = (FullHttpResponse) hc; - response.status(r.status()); - response.trailerHeaders(c -> r.trailingHeaders().forEach(h -> c.add(h.getKey(), h.getValue()))); - response.chunkedTransfer(false); - response.compression(true); - r.headers().forEach(h -> response.addHeader(h.getKey(), h.getValue())); - return Mono.from(response.sendObject(r.content())); - }); + request.receiveContent().switchIfEmpty(Mono.just(DefaultLastHttpContent.EMPTY_LAST_CONTENT)).subscribe(consumer); + + return Mono.from(consumer).flatMap(hc -> { + final FullHttpResponse r = (FullHttpResponse) hc; + response.status(r.status()); + response.trailerHeaders(c -> r.trailingHeaders().forEach(h -> c.add(h.getKey(), h.getValue()))); + response.chunkedTransfer(false); + response.compression(true); + r.headers().forEach(h -> response.addHeader(h.getKey(), h.getValue())); + return Mono.from(response.sendObject(r.content())); + }); + } } /** diff --git a/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/NonStreamingHttpChannel.java b/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4NonStreamingHttpChannel.java similarity index 92% rename from plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/NonStreamingHttpChannel.java rename to plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4NonStreamingHttpChannel.java index 98b359319ff1b..7df0b3c0c35fe 100644 --- a/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/NonStreamingHttpChannel.java +++ b/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4NonStreamingHttpChannel.java @@ -23,13 +23,13 @@ import reactor.netty.http.server.HttpServerRequest; import reactor.netty.http.server.HttpServerResponse; -class NonStreamingHttpChannel implements HttpChannel { +class ReactorNetty4NonStreamingHttpChannel implements HttpChannel { private final HttpServerRequest request; private final HttpServerResponse response; private final CompletableContext closeContext = new CompletableContext<>(); private final FluxSink emitter; - NonStreamingHttpChannel(HttpServerRequest request, HttpServerResponse response, FluxSink emitter) { + ReactorNetty4NonStreamingHttpChannel(HttpServerRequest request, HttpServerResponse response, FluxSink emitter) { this.request = request; this.response = response; this.emitter = emitter; diff --git a/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/NonStreamingRequestConsumer.java b/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4NonStreamingRequestConsumer.java similarity index 89% rename from plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/NonStreamingRequestConsumer.java rename to plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4NonStreamingRequestConsumer.java index d43e23e800e65..c09e7755b1670 100644 --- a/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/NonStreamingRequestConsumer.java +++ b/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4NonStreamingRequestConsumer.java @@ -25,7 +25,7 @@ import reactor.netty.http.server.HttpServerRequest; import reactor.netty.http.server.HttpServerResponse; -class NonStreamingRequestConsumer implements Consumer, Publisher, Disposable { +class ReactorNetty4NonStreamingRequestConsumer implements Consumer, Publisher, Disposable { private final HttpServerRequest request; private final HttpServerResponse response; private final CompositeByteBuf content; @@ -34,7 +34,7 @@ class NonStreamingRequestConsumer implements Consumer, private final AtomicBoolean disposed = new AtomicBoolean(false); private volatile FluxSink emitter; - NonStreamingRequestConsumer( + ReactorNetty4NonStreamingRequestConsumer( AbstractHttpServerTransport transport, HttpServerRequest request, HttpServerResponse response, @@ -64,12 +64,12 @@ public void accept(T message) { } } - public void process(HttpContent in, FluxSink emitter) { + void process(HttpContent in, FluxSink emitter) { // Consume request body in full before dispatching it content.addComponent(true, in.content().retain()); if (in instanceof LastHttpContent) { - final NonStreamingHttpChannel channel = new NonStreamingHttpChannel(request, response, emitter); + final ReactorNetty4NonStreamingHttpChannel channel = new ReactorNetty4NonStreamingHttpChannel(request, response, emitter); final HttpRequest r = createRequest(request, content); try { diff --git a/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4StreamingHttpChannel.java b/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4StreamingHttpChannel.java new file mode 100644 index 0000000000000..56dadea0477c5 --- /dev/null +++ b/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4StreamingHttpChannel.java @@ -0,0 +1,132 @@ +/* + * 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.http.reactor.netty4; + +import org.opensearch.common.concurrent.CompletableContext; +import org.opensearch.core.action.ActionListener; +import org.opensearch.core.common.bytes.BytesReference; +import org.opensearch.http.HttpChunk; +import org.opensearch.http.HttpResponse; +import org.opensearch.http.StreamingHttpChannel; +import org.opensearch.transport.reactor.netty4.Netty4Utils; + +import java.net.InetSocketAddress; +import java.util.List; +import java.util.Map; + +import io.netty.buffer.Unpooled; +import io.netty.handler.codec.http.DefaultHttpContent; +import io.netty.handler.codec.http.FullHttpResponse; +import io.netty.handler.codec.http.HttpContent; +import org.reactivestreams.Publisher; +import org.reactivestreams.Subscriber; +import reactor.core.publisher.Flux; +import reactor.core.publisher.FluxSink; +import reactor.netty.http.server.HttpServerRequest; +import reactor.netty.http.server.HttpServerResponse; + +class ReactorNetty4StreamingHttpChannel implements StreamingHttpChannel { + private final HttpServerRequest request; + private final HttpServerResponse response; + private final CompletableContext closeContext = new CompletableContext<>(); + private final Publisher receiver; + private final StreamingHttpContentSender sender; + private volatile FluxSink producer; + private volatile boolean lastChunkReceived = false; + + ReactorNetty4StreamingHttpChannel(HttpServerRequest request, HttpServerResponse response, StreamingHttpContentSender sender) { + this.request = request; + this.response = response; + this.sender = sender; + this.receiver = Flux.create(producer -> this.producer = producer); + this.request.withConnection(connection -> Netty4Utils.addListener(connection.channel().closeFuture(), closeContext)); + } + + @Override + public boolean isOpen() { + return true; + } + + @Override + public void close() { + request.withConnection(connection -> connection.channel().close()); + } + + @Override + public void addCloseListener(ActionListener listener) { + closeContext.addListener(ActionListener.toBiConsumer(listener)); + } + + @Override + public void sendChunk(HttpChunk chunk, ActionListener listener) { + sender.send(createContent(chunk), listener, chunk.isLast()); + } + + @Override + public void sendResponse(HttpResponse response, ActionListener listener) { + sender.send(createContent(response), listener, true); + } + + @Override + public void prepareResponse(int status, Map> headers) { + this.response.status(status); + headers.forEach((k, vs) -> vs.forEach(v -> this.response.addHeader(k, v))); + } + + @Override + public InetSocketAddress getRemoteAddress() { + return (InetSocketAddress) response.remoteAddress(); + } + + @Override + public InetSocketAddress getLocalAddress() { + return (InetSocketAddress) response.hostAddress(); + } + + @Override + public void receiveChunk(HttpChunk message) { + try { + if (lastChunkReceived) { + return; + } + + producer.next(message); + if (message.isLast()) { + lastChunkReceived = true; + producer.complete(); + } + } finally { + message.close(); + } + } + + @Override + public boolean isReadable() { + return producer != null; + } + + @Override + public boolean isWritable() { + return sender.isReady(); + } + + @Override + public void subscribe(Subscriber subscriber) { + receiver.subscribe(subscriber); + } + + private static HttpContent createContent(HttpResponse response) { + final FullHttpResponse fullHttpResponse = (FullHttpResponse) response; + return new DefaultHttpContent(fullHttpResponse.content()); + } + + private static HttpContent createContent(HttpChunk chunk) { + return new DefaultHttpContent(Unpooled.copiedBuffer(BytesReference.toByteBuffers(chunk.content()))); + } +} diff --git a/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4StreamingRequestConsumer.java b/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4StreamingRequestConsumer.java new file mode 100644 index 0000000000000..f34f54e561021 --- /dev/null +++ b/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4StreamingRequestConsumer.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.http.reactor.netty4; + +import org.opensearch.http.HttpChunk; +import org.opensearch.http.StreamingHttpChannel; + +import java.util.function.Consumer; + +import io.netty.handler.codec.http.HttpContent; +import io.netty.handler.codec.http.LastHttpContent; +import org.reactivestreams.Publisher; +import org.reactivestreams.Subscriber; +import reactor.netty.http.server.HttpServerRequest; +import reactor.netty.http.server.HttpServerResponse; + +class ReactorNetty4StreamingRequestConsumer implements Consumer, Publisher { + private final ReactorNetty4StreamingResponseProducer sender; + private final StreamingHttpChannel httpChannel; + + ReactorNetty4StreamingRequestConsumer(HttpServerRequest request, HttpServerResponse response) { + this.sender = new ReactorNetty4StreamingResponseProducer(); + this.httpChannel = new ReactorNetty4StreamingHttpChannel(request, response, sender); + } + + @Override + public void accept(T message) { + if (message instanceof LastHttpContent) { + httpChannel.receiveChunk(createChunk(message, true)); + } else if (message instanceof HttpContent) { + httpChannel.receiveChunk(createChunk(message, false)); + } + } + + @Override + public void subscribe(Subscriber s) { + sender.subscribe(s); + } + + HttpChunk createChunk(HttpContent chunk, boolean last) { + return new ReactorNetty4HttpChunk(chunk.content().retain(), last); + } + + StreamingHttpChannel httpChannel() { + return httpChannel; + } +} diff --git a/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4StreamingResponseProducer.java b/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4StreamingResponseProducer.java new file mode 100644 index 0000000000000..616edccdfc396 --- /dev/null +++ b/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4StreamingResponseProducer.java @@ -0,0 +1,54 @@ +/* + * 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.http.reactor.netty4; + +import org.opensearch.core.action.ActionListener; + +import io.netty.handler.codec.http.HttpContent; +import org.reactivestreams.Publisher; +import org.reactivestreams.Subscriber; +import reactor.core.publisher.Flux; +import reactor.core.publisher.FluxSink; + +class ReactorNetty4StreamingResponseProducer implements StreamingHttpContentSender, Publisher { + private final Publisher sender; + private volatile FluxSink emitter; + + ReactorNetty4StreamingResponseProducer() { + this.sender = Flux.create(emitter -> this.emitter = emitter); + } + + @Override + public void send(HttpContent content, ActionListener listener, boolean isLast) { + try { + emitter.next(content); + listener.onResponse(null); + if (isLast) { + emitter.complete(); + } + } catch (final Exception ex) { + emitter.error(ex); + listener.onFailure(ex); + } + } + + @Override + public void subscribe(Subscriber s) { + sender.subscribe(s); + } + + @Override + public boolean isReady() { + return emitter != null; + } + + FluxSink emitter() { + return emitter; + } +} diff --git a/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/StreamingHttpContentSender.java b/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/StreamingHttpContentSender.java new file mode 100644 index 0000000000000..f07d6fbb88349 --- /dev/null +++ b/plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/StreamingHttpContentSender.java @@ -0,0 +1,32 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.http.reactor.netty4; + +import org.opensearch.core.action.ActionListener; + +import io.netty.handler.codec.http.HttpContent; + +/** + * The generic interface for chunked {@link HttpContent} producers (response streaming). + */ +interface StreamingHttpContentSender { + /** + * Sends the next {@link HttpContent} over the wire + * @param content next {@link HttpContent} + * @param listener action listener + * @param isLast {@code true} if this is the last chunk, {@code false} otherwise + */ + void send(HttpContent content, ActionListener listener, boolean isLast); + + /** + * Returns {@code true} is this channel is ready for streaming response data, {@code false} otherwise + * @return {@code true} is this channel is ready for streaming response data, {@code false} otherwise + */ + boolean isReady(); +} diff --git a/plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ReactorHttpClient.java b/plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ReactorHttpClient.java index 920c895205023..0953e51484bd3 100644 --- a/plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ReactorHttpClient.java +++ b/plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ReactorHttpClient.java @@ -14,16 +14,22 @@ package org.opensearch.http.reactor.netty4; import org.opensearch.common.collect.Tuple; +import org.opensearch.common.xcontent.XContentType; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.tasks.Task; import org.opensearch.test.OpenSearchTestCase; import java.io.Closeable; +import java.io.IOException; +import java.io.UncheckedIOException; import java.net.InetSocketAddress; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.stream.Stream; import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; @@ -36,6 +42,7 @@ import io.netty.handler.codec.http.HttpContent; import io.netty.handler.codec.http.HttpHeaderNames; import io.netty.handler.codec.http.HttpMethod; +import io.netty.handler.codec.http.HttpRequest; import io.netty.handler.codec.http.HttpResponse; import io.netty.handler.codec.http.HttpVersion; import io.netty.handler.codec.http2.HttpConversionUtil; @@ -121,6 +128,11 @@ public final FullHttpResponse send(InetSocketAddress remoteAddress, FullHttpRequ return responses.get(0); } + public final FullHttpResponse stream(InetSocketAddress remoteAddress, HttpRequest httpRequest, Stream stream) + throws InterruptedException { + return sendRequestStream(remoteAddress, httpRequest, stream); + } + public final FullHttpResponse send(InetSocketAddress remoteAddress, FullHttpRequest httpRequest, HttpContent content) throws InterruptedException { final List responses = sendRequests( @@ -207,6 +219,46 @@ private List sendRequests( } } + private FullHttpResponse sendRequestStream( + final InetSocketAddress remoteAddress, + final HttpRequest request, + final Stream stream + ) { + final NioEventLoopGroup eventLoopGroup = new NioEventLoopGroup(1); + try { + final HttpClient client = createClient(remoteAddress, eventLoopGroup); + + return client.headers(h -> h.add(request.headers())) + .baseUrl(request.getUri()) + .request(request.method()) + .send(Flux.fromStream(stream).map(s -> { + try (XContentBuilder builder = XContentType.JSON.contentBuilder()) { + return Unpooled.wrappedBuffer( + s.toXContent(builder, ToXContent.EMPTY_PARAMS).toString().getBytes(StandardCharsets.UTF_8) + ); + } catch (final IOException ex) { + throw new UncheckedIOException(ex); + } + })) + .response( + (r, c) -> c.aggregate() + .map( + b -> new DefaultFullHttpResponse( + r.version(), + r.status(), + b.retain(), + r.responseHeaders(), + EmptyHttpHeaders.INSTANCE + ) + ) + ) + .blockLast(); + + } finally { + eventLoopGroup.shutdownGracefully().awaitUninterruptibly(); + } + } + private HttpClient createClient(final InetSocketAddress remoteAddress, final NioEventLoopGroup eventLoopGroup) { final HttpClient client = HttpClient.newConnection() .resolver(DefaultAddressResolverGroup.INSTANCE) diff --git a/plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ReactorNetty4HttpServerTransportStreamingTests.java b/plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ReactorNetty4HttpServerTransportStreamingTests.java new file mode 100644 index 0000000000000..a7bf71e58e9b6 --- /dev/null +++ b/plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ReactorNetty4HttpServerTransportStreamingTests.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.http.reactor.netty4; + +import org.apache.logging.log4j.message.ParameterizedMessage; +import org.opensearch.client.node.NodeClient; +import org.opensearch.common.lease.Releasable; +import org.opensearch.common.network.NetworkService; +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.MockBigArrays; +import org.opensearch.common.util.MockPageCacheRecycler; +import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.common.xcontent.XContentType; +import org.opensearch.common.xcontent.support.XContentHttpChunk; +import org.opensearch.core.common.transport.TransportAddress; +import org.opensearch.core.indices.breaker.NoneCircuitBreakerService; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.http.HttpServerTransport; +import org.opensearch.rest.RestChannel; +import org.opensearch.rest.RestHandler; +import org.opensearch.rest.RestRequest; +import org.opensearch.rest.RestRequest.Method; +import org.opensearch.rest.StreamingRestChannel; +import org.opensearch.telemetry.tracing.noop.NoopTracer; +import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.test.rest.FakeRestRequest; +import org.opensearch.threadpool.TestThreadPool; +import org.opensearch.threadpool.ThreadPool; +import org.opensearch.transport.reactor.SharedGroupFactory; +import org.junit.After; +import org.junit.Before; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.charset.StandardCharsets; +import java.time.Duration; +import java.util.Arrays; +import java.util.Collections; +import java.util.Map; +import java.util.Optional; +import java.util.function.Function; +import java.util.stream.Collectors; + +import io.netty.handler.codec.http.DefaultHttpRequest; +import io.netty.handler.codec.http.FullHttpResponse; +import io.netty.handler.codec.http.HttpMethod; +import io.netty.handler.codec.http.HttpRequest; +import io.netty.handler.codec.http.HttpResponseStatus; +import io.netty.handler.codec.http.HttpVersion; +import reactor.core.publisher.Flux; +import reactor.core.publisher.Mono; + +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.Matchers.equalTo; + +/** + * Tests for the {@link ReactorNetty4HttpServerTransport} class with streaming support. + */ +public class ReactorNetty4HttpServerTransportStreamingTests extends OpenSearchTestCase { + private static final Function XCONTENT_CONVERTER = (str) -> new ToXContent() { + @Override + public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException { + return builder.startObject().field("doc", str).endObject(); + } + }; + + private NetworkService networkService; + private ThreadPool threadPool; + private MockBigArrays bigArrays; + private ClusterSettings clusterSettings; + + @Before + public void setup() throws Exception { + networkService = new NetworkService(Collections.emptyList()); + threadPool = new TestThreadPool("test"); + bigArrays = new MockBigArrays(new MockPageCacheRecycler(Settings.EMPTY), new NoneCircuitBreakerService()); + clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + } + + @After + public void shutdown() throws Exception { + if (threadPool != null) { + threadPool.shutdownNow(); + } + threadPool = null; + networkService = null; + bigArrays = null; + clusterSettings = null; + } + + public void testRequestResponseStreaming() throws InterruptedException { + final String responseString = randomAlphaOfLength(4 * 1024); + final String url = "/stream/"; + + final ToXContent[] chunks = newChunks(responseString); + final HttpServerTransport.Dispatcher dispatcher = new HttpServerTransport.Dispatcher() { + @Override + public Optional dispatchHandler(String uri, String rawPath, Method method, Map params) { + return Optional.of(new RestHandler() { + @Override + public boolean supportsStreaming() { + return true; + } + + @Override + public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception { + logger.error("--> Unexpected request [{}]", request.uri()); + throw new AssertionError(); + } + }); + } + + @Override + public void dispatchRequest(final RestRequest request, final RestChannel channel, final ThreadContext threadContext) { + if (url.equals(request.uri())) { + assertThat(channel, instanceOf(StreamingRestChannel.class)); + final StreamingRestChannel streamingChannel = (StreamingRestChannel) channel; + + // Await at most 5 seconds till channel is ready for writing the response stream, fail otherwise + final Mono ready = Mono.fromRunnable(() -> { + while (!streamingChannel.isWritable()) { + Thread.onSpinWait(); + } + }).timeout(Duration.ofSeconds(5)); + + threadPool.executor(ThreadPool.Names.WRITE) + .execute(() -> Flux.concat(Flux.fromArray(newChunks(responseString)).map(e -> { + try (XContentBuilder builder = channel.newBuilder(XContentType.JSON, true)) { + return XContentHttpChunk.from(e.toXContent(builder, ToXContent.EMPTY_PARAMS)); + } catch (final IOException ex) { + throw new UncheckedIOException(ex); + } + }), Mono.just(XContentHttpChunk.last())) + .delaySubscription(ready) + .subscribe(streamingChannel::sendChunk, null, () -> { + if (channel.bytesOutput() instanceof Releasable) { + ((Releasable) channel.bytesOutput()).close(); + } + })); + } else { + logger.error("--> Unexpected successful uri [{}]", request.uri()); + throw new AssertionError(); + } + } + + @Override + public void dispatchBadRequest(final RestChannel channel, final ThreadContext threadContext, final Throwable cause) { + logger.error( + new ParameterizedMessage("--> Unexpected bad request [{}]", FakeRestRequest.requestToString(channel.request())), + cause + ); + throw new AssertionError(); + } + + }; + + try ( + ReactorNetty4HttpServerTransport transport = new ReactorNetty4HttpServerTransport( + Settings.EMPTY, + networkService, + bigArrays, + threadPool, + xContentRegistry(), + dispatcher, + clusterSettings, + new SharedGroupFactory(Settings.EMPTY), + NoopTracer.INSTANCE + ) + ) { + transport.start(); + final TransportAddress remoteAddress = randomFrom(transport.boundAddress().boundAddresses()); + + try (ReactorHttpClient client = ReactorHttpClient.create(false)) { + HttpRequest request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, url); + final FullHttpResponse response = client.stream(remoteAddress.address(), request, Arrays.stream(chunks)); + try { + assertThat(response.status(), equalTo(HttpResponseStatus.OK)); + byte[] bytes = new byte[response.content().readableBytes()]; + response.content().readBytes(bytes); + assertThat(new String(bytes, StandardCharsets.UTF_8), equalTo(Arrays.stream(newChunks(responseString)).map(s -> { + try (XContentBuilder builder = XContentType.JSON.contentBuilder()) { + return s.toXContent(builder, ToXContent.EMPTY_PARAMS).toString(); + } catch (final IOException ex) { + throw new UncheckedIOException(ex); + } + }).collect(Collectors.joining("")))); + } finally { + response.release(); + } + } + } + } + + private static ToXContent[] newChunks(final String responseString) { + final ToXContent[] chunks = new ToXContent[responseString.length() / 16]; + + for (int chunk = 0; chunk < responseString.length(); chunk += 16) { + chunks[chunk / 16] = XCONTENT_CONVERTER.apply(responseString.substring(chunk, chunk + 16)); + } + + return chunks; + } +} diff --git a/server/src/main/java/org/opensearch/action/ActionModule.java b/server/src/main/java/org/opensearch/action/ActionModule.java index 5e2b62614fc47..16c15f553951c 100644 --- a/server/src/main/java/org/opensearch/action/ActionModule.java +++ b/server/src/main/java/org/opensearch/action/ActionModule.java @@ -441,6 +441,7 @@ import org.opensearch.rest.action.cat.RestTemplatesAction; import org.opensearch.rest.action.cat.RestThreadPoolAction; import org.opensearch.rest.action.document.RestBulkAction; +import org.opensearch.rest.action.document.RestBulkStreamingAction; import org.opensearch.rest.action.document.RestDeleteAction; import org.opensearch.rest.action.document.RestGetAction; import org.opensearch.rest.action.document.RestGetSourceAction; @@ -887,6 +888,7 @@ public void initRestHandlers(Supplier nodesInCluster) { registerHandler.accept(new RestTermVectorsAction()); registerHandler.accept(new RestMultiTermVectorsAction()); registerHandler.accept(new RestBulkAction(settings)); + registerHandler.accept(new RestBulkStreamingAction(settings)); registerHandler.accept(new RestUpdateAction()); registerHandler.accept(new RestSearchAction()); diff --git a/server/src/main/java/org/opensearch/common/xcontent/support/XContentHttpChunk.java b/server/src/main/java/org/opensearch/common/xcontent/support/XContentHttpChunk.java new file mode 100644 index 0000000000000..15b63a0ac2030 --- /dev/null +++ b/server/src/main/java/org/opensearch/common/xcontent/support/XContentHttpChunk.java @@ -0,0 +1,65 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.common.xcontent.support; + +import org.opensearch.common.Nullable; +import org.opensearch.common.lease.Releasable; +import org.opensearch.core.common.bytes.BytesArray; +import org.opensearch.core.common.bytes.BytesReference; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.http.HttpChunk; + +/** + * Wraps the instance of the {@link XContentBuilder} into {@link HttpChunk} + */ +public final class XContentHttpChunk implements HttpChunk { + private final BytesReference content; + + /** + * Creates a new {@link HttpChunk} from {@link XContentBuilder} + * @param builder {@link XContentBuilder} instance + * @return new {@link HttpChunk} instance, if passed {@link XContentBuilder} us {@code null}, a last empty {@link HttpChunk} will be returned + */ + public static HttpChunk from(@Nullable final XContentBuilder builder) { + return new XContentHttpChunk(builder); + } + + /** + * Creates a new last empty {@link HttpChunk} + * @return last empty {@link HttpChunk} instance + */ + public static HttpChunk last() { + return new XContentHttpChunk(null); + } + + private XContentHttpChunk(@Nullable final XContentBuilder builder) { + if (builder == null /* no content */) { + content = BytesArray.EMPTY; + } else { + content = BytesReference.bytes(builder); + } + } + + @Override + public boolean isLast() { + return content == BytesArray.EMPTY; + } + + @Override + public BytesReference content() { + return content; + } + + @Override + public void close() { + if (content instanceof Releasable) { + ((Releasable) content).close(); + } + } +} diff --git a/server/src/main/java/org/opensearch/http/AbstractHttpServerTransport.java b/server/src/main/java/org/opensearch/http/AbstractHttpServerTransport.java index 257aca2b67990..991fbf12072be 100644 --- a/server/src/main/java/org/opensearch/http/AbstractHttpServerTransport.java +++ b/server/src/main/java/org/opensearch/http/AbstractHttpServerTransport.java @@ -357,6 +357,16 @@ protected void serverAcceptedChannel(HttpChannel httpChannel) { logger.trace(() -> new ParameterizedMessage("Http channel accepted: {}", httpChannel)); } + /** + * This method handles an incoming http request as a stream. + * + * @param httpRequest that is incoming + * @param httpChannel that received the http request + */ + public void incomingStream(HttpRequest httpRequest, final StreamingHttpChannel httpChannel) { + handleIncomingRequest(httpRequest, httpChannel, httpRequest.getInboundException()); + } + /** * This method handles an incoming http request. * @@ -438,29 +448,56 @@ private void handleIncomingRequest(final HttpRequest httpRequest, final HttpChan RestChannel innerChannel; ThreadContext threadContext = threadPool.getThreadContext(); try { - innerChannel = new DefaultRestChannel( - httpChannel, - httpRequest, - restRequest, - bigArrays, - handlingSettings, - threadContext, - corsHandler, - trace - ); + if (httpChannel instanceof StreamingHttpChannel) { + innerChannel = new DefaultStreamingRestChannel( + (StreamingHttpChannel) httpChannel, + httpRequest, + restRequest, + bigArrays, + handlingSettings, + threadContext, + corsHandler, + trace + ); + } else { + innerChannel = new DefaultRestChannel( + httpChannel, + httpRequest, + restRequest, + bigArrays, + handlingSettings, + threadContext, + corsHandler, + trace + ); + } } catch (final IllegalArgumentException e) { badRequestCause = ExceptionsHelper.useOrSuppress(badRequestCause, e); final RestRequest innerRequest = RestRequest.requestWithoutParameters(xContentRegistry, httpRequest, httpChannel); - innerChannel = new DefaultRestChannel( - httpChannel, - httpRequest, - innerRequest, - bigArrays, - handlingSettings, - threadContext, - corsHandler, - trace - ); + + if (httpChannel instanceof StreamingHttpChannel) { + innerChannel = new DefaultStreamingRestChannel( + (StreamingHttpChannel) httpChannel, + httpRequest, + innerRequest, + bigArrays, + handlingSettings, + threadContext, + corsHandler, + trace + ); + } else { + innerChannel = new DefaultRestChannel( + httpChannel, + httpRequest, + innerRequest, + bigArrays, + handlingSettings, + threadContext, + corsHandler, + trace + ); + } } channel = innerChannel; } diff --git a/server/src/main/java/org/opensearch/http/DefaultRestChannel.java b/server/src/main/java/org/opensearch/http/DefaultRestChannel.java index 7084600133a75..497ec799ff937 100644 --- a/server/src/main/java/org/opensearch/http/DefaultRestChannel.java +++ b/server/src/main/java/org/opensearch/http/DefaultRestChannel.java @@ -58,11 +58,11 @@ /** * The default rest channel for incoming requests. This class implements the basic logic for sending a rest - * response. It will set necessary headers nad ensure that bytes are released after the response is sent. + * response. It will set necessary headers and ensure that bytes are released after the response is sent. * * @opensearch.internal */ -public class DefaultRestChannel extends AbstractRestChannel implements RestChannel { +class DefaultRestChannel extends AbstractRestChannel implements RestChannel { static final String CLOSE = "close"; static final String CONNECTION = "connection"; diff --git a/server/src/main/java/org/opensearch/http/DefaultStreamingRestChannel.java b/server/src/main/java/org/opensearch/http/DefaultStreamingRestChannel.java new file mode 100644 index 0000000000000..7d8445294a4f3 --- /dev/null +++ b/server/src/main/java/org/opensearch/http/DefaultStreamingRestChannel.java @@ -0,0 +1,107 @@ +/* + * 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.http; + +import org.opensearch.common.Nullable; +import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.common.io.stream.ReleasableBytesStreamOutput; +import org.opensearch.common.lease.Releasable; +import org.opensearch.common.lease.Releasables; +import org.opensearch.common.util.BigArrays; +import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.core.action.ActionListener; +import org.opensearch.core.rest.RestStatus; +import org.opensearch.rest.RestRequest; +import org.opensearch.rest.StreamingRestChannel; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; + +import org.reactivestreams.Subscriber; + +import static org.opensearch.tasks.Task.X_OPAQUE_ID; + +/** + * The streaming rest channel for incoming requests. This class implements the logic for sending a streaming + * rest in chunks response. It will set necessary headers and ensure that bytes are released after the full + * response is sent. + * + * @opensearch.internal + */ +class DefaultStreamingRestChannel extends DefaultRestChannel implements StreamingRestChannel { + private final StreamingHttpChannel streamingHttpChannel; + @Nullable + private final HttpTracer tracerLog; + + DefaultStreamingRestChannel( + StreamingHttpChannel streamingHttpChannel, + HttpRequest httpRequest, + RestRequest request, + BigArrays bigArrays, + HttpHandlingSettings settings, + ThreadContext threadContext, + CorsHandler corsHandler, + @Nullable HttpTracer tracerLog + ) { + super(streamingHttpChannel, httpRequest, request, bigArrays, settings, threadContext, corsHandler, tracerLog); + this.streamingHttpChannel = streamingHttpChannel; + this.tracerLog = tracerLog; + } + + @Override + public void subscribe(Subscriber subscriber) { + streamingHttpChannel.subscribe(subscriber); + } + + @Override + public void sendChunk(HttpChunk chunk) { + String opaque = null; + boolean success = false; + final List toClose = new ArrayList<>(3); + String contentLength = null; + + try { + opaque = request.header(X_OPAQUE_ID); + contentLength = String.valueOf(chunk.content().length()); + toClose.add(chunk); + + BytesStreamOutput bytesStreamOutput = newBytesOutput(); + if (bytesStreamOutput instanceof ReleasableBytesStreamOutput) { + toClose.add((Releasable) bytesStreamOutput); + } + + ActionListener listener = ActionListener.wrap(() -> Releasables.close(toClose)); + streamingHttpChannel.sendChunk(chunk, listener); + success = true; + } finally { + if (success == false) { + Releasables.close(toClose); + } + if (tracerLog != null) { + tracerLog.traceChunk(chunk, streamingHttpChannel, contentLength, opaque, request.getRequestId(), success); + } + } + } + + @Override + public void prepareResponse(RestStatus status, Map> headers) { + streamingHttpChannel.prepareResponse(status.getStatus(), headers); + } + + @Override + public boolean isReadable() { + return streamingHttpChannel.isReadable(); + } + + @Override + public boolean isWritable() { + return streamingHttpChannel.isWritable(); + } +} diff --git a/server/src/main/java/org/opensearch/http/HttpChunk.java b/server/src/main/java/org/opensearch/http/HttpChunk.java new file mode 100644 index 0000000000000..7bcb526fe17bb --- /dev/null +++ b/server/src/main/java/org/opensearch/http/HttpChunk.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.http; + +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.common.lease.Releasable; +import org.opensearch.core.common.bytes.BytesReference; + +/** + * Represents a chunk of the HTTP request / response stream + * + * @opensearch.experimental + */ +@ExperimentalApi +public interface HttpChunk extends Releasable { + /** + * Signals this is the last chunk of the stream. + * @return "true" if this is the last chunk of the stream, "false" otherwise + */ + boolean isLast(); + + /** + * Returns the content of this chunk + * @return the content of this chunk + */ + BytesReference content(); +} diff --git a/server/src/main/java/org/opensearch/http/HttpServerTransport.java b/server/src/main/java/org/opensearch/http/HttpServerTransport.java index 012b69c29c1d4..f58d604151fd0 100644 --- a/server/src/main/java/org/opensearch/http/HttpServerTransport.java +++ b/server/src/main/java/org/opensearch/http/HttpServerTransport.java @@ -38,8 +38,12 @@ import org.opensearch.core.common.transport.BoundTransportAddress; import org.opensearch.core.service.ReportingService; import org.opensearch.rest.RestChannel; +import org.opensearch.rest.RestHandler; import org.opensearch.rest.RestRequest; +import java.util.Map; +import java.util.Optional; + /** * HTTP Transport server * @@ -61,6 +65,17 @@ public interface HttpServerTransport extends LifecycleComponent, ReportingServic * Dispatches HTTP requests. */ interface Dispatcher { + /** + * Finds the matching {@link RestHandler} that the request is going to be dispatched to, if any. + * @param uri request URI + * @param rawPath request raw path + * @param method request HTTP method + * @param params request parameters + * @return matching {@link RestHandler} that the request is going to be dispatched to, {@code Optional.empty()} if none match + */ + default Optional dispatchHandler(String uri, String rawPath, RestRequest.Method method, Map params) { + return Optional.empty(); + } /** * Dispatches the {@link RestRequest} to the relevant request handler or responds to the given rest channel directly if diff --git a/server/src/main/java/org/opensearch/http/HttpTracer.java b/server/src/main/java/org/opensearch/http/HttpTracer.java index 7a763b9ffb790..de1da4a20e294 100644 --- a/server/src/main/java/org/opensearch/http/HttpTracer.java +++ b/server/src/main/java/org/opensearch/http/HttpTracer.java @@ -128,6 +128,36 @@ void traceResponse( ); } + /** + * Logs the response chunk to a request that was logged by {@link #maybeTraceRequest(RestRequest, Exception)}. + * + * @param chunk response chunk + * @param httpChannel HttpChannel the response was sent on + * @param contentLength Value of the response content length header + * @param opaqueHeader Value of HTTP header {@link Task#X_OPAQUE_ID} + * @param requestId Request id as returned by {@link RestRequest#getRequestId()} + * @param success Whether the response was successfully sent + */ + void traceChunk( + HttpChunk chunk, + StreamingHttpChannel httpChannel, + String contentLength, + String opaqueHeader, + long requestId, + boolean success + ) { + logger.trace( + new ParameterizedMessage( + "[{}][{}][{}] sent next chunk to [{}] success [{}]", + requestId, + opaqueHeader, + contentLength, + httpChannel, + success + ) + ); + } + private void setTracerLogInclude(List tracerLogInclude) { this.tracerLogInclude = tracerLogInclude.toArray(Strings.EMPTY_ARRAY); } diff --git a/server/src/main/java/org/opensearch/http/StreamingHttpChannel.java b/server/src/main/java/org/opensearch/http/StreamingHttpChannel.java new file mode 100644 index 0000000000000..9bab25cb537ed --- /dev/null +++ b/server/src/main/java/org/opensearch/http/StreamingHttpChannel.java @@ -0,0 +1,56 @@ +/* + * 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.http; + +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.core.action.ActionListener; + +import java.util.List; +import java.util.Map; + +import org.reactivestreams.Publisher; + +/** + * Represents an HTTP communication channel with streaming capabilities. + * + * @opensearch.experimental + */ +@ExperimentalApi +public interface StreamingHttpChannel extends HttpChannel, Publisher { + /** + * Sends the next {@link HttpChunk} to the response stream + * @param chunk response chunk to send to channel + */ + void sendChunk(HttpChunk chunk, ActionListener listener); + + /** + * Receives the next {@link HttpChunk} from the request stream + * @param chunk next {@link HttpChunk} + */ + void receiveChunk(HttpChunk chunk); + + /** + * Prepares response before kicking of content streaming + * @param status response status + * @param headers response headers + */ + void prepareResponse(int status, Map> headers); + + /** + * Returns {@code true} is this channel is ready for streaming request data, {@code false} otherwise + * @return {@code true} is this channel is ready for streaming request data, {@code false} otherwise + */ + boolean isReadable(); + + /** + * Returns {@code true} is this channel is ready for streaming response data, {@code false} otherwise + * @return {@code true} is this channel is ready for streaming response data, {@code false} otherwise + */ + boolean isWritable(); +} diff --git a/server/src/main/java/org/opensearch/rest/BaseRestHandler.java b/server/src/main/java/org/opensearch/rest/BaseRestHandler.java index fc150405747ec..a5c9e6bcd286b 100644 --- a/server/src/main/java/org/opensearch/rest/BaseRestHandler.java +++ b/server/src/main/java/org/opensearch/rest/BaseRestHandler.java @@ -40,6 +40,7 @@ import org.opensearch.action.support.clustermanager.ClusterManagerNodeRequest; import org.opensearch.client.node.NodeClient; import org.opensearch.common.CheckedConsumer; +import org.opensearch.common.annotation.ExperimentalApi; import org.opensearch.common.annotation.PublicApi; import org.opensearch.common.collect.Tuple; import org.opensearch.common.logging.DeprecationLogger; @@ -200,6 +201,16 @@ protected final String unrecognized( @PublicApi(since = "1.0.0") protected interface RestChannelConsumer extends CheckedConsumer {} + /** + * Streaming REST requests are handled by preparing a streaming channel consumer that represents the execution of + * the request against a channel. + * + * @opensearch.experimental + */ + @FunctionalInterface + @ExperimentalApi + protected interface StreamingRestChannelConsumer extends CheckedConsumer {} + /** * Prepare the request for execution. Implementations should consume all request params before * returning the runnable for actual execution. Unconsumed params will immediately terminate @@ -317,6 +328,11 @@ public boolean allowsUnsafeBuffers() { public boolean allowSystemIndexAccessByDefault() { return delegate.allowSystemIndexAccessByDefault(); } + + @Override + public boolean supportsStreaming() { + return delegate.supportsStreaming(); + } } /** diff --git a/server/src/main/java/org/opensearch/rest/RestController.java b/server/src/main/java/org/opensearch/rest/RestController.java index 95abb9b3daeca..0c173523fa7cd 100644 --- a/server/src/main/java/org/opensearch/rest/RestController.java +++ b/server/src/main/java/org/opensearch/rest/RestController.java @@ -54,6 +54,7 @@ import org.opensearch.core.xcontent.MediaType; import org.opensearch.core.xcontent.MediaTypeRegistry; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.http.HttpChunk; import org.opensearch.http.HttpServerTransport; import org.opensearch.identity.IdentityService; import org.opensearch.identity.Subject; @@ -71,12 +72,16 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; import java.util.function.UnaryOperator; import java.util.stream.Collectors; +import org.reactivestreams.Subscriber; +import reactor.core.publisher.Mono; + import static org.opensearch.cluster.metadata.IndexNameExpressionResolver.SYSTEM_INDEX_ACCESS_CONTROL_HEADER_KEY; import static org.opensearch.core.rest.RestStatus.BAD_REQUEST; import static org.opensearch.core.rest.RestStatus.INTERNAL_SERVER_ERROR; @@ -257,6 +262,32 @@ public void registerHandler(final RestHandler restHandler) { ); } + @Override + public Optional dispatchHandler(String uri, String rawPath, RestRequest.Method method, Map params) { + // Loop through all possible handlers, attempting to dispatch the request + final Iterator allHandlers = getAllRestMethodHandlers(params, rawPath); + + while (allHandlers.hasNext()) { + final RestHandler handler; + final RestMethodHandlers handlers = allHandlers.next(); + if (handlers == null) { + handler = null; + } else { + handler = handlers.getHandler(method); + } + if (handler == null) { + final Set validMethodSet = getValidHandlerMethodSet(rawPath); + if (validMethodSet.contains(method) == false) { + return Optional.empty(); + } + } else { + return Optional.of(handler); + } + } + + return Optional.empty(); + } + @Override public void dispatchRequest(RestRequest request, RestChannel channel, ThreadContext threadContext) { try { @@ -318,8 +349,25 @@ private void dispatchRequest(RestRequest request, RestChannel channel, RestHandl } else { inFlightRequestsBreaker(circuitBreakerService).addWithoutBreaking(contentLength); } - // iff we could reserve bytes for the request we need to send the response also over this channel - responseChannel = new ResourceHandlingHttpChannel(channel, circuitBreakerService, contentLength); + + if (handler.supportsStreaming()) { + // The handler may support streaming but not the engine, in this case we fail with the bad request + if (channel instanceof StreamingRestChannel) { + responseChannel = new StreamHandlingHttpChannel((StreamingRestChannel) channel, circuitBreakerService, contentLength); + } else { + throw new IllegalStateException( + "The engine does not support HTTP streaming, unable to serve uri [" + + request.getHttpRequest().uri() + + "] and method [" + + request.getHttpRequest().method() + + "]" + ); + } + } else { + // if we could reserve bytes for the request we need to send the response also over this channel + responseChannel = new ResourceHandlingHttpChannel(channel, circuitBreakerService, contentLength); + } + // TODO: Count requests double in the circuit breaker if they need copying? if (handler.allowsUnsafeBuffers() == false) { request.ensureSafeBuffers(); @@ -642,7 +690,101 @@ private void close() { } inFlightRequestsBreaker(circuitBreakerService).addWithoutBreaking(-contentLength); } + } + + private static final class StreamHandlingHttpChannel implements StreamingRestChannel { + private final StreamingRestChannel delegate; + private final CircuitBreakerService circuitBreakerService; + private final int contentLength; + private final AtomicBoolean closed = new AtomicBoolean(); + private final AtomicBoolean subscribed = new AtomicBoolean(); + + StreamHandlingHttpChannel(StreamingRestChannel delegate, CircuitBreakerService circuitBreakerService, int contentLength) { + this.delegate = delegate; + this.circuitBreakerService = circuitBreakerService; + this.contentLength = contentLength; + } + + @Override + public XContentBuilder newBuilder() throws IOException { + return delegate.newBuilder(); + } + + @Override + public XContentBuilder newErrorBuilder() throws IOException { + return delegate.newErrorBuilder(); + } + + @Override + public XContentBuilder newBuilder(@Nullable MediaType mediaType, boolean useFiltering) throws IOException { + return delegate.newBuilder(mediaType, useFiltering); + } + + @Override + public XContentBuilder newBuilder(MediaType mediaType, MediaType responseContentType, boolean useFiltering) throws IOException { + return delegate.newBuilder(mediaType, responseContentType, useFiltering); + } + + @Override + public BytesStreamOutput bytesOutput() { + return delegate.bytesOutput(); + } + + @Override + public RestRequest request() { + return delegate.request(); + } + + @Override + public boolean detailedErrorsEnabled() { + return delegate.detailedErrorsEnabled(); + } + + @Override + public void sendResponse(RestResponse response) { + close(); + + // Check if subscribe() is already called, the headers and status are going to be sent + // over so we need to populate those **before** that, if possible. + if (subscribed.get() == false) { + prepareResponse(response.status(), Map.of("Content-Type", List.of(response.contentType()))); + Mono.ignoreElements(this).then(Mono.just(response)).subscribe(delegate::sendResponse); + } + } + + @Override + public void sendChunk(HttpChunk chunk) { + delegate.sendChunk(chunk); + } + + @Override + public void prepareResponse(RestStatus status, Map> headers) { + delegate.prepareResponse(status, headers); + } + + @Override + public void subscribe(Subscriber subscriber) { + subscribed.set(true); + delegate.subscribe(subscriber); + } + + private void close() { + // attempt to close once atomically + if (closed.compareAndSet(false, true) == false) { + throw new IllegalStateException("Channel is already closed"); + } + inFlightRequestsBreaker(circuitBreakerService).addWithoutBreaking(-contentLength); + } + @Override + public boolean isReadable() { + return delegate.isReadable(); + } + + @Override + public boolean isWritable() { + return delegate.isWritable(); + } } private static CircuitBreaker inFlightRequestsBreaker(CircuitBreakerService circuitBreakerService) { diff --git a/server/src/main/java/org/opensearch/rest/RestHandler.java b/server/src/main/java/org/opensearch/rest/RestHandler.java index 877afdd951088..1139e5fc65f31 100644 --- a/server/src/main/java/org/opensearch/rest/RestHandler.java +++ b/server/src/main/java/org/opensearch/rest/RestHandler.java @@ -72,6 +72,14 @@ default boolean supportsContentStream() { return false; } + /** + * Indicates if the RestHandler supports request / response streaming. Please note that the transport engine has to support + * streaming as well. + */ + default boolean supportsStreaming() { + return false; + } + /** * Indicates if the RestHandler supports working with pooled buffers. If the request handler will not escape the return * {@link RestRequest#content()} or any buffers extracted from it then there is no need to make a copies of any pooled buffers in the diff --git a/server/src/main/java/org/opensearch/rest/StreamingRestChannel.java b/server/src/main/java/org/opensearch/rest/StreamingRestChannel.java new file mode 100644 index 0000000000000..9460fa5a226c2 --- /dev/null +++ b/server/src/main/java/org/opensearch/rest/StreamingRestChannel.java @@ -0,0 +1,51 @@ +/* + * 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.rest; + +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.core.rest.RestStatus; +import org.opensearch.http.HttpChunk; + +import java.util.List; +import java.util.Map; + +import org.reactivestreams.Publisher; + +/** + * A streaming channel used to prepare response and sending the response in chunks. + * + * @opensearch.experimental + */ +@ExperimentalApi +public interface StreamingRestChannel extends RestChannel, Publisher { + /** + * Sends the next {@link HttpChunk} to the response stream + * @param chunk response chunk + */ + void sendChunk(HttpChunk chunk); + + /** + * Prepares response before kicking of content streaming + * @param status response status + * @param headers response headers + */ + void prepareResponse(RestStatus status, Map> headers); + + /** + * Returns {@code true} is this channel is ready for streaming request data, {@code false} otherwise + * @return {@code true} is this channel is ready for streaming request data, {@code false} otherwise + */ + boolean isReadable(); + + /** + * Returns {@code true} is this channel is ready for streaming response data, {@code false} otherwise + * @return {@code true} is this channel is ready for streaming response data, {@code false} otherwise + */ + boolean isWritable(); +} diff --git a/server/src/main/java/org/opensearch/rest/action/document/RestBulkStreamingAction.java b/server/src/main/java/org/opensearch/rest/action/document/RestBulkStreamingAction.java new file mode 100644 index 0000000000000..ce6e32a7824c9 --- /dev/null +++ b/server/src/main/java/org/opensearch/rest/action/document/RestBulkStreamingAction.java @@ -0,0 +1,199 @@ +/* + * 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.rest.action.document; + +import org.opensearch.action.ActionRequestValidationException; +import org.opensearch.action.DocWriteRequest; +import org.opensearch.action.bulk.BulkItemResponse; +import org.opensearch.action.bulk.BulkRequest; +import org.opensearch.action.bulk.BulkResponse; +import org.opensearch.action.bulk.BulkShardRequest; +import org.opensearch.action.support.ActiveShardCount; +import org.opensearch.client.Requests; +import org.opensearch.client.node.NodeClient; +import org.opensearch.common.collect.Tuple; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.unit.TimeValue; +import org.opensearch.common.xcontent.support.XContentHttpChunk; +import org.opensearch.core.action.ActionListener; +import org.opensearch.core.rest.RestStatus; +import org.opensearch.core.xcontent.MediaType; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.rest.BaseRestHandler; +import org.opensearch.rest.BytesRestResponse; +import org.opensearch.rest.RestRequest; +import org.opensearch.rest.StreamingRestChannel; +import org.opensearch.search.fetch.subphase.FetchSourceContext; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CompletableFuture; + +import reactor.core.publisher.Flux; +import reactor.core.publisher.Mono; + +import static java.util.Arrays.asList; +import static java.util.Collections.unmodifiableList; +import static org.opensearch.rest.RestRequest.Method.POST; +import static org.opensearch.rest.RestRequest.Method.PUT; + +/** + * 
+ * { "index" : { "_index" : "test", "_id" : "1" }
+ * { "type1" : { "field1" : "value1" } }
+ * { "delete" : { "_index" : "test", "_id" : "2" } }
+ * { "create" : { "_index" : "test", "_id" : "1" }
+ * { "type1" : { "field1" : "value1" } }
+ *
+ * + * @opensearch.api + */ +public class RestBulkStreamingAction extends BaseRestHandler { + private static final BulkResponse EMPTY = new BulkResponse(new BulkItemResponse[0], 0L); + private final boolean allowExplicitIndex; + + public RestBulkStreamingAction(Settings settings) { + this.allowExplicitIndex = MULTI_ALLOW_EXPLICIT_INDEX.get(settings); + } + + @Override + public List routes() { + return unmodifiableList( + asList( + new Route(POST, "/_bulk/stream"), + new Route(PUT, "/_bulk/stream"), + new Route(POST, "/{index}/_bulk/stream"), + new Route(PUT, "/{index}/_bulk/stream") + ) + ); + } + + @Override + public String getName() { + return "streaming_bulk_action"; + } + + @Override + public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { + final String defaultIndex = request.param("index"); + final String defaultRouting = request.param("routing"); + final String defaultPipeline = request.param("pipeline"); + final String waitForActiveShards = request.param("wait_for_active_shards"); + final Boolean defaultRequireAlias = request.paramAsBoolean(DocWriteRequest.REQUIRE_ALIAS, null); + final TimeValue timeout = request.paramAsTime("timeout", BulkShardRequest.DEFAULT_TIMEOUT); + final String refresh = request.param("refresh"); + + final StreamingRestChannelConsumer consumer = (channel) -> { + final MediaType mediaType = request.getMediaType(); + + // Set the content type and the status code before sending the response stream over + channel.prepareResponse(RestStatus.OK, Map.of("Content-Type", List.of(mediaType.mediaTypeWithoutParameters()))); + + // This is initial implementation at the moment which transforms each single request stream chunk into + // individual bulk request and streams each response back. Another source of inefficiency comes from converting + // bulk response from raw (json/yaml/...) to model and back to raw (json/yaml/...). + + // TODOs: + // - add batching (by interval and/or count) + // - eliminate serialization inefficiencies + Flux.from(channel).map(chunk -> { + FetchSourceContext defaultFetchSourceContext = FetchSourceContext.parseFromRestRequest(request); + BulkRequest bulkRequest = Requests.bulkRequest(); + if (waitForActiveShards != null) { + bulkRequest.waitForActiveShards(ActiveShardCount.parseString(waitForActiveShards)); + } + + bulkRequest.timeout(timeout); + bulkRequest.setRefreshPolicy(refresh); + + try { + bulkRequest.add( + chunk.content(), + defaultIndex, + defaultRouting, + defaultFetchSourceContext, + defaultPipeline, + defaultRequireAlias, + allowExplicitIndex, + request.getMediaType() + ); + } catch (final IOException ex) { + throw new UncheckedIOException(ex); + } + + return Tuple.tuple(chunk.isLast(), bulkRequest); + }).flatMap(tuple -> { + final CompletableFuture f = new CompletableFuture<>(); + + if (tuple.v2().requests().isEmpty()) { + // this is the last request with no items + f.complete(EMPTY); + } else { + client.bulk(tuple.v2(), new ActionListener() { + @Override + public void onResponse(BulkResponse response) { + f.complete(response); + } + + @Override + public void onFailure(Exception ex) { + f.completeExceptionally(ex); + } + }); + + if (tuple.v1() == true /* last chunk */ ) { + return Flux.just(f, CompletableFuture.completedFuture(EMPTY)); + } + } + + return Mono.just(f); + }).concatMap(f -> Mono.fromFuture(f).doOnNext(r -> { + try { + if (r == EMPTY) { + channel.sendChunk(XContentHttpChunk.last()); + } else { + try (XContentBuilder builder = channel.newBuilder(mediaType, true)) { + channel.sendChunk(XContentHttpChunk.from(r.toXContent(builder, ToXContent.EMPTY_PARAMS))); + } + } + } catch (IOException ex) { + throw new UncheckedIOException(ex); + } + })).subscribe(); + }; + + return channel -> { + if (channel instanceof StreamingRestChannel) { + consumer.accept((StreamingRestChannel) channel); + } else { + final ActionRequestValidationException validationError = new ActionRequestValidationException(); + validationError.addValidationError("Unable to initiate request / response streaming over non-streaming channel"); + channel.sendResponse(new BytesRestResponse(channel, validationError)); + } + }; + } + + @Override + public boolean supportsContentStream() { + return true; + } + + @Override + public boolean supportsStreaming() { + return true; + } + + @Override + public boolean allowsUnsafeBuffers() { + return true; + } +} diff --git a/server/src/test/java/org/opensearch/rest/action/document/RestBulkStreamingActionTests.java b/server/src/test/java/org/opensearch/rest/action/document/RestBulkStreamingActionTests.java new file mode 100644 index 0000000000000..aa52ffe08927c --- /dev/null +++ b/server/src/test/java/org/opensearch/rest/action/document/RestBulkStreamingActionTests.java @@ -0,0 +1,89 @@ +/* + * 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.rest.action.document; + +import org.opensearch.Version; +import org.opensearch.action.bulk.BulkRequest; +import org.opensearch.action.bulk.BulkResponse; +import org.opensearch.client.node.NodeClient; +import org.opensearch.common.SetOnce; +import org.opensearch.common.xcontent.XContentType; +import org.opensearch.core.action.ActionListener; +import org.opensearch.core.common.bytes.BytesArray; +import org.opensearch.core.rest.RestStatus; +import org.opensearch.core.xcontent.MediaTypeRegistry; +import org.opensearch.rest.RestChannel; +import org.opensearch.rest.RestRequest; +import org.opensearch.rest.RestResponse; +import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.test.client.NoOpNodeClient; +import org.opensearch.test.rest.FakeRestRequest; + +import java.util.HashMap; +import java.util.Map; + +import org.mockito.ArgumentCaptor; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Tests for {@link RestBulkStreamingAction}. + */ +public class RestBulkStreamingActionTests extends OpenSearchTestCase { + public void testBulkStreamingPipelineUpsert() throws Exception { + SetOnce bulkCalled = new SetOnce<>(); + try (NodeClient verifyingClient = new NoOpNodeClient(this.getTestName()) { + @Override + public void bulk(BulkRequest request, ActionListener listener) { + bulkCalled.set(true); + } + }) { + final Map params = new HashMap<>(); + params.put("pipeline", "timestamps"); + + final FakeRestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withPath("my_index/_bulk/streaming") + .withParams(params) + .withContent( + new BytesArray( + "{\"index\":{\"_id\":\"1\"}}\n" + + "{\"field1\":\"val1\"}\n" + + "{\"update\":{\"_id\":\"2\"}}\n" + + "{\"script\":{\"source\":\"ctx._source.counter++;\"},\"upsert\":{\"field1\":\"upserted_val\"}}\n" + ), + MediaTypeRegistry.JSON + ) + .withMethod(RestRequest.Method.POST) + .build(); + request.param("error_trace", "false"); + request.param("rest.exception.stacktrace.skip", "false"); + + final RestChannel channel = mock(RestChannel.class); + when(channel.request()).thenReturn(request); + when(channel.newErrorBuilder()).thenReturn(XContentType.YAML.contentBuilder()); + when(channel.detailedErrorsEnabled()).thenReturn(true); + + new RestBulkStreamingAction(settings(Version.CURRENT).build()).handleRequest(request, channel, verifyingClient); + + final ArgumentCaptor responseCaptor = ArgumentCaptor.captor(); + verify(channel).sendResponse(responseCaptor.capture()); + + // We do not expect `bulk` action to be called since the default HTTP transport (netty4) does not support streaming + assertThat(bulkCalled.get(), equalTo(null)); + assertThat(responseCaptor.getValue().status(), equalTo(RestStatus.BAD_REQUEST)); + assertThat( + responseCaptor.getValue().content().utf8ToString(), + containsString("Unable to initiate request / response streaming") + ); + } + } +} From 1084ba9b167af8342be0acea075c83fa1673dcb1 Mon Sep 17 00:00:00 2001 From: Himshikha Gupta Date: Tue, 11 Jun 2024 04:51:32 +0530 Subject: [PATCH 7/7] [Remote Routing Table] Add write flow for remote routing table (#13870) * Introduce RemoteRoutingTableService for shard routing table management Signed-off-by: Himshikha Gupta Co-authored-by: Bukhtawar Khan Co-authored-by: Arpit Bandejiya --- CHANGELOG.md | 1 + .../routing/IndexShardRoutingTable.java | 4 +- .../InternalRemoteRoutingTableService.java | 303 +++++++++++ .../remote/NoopRemoteRoutingTableService.java | 77 +++ .../remote/RemoteRoutingTableService.java | 87 ++-- .../RemoteRoutingTableServiceFactory.java | 41 ++ .../opensearch/common/blobstore/BlobPath.java | 14 + .../common/settings/ClusterSettings.java | 3 + .../remote/ClusterMetadataManifest.java | 30 +- .../remote/RemoteClusterStateService.java | 98 +++- .../index/remote/RemoteIndexPath.java | 3 +- .../index/remote/RemoteStoreEnums.java | 45 +- .../index/remote/RemoteStorePathStrategy.java | 114 ++++- .../RemoteSegmentStoreDirectoryFactory.java | 4 +- .../RemoteStoreLockManagerFactory.java | 2 +- .../index/translog/RemoteFsTranslog.java | 4 +- .../routing/IndexShardRoutingTableTests.java | 6 + ...RemoteRoutingTableServiceFactoryTests.java | 51 ++ .../RemoteRoutingTableServiceTests.java | 484 +++++++++++++++++- .../remote/ClusterMetadataManifestTests.java | 10 +- .../RemoteClusterStateServiceTests.java | 177 ++++++- .../index/remote/RemoteStoreEnumsTests.java | 150 ++---- .../remote/RemoteStorePathStrategyTests.java | 87 ++++ .../opensearch/test/OpenSearchTestCase.java | 2 +- 24 files changed, 1544 insertions(+), 253 deletions(-) create mode 100644 server/src/main/java/org/opensearch/cluster/routing/remote/InternalRemoteRoutingTableService.java create mode 100644 server/src/main/java/org/opensearch/cluster/routing/remote/NoopRemoteRoutingTableService.java create mode 100644 server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceFactory.java create mode 100644 server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceFactoryTests.java create mode 100644 server/src/test/java/org/opensearch/index/remote/RemoteStorePathStrategyTests.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 681550cedcc0f..c6b9822d9a8f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Allow setting query parameters on requests ([#13776](https://github.com/opensearch-project/OpenSearch/issues/13776)) - Add capability to disable source recovery_source for an index ([#13590](https://github.com/opensearch-project/OpenSearch/pull/13590)) - Add remote routing table for remote state publication with experimental feature flag ([#13304](https://github.com/opensearch-project/OpenSearch/pull/13304)) +- Add upload flow for writing routing table to remote store ([#13870](https://github.com/opensearch-project/OpenSearch/pull/13870)) - Add dynamic action retry timeout setting ([#14022](https://github.com/opensearch-project/OpenSearch/issues/14022)) - [Remote Store] Add support to disable flush based on translog reader count ([#14027](https://github.com/opensearch-project/OpenSearch/pull/14027)) - Add recovery chunk size setting ([#13997](https://github.com/opensearch-project/OpenSearch/pull/13997)) diff --git a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java index fd8cbea42c12f..479143fa9a2f0 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java +++ b/server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java @@ -738,9 +738,7 @@ public boolean equals(Object o) { IndexShardRoutingTable that = (IndexShardRoutingTable) o; if (!shardId.equals(that.shardId)) return false; - if (!shards.equals(that.shards)) return false; - - return true; + return shards.size() == that.shards.size() && shards.containsAll(that.shards) && that.shards.containsAll(shards); } @Override diff --git a/server/src/main/java/org/opensearch/cluster/routing/remote/InternalRemoteRoutingTableService.java b/server/src/main/java/org/opensearch/cluster/routing/remote/InternalRemoteRoutingTableService.java new file mode 100644 index 0000000000000..dcf106914c571 --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/routing/remote/InternalRemoteRoutingTableService.java @@ -0,0 +1,303 @@ +/* + * 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.routing.remote; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.lucene.store.IndexInput; +import org.opensearch.action.LatchedActionListener; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.DiffableUtils; +import org.opensearch.cluster.routing.IndexRoutingTable; +import org.opensearch.cluster.routing.RoutingTable; +import org.opensearch.common.CheckedRunnable; +import org.opensearch.common.blobstore.AsyncMultiStreamBlobContainer; +import org.opensearch.common.blobstore.BlobContainer; +import org.opensearch.common.blobstore.BlobPath; +import org.opensearch.common.blobstore.stream.write.WritePriority; +import org.opensearch.common.blobstore.transfer.RemoteTransferContainer; +import org.opensearch.common.blobstore.transfer.stream.OffsetRangeIndexInputStream; +import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.common.lifecycle.AbstractLifecycleComponent; +import org.opensearch.common.lucene.store.ByteArrayIndexInput; +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Setting; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.io.IOUtils; +import org.opensearch.core.action.ActionListener; +import org.opensearch.core.common.bytes.BytesReference; +import org.opensearch.gateway.remote.ClusterMetadataManifest; +import org.opensearch.gateway.remote.RemoteClusterStateService; +import org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTable; +import org.opensearch.index.remote.RemoteStoreEnums; +import org.opensearch.index.remote.RemoteStorePathStrategy; +import org.opensearch.index.remote.RemoteStoreUtils; +import org.opensearch.node.Node; +import org.opensearch.node.remotestore.RemoteStoreNodeAttribute; +import org.opensearch.repositories.RepositoriesService; +import org.opensearch.repositories.Repository; +import org.opensearch.repositories.blobstore.BlobStoreRepository; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.function.Function; +import java.util.function.Supplier; +import java.util.stream.Collectors; + +import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteRoutingTableEnabled; + +/** + * A Service which provides APIs to upload and download routing table from remote store. + * + * @opensearch.internal + */ +public class InternalRemoteRoutingTableService extends AbstractLifecycleComponent implements RemoteRoutingTableService { + + /** + * This setting is used to set the remote routing table store blob store path type strategy. + */ + public static final Setting REMOTE_ROUTING_TABLE_PATH_TYPE_SETTING = new Setting<>( + "cluster.remote_store.routing_table.path_type", + RemoteStoreEnums.PathType.HASHED_PREFIX.toString(), + RemoteStoreEnums.PathType::parseString, + Setting.Property.NodeScope, + Setting.Property.Dynamic + ); + + /** + * This setting is used to set the remote routing table store blob store path hash algorithm strategy. + * This setting will come to effect if the {@link #REMOTE_ROUTING_TABLE_PATH_TYPE_SETTING} + * is either {@code HASHED_PREFIX} or {@code HASHED_INFIX}. + */ + public static final Setting REMOTE_ROUTING_TABLE_PATH_HASH_ALGO_SETTING = new Setting<>( + "cluster.remote_store.routing_table.path_hash_algo", + RemoteStoreEnums.PathHashAlgorithm.FNV_1A_BASE64.toString(), + RemoteStoreEnums.PathHashAlgorithm::parseString, + Setting.Property.NodeScope, + Setting.Property.Dynamic + ); + + public static final String INDEX_ROUTING_PATH_TOKEN = "index-routing"; + public static final String INDEX_ROUTING_FILE_PREFIX = "index_routing"; + public static final String DELIMITER = "__"; + public static final String INDEX_ROUTING_METADATA_PREFIX = "indexRouting--"; + + private static final Logger logger = LogManager.getLogger(InternalRemoteRoutingTableService.class); + private final Settings settings; + private final Supplier repositoriesService; + private BlobStoreRepository blobStoreRepository; + private RemoteStoreEnums.PathType pathType; + private RemoteStoreEnums.PathHashAlgorithm pathHashAlgo; + + public InternalRemoteRoutingTableService( + Supplier repositoriesService, + Settings settings, + ClusterSettings clusterSettings + ) { + assert isRemoteRoutingTableEnabled(settings) : "Remote routing table is not enabled"; + this.repositoriesService = repositoriesService; + this.settings = settings; + this.pathType = clusterSettings.get(REMOTE_ROUTING_TABLE_PATH_TYPE_SETTING); + this.pathHashAlgo = clusterSettings.get(REMOTE_ROUTING_TABLE_PATH_HASH_ALGO_SETTING); + clusterSettings.addSettingsUpdateConsumer(REMOTE_ROUTING_TABLE_PATH_TYPE_SETTING, this::setPathTypeSetting); + clusterSettings.addSettingsUpdateConsumer(REMOTE_ROUTING_TABLE_PATH_HASH_ALGO_SETTING, this::setPathHashAlgoSetting); + } + + private void setPathTypeSetting(RemoteStoreEnums.PathType pathType) { + this.pathType = pathType; + } + + private void setPathHashAlgoSetting(RemoteStoreEnums.PathHashAlgorithm pathHashAlgo) { + this.pathHashAlgo = pathHashAlgo; + } + + public List getIndicesRouting(RoutingTable routingTable) { + return new ArrayList<>(routingTable.indicesRouting().values()); + } + + /** + * Returns diff between the two routing tables, which includes upserts and deletes. + * @param before previous routing table + * @param after current routing table + * @return diff of the previous and current routing table + */ + public DiffableUtils.MapDiff> getIndicesRoutingMapDiff( + RoutingTable before, + RoutingTable after + ) { + return DiffableUtils.diff( + before.getIndicesRouting(), + after.getIndicesRouting(), + DiffableUtils.getStringKeySerializer(), + CUSTOM_ROUTING_TABLE_VALUE_SERIALIZER + ); + } + + /** + * Create async action for writing one {@code IndexRoutingTable} to remote store + * @param clusterState current cluster state + * @param indexRouting indexRoutingTable to write to remote store + * @param latchedActionListener listener for handling async action response + * @param clusterBasePath base path for remote file + * @return returns runnable async action + */ + public CheckedRunnable getIndexRoutingAsyncAction( + ClusterState clusterState, + IndexRoutingTable indexRouting, + LatchedActionListener latchedActionListener, + BlobPath clusterBasePath + ) { + + BlobPath indexRoutingPath = clusterBasePath.add(INDEX_ROUTING_PATH_TOKEN); + BlobPath path = pathType.path( + RemoteStorePathStrategy.PathInput.builder().basePath(indexRoutingPath).indexUUID(indexRouting.getIndex().getUUID()).build(), + pathHashAlgo + ); + final BlobContainer blobContainer = blobStoreRepository.blobStore().blobContainer(path); + + final String fileName = getIndexRoutingFileName(clusterState.term(), clusterState.version()); + + ActionListener completionListener = ActionListener.wrap( + resp -> latchedActionListener.onResponse( + new ClusterMetadataManifest.UploadedIndexMetadata( + indexRouting.getIndex().getName(), + indexRouting.getIndex().getUUID(), + path.buildAsString() + fileName, + INDEX_ROUTING_METADATA_PREFIX + ) + ), + ex -> latchedActionListener.onFailure( + new RemoteClusterStateService.RemoteStateTransferException( + "Exception in writing index to remote store: " + indexRouting.getIndex().toString(), + ex + ) + ) + ); + + return () -> uploadIndex(indexRouting, fileName, blobContainer, completionListener); + } + + /** + * Combines IndicesRoutingMetadata from previous manifest and current uploaded indices, removes deleted indices. + * @param previousManifest previous manifest, used to get all existing indices routing paths + * @param indicesRoutingUploaded current uploaded indices routings + * @param indicesRoutingToDelete indices to delete + * @return combined list of metadata + */ + public List getAllUploadedIndicesRouting( + ClusterMetadataManifest previousManifest, + List indicesRoutingUploaded, + List indicesRoutingToDelete + ) { + final Map allUploadedIndicesRouting = previousManifest.getIndicesRouting() + .stream() + .collect(Collectors.toMap(ClusterMetadataManifest.UploadedIndexMetadata::getIndexName, Function.identity())); + + indicesRoutingUploaded.forEach( + uploadedIndexRouting -> allUploadedIndicesRouting.put(uploadedIndexRouting.getIndexName(), uploadedIndexRouting) + ); + indicesRoutingToDelete.forEach(allUploadedIndicesRouting::remove); + + return new ArrayList<>(allUploadedIndicesRouting.values()); + } + + private void uploadIndex( + IndexRoutingTable indexRouting, + String fileName, + BlobContainer blobContainer, + ActionListener completionListener + ) { + RemoteIndexRoutingTable indexRoutingInput = new RemoteIndexRoutingTable(indexRouting); + BytesReference bytesInput = null; + try (BytesStreamOutput streamOutput = new BytesStreamOutput()) { + indexRoutingInput.writeTo(streamOutput); + bytesInput = streamOutput.bytes(); + } catch (IOException e) { + logger.error("Failed to serialize IndexRoutingTable for [{}]: [{}]", indexRouting, e); + completionListener.onFailure(e); + return; + } + + if (blobContainer instanceof AsyncMultiStreamBlobContainer == false) { + try { + blobContainer.writeBlob(fileName, bytesInput.streamInput(), bytesInput.length(), true); + completionListener.onResponse(null); + } catch (IOException e) { + logger.error("Failed to write IndexRoutingTable to remote store for indexRouting [{}]: [{}]", indexRouting, e); + completionListener.onFailure(e); + } + return; + } + + try (IndexInput input = new ByteArrayIndexInput("indexrouting", BytesReference.toBytes(bytesInput))) { + try ( + RemoteTransferContainer remoteTransferContainer = new RemoteTransferContainer( + fileName, + fileName, + input.length(), + true, + WritePriority.URGENT, + (size, position) -> new OffsetRangeIndexInputStream(input, size, position), + null, + false + ) + ) { + ((AsyncMultiStreamBlobContainer) blobContainer).asyncBlobUpload( + remoteTransferContainer.createWriteContext(), + completionListener + ); + } catch (IOException e) { + logger.error("Failed to write IndexRoutingTable to remote store for indexRouting [{}]: [{}]", indexRouting, e); + completionListener.onFailure(e); + } + } catch (IOException e) { + logger.error( + "Failed to create transfer object for IndexRoutingTable for remote store upload for indexRouting [{}]: [{}]", + indexRouting, + e + ); + completionListener.onFailure(e); + } + } + + private String getIndexRoutingFileName(long term, long version) { + return String.join( + DELIMITER, + INDEX_ROUTING_FILE_PREFIX, + RemoteStoreUtils.invertLong(term), + RemoteStoreUtils.invertLong(version), + RemoteStoreUtils.invertLong(System.currentTimeMillis()) + ); + } + + @Override + protected void doClose() throws IOException { + if (blobStoreRepository != null) { + IOUtils.close(blobStoreRepository); + } + } + + @Override + protected void doStart() { + assert isRemoteRoutingTableEnabled(settings) == true : "Remote routing table is not enabled"; + final String remoteStoreRepo = settings.get( + Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY + ); + assert remoteStoreRepo != null : "Remote routing table repository is not configured"; + final Repository repository = repositoriesService.get().repository(remoteStoreRepo); + assert repository instanceof BlobStoreRepository : "Repository should be instance of BlobStoreRepository"; + blobStoreRepository = (BlobStoreRepository) repository; + } + + @Override + protected void doStop() {} + +} diff --git a/server/src/main/java/org/opensearch/cluster/routing/remote/NoopRemoteRoutingTableService.java b/server/src/main/java/org/opensearch/cluster/routing/remote/NoopRemoteRoutingTableService.java new file mode 100644 index 0000000000000..b52c00f1f8576 --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/routing/remote/NoopRemoteRoutingTableService.java @@ -0,0 +1,77 @@ +/* + * 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.routing.remote; + +import org.opensearch.action.LatchedActionListener; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.DiffableUtils; +import org.opensearch.cluster.routing.IndexRoutingTable; +import org.opensearch.cluster.routing.RoutingTable; +import org.opensearch.common.CheckedRunnable; +import org.opensearch.common.blobstore.BlobPath; +import org.opensearch.common.lifecycle.AbstractLifecycleComponent; +import org.opensearch.gateway.remote.ClusterMetadataManifest; + +import java.io.IOException; +import java.util.List; +import java.util.Map; + +/** + * Noop impl for RemoteRoutingTableService. + */ +public class NoopRemoteRoutingTableService extends AbstractLifecycleComponent implements RemoteRoutingTableService { + + @Override + public List getIndicesRouting(RoutingTable routingTable) { + return List.of(); + } + + @Override + public DiffableUtils.MapDiff> getIndicesRoutingMapDiff( + RoutingTable before, + RoutingTable after + ) { + return DiffableUtils.diff(Map.of(), Map.of(), DiffableUtils.getStringKeySerializer(), CUSTOM_ROUTING_TABLE_VALUE_SERIALIZER); + } + + @Override + public CheckedRunnable getIndexRoutingAsyncAction( + ClusterState clusterState, + IndexRoutingTable indexRouting, + LatchedActionListener latchedActionListener, + BlobPath clusterBasePath + ) { + // noop + return () -> {}; + } + + @Override + public List getAllUploadedIndicesRouting( + ClusterMetadataManifest previousManifest, + List indicesRoutingUploaded, + List indicesRoutingToDelete + ) { + return List.of(); + } + + @Override + protected void doStart() { + // noop + } + + @Override + protected void doStop() { + // noop + } + + @Override + protected void doClose() throws IOException { + // noop + } +} diff --git a/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java b/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java index ba2208e17df1f..dbf01904116ed 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java @@ -8,60 +8,57 @@ package org.opensearch.cluster.routing.remote; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.opensearch.common.lifecycle.AbstractLifecycleComponent; -import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.io.IOUtils; -import org.opensearch.node.Node; -import org.opensearch.node.remotestore.RemoteStoreNodeAttribute; -import org.opensearch.repositories.RepositoriesService; -import org.opensearch.repositories.Repository; -import org.opensearch.repositories.blobstore.BlobStoreRepository; +import org.opensearch.action.LatchedActionListener; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.DiffableUtils; +import org.opensearch.cluster.routing.IndexRoutingTable; +import org.opensearch.cluster.routing.RoutingTable; +import org.opensearch.common.CheckedRunnable; +import org.opensearch.common.blobstore.BlobPath; +import org.opensearch.common.lifecycle.LifecycleComponent; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.gateway.remote.ClusterMetadataManifest; import java.io.IOException; -import java.util.function.Supplier; - -import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteRoutingTableEnabled; +import java.util.List; +import java.util.Map; /** - * A Service which provides APIs to upload and download routing table from remote store. - * - * @opensearch.internal + * Interface for RemoteRoutingTableService. Exposes methods to orchestrate upload and download of routing table from remote store. */ -public class RemoteRoutingTableService extends AbstractLifecycleComponent { +public interface RemoteRoutingTableService extends LifecycleComponent { + static final DiffableUtils.NonDiffableValueSerializer CUSTOM_ROUTING_TABLE_VALUE_SERIALIZER = + new DiffableUtils.NonDiffableValueSerializer() { + @Override + public void write(IndexRoutingTable value, StreamOutput out) throws IOException { + value.writeTo(out); + } - private static final Logger logger = LogManager.getLogger(RemoteRoutingTableService.class); - private final Settings settings; - private final Supplier repositoriesService; - private BlobStoreRepository blobStoreRepository; + @Override + public IndexRoutingTable read(StreamInput in, String key) throws IOException { + return IndexRoutingTable.readFrom(in); + } + }; - public RemoteRoutingTableService(Supplier repositoriesService, Settings settings) { - assert isRemoteRoutingTableEnabled(settings) : "Remote routing table is not enabled"; - this.repositoriesService = repositoriesService; - this.settings = settings; - } + List getIndicesRouting(RoutingTable routingTable); - @Override - protected void doClose() throws IOException { - if (blobStoreRepository != null) { - IOUtils.close(blobStoreRepository); - } - } + DiffableUtils.MapDiff> getIndicesRoutingMapDiff( + RoutingTable before, + RoutingTable after + ); - @Override - protected void doStart() { - assert isRemoteRoutingTableEnabled(settings) == true : "Remote routing table is not enabled"; - final String remoteStoreRepo = settings.get( - Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY - ); - assert remoteStoreRepo != null : "Remote routing table repository is not configured"; - final Repository repository = repositoriesService.get().repository(remoteStoreRepo); - assert repository instanceof BlobStoreRepository : "Repository should be instance of BlobStoreRepository"; - blobStoreRepository = (BlobStoreRepository) repository; - } + CheckedRunnable getIndexRoutingAsyncAction( + ClusterState clusterState, + IndexRoutingTable indexRouting, + LatchedActionListener latchedActionListener, + BlobPath clusterBasePath + ); - @Override - protected void doStop() {} + List getAllUploadedIndicesRouting( + ClusterMetadataManifest previousManifest, + List indicesRoutingUploaded, + List indicesRoutingToDelete + ); } diff --git a/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceFactory.java b/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceFactory.java new file mode 100644 index 0000000000000..49f90fa261f27 --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceFactory.java @@ -0,0 +1,41 @@ +/* + * 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.routing.remote; + +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Settings; +import org.opensearch.repositories.RepositoriesService; + +import java.util.function.Supplier; + +import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteRoutingTableEnabled; + +/** + * Factory to provide impl for RemoteRoutingTableService based on settings. + */ +public class RemoteRoutingTableServiceFactory { + + /** + * Returns {@code DefaultRemoteRoutingTableService} if the feature is enabled, otherwise {@code NoopRemoteRoutingTableService} + * @param repositoriesService repositoriesService + * @param settings settings + * @param clusterSettings clusterSettings + * @return RemoteRoutingTableService + */ + public static RemoteRoutingTableService getService( + Supplier repositoriesService, + Settings settings, + ClusterSettings clusterSettings + ) { + if (isRemoteRoutingTableEnabled(settings)) { + return new InternalRemoteRoutingTableService(repositoriesService, settings, clusterSettings); + } + return new NoopRemoteRoutingTableService(); + } +} diff --git a/server/src/main/java/org/opensearch/common/blobstore/BlobPath.java b/server/src/main/java/org/opensearch/common/blobstore/BlobPath.java index 6f3e8be7c28b8..68af77714a319 100644 --- a/server/src/main/java/org/opensearch/common/blobstore/BlobPath.java +++ b/server/src/main/java/org/opensearch/common/blobstore/BlobPath.java @@ -39,6 +39,7 @@ import java.util.Collections; import java.util.Iterator; import java.util.List; +import java.util.Objects; /** * The list of paths where a blob can reside. The contents of the paths are dependent upon the implementation of {@link BlobContainer}. @@ -110,6 +111,19 @@ public BlobPath parent() { } } + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + BlobPath that = (BlobPath) o; + return Objects.equals(paths, that.paths); + } + + @Override + public int hashCode() { + return Objects.hashCode(paths); + } + @Override public String toString() { StringBuilder sb = new StringBuilder(); 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 34b32edfd896b..335615a6affb7 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -77,6 +77,7 @@ import org.opensearch.cluster.routing.allocation.decider.SameShardAllocationDecider; import org.opensearch.cluster.routing.allocation.decider.ShardsLimitAllocationDecider; import org.opensearch.cluster.routing.allocation.decider.ThrottlingAllocationDecider; +import org.opensearch.cluster.routing.remote.InternalRemoteRoutingTableService; import org.opensearch.cluster.service.ClusterApplierService; import org.opensearch.cluster.service.ClusterManagerService; import org.opensearch.cluster.service.ClusterManagerTaskThrottler; @@ -723,6 +724,8 @@ public void apply(Settings value, Settings current, Settings previous) { RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING, IndicesService.CLUSTER_REMOTE_INDEX_RESTRICT_ASYNC_DURABILITY_SETTING, IndicesService.CLUSTER_INDEX_RESTRICT_REPLICATION_TYPE_SETTING, + InternalRemoteRoutingTableService.REMOTE_ROUTING_TABLE_PATH_TYPE_SETTING, + InternalRemoteRoutingTableService.REMOTE_ROUTING_TABLE_PATH_HASH_ALGO_SETTING, // Admission Control Settings AdmissionControlSettings.ADMISSION_CONTROL_TRANSPORT_LAYER_MODE, diff --git a/server/src/main/java/org/opensearch/gateway/remote/ClusterMetadataManifest.java b/server/src/main/java/org/opensearch/gateway/remote/ClusterMetadataManifest.java index 503d86dbe5b7e..4f18f1591a3fd 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/ClusterMetadataManifest.java +++ b/server/src/main/java/org/opensearch/gateway/remote/ClusterMetadataManifest.java @@ -812,6 +812,7 @@ public static class UploadedIndexMetadata implements UploadedMetadata, Writeable private static final ParseField INDEX_NAME_FIELD = new ParseField("index_name"); private static final ParseField INDEX_UUID_FIELD = new ParseField("index_uuid"); private static final ParseField UPLOADED_FILENAME_FIELD = new ParseField("uploaded_filename"); + private static final ParseField COMPONENT_PREFIX_FIELD = new ParseField("component_prefix"); private static String indexName(Object[] fields) { return (String) fields[0]; @@ -825,23 +826,34 @@ private static String uploadedFilename(Object[] fields) { return (String) fields[2]; } + private static String componentPrefix(Object[] fields) { + return (String) fields[3]; + } + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( "uploaded_index_metadata", - fields -> new UploadedIndexMetadata(indexName(fields), indexUUID(fields), uploadedFilename(fields)) + fields -> new UploadedIndexMetadata(indexName(fields), indexUUID(fields), uploadedFilename(fields), componentPrefix(fields)) ); static { PARSER.declareString(ConstructingObjectParser.constructorArg(), INDEX_NAME_FIELD); PARSER.declareString(ConstructingObjectParser.constructorArg(), INDEX_UUID_FIELD); PARSER.declareString(ConstructingObjectParser.constructorArg(), UPLOADED_FILENAME_FIELD); + PARSER.declareString(ConstructingObjectParser.constructorArg(), COMPONENT_PREFIX_FIELD); } static final String COMPONENT_PREFIX = "index--"; + private final String componentPrefix; private final String indexName; private final String indexUUID; private final String uploadedFilename; public UploadedIndexMetadata(String indexName, String indexUUID, String uploadedFileName) { + this(indexName, indexUUID, uploadedFileName, COMPONENT_PREFIX); + } + + public UploadedIndexMetadata(String indexName, String indexUUID, String uploadedFileName, String componentPrefix) { + this.componentPrefix = componentPrefix; this.indexName = indexName; this.indexUUID = indexUUID; this.uploadedFilename = uploadedFileName; @@ -851,6 +863,7 @@ public UploadedIndexMetadata(StreamInput in) throws IOException { this.indexName = in.readString(); this.indexUUID = in.readString(); this.uploadedFilename = in.readString(); + this.componentPrefix = in.readString(); } public String getUploadedFilePath() { @@ -859,7 +872,7 @@ public String getUploadedFilePath() { @Override public String getComponent() { - return COMPONENT_PREFIX + getIndexName(); + return componentPrefix + getIndexName(); } public String getUploadedFilename() { @@ -875,11 +888,16 @@ public String getIndexUUID() { return indexUUID; } + public String getComponentPrefix() { + return componentPrefix; + } + @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { return builder.field(INDEX_NAME_FIELD.getPreferredName(), getIndexName()) .field(INDEX_UUID_FIELD.getPreferredName(), getIndexUUID()) - .field(UPLOADED_FILENAME_FIELD.getPreferredName(), getUploadedFilePath()); + .field(UPLOADED_FILENAME_FIELD.getPreferredName(), getUploadedFilePath()) + .field(COMPONENT_PREFIX_FIELD.getPreferredName(), getComponentPrefix()); } @Override @@ -887,6 +905,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeString(indexName); out.writeString(indexUUID); out.writeString(uploadedFilename); + out.writeString(componentPrefix); } @Override @@ -900,12 +919,13 @@ public boolean equals(Object o) { final UploadedIndexMetadata that = (UploadedIndexMetadata) o; return Objects.equals(indexName, that.indexName) && Objects.equals(indexUUID, that.indexUUID) - && Objects.equals(uploadedFilename, that.uploadedFilename); + && Objects.equals(uploadedFilename, that.uploadedFilename) + && Objects.equals(componentPrefix, that.componentPrefix); } @Override public int hashCode() { - return Objects.hash(indexName, indexUUID, uploadedFilename); + return Objects.hash(indexName, indexUUID, uploadedFilename, componentPrefix); } @Override diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java index 8128921de6d3f..1617efd6200c3 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -14,11 +14,15 @@ import org.opensearch.Version; import org.opensearch.action.LatchedActionListener; import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.DiffableUtils; import org.opensearch.cluster.coordination.CoordinationMetadata; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.Metadata; import org.opensearch.cluster.metadata.TemplatesMetadata; +import org.opensearch.cluster.routing.IndexRoutingTable; +import org.opensearch.cluster.routing.remote.InternalRemoteRoutingTableService; import org.opensearch.cluster.routing.remote.RemoteRoutingTableService; +import org.opensearch.cluster.routing.remote.RemoteRoutingTableServiceFactory; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.CheckedRunnable; import org.opensearch.common.Nullable; @@ -71,7 +75,6 @@ import static java.util.Objects.requireNonNull; import static org.opensearch.gateway.PersistedClusterStateService.SLOW_WRITE_LOGGING_THRESHOLD; -import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteRoutingTableEnabled; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteStoreClusterStateEnabled; /** @@ -168,6 +171,12 @@ public class RemoteClusterStateService implements Closeable { /** * Manifest format compatible with codec v2, where global metadata file is replaced with multiple metadata attribute files */ + public static final ChecksumBlobStoreFormat CLUSTER_METADATA_MANIFEST_FORMAT_V2 = + new ChecksumBlobStoreFormat<>("cluster-metadata-manifest", METADATA_MANIFEST_NAME_FORMAT, ClusterMetadataManifest::fromXContentV2); + + /** + * Manifest format compatible with codec v3, where global metadata file is replaced with multiple metadata attribute files + */ public static final ChecksumBlobStoreFormat CLUSTER_METADATA_MANIFEST_FORMAT = new ChecksumBlobStoreFormat<>( "cluster-metadata-manifest", METADATA_MANIFEST_NAME_FORMAT, @@ -204,7 +213,7 @@ public class RemoteClusterStateService implements Closeable { private final List indexMetadataUploadListeners; private BlobStoreRepository blobStoreRepository; private BlobStoreTransferService blobStoreTransferService; - private Optional remoteRoutingTableService; + private final RemoteRoutingTableService remoteRoutingTableService; private volatile TimeValue slowWriteLoggingThreshold; private volatile TimeValue indexMetadataUploadTimeout; @@ -215,7 +224,7 @@ public class RemoteClusterStateService implements Closeable { private final String CLUSTER_STATE_UPLOAD_TIME_LOG_STRING = "writing cluster state for version [{}] took [{}ms]"; private final String METADATA_UPDATE_LOG_STRING = "wrote metadata for [{}] indices and skipped [{}] unchanged " + "indices, coordination metadata updated : [{}], settings metadata updated : [{}], templates metadata " - + "updated : [{}], custom metadata updated : [{}]"; + + "updated : [{}], custom metadata updated : [{}], indices routing updated : [{}]"; public static final int INDEX_METADATA_CURRENT_CODEC_VERSION = 1; public static final int MANIFEST_CURRENT_CODEC_VERSION = ClusterMetadataManifest.CODEC_V3; public static final int GLOBAL_METADATA_CURRENT_CODEC_VERSION = 2; @@ -257,9 +266,7 @@ public RemoteClusterStateService( this.remoteStateStats = new RemotePersistenceStats(); this.remoteClusterStateCleanupManager = new RemoteClusterStateCleanupManager(this, clusterService); this.indexMetadataUploadListeners = indexMetadataUploadListeners; - this.remoteRoutingTableService = isRemoteRoutingTableEnabled(settings) - ? Optional.of(new RemoteRoutingTableService(repositoriesService, settings)) - : Optional.empty(); + this.remoteRoutingTableService = RemoteRoutingTableServiceFactory.getService(repositoriesService, settings, clusterSettings); } /** @@ -283,7 +290,8 @@ public RemoteClusterStateManifestInfo writeFullMetadata(ClusterState clusterStat clusterState.metadata().customs(), true, true, - true + true, + remoteRoutingTableService.getIndicesRouting(clusterState.getRoutingTable()) ); final RemoteClusterStateManifestInfo manifestDetails = uploadManifest( clusterState, @@ -293,6 +301,7 @@ public RemoteClusterStateManifestInfo writeFullMetadata(ClusterState clusterStat uploadedMetadataResults.uploadedSettingsMetadata, uploadedMetadataResults.uploadedTemplatesMetadata, uploadedMetadataResults.uploadedCustomMetadataMap, + uploadedMetadataResults.uploadedIndicesRoutingMetadata, false ); final long durationMillis = TimeValue.nsecToMSec(relativeTimeNanosSupplier.getAsLong() - startTimeNanos); @@ -300,16 +309,19 @@ public RemoteClusterStateManifestInfo writeFullMetadata(ClusterState clusterStat remoteStateStats.stateTook(durationMillis); if (durationMillis >= slowWriteLoggingThreshold.getMillis()) { logger.warn( - "writing cluster state took [{}ms] which is above the warn threshold of [{}]; " + "wrote full state with [{}] indices", + "writing cluster state took [{}ms] which is above the warn threshold of [{}]; " + + "wrote full state with [{}] indices and [{}] indicesRouting", durationMillis, slowWriteLoggingThreshold, - uploadedMetadataResults.uploadedIndexMetadata.size() + uploadedMetadataResults.uploadedIndexMetadata.size(), + uploadedMetadataResults.uploadedIndicesRoutingMetadata.size() ); } else { logger.info( - "writing cluster state took [{}ms]; " + "wrote full state with [{}] indices and global metadata", + "writing cluster state took [{}ms]; " + "wrote full state with [{}] indices, [{}] indicesRouting and global metadata", durationMillis, - uploadedMetadataResults.uploadedIndexMetadata.size() + uploadedMetadataResults.uploadedIndexMetadata.size(), + uploadedMetadataResults.uploadedIndicesRoutingMetadata.size() ); } return manifestDetails; @@ -374,6 +386,12 @@ public RemoteClusterStateManifestInfo writeIncrementalMetadata( // index present in current cluster state indicesToBeDeletedFromRemote.remove(indexMetadata.getIndex().getName()); } + + DiffableUtils.MapDiff> routingTableDiff = remoteRoutingTableService + .getIndicesRoutingMapDiff(previousClusterState.getRoutingTable(), clusterState.getRoutingTable()); + List indicesRoutingToUpload = new ArrayList<>(); + routingTableDiff.getUpserts().forEach((k, v) -> indicesRoutingToUpload.add(v)); + UploadedMetadataResults uploadedMetadataResults; // For migration case from codec V0 or V1 to V2, we have added null check on metadata attribute files, // If file is empty and codec is 1 then write global metadata. @@ -393,7 +411,8 @@ public RemoteClusterStateManifestInfo writeIncrementalMetadata( firstUploadForSplitGlobalMetadata ? clusterState.metadata().customs() : customsToUpload, updateCoordinationMetadata, updateSettingsMetadata, - updateTemplatesMetadata + updateTemplatesMetadata, + indicesRoutingToUpload ); // update the map if the metadata was uploaded @@ -405,6 +424,13 @@ public RemoteClusterStateManifestInfo writeIncrementalMetadata( customsToBeDeletedFromRemote.keySet().forEach(allUploadedCustomMap::remove); indicesToBeDeletedFromRemote.keySet().forEach(allUploadedIndexMetadata::remove); + List allUploadedIndicesRouting = new ArrayList<>(); + allUploadedIndicesRouting = remoteRoutingTableService.getAllUploadedIndicesRouting( + previousManifest, + uploadedMetadataResults.uploadedIndicesRoutingMetadata, + routingTableDiff.getDeletes() + ); + final RemoteClusterStateManifestInfo manifestDetails = uploadManifest( clusterState, new ArrayList<>(allUploadedIndexMetadata.values()), @@ -415,6 +441,7 @@ public RemoteClusterStateManifestInfo writeIncrementalMetadata( firstUploadForSplitGlobalMetadata || !customsToUpload.isEmpty() ? allUploadedCustomMap : previousManifest.getCustomMetadataMap(), + allUploadedIndicesRouting, false ); @@ -433,7 +460,8 @@ public RemoteClusterStateManifestInfo writeIncrementalMetadata( updateCoordinationMetadata, updateSettingsMetadata, updateTemplatesMetadata, - customsToUpload.size() + customsToUpload.size(), + indicesRoutingToUpload.size() ); if (durationMillis >= slowWriteLoggingThreshold.getMillis()) { logger.warn( @@ -455,11 +483,13 @@ private UploadedMetadataResults writeMetadataInParallel( Map customToUpload, boolean uploadCoordinationMetadata, boolean uploadSettingsMetadata, - boolean uploadTemplateMetadata + boolean uploadTemplateMetadata, + List indicesRoutingToUpload ) throws IOException { assert Objects.nonNull(indexMetadataUploadListeners) : "indexMetadataUploadListeners can not be null"; int totalUploadTasks = indexToUpload.size() + indexMetadataUploadListeners.size() + customToUpload.size() - + (uploadCoordinationMetadata ? 1 : 0) + (uploadSettingsMetadata ? 1 : 0) + (uploadTemplateMetadata ? 1 : 0); + + (uploadCoordinationMetadata ? 1 : 0) + (uploadSettingsMetadata ? 1 : 0) + (uploadTemplateMetadata ? 1 : 0) + + indicesRoutingToUpload.size(); CountDownLatch latch = new CountDownLatch(totalUploadTasks); Map> uploadTasks = new HashMap<>(totalUploadTasks); Map results = new HashMap<>(totalUploadTasks); @@ -526,6 +556,18 @@ private UploadedMetadataResults writeMetadataInParallel( uploadTasks.put(indexMetadata.getIndex().getName(), getIndexMetadataAsyncAction(clusterState, indexMetadata, listener)); }); + indicesRoutingToUpload.forEach(indexRoutingTable -> { + uploadTasks.put( + InternalRemoteRoutingTableService.INDEX_ROUTING_METADATA_PREFIX + indexRoutingTable.getIndex().getName(), + remoteRoutingTableService.getIndexRoutingAsyncAction( + clusterState, + indexRoutingTable, + listener, + getCusterMetadataBasePath(clusterState.getClusterName().value(), clusterState.metadata().clusterUUID()) + ) + ); + }); + // start async upload of all required metadata files for (CheckedRunnable uploadTask : uploadTasks.values()) { uploadTask.run(); @@ -572,7 +614,10 @@ private UploadedMetadataResults writeMetadataInParallel( } UploadedMetadataResults response = new UploadedMetadataResults(); results.forEach((name, uploadedMetadata) -> { - if (name.contains(CUSTOM_METADATA)) { + if (uploadedMetadata.getClass().equals(UploadedIndexMetadata.class) + && uploadedMetadata.getComponent().contains(InternalRemoteRoutingTableService.INDEX_ROUTING_METADATA_PREFIX)) { + response.uploadedIndicesRoutingMetadata.add((UploadedIndexMetadata) uploadedMetadata); + } else if (name.contains(CUSTOM_METADATA)) { // component name for custom metadata will look like custom-- String custom = name.split(DELIMITER)[0].split(CUSTOM_DELIMITER)[1]; response.uploadedCustomMetadataMap.put( @@ -741,6 +786,7 @@ public RemoteClusterStateManifestInfo markLastStateAsCommitted(ClusterState clus previousManifest.getSettingsMetadata(), previousManifest.getTemplatesMetadata(), previousManifest.getCustomMetadataMap(), + previousManifest.getIndicesRouting(), true ); if (!previousManifest.isClusterUUIDCommitted() && committedManifestDetails.getClusterMetadataManifest().isClusterUUIDCommitted()) { @@ -756,9 +802,7 @@ public void close() throws IOException { if (blobStoreRepository != null) { IOUtils.close(blobStoreRepository); } - if (this.remoteRoutingTableService.isPresent()) { - this.remoteRoutingTableService.get().close(); - } + this.remoteRoutingTableService.close(); } public void start() { @@ -771,7 +815,7 @@ public void start() { assert repository instanceof BlobStoreRepository : "Repository should be instance of BlobStoreRepository"; blobStoreRepository = (BlobStoreRepository) repository; remoteClusterStateCleanupManager.start(); - this.remoteRoutingTableService.ifPresent(RemoteRoutingTableService::start); + this.remoteRoutingTableService.start(); } private RemoteClusterStateManifestInfo uploadManifest( @@ -782,6 +826,7 @@ private RemoteClusterStateManifestInfo uploadManifest( UploadedMetadataAttribute uploadedSettingsMetadata, UploadedMetadataAttribute uploadedTemplatesMetadata, Map uploadedCustomMetadataMap, + List uploadedIndicesRouting, boolean committed ) throws IOException { synchronized (this) { @@ -809,8 +854,7 @@ private RemoteClusterStateManifestInfo uploadManifest( uploadedTemplatesMetadata, uploadedCustomMetadataMap, clusterState.routingTable().version(), - // TODO: Add actual list of changed indices routing with index routing upload flow. - new ArrayList<>() + uploadedIndicesRouting ); writeMetadataManifest(clusterState.getClusterName().value(), clusterState.metadata().clusterUUID(), manifest, manifestFileName); return new RemoteClusterStateManifestInfo(manifest, manifestFileName); @@ -957,7 +1001,7 @@ public TimeValue getMetadataManifestUploadTimeout() { } // Package private for unit test - Optional getRemoteRoutingTableService() { + RemoteRoutingTableService getRemoteRoutingTableService() { return this.remoteRoutingTableService; } @@ -1496,6 +1540,8 @@ private ChecksumBlobStoreFormat getClusterMetadataManif long codecVersion = getManifestCodecVersion(fileName); if (codecVersion == MANIFEST_CURRENT_CODEC_VERSION) { return CLUSTER_METADATA_MANIFEST_FORMAT; + } else if (codecVersion == ClusterMetadataManifest.CODEC_V2) { + return CLUSTER_METADATA_MANIFEST_FORMAT_V2; } else if (codecVersion == ClusterMetadataManifest.CODEC_V1) { return CLUSTER_METADATA_MANIFEST_FORMAT_V1; } else if (codecVersion == ClusterMetadataManifest.CODEC_V0) { @@ -1549,19 +1595,22 @@ private static class UploadedMetadataResults { UploadedMetadataAttribute uploadedCoordinationMetadata; UploadedMetadataAttribute uploadedSettingsMetadata; UploadedMetadataAttribute uploadedTemplatesMetadata; + List uploadedIndicesRoutingMetadata; public UploadedMetadataResults( List uploadedIndexMetadata, Map uploadedCustomMetadataMap, UploadedMetadataAttribute uploadedCoordinationMetadata, UploadedMetadataAttribute uploadedSettingsMetadata, - UploadedMetadataAttribute uploadedTemplatesMetadata + UploadedMetadataAttribute uploadedTemplatesMetadata, + List uploadedIndicesRoutingMetadata ) { this.uploadedIndexMetadata = uploadedIndexMetadata; this.uploadedCustomMetadataMap = uploadedCustomMetadataMap; this.uploadedCoordinationMetadata = uploadedCoordinationMetadata; this.uploadedSettingsMetadata = uploadedSettingsMetadata; this.uploadedTemplatesMetadata = uploadedTemplatesMetadata; + this.uploadedIndicesRoutingMetadata = uploadedIndicesRoutingMetadata; } public UploadedMetadataResults() { @@ -1570,6 +1619,7 @@ public UploadedMetadataResults() { this.uploadedCoordinationMetadata = null; this.uploadedSettingsMetadata = null; this.uploadedTemplatesMetadata = null; + this.uploadedIndicesRoutingMetadata = new ArrayList<>(); } } } diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteIndexPath.java b/server/src/main/java/org/opensearch/index/remote/RemoteIndexPath.java index 89b642b79df86..899ff16c9d607 100644 --- a/server/src/main/java/org/opensearch/index/remote/RemoteIndexPath.java +++ b/server/src/main/java/org/opensearch/index/remote/RemoteIndexPath.java @@ -19,6 +19,7 @@ import org.opensearch.index.remote.RemoteStoreEnums.PathHashAlgorithm; import org.opensearch.index.remote.RemoteStoreEnums.PathType; import org.opensearch.index.remote.RemoteStorePathStrategy.PathInput; +import org.opensearch.index.remote.RemoteStorePathStrategy.ShardDataPathInput; import java.io.IOException; import java.util.Collections; @@ -141,7 +142,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws DataCategory dataCategory = entry.getKey(); for (DataType type : entry.getValue()) { for (int shardNo = 0; shardNo < shardCount; shardNo++) { - PathInput pathInput = PathInput.builder() + PathInput pathInput = ShardDataPathInput.builder() .basePath(new BlobPath().add(basePath)) .indexUUID(indexUUID) .shardId(Integer.toString(shardNo)) diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteStoreEnums.java b/server/src/main/java/org/opensearch/index/remote/RemoteStoreEnums.java index c1ac74724e405..b0376c97e6994 100644 --- a/server/src/main/java/org/opensearch/index/remote/RemoteStoreEnums.java +++ b/server/src/main/java/org/opensearch/index/remote/RemoteStoreEnums.java @@ -95,11 +95,7 @@ public enum PathType { public BlobPath generatePath(PathInput pathInput, PathHashAlgorithm hashAlgorithm) { assert Objects.isNull(hashAlgorithm) : "hashAlgorithm is expected to be null with fixed remote store path type"; // Hash algorithm is not used in FIXED path type - return pathInput.basePath() - .add(pathInput.indexUUID()) - .add(pathInput.shardId()) - .add(pathInput.dataCategory().getName()) - .add(pathInput.dataType().getName()); + return pathInput.basePath().add(pathInput.fixedSubPath()); } @Override @@ -111,13 +107,7 @@ boolean requiresHashAlgorithm() { @Override public BlobPath generatePath(PathInput pathInput, PathHashAlgorithm hashAlgorithm) { assert Objects.nonNull(hashAlgorithm) : "hashAlgorithm is expected to be non-null"; - return BlobPath.cleanPath() - .add(hashAlgorithm.hash(pathInput)) - .add(pathInput.basePath()) - .add(pathInput.indexUUID()) - .add(pathInput.shardId()) - .add(pathInput.dataCategory().getName()) - .add(pathInput.dataType().getName()); + return BlobPath.cleanPath().add(hashAlgorithm.hash(pathInput)).add(pathInput.basePath()).add(pathInput.fixedSubPath()); } @Override @@ -129,12 +119,7 @@ boolean requiresHashAlgorithm() { @Override public BlobPath generatePath(PathInput pathInput, PathHashAlgorithm hashAlgorithm) { assert Objects.nonNull(hashAlgorithm) : "hashAlgorithm is expected to be non-null"; - return pathInput.basePath() - .add(hashAlgorithm.hash(pathInput)) - .add(pathInput.indexUUID()) - .add(pathInput.shardId()) - .add(pathInput.dataCategory().getName()) - .add(pathInput.dataType().getName()); + return pathInput.basePath().add(hashAlgorithm.hash(pathInput)).add(pathInput.fixedSubPath()); } @Override @@ -186,13 +171,7 @@ public static PathType fromCode(int code) { * @return the blob path for the path input. */ public BlobPath path(PathInput pathInput, PathHashAlgorithm hashAlgorithm) { - DataCategory dataCategory = pathInput.dataCategory(); - DataType dataType = pathInput.dataType(); - assert dataCategory.isSupportedDataType(dataType) : "category:" - + dataCategory - + " type:" - + dataType - + " are not supported together"; + pathInput.assertIsValid(); return generatePath(pathInput, hashAlgorithm); } @@ -227,9 +206,11 @@ public enum PathHashAlgorithm { FNV_1A_BASE64(0) { @Override String hash(PathInput pathInput) { - String input = pathInput.indexUUID() + pathInput.shardId() + pathInput.dataCategory().getName() + pathInput.dataType() - .getName(); - long hash = FNV1a.hash64(input); + StringBuilder input = new StringBuilder(); + for (String path : pathInput.fixedSubPath().toArray()) { + input.append(path); + } + long hash = FNV1a.hash64(input.toString()); return longToUrlBase64(hash); } }, @@ -240,9 +221,11 @@ String hash(PathInput pathInput) { FNV_1A_COMPOSITE_1(1) { @Override String hash(PathInput pathInput) { - String input = pathInput.indexUUID() + pathInput.shardId() + pathInput.dataCategory().getName() + pathInput.dataType() - .getName(); - long hash = FNV1a.hash64(input); + StringBuilder input = new StringBuilder(); + for (String path : pathInput.fixedSubPath().toArray()) { + input.append(path); + } + long hash = FNV1a.hash64(input.toString()); return longToCompositeBase64AndBinaryEncoding(hash, 20); } }; diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteStorePathStrategy.java b/server/src/main/java/org/opensearch/index/remote/RemoteStorePathStrategy.java index c58f6c3faac84..d0250790068f7 100644 --- a/server/src/main/java/org/opensearch/index/remote/RemoteStorePathStrategy.java +++ b/server/src/main/java/org/opensearch/index/remote/RemoteStorePathStrategy.java @@ -72,7 +72,9 @@ public BlobPath generatePath(PathInput pathInput) { } /** - * Wrapper class for the input required to generate path for remote store uploads. + * Wrapper class for the path input required to generate path for remote store uploads. This input is composed of + * basePath and indexUUID. + * * @opensearch.internal */ @PublicApi(since = "2.14.0") @@ -80,16 +82,10 @@ public BlobPath generatePath(PathInput pathInput) { public static class PathInput { private final BlobPath basePath; private final String indexUUID; - private final String shardId; - private final DataCategory dataCategory; - private final DataType dataType; - public PathInput(BlobPath basePath, String indexUUID, String shardId, DataCategory dataCategory, DataType dataType) { - this.basePath = Objects.requireNonNull(basePath); - this.indexUUID = Objects.requireNonNull(indexUUID); - this.shardId = Objects.requireNonNull(shardId); - this.dataCategory = Objects.requireNonNull(dataCategory); - this.dataType = Objects.requireNonNull(dataType); + public PathInput(Builder builder) { + this.basePath = Objects.requireNonNull(builder.basePath); + this.indexUUID = Objects.requireNonNull(builder.indexUUID); } BlobPath basePath() { @@ -100,6 +96,78 @@ String indexUUID() { return indexUUID; } + BlobPath fixedSubPath() { + return BlobPath.cleanPath().add(indexUUID); + } + + /** + * Returns a new builder for {@link PathInput}. + */ + public static Builder builder() { + return new Builder<>(); + } + + public void assertIsValid() { + // Input is always valid here. + } + + /** + * Builder for {@link PathInput}. + * + * @opensearch.internal + */ + @PublicApi(since = "2.14.0") + @ExperimentalApi + public static class Builder> { + private BlobPath basePath; + private String indexUUID; + + public T basePath(BlobPath basePath) { + this.basePath = basePath; + return self(); + } + + public Builder indexUUID(String indexUUID) { + this.indexUUID = indexUUID; + return self(); + } + + protected T self() { + return (T) this; + } + + public PathInput build() { + return new PathInput(this); + } + } + } + + /** + * Wrapper class for the data aware path input required to generate path for remote store uploads. This input is + * composed of the parent inputs, shard id, data category and data type. + * + * @opensearch.internal + */ + @PublicApi(since = "2.14.0") + @ExperimentalApi + public static class ShardDataPathInput extends PathInput { + private final String shardId; + private final DataCategory dataCategory; + private final DataType dataType; + + public ShardDataPathInput(Builder builder) { + super(builder); + this.shardId = Objects.requireNonNull(builder.shardId); + this.dataCategory = Objects.requireNonNull(builder.dataCategory); + this.dataType = Objects.requireNonNull(builder.dataType); + assert dataCategory.isSupportedDataType(dataType) : "category:" + + dataCategory + + " type:" + + dataType + + " are not supported together"; + + } + String shardId() { return shardId; } @@ -112,34 +180,37 @@ DataType dataType() { return dataType; } + @Override + BlobPath fixedSubPath() { + return super.fixedSubPath().add(shardId).add(dataCategory.getName()).add(dataType.getName()); + } + /** - * Returns a new builder for {@link PathInput}. + * Returns a new builder for {@link ShardDataPathInput}. */ public static Builder builder() { return new Builder(); } /** - * Builder for {@link PathInput}. + * Builder for {@link ShardDataPathInput}. * * @opensearch.internal */ @PublicApi(since = "2.14.0") @ExperimentalApi - public static class Builder { - private BlobPath basePath; - private String indexUUID; + public static class Builder extends PathInput.Builder { private String shardId; private DataCategory dataCategory; private DataType dataType; public Builder basePath(BlobPath basePath) { - this.basePath = basePath; + super.basePath = basePath; return this; } public Builder indexUUID(String indexUUID) { - this.indexUUID = indexUUID; + super.indexUUID = indexUUID; return this; } @@ -158,8 +229,13 @@ public Builder dataType(DataType dataType) { return this; } - public PathInput build() { - return new PathInput(basePath, indexUUID, shardId, dataCategory, dataType); + @Override + protected Builder self() { + return this; + } + + public ShardDataPathInput build() { + return new ShardDataPathInput(this); } } } diff --git a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryFactory.java b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryFactory.java index e462f6d4ac011..b965d7ce73ae6 100644 --- a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryFactory.java +++ b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryFactory.java @@ -65,7 +65,7 @@ public Directory newDirectory(String repositoryName, String indexUUID, ShardId s BlobPath repositoryBasePath = blobStoreRepository.basePath(); String shardIdStr = String.valueOf(shardId.id()); - RemoteStorePathStrategy.PathInput dataPathInput = RemoteStorePathStrategy.PathInput.builder() + RemoteStorePathStrategy.ShardDataPathInput dataPathInput = RemoteStorePathStrategy.ShardDataPathInput.builder() .basePath(repositoryBasePath) .indexUUID(indexUUID) .shardId(shardIdStr) @@ -80,7 +80,7 @@ public Directory newDirectory(String repositoryName, String indexUUID, ShardId s blobStoreRepository::maybeRateLimitRemoteDownloadTransfers ); - RemoteStorePathStrategy.PathInput mdPathInput = RemoteStorePathStrategy.PathInput.builder() + RemoteStorePathStrategy.ShardDataPathInput mdPathInput = RemoteStorePathStrategy.ShardDataPathInput.builder() .basePath(repositoryBasePath) .indexUUID(indexUUID) .shardId(shardIdStr) diff --git a/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreLockManagerFactory.java b/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreLockManagerFactory.java index 45d466d3a8ce8..993c1bbdf033f 100644 --- a/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreLockManagerFactory.java +++ b/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreLockManagerFactory.java @@ -56,7 +56,7 @@ public static RemoteStoreMetadataLockManager newLockManager( assert repository instanceof BlobStoreRepository : "repository should be instance of BlobStoreRepository"; BlobPath repositoryBasePath = ((BlobStoreRepository) repository).basePath(); - RemoteStorePathStrategy.PathInput lockFilesPathInput = RemoteStorePathStrategy.PathInput.builder() + RemoteStorePathStrategy.ShardDataPathInput lockFilesPathInput = RemoteStorePathStrategy.ShardDataPathInput.builder() .basePath(repositoryBasePath) .indexUUID(indexUUID) .shardId(shardId) diff --git a/server/src/main/java/org/opensearch/index/translog/RemoteFsTranslog.java b/server/src/main/java/org/opensearch/index/translog/RemoteFsTranslog.java index f29b6fba6537f..c533a31c310c7 100644 --- a/server/src/main/java/org/opensearch/index/translog/RemoteFsTranslog.java +++ b/server/src/main/java/org/opensearch/index/translog/RemoteFsTranslog.java @@ -304,7 +304,7 @@ public static TranslogTransferManager buildTranslogTransferManager( assert Objects.nonNull(pathStrategy); String indexUUID = shardId.getIndex().getUUID(); String shardIdStr = String.valueOf(shardId.id()); - RemoteStorePathStrategy.PathInput dataPathInput = RemoteStorePathStrategy.PathInput.builder() + RemoteStorePathStrategy.ShardDataPathInput dataPathInput = RemoteStorePathStrategy.ShardDataPathInput.builder() .basePath(blobStoreRepository.basePath()) .indexUUID(indexUUID) .shardId(shardIdStr) @@ -312,7 +312,7 @@ public static TranslogTransferManager buildTranslogTransferManager( .dataType(DATA) .build(); BlobPath dataPath = pathStrategy.generatePath(dataPathInput); - RemoteStorePathStrategy.PathInput mdPathInput = RemoteStorePathStrategy.PathInput.builder() + RemoteStorePathStrategy.ShardDataPathInput mdPathInput = RemoteStorePathStrategy.ShardDataPathInput.builder() .basePath(blobStoreRepository.basePath()) .indexUUID(indexUUID) .shardId(shardIdStr) diff --git a/server/src/test/java/org/opensearch/cluster/routing/IndexShardRoutingTableTests.java b/server/src/test/java/org/opensearch/cluster/routing/IndexShardRoutingTableTests.java index e881016fb9305..6bfe60980adf3 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/IndexShardRoutingTableTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/IndexShardRoutingTableTests.java @@ -69,6 +69,12 @@ public void testEquals() { assertNotEquals(table1, null); assertNotEquals(table1, s); assertNotEquals(table1, table3); + + ShardRouting primary = TestShardRouting.newShardRouting(shardId, "node-1", true, ShardRoutingState.STARTED); + ShardRouting replica = TestShardRouting.newShardRouting(shardId, "node-2", false, ShardRoutingState.STARTED); + IndexShardRoutingTable table4 = new IndexShardRoutingTable(shardId, Arrays.asList(primary, replica)); + IndexShardRoutingTable table5 = new IndexShardRoutingTable(shardId, Arrays.asList(replica, primary)); + assertEquals(table4, table5); } public void testShardsMatchingPredicate() { diff --git a/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceFactoryTests.java b/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceFactoryTests.java new file mode 100644 index 0000000000000..d0c2cca4b46f0 --- /dev/null +++ b/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceFactoryTests.java @@ -0,0 +1,51 @@ +/* + * 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.routing.remote; + +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.FeatureFlags; +import org.opensearch.repositories.RepositoriesService; +import org.opensearch.repositories.fs.FsRepository; +import org.opensearch.test.OpenSearchTestCase; + +import java.util.function.Supplier; + +import static org.opensearch.common.util.FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL; +import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY; + +public class RemoteRoutingTableServiceFactoryTests extends OpenSearchTestCase { + + Supplier repositoriesService; + + public void testGetServiceWhenRemoteRoutingDisabled() { + Settings settings = Settings.builder().build(); + RemoteRoutingTableService service = RemoteRoutingTableServiceFactory.getService( + repositoriesService, + settings, + new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) + ); + assertTrue(service instanceof NoopRemoteRoutingTableService); + } + + public void testGetServiceWhenRemoteRoutingEnabled() { + Settings settings = Settings.builder() + .put("node.attr." + REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, "routing_repository") + .put(FsRepository.REPOSITORIES_COMPRESS_SETTING.getKey(), false) + .build(); + Settings nodeSettings = Settings.builder().put(REMOTE_PUBLICATION_EXPERIMENTAL, "true").build(); + FeatureFlags.initializeFeatureFlags(nodeSettings); + RemoteRoutingTableService service = RemoteRoutingTableServiceFactory.getService( + repositoriesService, + settings, + new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) + ); + assertTrue(service instanceof InternalRemoteRoutingTableService); + } +} diff --git a/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java b/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java index 9a9cbfa153259..8fd410e774332 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java @@ -8,30 +8,78 @@ package org.opensearch.cluster.routing.remote; +import org.opensearch.Version; +import org.opensearch.action.LatchedActionListener; +import org.opensearch.cluster.ClusterName; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.DiffableUtils; +import org.opensearch.cluster.coordination.CoordinationMetadata; +import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.cluster.metadata.Metadata; +import org.opensearch.cluster.routing.IndexRoutingTable; +import org.opensearch.cluster.routing.RoutingTable; +import org.opensearch.common.CheckedRunnable; +import org.opensearch.common.blobstore.AsyncMultiStreamBlobContainer; +import org.opensearch.common.blobstore.BlobContainer; +import org.opensearch.common.blobstore.BlobPath; +import org.opensearch.common.blobstore.BlobStore; +import org.opensearch.common.blobstore.stream.write.WriteContext; +import org.opensearch.common.blobstore.stream.write.WritePriority; +import org.opensearch.common.compress.DeflateCompressor; +import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.FeatureFlags; +import org.opensearch.core.action.ActionListener; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.index.Index; +import org.opensearch.gateway.remote.ClusterMetadataManifest; +import org.opensearch.gateway.remote.RemoteClusterStateService; +import org.opensearch.index.remote.RemoteStoreEnums; +import org.opensearch.index.remote.RemoteStorePathStrategy; +import org.opensearch.index.remote.RemoteStoreUtils; import org.opensearch.repositories.FilterRepository; import org.opensearch.repositories.RepositoriesService; import org.opensearch.repositories.RepositoryMissingException; import org.opensearch.repositories.blobstore.BlobStoreRepository; +import org.opensearch.repositories.fs.FsRepository; import org.opensearch.test.OpenSearchTestCase; import org.junit.After; import org.junit.Before; +import java.io.IOException; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.function.Supplier; +import org.mockito.ArgumentCaptor; + +import static org.opensearch.cluster.routing.remote.InternalRemoteRoutingTableService.INDEX_ROUTING_FILE_PREFIX; +import static org.opensearch.cluster.routing.remote.InternalRemoteRoutingTableService.INDEX_ROUTING_PATH_TOKEN; import static org.opensearch.common.util.FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL; +import static org.opensearch.gateway.remote.RemoteClusterStateService.DELIMITER; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.startsWith; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; public class RemoteRoutingTableServiceTests extends OpenSearchTestCase { - private RemoteRoutingTableService remoteRoutingTableService; + private InternalRemoteRoutingTableService remoteRoutingTableService; private Supplier repositoriesServiceSupplier; private RepositoriesService repositoriesService; private BlobStoreRepository blobStoreRepository; + private BlobStore blobStore; + private BlobContainer blobContainer; + private BlobPath basePath; @Before public void setup() { @@ -41,15 +89,26 @@ public void setup() { Settings settings = Settings.builder() .put("node.attr." + REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, "routing_repository") + .put(FsRepository.REPOSITORIES_COMPRESS_SETTING.getKey(), false) .build(); blobStoreRepository = mock(BlobStoreRepository.class); + when(blobStoreRepository.getCompressor()).thenReturn(new DeflateCompressor()); + blobStore = mock(BlobStore.class); + blobContainer = mock(BlobContainer.class); when(repositoriesService.repository("routing_repository")).thenReturn(blobStoreRepository); + when(blobStoreRepository.blobStore()).thenReturn(blobStore); Settings nodeSettings = Settings.builder().put(REMOTE_PUBLICATION_EXPERIMENTAL, "true").build(); FeatureFlags.initializeFeatureFlags(nodeSettings); - remoteRoutingTableService = new RemoteRoutingTableService(repositoriesServiceSupplier, settings); + basePath = BlobPath.cleanPath().add("base-path"); + + remoteRoutingTableService = new InternalRemoteRoutingTableService( + repositoriesServiceSupplier, + settings, + new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) + ); } @After @@ -60,7 +119,14 @@ public void teardown() throws Exception { public void testFailInitializationWhenRemoteRoutingDisabled() { final Settings settings = Settings.builder().build(); - assertThrows(AssertionError.class, () -> new RemoteRoutingTableService(repositoriesServiceSupplier, settings)); + assertThrows( + AssertionError.class, + () -> new InternalRemoteRoutingTableService( + repositoriesServiceSupplier, + settings, + new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) + ) + ); } public void testFailStartWhenRepositoryNotSet() { @@ -74,4 +140,416 @@ public void testFailStartWhenNotBlobRepository() { assertThrows(AssertionError.class, () -> remoteRoutingTableService.start()); } + public void testGetIndicesRoutingMapDiff() { + String indexName = randomAlphaOfLength(randomIntBetween(1, 50)); + final Index index = new Index(indexName, "uuid"); + final IndexMetadata indexMetadata = new IndexMetadata.Builder(indexName).settings( + Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) + .put(IndexMetadata.SETTING_INDEX_UUID, "uuid") + .build() + ).numberOfShards(1).numberOfReplicas(1).build(); + + RoutingTable routingTable = RoutingTable.builder().addAsNew(indexMetadata).build(); + + DiffableUtils.MapDiff> diff = remoteRoutingTableService + .getIndicesRoutingMapDiff(routingTable, routingTable); + assertEquals(0, diff.getUpserts().size()); + assertEquals(0, diff.getDeletes().size()); + + // Reversing order to check for equality without order. + IndexRoutingTable indexRouting = routingTable.getIndicesRouting().get(indexName); + IndexRoutingTable indexRoutingTableReversed = IndexRoutingTable.builder(index) + .addShard(indexRouting.getShards().get(0).replicaShards().get(0)) + .addShard(indexRouting.getShards().get(0).primaryShard()) + .build(); + RoutingTable routingTable2 = RoutingTable.builder().add(indexRoutingTableReversed).build(); + + diff = remoteRoutingTableService.getIndicesRoutingMapDiff(routingTable, routingTable2); + assertEquals(0, diff.getUpserts().size()); + assertEquals(0, diff.getDeletes().size()); + } + + public void testGetIndicesRoutingMapDiffIndexAdded() { + String indexName = randomAlphaOfLength(randomIntBetween(1, 50)); + final IndexMetadata indexMetadata = new IndexMetadata.Builder(indexName).settings( + Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) + .put(IndexMetadata.SETTING_INDEX_UUID, "uuid") + .build() + ).numberOfShards(randomInt(1000)).numberOfReplicas(randomInt(10)).build(); + RoutingTable routingTable = RoutingTable.builder().addAsNew(indexMetadata).build(); + + String indexName2 = randomAlphaOfLength(randomIntBetween(1, 50)); + int noOfShards = randomInt(1000); + int noOfReplicas = randomInt(10); + final IndexMetadata indexMetadata2 = new IndexMetadata.Builder(indexName2).settings( + Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) + .put(IndexMetadata.SETTING_INDEX_UUID, "uuid2") + .build() + ).numberOfShards(noOfShards).numberOfReplicas(noOfReplicas).build(); + RoutingTable routingTable2 = RoutingTable.builder(routingTable).addAsNew(indexMetadata2).build(); + + DiffableUtils.MapDiff> diff = remoteRoutingTableService + .getIndicesRoutingMapDiff(routingTable, routingTable2); + assertEquals(1, diff.getUpserts().size()); + assertNotNull(diff.getUpserts().get(indexName2)); + assertEquals(noOfShards, diff.getUpserts().get(indexName2).getShards().size()); + + assertEquals(0, diff.getDeletes().size()); + } + + public void testGetIndicesRoutingMapDiffShardChanged() { + String indexName = randomAlphaOfLength(randomIntBetween(1, 50)); + final Index index = new Index(indexName, "uuid"); + int noOfShards = randomInt(1000); + int noOfReplicas = randomInt(10); + final IndexMetadata indexMetadata = new IndexMetadata.Builder(indexName).settings( + Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) + .put(IndexMetadata.SETTING_INDEX_UUID, "uuid") + .build() + ).numberOfShards(noOfShards).numberOfReplicas(noOfReplicas).build(); + + RoutingTable routingTable = RoutingTable.builder().addAsNew(indexMetadata).build(); + + final IndexMetadata indexMetadata2 = new IndexMetadata.Builder(indexName).settings( + Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) + .put(IndexMetadata.SETTING_INDEX_UUID, "uuid") + .build() + ).numberOfShards(noOfShards + 1).numberOfReplicas(noOfReplicas).build(); + RoutingTable routingTable2 = RoutingTable.builder().addAsNew(indexMetadata2).build(); + + DiffableUtils.MapDiff> diff = remoteRoutingTableService + .getIndicesRoutingMapDiff(routingTable, routingTable2); + assertEquals(1, diff.getUpserts().size()); + assertNotNull(diff.getUpserts().get(indexName)); + assertEquals(noOfShards + 1, diff.getUpserts().get(indexName).getShards().size()); + assertEquals(noOfReplicas + 1, diff.getUpserts().get(indexName).getShards().get(0).getSize()); + assertEquals(0, diff.getDeletes().size()); + + final IndexMetadata indexMetadata3 = new IndexMetadata.Builder(indexName).settings( + Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) + .put(IndexMetadata.SETTING_INDEX_UUID, "uuid") + .build() + ).numberOfShards(noOfShards + 1).numberOfReplicas(noOfReplicas + 1).build(); + RoutingTable routingTable3 = RoutingTable.builder().addAsNew(indexMetadata3).build(); + + diff = remoteRoutingTableService.getIndicesRoutingMapDiff(routingTable2, routingTable3); + assertEquals(1, diff.getUpserts().size()); + assertNotNull(diff.getUpserts().get(indexName)); + assertEquals(noOfShards + 1, diff.getUpserts().get(indexName).getShards().size()); + assertEquals(noOfReplicas + 2, diff.getUpserts().get(indexName).getShards().get(0).getSize()); + + assertEquals(0, diff.getDeletes().size()); + } + + public void testGetIndicesRoutingMapDiffShardDetailChanged() { + String indexName = randomAlphaOfLength(randomIntBetween(1, 50)); + final Index index = new Index(indexName, "uuid"); + int noOfShards = randomInt(1000); + int noOfReplicas = randomInt(10); + final IndexMetadata indexMetadata = new IndexMetadata.Builder(indexName).settings( + Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) + .put(IndexMetadata.SETTING_INDEX_UUID, "uuid") + .build() + ).numberOfShards(noOfShards).numberOfReplicas(noOfReplicas).build(); + + RoutingTable routingTable = RoutingTable.builder().addAsNew(indexMetadata).build(); + RoutingTable routingTable2 = RoutingTable.builder().addAsRecovery(indexMetadata).build(); + + DiffableUtils.MapDiff> diff = remoteRoutingTableService + .getIndicesRoutingMapDiff(routingTable, routingTable2); + assertEquals(1, diff.getUpserts().size()); + assertNotNull(diff.getUpserts().get(indexName)); + assertEquals(noOfShards, diff.getUpserts().get(indexName).getShards().size()); + assertEquals(noOfReplicas + 1, diff.getUpserts().get(indexName).getShards().get(0).getSize()); + assertEquals(0, diff.getDeletes().size()); + } + + public void testGetIndicesRoutingMapDiffIndexDeleted() { + String indexName = randomAlphaOfLength(randomIntBetween(1, 50)); + final IndexMetadata indexMetadata = new IndexMetadata.Builder(indexName).settings( + Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) + .put(IndexMetadata.SETTING_INDEX_UUID, "uuid") + .build() + ).numberOfShards(randomInt(1000)).numberOfReplicas(randomInt(10)).build(); + RoutingTable routingTable = RoutingTable.builder().addAsNew(indexMetadata).build(); + + String indexName2 = randomAlphaOfLength(randomIntBetween(1, 50)); + final IndexMetadata indexMetadata2 = new IndexMetadata.Builder(indexName2).settings( + Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) + .put(IndexMetadata.SETTING_INDEX_UUID, "uuid2") + .build() + ).numberOfShards(randomInt(1000)).numberOfReplicas(randomInt(10)).build(); + RoutingTable routingTable2 = RoutingTable.builder().addAsNew(indexMetadata2).build(); + + DiffableUtils.MapDiff> diff = remoteRoutingTableService + .getIndicesRoutingMapDiff(routingTable, routingTable2); + assertEquals(1, diff.getUpserts().size()); + assertNotNull(diff.getUpserts().get(indexName2)); + + assertEquals(1, diff.getDeletes().size()); + assertEquals(indexName, diff.getDeletes().get(0)); + } + + public void testGetIndexRoutingAsyncAction() throws IOException { + String indexName = randomAlphaOfLength(randomIntBetween(1, 50)); + ClusterState clusterState = createClusterState(indexName); + BlobPath expectedPath = getPath(); + + LatchedActionListener listener = mock(LatchedActionListener.class); + when(blobStore.blobContainer(expectedPath)).thenReturn(blobContainer); + + remoteRoutingTableService.start(); + CheckedRunnable runnable = remoteRoutingTableService.getIndexRoutingAsyncAction( + clusterState, + clusterState.routingTable().getIndicesRouting().get(indexName), + listener, + basePath + ); + assertNotNull(runnable); + runnable.run(); + + String expectedFilePrefix = String.join( + DELIMITER, + INDEX_ROUTING_FILE_PREFIX, + RemoteStoreUtils.invertLong(clusterState.term()), + RemoteStoreUtils.invertLong(clusterState.version()) + ); + verify(blobContainer, times(1)).writeBlob(startsWith(expectedFilePrefix), any(StreamInput.class), anyLong(), eq(true)); + verify(listener, times(1)).onResponse(any(ClusterMetadataManifest.UploadedMetadata.class)); + } + + public void testGetIndexRoutingAsyncActionFailureInBlobRepo() throws IOException { + String indexName = randomAlphaOfLength(randomIntBetween(1, 50)); + ClusterState clusterState = createClusterState(indexName); + BlobPath expectedPath = getPath(); + + LatchedActionListener listener = mock(LatchedActionListener.class); + when(blobStore.blobContainer(expectedPath)).thenReturn(blobContainer); + doThrow(new IOException("testing failure")).when(blobContainer).writeBlob(anyString(), any(StreamInput.class), anyLong(), eq(true)); + + remoteRoutingTableService.start(); + CheckedRunnable runnable = remoteRoutingTableService.getIndexRoutingAsyncAction( + clusterState, + clusterState.routingTable().getIndicesRouting().get(indexName), + listener, + basePath + ); + assertNotNull(runnable); + runnable.run(); + String expectedFilePrefix = String.join( + DELIMITER, + INDEX_ROUTING_FILE_PREFIX, + RemoteStoreUtils.invertLong(clusterState.term()), + RemoteStoreUtils.invertLong(clusterState.version()) + ); + verify(blobContainer, times(1)).writeBlob(startsWith(expectedFilePrefix), any(StreamInput.class), anyLong(), eq(true)); + verify(listener, times(1)).onFailure(any(RemoteClusterStateService.RemoteStateTransferException.class)); + } + + public void testGetIndexRoutingAsyncActionAsyncRepo() throws IOException { + String indexName = randomAlphaOfLength(randomIntBetween(1, 50)); + ClusterState clusterState = createClusterState(indexName); + BlobPath expectedPath = getPath(); + + LatchedActionListener listener = mock(LatchedActionListener.class); + blobContainer = mock(AsyncMultiStreamBlobContainer.class); + when(blobStore.blobContainer(expectedPath)).thenReturn(blobContainer); + ArgumentCaptor> actionListenerArgumentCaptor = ArgumentCaptor.forClass(ActionListener.class); + ArgumentCaptor writeContextArgumentCaptor = ArgumentCaptor.forClass(WriteContext.class); + ConcurrentHashMap capturedWriteContext = new ConcurrentHashMap<>(); + + doAnswer((i) -> { + actionListenerArgumentCaptor.getValue().onResponse(null); + WriteContext writeContext = writeContextArgumentCaptor.getValue(); + capturedWriteContext.put(writeContext.getFileName().split(DELIMITER)[0], writeContextArgumentCaptor.getValue()); + return null; + }).when((AsyncMultiStreamBlobContainer) blobContainer) + .asyncBlobUpload(writeContextArgumentCaptor.capture(), actionListenerArgumentCaptor.capture()); + + remoteRoutingTableService.start(); + CheckedRunnable runnable = remoteRoutingTableService.getIndexRoutingAsyncAction( + clusterState, + clusterState.routingTable().getIndicesRouting().get(indexName), + listener, + basePath + ); + assertNotNull(runnable); + runnable.run(); + + String expectedFilePrefix = String.join( + DELIMITER, + INDEX_ROUTING_FILE_PREFIX, + RemoteStoreUtils.invertLong(clusterState.term()), + RemoteStoreUtils.invertLong(clusterState.version()) + ); + assertEquals(1, actionListenerArgumentCaptor.getAllValues().size()); + assertEquals(1, writeContextArgumentCaptor.getAllValues().size()); + assertNotNull(capturedWriteContext.get("index_routing")); + assertEquals(capturedWriteContext.get("index_routing").getWritePriority(), WritePriority.URGENT); + assertTrue(capturedWriteContext.get("index_routing").getFileName().startsWith(expectedFilePrefix)); + } + + public void testGetIndexRoutingAsyncActionAsyncRepoFailureInRepo() throws IOException { + String indexName = randomAlphaOfLength(randomIntBetween(1, 50)); + ClusterState clusterState = createClusterState(indexName); + BlobPath expectedPath = getPath(); + + LatchedActionListener listener = mock(LatchedActionListener.class); + blobContainer = mock(AsyncMultiStreamBlobContainer.class); + when(blobStore.blobContainer(expectedPath)).thenReturn(blobContainer); + + doThrow(new IOException("Testing failure")).when((AsyncMultiStreamBlobContainer) blobContainer) + .asyncBlobUpload(any(WriteContext.class), any(ActionListener.class)); + + remoteRoutingTableService.start(); + CheckedRunnable runnable = remoteRoutingTableService.getIndexRoutingAsyncAction( + clusterState, + clusterState.routingTable().getIndicesRouting().get(indexName), + listener, + basePath + ); + assertNotNull(runnable); + runnable.run(); + verify(listener, times(1)).onFailure(any(RemoteClusterStateService.RemoteStateTransferException.class)); + } + + public void testGetAllUploadedIndicesRouting() { + final ClusterMetadataManifest previousManifest = ClusterMetadataManifest.builder().build(); + final ClusterMetadataManifest.UploadedIndexMetadata uploadedIndexMetadata = new ClusterMetadataManifest.UploadedIndexMetadata( + "test-index", + "index-uuid", + "index-filename", + InternalRemoteRoutingTableService.INDEX_ROUTING_METADATA_PREFIX + ); + + List allIndiceRoutingMetadata = remoteRoutingTableService + .getAllUploadedIndicesRouting(previousManifest, List.of(uploadedIndexMetadata), List.of()); + assertNotNull(allIndiceRoutingMetadata); + assertEquals(1, allIndiceRoutingMetadata.size()); + assertEquals(uploadedIndexMetadata, allIndiceRoutingMetadata.get(0)); + } + + public void testGetAllUploadedIndicesRoutingExistingIndexInManifest() { + final ClusterMetadataManifest.UploadedIndexMetadata uploadedIndexMetadata = new ClusterMetadataManifest.UploadedIndexMetadata( + "test-index", + "index-uuid", + "index-filename", + InternalRemoteRoutingTableService.INDEX_ROUTING_METADATA_PREFIX + ); + final ClusterMetadataManifest previousManifest = ClusterMetadataManifest.builder() + .indicesRouting(List.of(uploadedIndexMetadata)) + .build(); + + List allIndiceRoutingMetadata = remoteRoutingTableService + .getAllUploadedIndicesRouting(previousManifest, List.of(uploadedIndexMetadata), List.of()); + assertNotNull(allIndiceRoutingMetadata); + assertEquals(1, allIndiceRoutingMetadata.size()); + assertEquals(uploadedIndexMetadata, allIndiceRoutingMetadata.get(0)); + } + + public void testGetAllUploadedIndicesRoutingNewIndexFromManifest() { + final ClusterMetadataManifest.UploadedIndexMetadata uploadedIndexMetadata = new ClusterMetadataManifest.UploadedIndexMetadata( + "test-index", + "index-uuid", + "index-filename", + InternalRemoteRoutingTableService.INDEX_ROUTING_METADATA_PREFIX + ); + final ClusterMetadataManifest previousManifest = ClusterMetadataManifest.builder() + .indicesRouting(List.of(uploadedIndexMetadata)) + .build(); + final ClusterMetadataManifest.UploadedIndexMetadata uploadedIndexMetadata2 = new ClusterMetadataManifest.UploadedIndexMetadata( + "test-index2", + "index-uuid", + "index-filename", + InternalRemoteRoutingTableService.INDEX_ROUTING_METADATA_PREFIX + ); + + List allIndiceRoutingMetadata = remoteRoutingTableService + .getAllUploadedIndicesRouting(previousManifest, List.of(uploadedIndexMetadata2), List.of()); + assertNotNull(allIndiceRoutingMetadata); + assertEquals(2, allIndiceRoutingMetadata.size()); + assertEquals(uploadedIndexMetadata, allIndiceRoutingMetadata.get(0)); + assertEquals(uploadedIndexMetadata2, allIndiceRoutingMetadata.get(1)); + } + + public void testGetAllUploadedIndicesRoutingIndexDeleted() { + final ClusterMetadataManifest.UploadedIndexMetadata uploadedIndexMetadata = new ClusterMetadataManifest.UploadedIndexMetadata( + "test-index", + "index-uuid", + "index-filename", + InternalRemoteRoutingTableService.INDEX_ROUTING_METADATA_PREFIX + ); + final ClusterMetadataManifest.UploadedIndexMetadata uploadedIndexMetadata2 = new ClusterMetadataManifest.UploadedIndexMetadata( + "test-index2", + "index-uuid", + "index-filename", + InternalRemoteRoutingTableService.INDEX_ROUTING_METADATA_PREFIX + ); + final ClusterMetadataManifest previousManifest = ClusterMetadataManifest.builder() + .indicesRouting(List.of(uploadedIndexMetadata, uploadedIndexMetadata2)) + .build(); + + List allIndiceRoutingMetadata = remoteRoutingTableService + .getAllUploadedIndicesRouting(previousManifest, List.of(uploadedIndexMetadata2), List.of("test-index")); + assertNotNull(allIndiceRoutingMetadata); + assertEquals(1, allIndiceRoutingMetadata.size()); + assertEquals(uploadedIndexMetadata2, allIndiceRoutingMetadata.get(0)); + } + + public void testGetAllUploadedIndicesRoutingNoChange() { + final ClusterMetadataManifest.UploadedIndexMetadata uploadedIndexMetadata = new ClusterMetadataManifest.UploadedIndexMetadata( + "test-index", + "index-uuid", + "index-filename", + InternalRemoteRoutingTableService.INDEX_ROUTING_METADATA_PREFIX + ); + final ClusterMetadataManifest.UploadedIndexMetadata uploadedIndexMetadata2 = new ClusterMetadataManifest.UploadedIndexMetadata( + "test-index2", + "index-uuid", + "index-filename", + InternalRemoteRoutingTableService.INDEX_ROUTING_METADATA_PREFIX + ); + final ClusterMetadataManifest previousManifest = ClusterMetadataManifest.builder() + .indicesRouting(List.of(uploadedIndexMetadata, uploadedIndexMetadata2)) + .build(); + + List allIndiceRoutingMetadata = remoteRoutingTableService + .getAllUploadedIndicesRouting(previousManifest, List.of(), List.of()); + assertNotNull(allIndiceRoutingMetadata); + assertEquals(2, allIndiceRoutingMetadata.size()); + assertEquals(uploadedIndexMetadata, allIndiceRoutingMetadata.get(0)); + assertEquals(uploadedIndexMetadata2, allIndiceRoutingMetadata.get(1)); + } + + private ClusterState createClusterState(String indexName) { + final IndexMetadata indexMetadata = new IndexMetadata.Builder(indexName).settings( + Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) + .put(IndexMetadata.SETTING_INDEX_UUID, "uuid") + .build() + ).numberOfShards(randomInt(1000)).numberOfReplicas(randomInt(10)).build(); + RoutingTable routingTable = RoutingTable.builder().addAsNew(indexMetadata).build(); + return ClusterState.builder(ClusterName.DEFAULT) + .routingTable(routingTable) + .metadata(Metadata.builder().coordinationMetadata(CoordinationMetadata.builder().term(1L).build())) + .version(2L) + .build(); + } + + private BlobPath getPath() { + BlobPath indexRoutingPath = basePath.add(INDEX_ROUTING_PATH_TOKEN); + return RemoteStoreEnums.PathType.HASHED_PREFIX.path( + RemoteStorePathStrategy.PathInput.builder().basePath(indexRoutingPath).indexUUID("uuid").build(), + RemoteStoreEnums.PathHashAlgorithm.FNV_1A_BASE64 + ); + } } diff --git a/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java b/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java index d1f559eb75f85..7cdadfe9e96c3 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java @@ -12,6 +12,7 @@ import org.opensearch.cluster.metadata.IndexGraveyard; import org.opensearch.cluster.metadata.RepositoriesMetadata; import org.opensearch.cluster.metadata.WeightedRoutingMetadata; +import org.opensearch.cluster.routing.remote.InternalRemoteRoutingTableService; import org.opensearch.common.xcontent.json.JsonXContent; import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.core.common.io.stream.NamedWriteableRegistry; @@ -366,6 +367,13 @@ public void testClusterMetadataManifestXContentV2() throws IOException { public void testClusterMetadataManifestXContentV3() throws IOException { UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "test-uuid", "/test/upload/path"); UploadedMetadataAttribute uploadedMetadataAttribute = new UploadedMetadataAttribute("attribute_name", "testing_attribute"); + UploadedIndexMetadata uploadedIndexRoutingMetadata = new UploadedIndexMetadata( + "test-index", + "test-uuid", + "routing-path", + InternalRemoteRoutingTableService.INDEX_ROUTING_METADATA_PREFIX + ); + ClusterMetadataManifest originalManifest = new ClusterMetadataManifest( 1L, 1L, @@ -400,7 +408,7 @@ public void testClusterMetadataManifestXContentV3() throws IOException { ) ).stream().collect(Collectors.toMap(UploadedMetadataAttribute::getAttributeName, Function.identity())), 1L, - Collections.singletonList(uploadedIndexMetadata) + Collections.singletonList(uploadedIndexRoutingMetadata) ); final XContentBuilder builder = JsonXContent.contentBuilder(); builder.startObject(); diff --git a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java index 890a4b478b502..e91eeb82d44b9 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -19,6 +19,9 @@ import org.opensearch.cluster.metadata.Metadata; import org.opensearch.cluster.metadata.TemplatesMetadata; import org.opensearch.cluster.node.DiscoveryNodes; +import org.opensearch.cluster.routing.RoutingTable; +import org.opensearch.cluster.routing.remote.InternalRemoteRoutingTableService; +import org.opensearch.cluster.routing.remote.NoopRemoteRoutingTableService; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.blobstore.AsyncMultiStreamBlobContainer; import org.opensearch.common.blobstore.BlobContainer; @@ -162,6 +165,8 @@ public void setup() { blobStore = mock(BlobStore.class); when(blobStoreRepository.blobStore()).thenReturn(blobStore); when(repositoriesService.repository("remote_store_repository")).thenReturn(blobStoreRepository); + when(repositoriesService.repository("routing_repository")).thenReturn(blobStoreRepository); + when(blobStoreRepository.getNamedXContentRegistry()).thenReturn(xContentRegistry); remoteClusterStateService = new RemoteClusterStateService( "test-node-id", @@ -1402,7 +1407,7 @@ public void testGlobalMetadataUploadWaitTimeSetting() { } public void testRemoteRoutingTableNotInitializedWhenDisabled() { - assertFalse(remoteClusterStateService.getRemoteRoutingTableService().isPresent()); + assertTrue(remoteClusterStateService.getRemoteRoutingTableService() instanceof NoopRemoteRoutingTableService); } public void testRemoteRoutingTableInitializedWhenEnabled() { @@ -1425,7 +1430,172 @@ public void testRemoteRoutingTableInitializedWhenEnabled() { threadPool, List.of(new RemoteIndexPathUploader(threadPool, newSettings, repositoriesServiceSupplier, clusterSettings)) ); - assertTrue(remoteClusterStateService.getRemoteRoutingTableService().isPresent()); + assertTrue(remoteClusterStateService.getRemoteRoutingTableService() instanceof InternalRemoteRoutingTableService); + } + + public void testWriteFullMetadataSuccessWithRoutingTable() throws IOException { + initializeRoutingTable(); + mockBlobStoreObjects(); + when((blobStoreRepository.basePath())).thenReturn(BlobPath.cleanPath().add("base-path")); + + final ClusterState clusterState = generateClusterStateWithOneIndex().nodes(nodesWithLocalNodeClusterManager()).build(); + remoteClusterStateService.start(); + final ClusterMetadataManifest manifest = remoteClusterStateService.writeFullMetadata(clusterState, "prev-cluster-uuid") + .getClusterMetadataManifest(); + final UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "index-uuid", "metadata-filename"); + final UploadedIndexMetadata uploadedIndiceRoutingMetadata = new UploadedIndexMetadata( + "test-index", + "index-uuid", + "routing-filename", + InternalRemoteRoutingTableService.INDEX_ROUTING_METADATA_PREFIX + ); + final ClusterMetadataManifest expectedManifest = ClusterMetadataManifest.builder() + .indices(List.of(uploadedIndexMetadata)) + .clusterTerm(1L) + .stateVersion(1L) + .stateUUID("state-uuid") + .clusterUUID("cluster-uuid") + .previousClusterUUID("prev-cluster-uuid") + .routingTableVersion(1L) + .indicesRouting(List.of(uploadedIndiceRoutingMetadata)) + .build(); + + assertThat(manifest.getIndices().size(), is(1)); + assertThat(manifest.getClusterTerm(), is(expectedManifest.getClusterTerm())); + assertThat(manifest.getStateVersion(), is(expectedManifest.getStateVersion())); + assertThat(manifest.getClusterUUID(), is(expectedManifest.getClusterUUID())); + assertThat(manifest.getStateUUID(), is(expectedManifest.getStateUUID())); + assertThat(manifest.getPreviousClusterUUID(), is(expectedManifest.getPreviousClusterUUID())); + assertThat(manifest.getRoutingTableVersion(), is(expectedManifest.getRoutingTableVersion())); + assertThat(manifest.getIndicesRouting().get(0).getIndexName(), is(uploadedIndiceRoutingMetadata.getIndexName())); + assertThat(manifest.getIndicesRouting().get(0).getIndexUUID(), is(uploadedIndiceRoutingMetadata.getIndexUUID())); + assertThat(manifest.getIndicesRouting().get(0).getUploadedFilename(), notNullValue()); + } + + public void testWriteFullMetadataInParallelSuccessWithRoutingTable() throws IOException { + initializeRoutingTable(); + final ClusterState clusterState = generateClusterStateWithOneIndex().nodes(nodesWithLocalNodeClusterManager()).build(); + AsyncMultiStreamBlobContainer container = (AsyncMultiStreamBlobContainer) mockBlobStoreObjects(AsyncMultiStreamBlobContainer.class); + + ArgumentCaptor> actionListenerArgumentCaptor = ArgumentCaptor.forClass(ActionListener.class); + ArgumentCaptor writeContextArgumentCaptor = ArgumentCaptor.forClass(WriteContext.class); + ConcurrentHashMap capturedWriteContext = new ConcurrentHashMap<>(); + doAnswer((i) -> { + actionListenerArgumentCaptor.getValue().onResponse(null); + WriteContext writeContext = writeContextArgumentCaptor.getValue(); + capturedWriteContext.put(writeContext.getFileName().split(DELIMITER)[0], writeContextArgumentCaptor.getValue()); + return null; + }).when(container).asyncBlobUpload(writeContextArgumentCaptor.capture(), actionListenerArgumentCaptor.capture()); + + when((blobStoreRepository.basePath())).thenReturn(BlobPath.cleanPath().add("base-path")); + + remoteClusterStateService.start(); + final ClusterMetadataManifest manifest = remoteClusterStateService.writeFullMetadata(clusterState, "prev-cluster-uuid") + .getClusterMetadataManifest(); + + final UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "index-uuid", "metadata-filename"); + final UploadedIndexMetadata uploadedIndiceRoutingMetadata = new UploadedIndexMetadata( + "test-index", + "index-uuid", + "routing-filename", + InternalRemoteRoutingTableService.INDEX_ROUTING_METADATA_PREFIX + ); + + final ClusterMetadataManifest expectedManifest = ClusterMetadataManifest.builder() + .indices(List.of(uploadedIndexMetadata)) + .clusterTerm(1L) + .stateVersion(1L) + .stateUUID("state-uuid") + .clusterUUID("cluster-uuid") + .previousClusterUUID("prev-cluster-uuid") + .routingTableVersion(1) + .indicesRouting(List.of(uploadedIndiceRoutingMetadata)) + .build(); + + assertThat(manifest.getIndices().size(), is(1)); + assertThat(manifest.getIndices().get(0).getIndexName(), is(uploadedIndexMetadata.getIndexName())); + assertThat(manifest.getIndices().get(0).getIndexUUID(), is(uploadedIndexMetadata.getIndexUUID())); + assertThat(manifest.getIndices().get(0).getUploadedFilename(), notNullValue()); + assertThat(manifest.getClusterTerm(), is(expectedManifest.getClusterTerm())); + assertThat(manifest.getStateVersion(), is(expectedManifest.getStateVersion())); + assertThat(manifest.getClusterUUID(), is(expectedManifest.getClusterUUID())); + assertThat(manifest.getStateUUID(), is(expectedManifest.getStateUUID())); + assertThat(manifest.getPreviousClusterUUID(), is(expectedManifest.getPreviousClusterUUID())); + assertThat(manifest.getRoutingTableVersion(), is(expectedManifest.getRoutingTableVersion())); + assertThat(manifest.getIndicesRouting().get(0).getIndexName(), is(uploadedIndiceRoutingMetadata.getIndexName())); + assertThat(manifest.getIndicesRouting().get(0).getIndexUUID(), is(uploadedIndiceRoutingMetadata.getIndexUUID())); + assertThat(manifest.getIndicesRouting().get(0).getUploadedFilename(), notNullValue()); + + assertEquals(8, actionListenerArgumentCaptor.getAllValues().size()); + assertEquals(8, writeContextArgumentCaptor.getAllValues().size()); + } + + public void testWriteIncrementalMetadataSuccessWithRoutingTable() throws IOException { + initializeRoutingTable(); + final ClusterState clusterState = generateClusterStateWithOneIndex().nodes(nodesWithLocalNodeClusterManager()).build(); + mockBlobStoreObjects(); + final CoordinationMetadata coordinationMetadata = CoordinationMetadata.builder().term(1L).build(); + final ClusterState previousClusterState = ClusterState.builder(ClusterName.DEFAULT) + .metadata(Metadata.builder().coordinationMetadata(coordinationMetadata)) + .build(); + + final ClusterMetadataManifest previousManifest = ClusterMetadataManifest.builder().indices(Collections.emptyList()).build(); + when((blobStoreRepository.basePath())).thenReturn(BlobPath.cleanPath().add("base-path")); + + remoteClusterStateService.start(); + final ClusterMetadataManifest manifest = remoteClusterStateService.writeIncrementalMetadata( + previousClusterState, + clusterState, + previousManifest + ).getClusterMetadataManifest(); + final UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "index-uuid", "metadata-filename"); + final UploadedIndexMetadata uploadedIndiceRoutingMetadata = new UploadedIndexMetadata( + "test-index", + "index-uuid", + "routing-filename", + InternalRemoteRoutingTableService.INDEX_ROUTING_METADATA_PREFIX + ); + final ClusterMetadataManifest expectedManifest = ClusterMetadataManifest.builder() + .indices(List.of(uploadedIndexMetadata)) + .clusterTerm(1L) + .stateVersion(1L) + .stateUUID("state-uuid") + .clusterUUID("cluster-uuid") + .previousClusterUUID("prev-cluster-uuid") + .routingTableVersion(1) + .indicesRouting(List.of(uploadedIndiceRoutingMetadata)) + .build(); + + assertThat(manifest.getIndices().size(), is(1)); + assertThat(manifest.getClusterTerm(), is(expectedManifest.getClusterTerm())); + assertThat(manifest.getStateVersion(), is(expectedManifest.getStateVersion())); + assertThat(manifest.getClusterUUID(), is(expectedManifest.getClusterUUID())); + assertThat(manifest.getStateUUID(), is(expectedManifest.getStateUUID())); + assertThat(manifest.getRoutingTableVersion(), is(expectedManifest.getRoutingTableVersion())); + assertThat(manifest.getIndicesRouting().get(0).getIndexName(), is(uploadedIndiceRoutingMetadata.getIndexName())); + assertThat(manifest.getIndicesRouting().get(0).getIndexUUID(), is(uploadedIndiceRoutingMetadata.getIndexUUID())); + assertThat(manifest.getIndicesRouting().get(0).getUploadedFilename(), notNullValue()); + } + + private void initializeRoutingTable() { + Settings newSettings = Settings.builder() + .put("node.attr." + REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, "routing_repository") + .put("node.attr." + REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY, "remote_store_repository") + .put(RemoteClusterStateService.REMOTE_CLUSTER_STATE_ENABLED_SETTING.getKey(), true) + .build(); + clusterSettings.applySettings(newSettings); + + Settings nodeSettings = Settings.builder().put(REMOTE_PUBLICATION_EXPERIMENTAL, "true").build(); + FeatureFlags.initializeFeatureFlags(nodeSettings); + remoteClusterStateService = new RemoteClusterStateService( + "test-node-id", + repositoriesServiceSupplier, + newSettings, + clusterService, + () -> 0L, + threadPool, + List.of(new RemoteIndexPathUploader(threadPool, newSettings, repositoriesServiceSupplier, clusterSettings)) + ); } private void mockObjectsForGettingPreviousClusterUUID(Map clusterUUIDsPointers) throws IOException { @@ -1850,7 +2020,8 @@ static ClusterState.Builder generateClusterStateWithOneIndex() { .templates(templatesMetadata) .putCustom(customMetadata1.getWriteableName(), customMetadata1) .build() - ); + ) + .routingTable(RoutingTable.builder().addAsNew(indexMetadata).version(1L).build()); } static DiscoveryNodes nodesWithLocalNodeClusterManager() { diff --git a/server/src/test/java/org/opensearch/index/remote/RemoteStoreEnumsTests.java b/server/src/test/java/org/opensearch/index/remote/RemoteStoreEnumsTests.java index 575b397382f24..481a0568ff0a7 100644 --- a/server/src/test/java/org/opensearch/index/remote/RemoteStoreEnumsTests.java +++ b/server/src/test/java/org/opensearch/index/remote/RemoteStoreEnumsTests.java @@ -13,7 +13,7 @@ import org.opensearch.index.remote.RemoteStoreEnums.DataCategory; import org.opensearch.index.remote.RemoteStoreEnums.DataType; import org.opensearch.index.remote.RemoteStoreEnums.PathType; -import org.opensearch.index.remote.RemoteStorePathStrategy.PathInput; +import org.opensearch.index.remote.RemoteStorePathStrategy.ShardDataPathInput; import org.opensearch.test.OpenSearchTestCase; import java.util.ArrayList; @@ -70,7 +70,7 @@ public void testGeneratePathForFixedType() { String basePath = getPath(pathList) + indexUUID + SEPARATOR + shardId + SEPARATOR; // Translog Data - PathInput pathInput = PathInput.builder() + ShardDataPathInput pathInput = ShardDataPathInput.builder() .basePath(blobPath) .indexUUID(indexUUID) .shardId(shardId) @@ -82,7 +82,7 @@ public void testGeneratePathForFixedType() { // Translog Metadata dataType = METADATA; - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(blobPath) .indexUUID(indexUUID) .shardId(shardId) @@ -92,21 +92,10 @@ public void testGeneratePathForFixedType() { result = FIXED.path(pathInput, null); assertEquals(basePath + dataCategory.getName() + SEPARATOR + dataType.getName() + SEPARATOR, result.buildAsString()); - // Translog Lock files - This is a negative case where the assertion will trip. - dataType = LOCK_FILES; - PathInput finalPathInput = PathInput.builder() - .basePath(blobPath) - .indexUUID(indexUUID) - .shardId(shardId) - .dataCategory(dataCategory) - .dataType(dataType) - .build(); - assertThrows(AssertionError.class, () -> FIXED.path(finalPathInput, null)); - // Segment Data dataCategory = SEGMENTS; dataType = DATA; - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(blobPath) .indexUUID(indexUUID) .shardId(shardId) @@ -118,7 +107,7 @@ public void testGeneratePathForFixedType() { // Segment Metadata dataType = METADATA; - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(blobPath) .indexUUID(indexUUID) .shardId(shardId) @@ -130,7 +119,7 @@ public void testGeneratePathForFixedType() { // Segment Metadata dataType = LOCK_FILES; - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(blobPath) .indexUUID(indexUUID) .shardId(shardId) @@ -155,7 +144,7 @@ public void testGeneratePathForHashedPrefixType() { String basePath = getPath(pathList) + indexUUID + SEPARATOR + shardId; // Translog Data - PathInput pathInput = PathInput.builder() + ShardDataPathInput pathInput = ShardDataPathInput.builder() .basePath(blobPath) .indexUUID(indexUUID) .shardId(shardId) @@ -172,7 +161,7 @@ public void testGeneratePathForHashedPrefixType() { BlobPath fixedBlobPath = BlobPath.cleanPath().add("xjsdhj").add("ddjsha").add("yudy7sd").add("32hdhua7").add("89jdij"); String fixedIndexUUID = "k2ijhe877d7yuhx7"; String fixedShardId = "10"; - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(fixedBlobPath) .indexUUID(fixedIndexUUID) .shardId(fixedShardId) @@ -184,7 +173,7 @@ public void testGeneratePathForHashedPrefixType() { // Translog Metadata dataType = METADATA; - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(blobPath) .indexUUID(indexUUID) .shardId(shardId) @@ -198,7 +187,7 @@ public void testGeneratePathForHashedPrefixType() { ); // assert with exact value for known base path - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(fixedBlobPath) .indexUUID(fixedIndexUUID) .shardId(fixedShardId) @@ -208,31 +197,10 @@ public void testGeneratePathForHashedPrefixType() { result = HASHED_PREFIX.path(pathInput, FNV_1A_BASE64); assertEquals("oKU5SjILiy4/xjsdhj/ddjsha/yudy7sd/32hdhua7/89jdij/k2ijhe877d7yuhx7/10/translog/metadata/", result.buildAsString()); - // Translog Lock files - This is a negative case where the assertion will trip. - dataType = LOCK_FILES; - PathInput finalPathInput = PathInput.builder() - .basePath(blobPath) - .indexUUID(indexUUID) - .shardId(shardId) - .dataCategory(dataCategory) - .dataType(dataType) - .build(); - assertThrows(AssertionError.class, () -> HASHED_PREFIX.path(finalPathInput, null)); - - // assert with exact value for known base path - pathInput = PathInput.builder() - .basePath(fixedBlobPath) - .indexUUID(fixedIndexUUID) - .shardId(fixedShardId) - .dataCategory(dataCategory) - .dataType(dataType) - .build(); - assertThrows(AssertionError.class, () -> HASHED_PREFIX.path(finalPathInput, null)); - // Segment Data dataCategory = SEGMENTS; dataType = DATA; - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(blobPath) .indexUUID(indexUUID) .shardId(shardId) @@ -246,7 +214,7 @@ public void testGeneratePathForHashedPrefixType() { ); // assert with exact value for known base path - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(fixedBlobPath) .indexUUID(fixedIndexUUID) .shardId(fixedShardId) @@ -258,7 +226,7 @@ public void testGeneratePathForHashedPrefixType() { // Segment Metadata dataType = METADATA; - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(blobPath) .indexUUID(indexUUID) .shardId(shardId) @@ -272,7 +240,7 @@ public void testGeneratePathForHashedPrefixType() { ); // assert with exact value for known base path - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(fixedBlobPath) .indexUUID(fixedIndexUUID) .shardId(fixedShardId) @@ -284,7 +252,7 @@ public void testGeneratePathForHashedPrefixType() { // Segment Lockfiles dataType = LOCK_FILES; - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(blobPath) .indexUUID(indexUUID) .shardId(shardId) @@ -298,7 +266,7 @@ public void testGeneratePathForHashedPrefixType() { ); // assert with exact value for known base path - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(fixedBlobPath) .indexUUID(fixedIndexUUID) .shardId(fixedShardId) @@ -323,7 +291,7 @@ public void testGeneratePathForHashedPrefixTypeAndFNVCompositeHashAlgorithm() { String basePath = getPath(pathList) + indexUUID + SEPARATOR + shardId; // Translog Data - PathInput pathInput = PathInput.builder() + ShardDataPathInput pathInput = ShardDataPathInput.builder() .basePath(blobPath) .indexUUID(indexUUID) .shardId(shardId) @@ -342,7 +310,7 @@ public void testGeneratePathForHashedPrefixTypeAndFNVCompositeHashAlgorithm() { BlobPath fixedBlobPath = BlobPath.cleanPath().add("xjsdhj").add("ddjsha").add("yudy7sd").add("32hdhua7").add("89jdij"); String fixedIndexUUID = "k2ijhe877d7yuhx7"; String fixedShardId = "10"; - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(fixedBlobPath) .indexUUID(fixedIndexUUID) .shardId(fixedShardId) @@ -354,7 +322,7 @@ public void testGeneratePathForHashedPrefixTypeAndFNVCompositeHashAlgorithm() { // Translog Metadata dataType = METADATA; - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(blobPath) .indexUUID(indexUUID) .shardId(shardId) @@ -370,7 +338,7 @@ public void testGeneratePathForHashedPrefixTypeAndFNVCompositeHashAlgorithm() { ); // assert with exact value for known base path - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(fixedBlobPath) .indexUUID(fixedIndexUUID) .shardId(fixedShardId) @@ -383,31 +351,10 @@ public void testGeneratePathForHashedPrefixTypeAndFNVCompositeHashAlgorithm() { result.buildAsString() ); - // Translog Lock files - This is a negative case where the assertion will trip. - dataType = LOCK_FILES; - PathInput finalPathInput = PathInput.builder() - .basePath(blobPath) - .indexUUID(indexUUID) - .shardId(shardId) - .dataCategory(dataCategory) - .dataType(dataType) - .build(); - assertThrows(AssertionError.class, () -> HASHED_PREFIX.path(finalPathInput, null)); - - // assert with exact value for known base path - pathInput = PathInput.builder() - .basePath(fixedBlobPath) - .indexUUID(fixedIndexUUID) - .shardId(fixedShardId) - .dataCategory(dataCategory) - .dataType(dataType) - .build(); - assertThrows(AssertionError.class, () -> HASHED_PREFIX.path(finalPathInput, null)); - // Segment Data dataCategory = SEGMENTS; dataType = DATA; - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(blobPath) .indexUUID(indexUUID) .shardId(shardId) @@ -423,7 +370,7 @@ public void testGeneratePathForHashedPrefixTypeAndFNVCompositeHashAlgorithm() { ); // assert with exact value for known base path - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(fixedBlobPath) .indexUUID(fixedIndexUUID) .shardId(fixedShardId) @@ -435,7 +382,7 @@ public void testGeneratePathForHashedPrefixTypeAndFNVCompositeHashAlgorithm() { // Segment Metadata dataType = METADATA; - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(blobPath) .indexUUID(indexUUID) .shardId(shardId) @@ -451,7 +398,7 @@ public void testGeneratePathForHashedPrefixTypeAndFNVCompositeHashAlgorithm() { ); // assert with exact value for known base path - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(fixedBlobPath) .indexUUID(fixedIndexUUID) .shardId(fixedShardId) @@ -466,7 +413,7 @@ public void testGeneratePathForHashedPrefixTypeAndFNVCompositeHashAlgorithm() { // Segment Lockfiles dataType = LOCK_FILES; - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(blobPath) .indexUUID(indexUUID) .shardId(shardId) @@ -482,7 +429,7 @@ public void testGeneratePathForHashedPrefixTypeAndFNVCompositeHashAlgorithm() { ); // assert with exact value for known base path - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(fixedBlobPath) .indexUUID(fixedIndexUUID) .shardId(fixedShardId) @@ -509,9 +456,9 @@ public void testGeneratePathForHashedInfixType() { DataType dataType = DATA; String basePath = getPath(pathList); - basePath = basePath.length() == 0 ? basePath : basePath.substring(0, basePath.length() - 1); + basePath = basePath.isEmpty() ? basePath : basePath.substring(0, basePath.length() - 1); // Translog Data - PathInput pathInput = PathInput.builder() + ShardDataPathInput pathInput = ShardDataPathInput.builder() .basePath(blobPath) .indexUUID(indexUUID) .shardId(shardId) @@ -527,7 +474,7 @@ public void testGeneratePathForHashedInfixType() { BlobPath fixedBlobPath = BlobPath.cleanPath().add("xjsdhj").add("ddjsha").add("yudy7sd").add("32hdhua7").add("89jdij"); String fixedIndexUUID = "k2ijhe877d7yuhx7"; String fixedShardId = "10"; - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(fixedBlobPath) .indexUUID(fixedIndexUUID) .shardId(fixedShardId) @@ -541,7 +488,7 @@ public void testGeneratePathForHashedInfixType() { // Translog Metadata dataType = METADATA; - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(blobPath) .indexUUID(indexUUID) .shardId(shardId) @@ -555,7 +502,7 @@ public void testGeneratePathForHashedInfixType() { assertTrue(new ParameterizedMessage("expected={} actual={}", expected, actual).getFormattedMessage(), actual.startsWith(expected)); // assert with exact value for known base path - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(fixedBlobPath) .indexUUID(fixedIndexUUID) .shardId(fixedShardId) @@ -567,31 +514,10 @@ public void testGeneratePathForHashedInfixType() { actual = result.buildAsString(); assertTrue(new ParameterizedMessage("expected={} actual={}", expected, actual).getFormattedMessage(), actual.startsWith(expected)); - // Translog Lock files - This is a negative case where the assertion will trip. - dataType = LOCK_FILES; - PathInput finalPathInput = PathInput.builder() - .basePath(blobPath) - .indexUUID(indexUUID) - .shardId(shardId) - .dataCategory(dataCategory) - .dataType(dataType) - .build(); - assertThrows(AssertionError.class, () -> HASHED_INFIX.path(finalPathInput, null)); - - // assert with exact value for known base path - pathInput = PathInput.builder() - .basePath(fixedBlobPath) - .indexUUID(fixedIndexUUID) - .shardId(fixedShardId) - .dataCategory(dataCategory) - .dataType(dataType) - .build(); - assertThrows(AssertionError.class, () -> HASHED_INFIX.path(finalPathInput, null)); - // Segment Data dataCategory = SEGMENTS; dataType = DATA; - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(blobPath) .indexUUID(indexUUID) .shardId(shardId) @@ -604,7 +530,7 @@ public void testGeneratePathForHashedInfixType() { assertTrue(new ParameterizedMessage("expected={} actual={}", expected, actual).getFormattedMessage(), actual.startsWith(expected)); // assert with exact value for known base path - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(fixedBlobPath) .indexUUID(fixedIndexUUID) .shardId(fixedShardId) @@ -618,7 +544,7 @@ public void testGeneratePathForHashedInfixType() { // Segment Metadata dataType = METADATA; - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(blobPath) .indexUUID(indexUUID) .shardId(shardId) @@ -631,7 +557,7 @@ public void testGeneratePathForHashedInfixType() { assertTrue(new ParameterizedMessage("expected={} actual={}", expected, actual).getFormattedMessage(), actual.startsWith(expected)); // assert with exact value for known base path - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(fixedBlobPath) .indexUUID(fixedIndexUUID) .shardId(fixedShardId) @@ -645,7 +571,7 @@ public void testGeneratePathForHashedInfixType() { // Segment Lockfiles dataType = LOCK_FILES; - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(blobPath) .indexUUID(indexUUID) .shardId(shardId) @@ -658,7 +584,7 @@ public void testGeneratePathForHashedInfixType() { assertTrue(new ParameterizedMessage("expected={} actual={}", expected, actual).getFormattedMessage(), actual.startsWith(expected)); // assert with exact value for known base path - pathInput = PathInput.builder() + pathInput = ShardDataPathInput.builder() .basePath(fixedBlobPath) .indexUUID(fixedIndexUUID) .shardId(fixedShardId) @@ -671,7 +597,7 @@ public void testGeneratePathForHashedInfixType() { assertTrue(new ParameterizedMessage("expected={} actual={}", expected, actual).getFormattedMessage(), actual.startsWith(expected)); } - private String derivePath(String basePath, PathInput pathInput) { + private String derivePath(String basePath, ShardDataPathInput pathInput) { return "".equals(basePath) ? String.join( SEPARATOR, diff --git a/server/src/test/java/org/opensearch/index/remote/RemoteStorePathStrategyTests.java b/server/src/test/java/org/opensearch/index/remote/RemoteStorePathStrategyTests.java new file mode 100644 index 0000000000000..cf5876cb5caf1 --- /dev/null +++ b/server/src/test/java/org/opensearch/index/remote/RemoteStorePathStrategyTests.java @@ -0,0 +1,87 @@ +/* + * 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.index.remote; + +import org.opensearch.common.blobstore.BlobPath; +import org.opensearch.test.OpenSearchTestCase; + +import static org.opensearch.index.remote.RemoteStoreEnums.DataCategory.TRANSLOG; +import static org.opensearch.index.remote.RemoteStoreEnums.DataType.DATA; +import static org.opensearch.index.remote.RemoteStoreEnums.DataType.LOCK_FILES; + +public class RemoteStorePathStrategyTests extends OpenSearchTestCase { + + private static final BlobPath BASE_PATH = BlobPath.cleanPath().add("base-path"); + private static final String INDEX_UUID = "indexUUID"; + private static final String SHARD_ID = "shardId"; + + public void testBasePathInput() { + assertThrows(NullPointerException.class, () -> RemoteStorePathStrategy.PathInput.builder().build()); + assertThrows(NullPointerException.class, () -> RemoteStorePathStrategy.PathInput.builder().basePath(BASE_PATH).build()); + assertThrows(NullPointerException.class, () -> RemoteStorePathStrategy.PathInput.builder().indexUUID(INDEX_UUID).build()); + RemoteStorePathStrategy.PathInput input = RemoteStorePathStrategy.PathInput.builder() + .basePath(BASE_PATH) + .indexUUID(INDEX_UUID) + .build(); + assertEquals(BASE_PATH, input.basePath()); + assertEquals(INDEX_UUID, input.indexUUID()); + } + + public void testPathInput() { + assertThrows(NullPointerException.class, () -> RemoteStorePathStrategy.ShardDataPathInput.builder().build()); + assertThrows(NullPointerException.class, () -> RemoteStorePathStrategy.ShardDataPathInput.builder().shardId(SHARD_ID).build()); + assertThrows( + NullPointerException.class, + () -> RemoteStorePathStrategy.ShardDataPathInput.builder().shardId(SHARD_ID).dataCategory(TRANSLOG).build() + ); + + // Translog Lock files - This is a negative case where the assertion will trip. + assertThrows( + AssertionError.class, + () -> RemoteStorePathStrategy.ShardDataPathInput.builder() + .basePath(BASE_PATH) + .indexUUID(INDEX_UUID) + .shardId(SHARD_ID) + .dataCategory(TRANSLOG) + .dataType(LOCK_FILES) + .build() + ); + + RemoteStorePathStrategy.ShardDataPathInput input = RemoteStorePathStrategy.ShardDataPathInput.builder() + .basePath(BASE_PATH) + .indexUUID(INDEX_UUID) + .shardId(SHARD_ID) + .dataCategory(TRANSLOG) + .dataType(DATA) + .build(); + assertEquals(BASE_PATH, input.basePath()); + assertEquals(INDEX_UUID, input.indexUUID()); + assertEquals(SHARD_ID, input.shardId()); + assertEquals(DATA, input.dataType()); + assertEquals(TRANSLOG, input.dataCategory()); + } + + public void testFixedSubPath() { + RemoteStorePathStrategy.PathInput input = RemoteStorePathStrategy.PathInput.builder() + .basePath(BASE_PATH) + .indexUUID(INDEX_UUID) + .build(); + assertEquals(BlobPath.cleanPath().add(INDEX_UUID), input.fixedSubPath()); + + RemoteStorePathStrategy.ShardDataPathInput input2 = RemoteStorePathStrategy.ShardDataPathInput.builder() + .basePath(BASE_PATH) + .indexUUID(INDEX_UUID) + .shardId(SHARD_ID) + .dataCategory(TRANSLOG) + .dataType(DATA) + .build(); + assertEquals(BlobPath.cleanPath().add(INDEX_UUID).add(SHARD_ID).add(TRANSLOG.getName()).add(DATA.getName()), input2.fixedSubPath()); + + } +} diff --git a/test/framework/src/main/java/org/opensearch/test/OpenSearchTestCase.java b/test/framework/src/main/java/org/opensearch/test/OpenSearchTestCase.java index 5ee65e7ea1a1c..6afc7c23d9e66 100644 --- a/test/framework/src/main/java/org/opensearch/test/OpenSearchTestCase.java +++ b/test/framework/src/main/java/org/opensearch/test/OpenSearchTestCase.java @@ -1828,7 +1828,7 @@ public static BlobPath getShardLevelBlobPath( ? RemoteStoreEnums.PathHashAlgorithm.valueOf(remoteCustomData.get(RemoteStoreEnums.PathHashAlgorithm.NAME)) : null : null; - RemoteStorePathStrategy.PathInput pathInput = RemoteStorePathStrategy.PathInput.builder() + RemoteStorePathStrategy.ShardDataPathInput pathInput = RemoteStorePathStrategy.ShardDataPathInput.builder() .basePath(basePath) .indexUUID(indexUUID) .shardId(shardId)