From 58303c61bfbe9bd6dd1d36afd59707a62e892931 Mon Sep 17 00:00:00 2001 From: Kaushal Kumar Date: Mon, 1 Jul 2024 12:29:50 -0700 Subject: [PATCH 01/20] add query group header propagator Signed-off-by: Kaushal Kumar --- ...ueryGroupThreadContextStatePropagator.java | 54 +++++++++++++++++++ .../java/org/opensearch/wlm/package-info.java | 13 +++++ ...GroupThreadContextStatePropagatorTest.java | 35 ++++++++++++ 3 files changed, 102 insertions(+) create mode 100644 server/src/main/java/org/opensearch/wlm/QueryGroupThreadContextStatePropagator.java create mode 100644 server/src/main/java/org/opensearch/wlm/package-info.java create mode 100644 server/src/test/java/org/opensearch/wlm/QueryGroupThreadContextStatePropagatorTest.java diff --git a/server/src/main/java/org/opensearch/wlm/QueryGroupThreadContextStatePropagator.java b/server/src/main/java/org/opensearch/wlm/QueryGroupThreadContextStatePropagator.java new file mode 100644 index 0000000000000..30bd45b709a97 --- /dev/null +++ b/server/src/main/java/org/opensearch/wlm/QueryGroupThreadContextStatePropagator.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.wlm; + +import org.opensearch.common.util.concurrent.ThreadContextStatePropagator; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +/** + * This class is used to propagate QueryGroup related headers to request and nodes + */ +public class QueryGroupThreadContextStatePropagator implements ThreadContextStatePropagator { + // TODO: move this constant to QueryGroupService class once the QueryGroup monitoring framework PR is ready + public static List PROPAGATED_HEADERS = List.of("queryGroupId"); + /** + * @param source current context transient headers + * @return the map of header and their values to be propagated across request threadContexts + */ + @Override + public Map transients(Map source) { + final Map transientHeaders = new HashMap<>(); + + for (String headerName: PROPAGATED_HEADERS) { + if (source.containsKey(headerName)) { + transientHeaders.put(headerName, source.get(headerName)); + } + } + return transientHeaders; + } + + /** + * @param source current context headers + * @return map of header and their values to be propagated across nodes + */ + @Override + public Map headers(Map source) { + final Map propagatedHeaders = new HashMap<>(); + + for (String headerName: PROPAGATED_HEADERS) { + if (source.containsKey(headerName)) { + propagatedHeaders.put(headerName, (String) source.get(headerName)); + } + } + return propagatedHeaders; + } +} diff --git a/server/src/main/java/org/opensearch/wlm/package-info.java b/server/src/main/java/org/opensearch/wlm/package-info.java new file mode 100644 index 0000000000000..f6a8008b6374e --- /dev/null +++ b/server/src/main/java/org/opensearch/wlm/package-info.java @@ -0,0 +1,13 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.wlm; + +/** + * This package contains workload management constructs + */ diff --git a/server/src/test/java/org/opensearch/wlm/QueryGroupThreadContextStatePropagatorTest.java b/server/src/test/java/org/opensearch/wlm/QueryGroupThreadContextStatePropagatorTest.java new file mode 100644 index 0000000000000..c10d75789681e --- /dev/null +++ b/server/src/test/java/org/opensearch/wlm/QueryGroupThreadContextStatePropagatorTest.java @@ -0,0 +1,35 @@ +/* + * 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.wlm; + +import org.junit.Test; +import org.opensearch.test.OpenSearchTestCase; + +import java.util.Map; + +import static org.junit.Assert.*; + +public class QueryGroupThreadContextStatePropagatorTest extends OpenSearchTestCase { + + + public void testTransients() { + QueryGroupThreadContextStatePropagator sut = new QueryGroupThreadContextStatePropagator(); + Map source = Map.of("queryGroupId", "adgarja0r235te"); + Map transients = sut.transients(source); + assertEquals("adgarja0r235te", transients.get("queryGroupId")); + } + + + public void testHeaders() { + QueryGroupThreadContextStatePropagator sut = new QueryGroupThreadContextStatePropagator(); + Map source = Map.of("queryGroupId", "adgarja0r235te"); + Map headers = sut.headers(source); + assertEquals("adgarja0r235te", headers.get("queryGroupId")); + } +} From 5efa3e351947fc77354ad725c76c8a547b265ee4 Mon Sep 17 00:00:00 2001 From: Kaushal Kumar Date: Mon, 1 Jul 2024 12:34:31 -0700 Subject: [PATCH 02/20] apply spotless check Signed-off-by: Kaushal Kumar --- .../wlm/QueryGroupThreadContextStatePropagator.java | 5 +++-- .../wlm/QueryGroupThreadContextStatePropagatorTest.java | 5 ----- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/org/opensearch/wlm/QueryGroupThreadContextStatePropagator.java b/server/src/main/java/org/opensearch/wlm/QueryGroupThreadContextStatePropagator.java index 30bd45b709a97..1408afa10c7cf 100644 --- a/server/src/main/java/org/opensearch/wlm/QueryGroupThreadContextStatePropagator.java +++ b/server/src/main/java/org/opensearch/wlm/QueryGroupThreadContextStatePropagator.java @@ -20,6 +20,7 @@ public class QueryGroupThreadContextStatePropagator implements ThreadContextStatePropagator { // TODO: move this constant to QueryGroupService class once the QueryGroup monitoring framework PR is ready public static List PROPAGATED_HEADERS = List.of("queryGroupId"); + /** * @param source current context transient headers * @return the map of header and their values to be propagated across request threadContexts @@ -28,7 +29,7 @@ public class QueryGroupThreadContextStatePropagator implements ThreadContextStat public Map transients(Map source) { final Map transientHeaders = new HashMap<>(); - for (String headerName: PROPAGATED_HEADERS) { + for (String headerName : PROPAGATED_HEADERS) { if (source.containsKey(headerName)) { transientHeaders.put(headerName, source.get(headerName)); } @@ -44,7 +45,7 @@ public Map transients(Map source) { public Map headers(Map source) { final Map propagatedHeaders = new HashMap<>(); - for (String headerName: PROPAGATED_HEADERS) { + for (String headerName : PROPAGATED_HEADERS) { if (source.containsKey(headerName)) { propagatedHeaders.put(headerName, (String) source.get(headerName)); } diff --git a/server/src/test/java/org/opensearch/wlm/QueryGroupThreadContextStatePropagatorTest.java b/server/src/test/java/org/opensearch/wlm/QueryGroupThreadContextStatePropagatorTest.java index c10d75789681e..60c8f2605914f 100644 --- a/server/src/test/java/org/opensearch/wlm/QueryGroupThreadContextStatePropagatorTest.java +++ b/server/src/test/java/org/opensearch/wlm/QueryGroupThreadContextStatePropagatorTest.java @@ -8,16 +8,12 @@ package org.opensearch.wlm; -import org.junit.Test; import org.opensearch.test.OpenSearchTestCase; import java.util.Map; -import static org.junit.Assert.*; - public class QueryGroupThreadContextStatePropagatorTest extends OpenSearchTestCase { - public void testTransients() { QueryGroupThreadContextStatePropagator sut = new QueryGroupThreadContextStatePropagator(); Map source = Map.of("queryGroupId", "adgarja0r235te"); @@ -25,7 +21,6 @@ public void testTransients() { assertEquals("adgarja0r235te", transients.get("queryGroupId")); } - public void testHeaders() { QueryGroupThreadContextStatePropagator sut = new QueryGroupThreadContextStatePropagator(); Map source = Map.of("queryGroupId", "adgarja0r235te"); From ede0efaccd90c51b06e100aaad8aecbca57bf58f Mon Sep 17 00:00:00 2001 From: Kaushal Kumar Date: Mon, 1 Jul 2024 12:45:40 -0700 Subject: [PATCH 03/20] add new propagator in ThreadContext Signed-off-by: Kaushal Kumar --- .../org/opensearch/common/util/concurrent/ThreadContext.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 906a27e9f398c..ac8cb7ec7802c 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 @@ -47,6 +47,7 @@ import org.opensearch.http.HttpTransportSettings; import org.opensearch.tasks.Task; import org.opensearch.tasks.TaskThreadContextStatePropagator; +import org.opensearch.wlm.QueryGroupThreadContextStatePropagator; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -128,7 +129,7 @@ public ThreadContext(Settings settings) { this.threadLocal = ThreadLocal.withInitial(() -> DEFAULT_CONTEXT); this.maxWarningHeaderCount = SETTING_HTTP_MAX_WARNING_HEADER_COUNT.get(settings); this.maxWarningHeaderSize = SETTING_HTTP_MAX_WARNING_HEADER_SIZE.get(settings).getBytes(); - this.propagators = new CopyOnWriteArrayList<>(List.of(new TaskThreadContextStatePropagator())); + this.propagators = new CopyOnWriteArrayList<>(List.of(new TaskThreadContextStatePropagator(), new QueryGroupThreadContextStatePropagator())); } public void registerThreadContextStatePropagator(final ThreadContextStatePropagator propagator) { From 51c52221521e73f8eef074f92862b08ca75eb0c5 Mon Sep 17 00:00:00 2001 From: Kaushal Kumar Date: Mon, 1 Jul 2024 12:47:21 -0700 Subject: [PATCH 04/20] spotlessApply Signed-off-by: Kaushal Kumar --- .../org/opensearch/common/util/concurrent/ThreadContext.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 ac8cb7ec7802c..3925c84ac7261 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 @@ -129,7 +129,9 @@ public ThreadContext(Settings settings) { this.threadLocal = ThreadLocal.withInitial(() -> DEFAULT_CONTEXT); this.maxWarningHeaderCount = SETTING_HTTP_MAX_WARNING_HEADER_COUNT.get(settings); this.maxWarningHeaderSize = SETTING_HTTP_MAX_WARNING_HEADER_SIZE.get(settings).getBytes(); - this.propagators = new CopyOnWriteArrayList<>(List.of(new TaskThreadContextStatePropagator(), new QueryGroupThreadContextStatePropagator())); + this.propagators = new CopyOnWriteArrayList<>( + List.of(new TaskThreadContextStatePropagator(), new QueryGroupThreadContextStatePropagator()) + ); } public void registerThreadContextStatePropagator(final ThreadContextStatePropagator propagator) { From dfa53b15f48a5223ae766ce7ecf12ac3a5efa136 Mon Sep 17 00:00:00 2001 From: Kaushal Kumar Date: Tue, 2 Jul 2024 16:55:01 -0700 Subject: [PATCH 05/20] address comments Signed-off-by: Kaushal Kumar --- .../wlm/QueryGroupThreadContextStatePropagator.java | 10 ++++------ .../src/main/java/org/opensearch/wlm/package-info.java | 4 ++-- ...> QueryGroupThreadContextStatePropagatorTests.java} | 2 +- 3 files changed, 7 insertions(+), 9 deletions(-) rename server/src/test/java/org/opensearch/wlm/{QueryGroupThreadContextStatePropagatorTest.java => QueryGroupThreadContextStatePropagatorTests.java} (92%) diff --git a/server/src/main/java/org/opensearch/wlm/QueryGroupThreadContextStatePropagator.java b/server/src/main/java/org/opensearch/wlm/QueryGroupThreadContextStatePropagator.java index 1408afa10c7cf..06d223907082e 100644 --- a/server/src/main/java/org/opensearch/wlm/QueryGroupThreadContextStatePropagator.java +++ b/server/src/main/java/org/opensearch/wlm/QueryGroupThreadContextStatePropagator.java @@ -26,13 +26,12 @@ public class QueryGroupThreadContextStatePropagator implements ThreadContextStat * @return the map of header and their values to be propagated across request threadContexts */ @Override + @SuppressWarnings("removal") public Map transients(Map source) { final Map transientHeaders = new HashMap<>(); for (String headerName : PROPAGATED_HEADERS) { - if (source.containsKey(headerName)) { - transientHeaders.put(headerName, source.get(headerName)); - } + transientHeaders.compute(headerName, (k, v) -> source.get(headerName)); } return transientHeaders; } @@ -42,13 +41,12 @@ public Map transients(Map source) { * @return map of header and their values to be propagated across nodes */ @Override + @SuppressWarnings("removal") public Map headers(Map source) { final Map propagatedHeaders = new HashMap<>(); for (String headerName : PROPAGATED_HEADERS) { - if (source.containsKey(headerName)) { - propagatedHeaders.put(headerName, (String) source.get(headerName)); - } + propagatedHeaders.compute(headerName, (k, v) -> (String) source.get(headerName)); } return propagatedHeaders; } diff --git a/server/src/main/java/org/opensearch/wlm/package-info.java b/server/src/main/java/org/opensearch/wlm/package-info.java index f6a8008b6374e..fa4731d95cc34 100644 --- a/server/src/main/java/org/opensearch/wlm/package-info.java +++ b/server/src/main/java/org/opensearch/wlm/package-info.java @@ -6,8 +6,8 @@ * compatible open source license. */ -package org.opensearch.wlm; - /** * This package contains workload management constructs */ + +package org.opensearch.wlm; diff --git a/server/src/test/java/org/opensearch/wlm/QueryGroupThreadContextStatePropagatorTest.java b/server/src/test/java/org/opensearch/wlm/QueryGroupThreadContextStatePropagatorTests.java similarity index 92% rename from server/src/test/java/org/opensearch/wlm/QueryGroupThreadContextStatePropagatorTest.java rename to server/src/test/java/org/opensearch/wlm/QueryGroupThreadContextStatePropagatorTests.java index 60c8f2605914f..ad5d7f569a56e 100644 --- a/server/src/test/java/org/opensearch/wlm/QueryGroupThreadContextStatePropagatorTest.java +++ b/server/src/test/java/org/opensearch/wlm/QueryGroupThreadContextStatePropagatorTests.java @@ -12,7 +12,7 @@ import java.util.Map; -public class QueryGroupThreadContextStatePropagatorTest extends OpenSearchTestCase { +public class QueryGroupThreadContextStatePropagatorTests extends OpenSearchTestCase { public void testTransients() { QueryGroupThreadContextStatePropagator sut = new QueryGroupThreadContextStatePropagator(); From 153134d4a0df279e56c94b50b6dc6eb80bce3036 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 1 Jul 2024 18:51:29 -0400 Subject: [PATCH 06/20] Bump com.microsoft.azure:msal4j from 1.15.1 to 1.16.0 in /plugins/repository-azure (#14610) * Bump com.microsoft.azure:msal4j in /plugins/repository-azure Bumps [com.microsoft.azure:msal4j](https://github.com/AzureAD/microsoft-authentication-library-for-java) from 1.15.1 to 1.16.0. - [Release notes](https://github.com/AzureAD/microsoft-authentication-library-for-java/releases) - [Changelog](https://github.com/AzureAD/microsoft-authentication-library-for-java/blob/dev/changelog.txt) - [Commits](https://github.com/AzureAD/microsoft-authentication-library-for-java/compare/v1.15.1...v1.16.0) --- updated-dependencies: - dependency-name: com.microsoft.azure:msal4j dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] * Updating SHAs 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] Signed-off-by: Kaushal Kumar --- CHANGELOG.md | 1 + plugins/repository-azure/build.gradle | 2 +- plugins/repository-azure/licenses/msal4j-1.15.1.jar.sha1 | 1 - plugins/repository-azure/licenses/msal4j-1.16.0.jar.sha1 | 1 + 4 files changed, 3 insertions(+), 2 deletions(-) delete mode 100644 plugins/repository-azure/licenses/msal4j-1.15.1.jar.sha1 create mode 100644 plugins/repository-azure/licenses/msal4j-1.16.0.jar.sha1 diff --git a/CHANGELOG.md b/CHANGELOG.md index e9470d9bb4727..a8e8f120ddd2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Bump `opentelemetry` from 1.36.0 to 1.39.0 ([#14457](https://github.com/opensearch-project/OpenSearch/pull/14457)) - Bump `azure-identity` from 1.11.4 to 1.13.0, Bump `msal4j` from 1.14.3 to 1.15.1, Bump `msal4j-persistence-extension` from 1.2.0 to 1.3.0 ([#14506](https://github.com/opensearch-project/OpenSearch/pull/14506)) - Bump `com.azure:azure-storage-common` from 12.21.2 to 12.25.1 ([#14517](https://github.com/opensearch-project/OpenSearch/pull/14517)) +- Bump `com.microsoft.azure:msal4j` from 1.15.1 to 1.16.0 ([#14610](https://github.com/opensearch-project/OpenSearch/pull/14610)) ### Changed - [Tiered Caching] Move query recomputation logic outside write lock ([#14187](https://github.com/opensearch-project/OpenSearch/pull/14187)) diff --git a/plugins/repository-azure/build.gradle b/plugins/repository-azure/build.gradle index f88d291a8eb4a..13b711019ff2a 100644 --- a/plugins/repository-azure/build.gradle +++ b/plugins/repository-azure/build.gradle @@ -61,7 +61,7 @@ dependencies { // Start of transitive dependencies for azure-identity api 'com.microsoft.azure:msal4j-persistence-extension:1.3.0' api "net.java.dev.jna:jna-platform:${versions.jna}" - api 'com.microsoft.azure:msal4j:1.15.1' + api 'com.microsoft.azure:msal4j:1.16.0' api 'com.nimbusds:oauth2-oidc-sdk:11.9.1' api 'com.nimbusds:nimbus-jose-jwt:9.40' api 'com.nimbusds:content-type:2.3' diff --git a/plugins/repository-azure/licenses/msal4j-1.15.1.jar.sha1 b/plugins/repository-azure/licenses/msal4j-1.15.1.jar.sha1 deleted file mode 100644 index 797f21d0d4995..0000000000000 --- a/plugins/repository-azure/licenses/msal4j-1.15.1.jar.sha1 +++ /dev/null @@ -1 +0,0 @@ -cd1daa94b81bd97153536b661c31295f99cbb8e7 \ No newline at end of file diff --git a/plugins/repository-azure/licenses/msal4j-1.16.0.jar.sha1 b/plugins/repository-azure/licenses/msal4j-1.16.0.jar.sha1 new file mode 100644 index 0000000000000..29fe5022a1570 --- /dev/null +++ b/plugins/repository-azure/licenses/msal4j-1.16.0.jar.sha1 @@ -0,0 +1 @@ +708a0a986ed091054f1c08866712e5b41aec6700 \ No newline at end of file From ec9d435279a36048e8cdd2b7fe53e2274ae36d16 Mon Sep 17 00:00:00 2001 From: Peter Alfonsi Date: Mon, 1 Jul 2024 15:54:39 -0700 Subject: [PATCH 07/20] [Bugfix] Fix ICacheKeySerializerTests flakiness (#14564) * Fix testInvalidInput flakiness Signed-off-by: Peter Alfonsi * Addressed andrross's comment Signed-off-by: Peter Alfonsi * rerun security check Signed-off-by: Peter Alfonsi --------- Signed-off-by: Peter Alfonsi Co-authored-by: Peter Alfonsi Signed-off-by: Kaushal Kumar --- .../common/cache/serializer/ICacheKeySerializerTests.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/server/src/test/java/org/opensearch/common/cache/serializer/ICacheKeySerializerTests.java b/server/src/test/java/org/opensearch/common/cache/serializer/ICacheKeySerializerTests.java index 7713fdf1d0adc..4b0fc3d2a7366 100644 --- a/server/src/test/java/org/opensearch/common/cache/serializer/ICacheKeySerializerTests.java +++ b/server/src/test/java/org/opensearch/common/cache/serializer/ICacheKeySerializerTests.java @@ -43,10 +43,9 @@ public void testInvalidInput() throws Exception { ICacheKeySerializer serializer = new ICacheKeySerializer<>(keySer); Random rand = Randomness.get(); - byte[] randomInput = new byte[1000]; - rand.nextBytes(randomInput); - - assertThrows(OpenSearchException.class, () -> serializer.deserialize(randomInput)); + // The first thing the serializer reads is a VInt for the number of dimensions. + // This is an invalid input for StreamInput.readVInt(), so we are guaranteed to have an exception + assertThrows(OpenSearchException.class, () -> serializer.deserialize(new byte[] { -1, -1, -1, -1, -1 })); } public void testDimNumbers() throws Exception { From bda1bfac12bf7338ef273879828930d543559fa6 Mon Sep 17 00:00:00 2001 From: Vatsal <36672090+imvtsl@users.noreply.github.com> Date: Tue, 2 Jul 2024 09:40:14 -0700 Subject: [PATCH 08/20] Correct typo in method name (#14621) Signed-off-by: vatsal Signed-off-by: Kaushal Kumar --- .../src/test/java/org/opensearch/index/get/GetResultTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/opensearch/index/get/GetResultTests.java b/server/src/test/java/org/opensearch/index/get/GetResultTests.java index 2001bb84454cd..ef8c48f2753c7 100644 --- a/server/src/test/java/org/opensearch/index/get/GetResultTests.java +++ b/server/src/test/java/org/opensearch/index/get/GetResultTests.java @@ -224,7 +224,7 @@ public void testEqualsAndHashcode() { ); } - public void testFomXContentEmbeddedFoundParsingException() throws IOException { + public void testFromXContentEmbeddedFoundParsingException() throws IOException { String json = "{\"_index\":\"foo\",\"_id\":\"bar\"}"; try ( XContentParser parser = JsonXContent.jsonXContent.createParser( From 6abdba2ea6dbf837d48f55c4d10a8e49167f6d28 Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Tue, 2 Jul 2024 13:18:39 -0700 Subject: [PATCH 09/20] Refactoring FilterPath.parse by using an iterative approach instead of recursion. (#14200) * Refactor FilterPath parse function (#12067) Signed-off-by: Robin Friedmann * Implement unit tests for FilterPathTests (#12067) Signed-off-by: Robin Friedmann * Write warn log if Filter is empty; Add comments (#12067) Signed-off-by: Robin Friedmann * Add changelog Signed-off-by: Siddhant Deshmukh * Remove unnecessary log statement Signed-off-by: Siddhant Deshmukh * Remove unused logger Signed-off-by: Siddhant Deshmukh * Spotless apply Signed-off-by: Siddhant Deshmukh * Remove incorrect changelog Signed-off-by: Siddhant Deshmukh --------- Signed-off-by: Siddhant Deshmukh Co-authored-by: Robin Friedmann Signed-off-by: Kaushal Kumar --- CHANGELOG.md | 1 + .../core/xcontent/filtering/FilterPath.java | 30 ++++++++----------- .../xcontent/filtering/FilterPathTests.java | 17 +++++++++++ 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a8e8f120ddd2f..14804bbd5974a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fix FuzzyQuery in keyword field will use IndexOrDocValuesQuery when both of index and doc_value are true ([#14378](https://github.com/opensearch-project/OpenSearch/pull/14378)) - Fix file cache initialization ([#14004](https://github.com/opensearch-project/OpenSearch/pull/14004)) - Handle NPE in GetResult if "found" field is missing ([#14552](https://github.com/opensearch-project/OpenSearch/pull/14552)) +- Refactoring FilterPath.parse by using an iterative approach ([#14200](https://github.com/opensearch-project/OpenSearch/pull/14200)) ### Security diff --git a/libs/core/src/main/java/org/opensearch/core/xcontent/filtering/FilterPath.java b/libs/core/src/main/java/org/opensearch/core/xcontent/filtering/FilterPath.java index 5389538a8c7dd..b8da9787165f8 100644 --- a/libs/core/src/main/java/org/opensearch/core/xcontent/filtering/FilterPath.java +++ b/libs/core/src/main/java/org/opensearch/core/xcontent/filtering/FilterPath.java @@ -46,7 +46,6 @@ public class FilterPath { static final FilterPath EMPTY = new FilterPath(); - private final String filter; private final String segment; private final FilterPath next; @@ -99,32 +98,29 @@ public static FilterPath[] compile(Set filters) { List paths = new ArrayList<>(); for (String filter : filters) { - if (filter != null) { + if (filter != null && !filter.isEmpty()) { filter = filter.trim(); if (filter.length() > 0) { - paths.add(parse(filter, filter)); + paths.add(parse(filter)); } } } return paths.toArray(new FilterPath[0]); } - private static FilterPath parse(final String filter, final String segment) { - int end = segment.length(); - - for (int i = 0; i < end;) { - char c = segment.charAt(i); + private static FilterPath parse(final String filter) { + // Split the filter into segments using a regex + // that avoids splitting escaped dots. + String[] segments = filter.split("(?= 0; i--) { + // Replace escaped dots with actual dots in the current segment. + String segment = segments[i].replaceAll("\\\\.", "."); + next = new FilterPath(filter, segment, next); } - return new FilterPath(filter, segment.replaceAll("\\\\.", "."), EMPTY); + + return next; } @Override diff --git a/libs/core/src/test/java/org/opensearch/core/xcontent/filtering/FilterPathTests.java b/libs/core/src/test/java/org/opensearch/core/xcontent/filtering/FilterPathTests.java index 0c5a17b70a956..d3191609f6119 100644 --- a/libs/core/src/test/java/org/opensearch/core/xcontent/filtering/FilterPathTests.java +++ b/libs/core/src/test/java/org/opensearch/core/xcontent/filtering/FilterPathTests.java @@ -35,6 +35,7 @@ import org.opensearch.common.util.set.Sets; import org.opensearch.test.OpenSearchTestCase; +import java.util.HashSet; import java.util.Set; import static java.util.Collections.singleton; @@ -369,4 +370,20 @@ public void testMultipleFilterPaths() { assertThat(filterPath.getSegment(), is(emptyString())); assertSame(filterPath, FilterPath.EMPTY); } + + public void testCompileWithEmptyString() { + Set filters = new HashSet<>(); + filters.add(""); + FilterPath[] filterPaths = FilterPath.compile(filters); + assertNotNull(filterPaths); + assertEquals(0, filterPaths.length); + } + + public void testCompileWithNull() { + Set filters = new HashSet<>(); + filters.add(null); + FilterPath[] filterPaths = FilterPath.compile(filters); + assertNotNull(filterPaths); + assertEquals(0, filterPaths.length); + } } From 38c9f82144575b70c0537553a76e5e929a00e4e0 Mon Sep 17 00:00:00 2001 From: rishavz_sagar Date: Wed, 3 Jul 2024 15:25:51 +0530 Subject: [PATCH 10/20] Removing String format in RemoteStoreMigrationAllocationDecider to optimise performance(#14612) Signed-off-by: RS146BIJAY Signed-off-by: Kaushal Kumar --- ...RemoteStoreMigrationAllocationDecider.java | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/allocation/decider/RemoteStoreMigrationAllocationDecider.java b/server/src/main/java/org/opensearch/cluster/routing/allocation/decider/RemoteStoreMigrationAllocationDecider.java index 4fc5fff805663..67fe4ea1dcb1b 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/allocation/decider/RemoteStoreMigrationAllocationDecider.java +++ b/server/src/main/java/org/opensearch/cluster/routing/allocation/decider/RemoteStoreMigrationAllocationDecider.java @@ -44,8 +44,6 @@ import org.opensearch.node.remotestore.RemoteStoreNodeService.CompatibilityMode; import org.opensearch.node.remotestore.RemoteStoreNodeService.Direction; -import java.util.Locale; - /** * A new allocation decider for migration of document replication clusters to remote store backed clusters: * - For STRICT compatibility mode, the decision is always YES @@ -101,7 +99,7 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, Routing if (migrationDirection.equals(Direction.NONE)) { // remote backed indices on docrep nodes and non remote backed indices on remote nodes are not allowed boolean isNoDecision = remoteSettingsBackedIndex ^ targetNode.isRemoteStoreNode(); - String reason = String.format(Locale.ROOT, " for %sremote store backed index", remoteSettingsBackedIndex ? "" : "non "); + String reason = " for " + (remoteSettingsBackedIndex ? "" : "non ") + "remote store backed index"; return allocation.decision( isNoDecision ? Decision.NO : Decision.YES, NAME, @@ -114,11 +112,9 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, Routing // check for remote store backed indices if (remoteSettingsBackedIndex && targetNode.isRemoteStoreNode() == false) { // allocations and relocations must be to a remote node - String reason = String.format( - Locale.ROOT, - " because a remote store backed index's shard copy can only be %s to a remote node", - ((shardRouting.assignedToNode() == false) ? "allocated" : "relocated") - ); + String reason = new StringBuilder(" because a remote store backed index's shard copy can only be ").append( + (shardRouting.assignedToNode() == false) ? "allocated" : "relocated" + ).append(" to a remote node").toString(); return allocation.decision(Decision.NO, NAME, getDecisionDetails(false, shardRouting, targetNode, reason)); } @@ -168,16 +164,18 @@ private Decision replicaShardDecision(ShardRouting replicaShardRouting, Discover // get detailed reason for the decision private String getDecisionDetails(boolean isYes, ShardRouting shardRouting, DiscoveryNode targetNode, String reason) { - return String.format( - Locale.ROOT, - "[%s migration_direction]: %s shard copy %s be %s to a %s node%s", - migrationDirection.direction, - (shardRouting.primary() ? "primary" : "replica"), - (isYes ? "can" : "can not"), - ((shardRouting.assignedToNode() == false) ? "allocated" : "relocated"), - (targetNode.isRemoteStoreNode() ? "remote" : "non-remote"), - reason - ); + return new StringBuilder("[").append(migrationDirection.direction) + .append(" migration_direction]: ") + .append(shardRouting.primary() ? "primary" : "replica") + .append(" shard copy ") + .append(isYes ? "can" : "can not") + .append(" be ") + .append((shardRouting.assignedToNode() == false) ? "allocated" : "relocated") + .append(" to a ") + .append(targetNode.isRemoteStoreNode() ? "remote" : "non-remote") + .append(" node") + .append(reason) + .toString(); } } From a5b991edf9542d78e0cce3ba441b58544df2fde5 Mon Sep 17 00:00:00 2001 From: Sooraj Sinha <81695996+soosinha@users.noreply.github.com> Date: Wed, 3 Jul 2024 22:33:54 +0530 Subject: [PATCH 11/20] Clear templates before Adding; Use NamedWriteableAwareStreamInput for RemoteCustomMetadata; Correct the check for deciding upload of HashesOfConsistentSettings (#14513) * Clear templates before Adding; Use NamedWriteableAwareStreamInput for RemoteCustomMetadata * Correct the check for deciding upload of hashes of consistent settings Signed-off-by: Sooraj Sinha Signed-off-by: Kaushal Kumar --- .../opensearch/cluster/metadata/Metadata.java | 1 + .../remote/RemoteClusterStateService.java | 5 +- .../remote/model/RemoteCustomMetadata.java | 5 +- .../cluster/metadata/MetadataTests.java | 27 ++++ .../RemoteClusterStateServiceTests.java | 125 +++++++++++++++++- .../model/RemoteCustomMetadataTests.java | 87 +++++++++++- 6 files changed, 239 insertions(+), 11 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java b/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java index a0ef8de07fbf2..e3f63b1c27b83 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java @@ -1287,6 +1287,7 @@ public Builder templates(Map templates) { } public Builder templates(TemplatesMetadata templatesMetadata) { + this.templates.clear(); this.templates.putAll(templatesMetadata.getTemplates()); return this; } 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 ef14adeb42b68..74abe9cd257b4 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -356,7 +356,7 @@ public RemoteClusterStateManifestInfo writeIncrementalMetadata( && clusterState.getNodes().delta(previousClusterState.getNodes()).hasChanges(); final boolean updateClusterBlocks = isPublicationEnabled && !clusterState.blocks().equals(previousClusterState.blocks()); final boolean updateHashesOfConsistentSettings = isPublicationEnabled - || Metadata.isHashesOfConsistentSettingsEqual(previousClusterState.metadata(), clusterState.metadata()) == false; + && Metadata.isHashesOfConsistentSettingsEqual(previousClusterState.metadata(), clusterState.metadata()) == false; uploadedMetadataResults = writeMetadataInParallel( clusterState, @@ -476,7 +476,8 @@ public RemoteClusterStateManifestInfo writeIncrementalMetadata( return manifestDetails; } - private UploadedMetadataResults writeMetadataInParallel( + // package private for testing + UploadedMetadataResults writeMetadataInParallel( ClusterState clusterState, List indexToUpload, Map prevIndexMetadataByName, diff --git a/server/src/main/java/org/opensearch/gateway/remote/model/RemoteCustomMetadata.java b/server/src/main/java/org/opensearch/gateway/remote/model/RemoteCustomMetadata.java index 4c7069ee8be9e..ec5dfbec820d4 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/model/RemoteCustomMetadata.java +++ b/server/src/main/java/org/opensearch/gateway/remote/model/RemoteCustomMetadata.java @@ -12,6 +12,7 @@ import org.opensearch.common.io.Streams; import org.opensearch.common.remote.AbstractRemoteWritableBlobEntity; import org.opensearch.common.remote.BlobPathParameters; +import org.opensearch.core.common.io.stream.NamedWriteableAwareStreamInput; import org.opensearch.core.common.io.stream.NamedWriteableRegistry; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.compress.Compressor; @@ -122,6 +123,8 @@ public UploadedMetadata getUploadedMetadata() { public static Custom readFrom(StreamInput streamInput, NamedWriteableRegistry namedWriteableRegistry, String customType) throws IOException { - return namedWriteableRegistry.getReader(Custom.class, customType).read(streamInput); + try (StreamInput in = new NamedWriteableAwareStreamInput(streamInput, namedWriteableRegistry)) { + return namedWriteableRegistry.getReader(Custom.class, customType).read(in); + } } } diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataTests.java index 618fcb923bc60..a434a713f330b 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/MetadataTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/MetadataTests.java @@ -1482,6 +1482,33 @@ public void testIsSegmentReplicationDisabled() { assertFalse(metadata.isSegmentReplicationEnabled(indexName)); } + public void testTemplatesMetadata() { + TemplatesMetadata templatesMetadata1 = TemplatesMetadata.builder() + .put( + IndexTemplateMetadata.builder("template_1") + .patterns(Arrays.asList("bar-*", "foo-*")) + .settings(Settings.builder().put("random_index_setting_" + randomAlphaOfLength(3), randomAlphaOfLength(5)).build()) + .build() + ) + .build(); + Metadata metadata1 = Metadata.builder().templates(templatesMetadata1).build(); + assertThat(metadata1.templates(), is(templatesMetadata1.getTemplates())); + + TemplatesMetadata templatesMetadata2 = TemplatesMetadata.builder() + .put( + IndexTemplateMetadata.builder("template_2") + .patterns(Arrays.asList("bar-*", "foo-*")) + .settings(Settings.builder().put("random_index_setting_" + randomAlphaOfLength(3), randomAlphaOfLength(5)).build()) + .build() + ) + .build(); + + Metadata metadata2 = Metadata.builder(metadata1).templates(templatesMetadata2).build(); + + assertThat(metadata2.templates(), is(templatesMetadata2.getTemplates())); + + } + public static Metadata randomMetadata() { Metadata.Builder md = Metadata.builder() .put(buildIndexMetadata("index", "alias", randomBoolean() ? null : randomBoolean()).build(), randomBoolean()) 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 c8fd982fec1e1..d983a4d8c4027 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -20,6 +20,7 @@ import org.opensearch.cluster.metadata.IndexTemplateMetadata; import org.opensearch.cluster.metadata.Metadata; import org.opensearch.cluster.metadata.TemplatesMetadata; +import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.node.DiscoveryNodes; import org.opensearch.cluster.routing.RoutingTable; import org.opensearch.cluster.routing.remote.InternalRemoteRoutingTableService; @@ -92,6 +93,7 @@ import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatchers; +import org.mockito.Mockito; import static java.util.stream.Collectors.toList; import static org.opensearch.common.util.FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL; @@ -111,6 +113,7 @@ import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_REPOSITORY_SETTINGS_ATTRIBUTE_KEY_PREFIX; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_REPOSITORY_TYPE_ATTRIBUTE_KEY_FORMAT; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY; +import static org.hamcrest.Matchers.anEmptyMap; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; @@ -118,6 +121,7 @@ import static org.hamcrest.Matchers.nullValue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; @@ -518,11 +522,13 @@ public void testWriteIncrementalMetadataSuccess() throws IOException { final ClusterMetadataManifest previousManifest = ClusterMetadataManifest.builder().indices(Collections.emptyList()).build(); remoteClusterStateService.start(); - final ClusterMetadataManifest manifest = remoteClusterStateService.writeIncrementalMetadata( + final RemoteClusterStateService rcssSpy = Mockito.spy(remoteClusterStateService); + final RemoteClusterStateManifestInfo manifestInfo = rcssSpy.writeIncrementalMetadata( previousClusterState, clusterState, previousManifest - ).getClusterMetadataManifest(); + ); + final ClusterMetadataManifest manifest = manifestInfo.getClusterMetadataManifest(); final UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "index-uuid", "metadata-filename__2"); final List indices = List.of(uploadedIndexMetadata); @@ -535,6 +541,24 @@ public void testWriteIncrementalMetadataSuccess() throws IOException { .previousClusterUUID("prev-cluster-uuid") .build(); + Mockito.verify(rcssSpy) + .writeMetadataInParallel( + eq(clusterState), + eq(new ArrayList(clusterState.metadata().indices().values())), + eq(Collections.singletonMap(indices.get(0).getIndexName(), null)), + eq(clusterState.metadata().customs()), + eq(true), + eq(true), + eq(true), + eq(false), + eq(false), + eq(false), + eq(Collections.emptyMap()), + eq(false), + eq(Collections.emptyList()) + ); + + assertThat(manifestInfo.getManifestFileName(), notNullValue()); assertThat(manifest.getIndices().size(), is(1)); assertThat(manifest.getIndices().get(0).getIndexName(), is(uploadedIndexMetadata.getIndexName())); assertThat(manifest.getIndices().get(0).getIndexUUID(), is(uploadedIndexMetadata.getIndexUUID())); @@ -543,6 +567,95 @@ public void testWriteIncrementalMetadataSuccess() throws IOException { assertThat(manifest.getStateVersion(), is(expectedManifest.getStateVersion())); assertThat(manifest.getClusterUUID(), is(expectedManifest.getClusterUUID())); assertThat(manifest.getStateUUID(), is(expectedManifest.getStateUUID())); + assertThat(manifest.getHashesOfConsistentSettings(), nullValue()); + assertThat(manifest.getDiscoveryNodesMetadata(), nullValue()); + assertThat(manifest.getClusterBlocksMetadata(), nullValue()); + assertThat(manifest.getClusterStateCustomMap(), anEmptyMap()); + assertThat(manifest.getTransientSettingsMetadata(), nullValue()); + assertThat(manifest.getTemplatesMetadata(), notNullValue()); + assertThat(manifest.getCoordinationMetadata(), notNullValue()); + assertThat(manifest.getCustomMetadataMap().size(), is(2)); + assertThat(manifest.getIndicesRouting().size(), is(0)); + } + + public void testWriteIncrementalMetadataSuccessWhenPublicationEnabled() throws IOException { + publicationEnabled = true; + Settings nodeSettings = Settings.builder().put(REMOTE_PUBLICATION_EXPERIMENTAL, publicationEnabled).build(); + FeatureFlags.initializeFeatureFlags(nodeSettings); + remoteClusterStateService = new RemoteClusterStateService( + "test-node-id", + repositoriesServiceSupplier, + settings, + clusterService, + () -> 0L, + threadPool, + List.of(new RemoteIndexPathUploader(threadPool, settings, repositoriesServiceSupplier, clusterSettings)), + writableRegistry() + ); + 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(); + + remoteClusterStateService.start(); + final RemoteClusterStateService rcssSpy = Mockito.spy(remoteClusterStateService); + final RemoteClusterStateManifestInfo manifestInfo = rcssSpy.writeIncrementalMetadata( + previousClusterState, + clusterState, + previousManifest + ); + final ClusterMetadataManifest manifest = manifestInfo.getClusterMetadataManifest(); + final UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "index-uuid", "metadata-filename__2"); + final List indices = List.of(uploadedIndexMetadata); + + final ClusterMetadataManifest expectedManifest = ClusterMetadataManifest.builder() + .indices(indices) + .clusterTerm(1L) + .stateVersion(1L) + .stateUUID("state-uuid") + .clusterUUID("cluster-uuid") + .previousClusterUUID("prev-cluster-uuid") + .build(); + + Mockito.verify(rcssSpy) + .writeMetadataInParallel( + eq(clusterState), + eq(new ArrayList(clusterState.metadata().indices().values())), + eq(Collections.singletonMap(indices.get(0).getIndexName(), null)), + eq(clusterState.metadata().customs()), + eq(true), + eq(true), + eq(true), + eq(true), + eq(false), + eq(false), + eq(Collections.emptyMap()), + eq(true), + Mockito.anyList() + ); + + assertThat(manifestInfo.getManifestFileName(), notNullValue()); + 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.getHashesOfConsistentSettings(), notNullValue()); + assertThat(manifest.getDiscoveryNodesMetadata(), notNullValue()); + assertThat(manifest.getClusterBlocksMetadata(), nullValue()); + assertThat(manifest.getClusterStateCustomMap(), anEmptyMap()); + assertThat(manifest.getTransientSettingsMetadata(), nullValue()); + assertThat(manifest.getTemplatesMetadata(), notNullValue()); + assertThat(manifest.getCoordinationMetadata(), notNullValue()); + assertThat(manifest.getCustomMetadataMap().size(), is(2)); + assertThat(manifest.getIndicesRouting().size(), is(1)); } /* @@ -2012,7 +2125,9 @@ static ClusterState.Builder generateClusterStateWithOneIndex() { .build(); final CoordinationMetadata coordinationMetadata = CoordinationMetadata.builder().term(1L).build(); final Settings settings = Settings.builder().put("mock-settings", true).build(); - final TemplatesMetadata templatesMetadata = TemplatesMetadata.EMPTY_METADATA; + final TemplatesMetadata templatesMetadata = TemplatesMetadata.builder() + .put(IndexTemplateMetadata.builder("template1").settings(idxSettings).patterns(List.of("test*")).build()) + .build(); final CustomMetadata1 customMetadata1 = new CustomMetadata1("custom-metadata-1"); return ClusterState.builder(ClusterName.DEFAULT) .version(1L) @@ -2025,6 +2140,7 @@ static ClusterState.Builder generateClusterStateWithOneIndex() { .coordinationMetadata(coordinationMetadata) .persistentSettings(settings) .templates(templatesMetadata) + .hashesOfConsistentSettings(Map.of("key1", "value1", "key2", "value2")) .putCustom(customMetadata1.getWriteableName(), customMetadata1) .build() ) @@ -2032,7 +2148,8 @@ static ClusterState.Builder generateClusterStateWithOneIndex() { } static DiscoveryNodes nodesWithLocalNodeClusterManager() { - return DiscoveryNodes.builder().clusterManagerNodeId("cluster-manager-id").localNodeId("cluster-manager-id").build(); + final DiscoveryNode localNode = new DiscoveryNode("cluster-manager-id", buildNewFakeTransportAddress(), Version.CURRENT); + return DiscoveryNodes.builder().clusterManagerNodeId("cluster-manager-id").localNodeId("cluster-manager-id").add(localNode).build(); } private static class CustomMetadata1 extends TestCustomMetadata { diff --git a/server/src/test/java/org/opensearch/gateway/remote/model/RemoteCustomMetadataTests.java b/server/src/test/java/org/opensearch/gateway/remote/model/RemoteCustomMetadataTests.java index 1e28817be79f2..60cceb205f43d 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/model/RemoteCustomMetadataTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/model/RemoteCustomMetadataTests.java @@ -8,6 +8,8 @@ package org.opensearch.gateway.remote.model; +import org.opensearch.Version; +import org.opensearch.cluster.ClusterModule; import org.opensearch.cluster.metadata.IndexGraveyard; import org.opensearch.cluster.metadata.Metadata.Custom; import org.opensearch.common.blobstore.BlobPath; @@ -16,13 +18,20 @@ import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; import org.opensearch.core.common.io.stream.NamedWriteableRegistry; +import org.opensearch.core.common.io.stream.NamedWriteableRegistry.Entry; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; import org.opensearch.core.compress.Compressor; import org.opensearch.core.compress.NoneCompressor; import org.opensearch.core.index.Index; +import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.gateway.remote.ClusterMetadataManifest.UploadedMetadata; import org.opensearch.gateway.remote.RemoteClusterStateUtils; import org.opensearch.index.remote.RemoteStoreUtils; import org.opensearch.index.translog.transfer.BlobStoreTransferService; +import org.opensearch.persistent.PersistentTaskParams; +import org.opensearch.persistent.PersistentTasksCustomMetadata; +import org.opensearch.persistent.PersistentTasksCustomMetadata.Assignment; import org.opensearch.repositories.blobstore.BlobStoreRepository; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.threadpool.TestThreadPool; @@ -33,6 +42,7 @@ import java.io.IOException; import java.io.InputStream; import java.util.List; +import java.util.Objects; import static org.opensearch.gateway.remote.RemoteClusterStateUtils.GLOBAL_METADATA_CURRENT_CODEC_VERSION; import static org.opensearch.gateway.remote.model.RemoteCustomMetadata.CUSTOM_DELIMITER; @@ -216,24 +226,93 @@ public void testGetUploadedMetadata() throws IOException { public void testSerDe() throws IOException { Custom customMetadata = getCustomMetadata(); + verifySerDe(customMetadata, IndexGraveyard.TYPE); + } + + public void testSerDeForPersistentTasks() throws IOException { + Custom customMetadata = getPersistentTasksMetadata(); + verifySerDe(customMetadata, PersistentTasksCustomMetadata.TYPE); + } + + private void verifySerDe(Custom objectToUpload, String objectType) throws IOException { RemoteCustomMetadata remoteObjectForUpload = new RemoteCustomMetadata( - customMetadata, - IndexGraveyard.TYPE, + objectToUpload, + objectType, METADATA_VERSION, clusterUUID, compressor, - namedWriteableRegistry + customWritableRegistry() ); try (InputStream inputStream = remoteObjectForUpload.serialize()) { remoteObjectForUpload.setFullBlobName(BlobPath.cleanPath()); assertThat(inputStream.available(), greaterThan(0)); Custom readCustomMetadata = remoteObjectForUpload.deserialize(inputStream); - assertThat(readCustomMetadata, is(customMetadata)); + assertThat(readCustomMetadata, is(objectToUpload)); } } + private NamedWriteableRegistry customWritableRegistry() { + List entries = ClusterModule.getNamedWriteables(); + entries.add(new Entry(PersistentTaskParams.class, TestPersistentTaskParams.PARAM_NAME, TestPersistentTaskParams::new)); + return new NamedWriteableRegistry(entries); + } + public static Custom getCustomMetadata() { return IndexGraveyard.builder().addTombstone(new Index("test-index", "3q2423")).build(); } + private static Custom getPersistentTasksMetadata() { + return PersistentTasksCustomMetadata.builder() + .addTask("_task_1", "testTaskName", new TestPersistentTaskParams("task param data"), new Assignment(null, "_reason")) + .build(); + } + + public static class TestPersistentTaskParams implements PersistentTaskParams { + + private static final String PARAM_NAME = "testTaskName"; + + private final String data; + + public TestPersistentTaskParams(String data) { + this.data = data; + } + + public TestPersistentTaskParams(StreamInput in) throws IOException { + this(in.readString()); + } + + @Override + public String getWriteableName() { + return PARAM_NAME; + } + + @Override + public Version getMinimalSupportedVersion() { + return Version.V_2_13_0; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeString(data); + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + return builder.startObject().field("data_field", data); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + TestPersistentTaskParams that = (TestPersistentTaskParams) o; + return Objects.equals(data, that.data); + } + + @Override + public int hashCode() { + return Objects.hash(data); + } + } + } From b1b5f87b9f7830f933ddd73bf1159474710e4625 Mon Sep 17 00:00:00 2001 From: Kaushal Kumar Date: Wed, 3 Jul 2024 12:12:57 -0700 Subject: [PATCH 12/20] add changelog Signed-off-by: Kaushal Kumar --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14804bbd5974a..8400bdcf3848e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add batching supported processor base type AbstractBatchingProcessor ([#14554](https://github.com/opensearch-project/OpenSearch/pull/14554)) - Fix race condition while parsing derived fields from search definition ([14445](https://github.com/opensearch-project/OpenSearch/pull/14445)) - Add allowlist setting for ingest-common and search-pipeline-common processors ([#14439](https://github.com/opensearch-project/OpenSearch/issues/14439)) +- [Workload Management] add queryGroupId header propagator across requests and nodes ### Dependencies - Bump `org.gradle.test-retry` from 1.5.8 to 1.5.9 ([#13442](https://github.com/opensearch-project/OpenSearch/pull/13442)) From 9a1c7b95a37a8218eda75aec7171b60f183d75f0 Mon Sep 17 00:00:00 2001 From: Kaushal Kumar Date: Wed, 3 Jul 2024 12:14:21 -0700 Subject: [PATCH 13/20] add PR link changelog Signed-off-by: Kaushal Kumar --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8400bdcf3848e..892ab8ac49e68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add batching supported processor base type AbstractBatchingProcessor ([#14554](https://github.com/opensearch-project/OpenSearch/pull/14554)) - Fix race condition while parsing derived fields from search definition ([14445](https://github.com/opensearch-project/OpenSearch/pull/14445)) - Add allowlist setting for ingest-common and search-pipeline-common processors ([#14439](https://github.com/opensearch-project/OpenSearch/issues/14439)) -- [Workload Management] add queryGroupId header propagator across requests and nodes +- [Workload Management] add queryGroupId header propagator across requests and nodes ([#14614](https://github.com/opensearch-project/OpenSearch/pull/14614)) ### Dependencies - Bump `org.gradle.test-retry` from 1.5.8 to 1.5.9 ([#13442](https://github.com/opensearch-project/OpenSearch/pull/13442)) From cbcbb0ea54f459ce1220af13a64144ca2ec1934d Mon Sep 17 00:00:00 2001 From: rishavz_sagar Date: Thu, 4 Jul 2024 11:50:47 +0530 Subject: [PATCH 14/20] Improve reroute performance by optimising List.removeAll in LocalShardsBalancer to filter remote search shard from relocation decision (#14613) Signed-off-by: RS146BIJAY Signed-off-by: Kaushal Kumar --- .../allocator/LocalShardsBalancer.java | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/allocation/allocator/LocalShardsBalancer.java b/server/src/main/java/org/opensearch/cluster/routing/allocation/allocator/LocalShardsBalancer.java index ec25d041bda43..6978c988fd648 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/allocation/allocator/LocalShardsBalancer.java +++ b/server/src/main/java/org/opensearch/cluster/routing/allocation/allocator/LocalShardsBalancer.java @@ -32,7 +32,6 @@ import org.opensearch.gateway.PriorityComparator; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; @@ -41,7 +40,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.stream.Collectors; import java.util.stream.Stream; import java.util.stream.StreamSupport; @@ -779,15 +777,16 @@ void allocateUnassigned() { * if we allocate for instance (0, R, IDX1) we move the second replica to the secondary array and proceed with * the next replica. If we could not find a node to allocate (0,R,IDX1) we move all it's replicas to ignoreUnassigned. */ - ShardRouting[] unassignedShards = unassigned.drain(); - List allUnassignedShards = Arrays.stream(unassignedShards).collect(Collectors.toList()); - List localUnassignedShards = allUnassignedShards.stream() - .filter(shard -> RoutingPool.LOCAL_ONLY.equals(RoutingPool.getShardPool(shard, allocation))) - .collect(Collectors.toList()); - allUnassignedShards.removeAll(localUnassignedShards); - allUnassignedShards.forEach(shard -> routingNodes.unassigned().add(shard)); - unassignedShards = localUnassignedShards.toArray(new ShardRouting[0]); - ShardRouting[] primary = unassignedShards; + List primaryList = new ArrayList<>(); + for (ShardRouting shard : unassigned.drain()) { + if (RoutingPool.LOCAL_ONLY.equals(RoutingPool.getShardPool(shard, allocation))) { + primaryList.add(shard); + } else { + routingNodes.unassigned().add(shard); + } + } + + ShardRouting[] primary = primaryList.toArray(new ShardRouting[0]); ShardRouting[] secondary = new ShardRouting[primary.length]; int secondaryLength = 0; int primaryLength = primary.length; From 1abe6b1e1bbab5b8f27ce90b4c38da16d2ce7f81 Mon Sep 17 00:00:00 2001 From: Sachin Kale Date: Fri, 5 Jul 2024 09:16:37 +0530 Subject: [PATCH 15/20] Fix assertion failure while deleting remote backed index (#14601) Signed-off-by: Sachin Kale Signed-off-by: Kaushal Kumar --- .../RemoteMigrationIndexMetadataUpdateIT.java | 2 -- .../opensearch/remotestore/RemoteStoreIT.java | 16 +++++++++++++++- .../java/org/opensearch/index/IndexService.java | 17 +++++++++++++++-- 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteMigrationIndexMetadataUpdateIT.java b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteMigrationIndexMetadataUpdateIT.java index 6885d37c4aab0..216c104dfecc1 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteMigrationIndexMetadataUpdateIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteMigrationIndexMetadataUpdateIT.java @@ -275,7 +275,6 @@ initalMetadataVersion < internalCluster().client() * After shard relocation completes, shuts down the docrep nodes and asserts remote * index settings are applied even when the index is in YELLOW state */ - @AwaitsFix(bugUrl = "https://github.com/opensearch-project/OpenSearch/issues/13737") public void testIndexSettingsUpdatedEvenForMisconfiguredReplicas() throws Exception { internalCluster().startClusterManagerOnlyNode(); @@ -332,7 +331,6 @@ public void testIndexSettingsUpdatedEvenForMisconfiguredReplicas() throws Except * After shard relocation completes, restarts the docrep node holding extra replica shard copy * and asserts remote index settings are applied as soon as the docrep replica copy is unassigned */ - @AwaitsFix(bugUrl = "https://github.com/opensearch-project/OpenSearch/issues/13871") public void testIndexSettingsUpdatedWhenDocrepNodeIsRestarted() throws Exception { internalCluster().startClusterManagerOnlyNode(); diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreIT.java index 96d6338e5913b..194dce5f4a57a 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreIT.java @@ -65,7 +65,6 @@ import static org.opensearch.index.remote.RemoteStoreEnums.DataType.METADATA; import static org.opensearch.index.shard.IndexShardTestCase.getTranslog; import static org.opensearch.indices.RemoteStoreSettings.CLUSTER_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING; -import static org.opensearch.test.OpenSearchTestCase.getShardLevelBlobPath; import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked; import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertHitCount; import static org.hamcrest.Matchers.comparesEqualTo; @@ -133,6 +132,21 @@ private void testPeerRecovery(int numberOfIterations, boolean invokeFlush) throw ); } + public void testRemoteStoreIndexCreationAndDeletionWithReferencedStore() throws InterruptedException, ExecutionException { + String dataNode = internalCluster().startNodes(1).get(0); + createIndex(INDEX_NAME, remoteStoreIndexSettings(0)); + ensureYellowAndNoInitializingShards(INDEX_NAME); + ensureGreen(INDEX_NAME); + + IndexShard indexShard = getIndexShard(dataNode, INDEX_NAME); + + // Simulating a condition where store is already in use by increasing ref count, this helps in testing index + // deletion when refresh is in-progress. + indexShard.store().incRef(); + assertAcked(client().admin().indices().prepareDelete(INDEX_NAME)); + indexShard.store().decRef(); + } + public void testPeerRecoveryWithRemoteStoreAndRemoteTranslogNoDataFlush() throws Exception { testPeerRecovery(1, true); } diff --git a/server/src/main/java/org/opensearch/index/IndexService.java b/server/src/main/java/org/opensearch/index/IndexService.java index 1c0db0095bb98..12b02d3dbd6fa 100644 --- a/server/src/main/java/org/opensearch/index/IndexService.java +++ b/server/src/main/java/org/opensearch/index/IndexService.java @@ -602,7 +602,21 @@ public synchronized IndexShard createShard( this.indexSettings.getRemoteStorePathStrategy() ); } - remoteStore = new Store(shardId, this.indexSettings, remoteDirectory, lock, Store.OnClose.EMPTY, path); + // When an instance of Store is created, a shardlock is created which is released on closing the instance of store. + // Currently, we create 2 instances of store for remote store backed indices: store and remoteStore. + // As there can be only one shardlock acquired for a given shard, the lock is shared between store and remoteStore. + // This creates an issue when we are deleting the index as it results in closing both store and remoteStore. + // Sample test failure: https://github.com/opensearch-project/OpenSearch/issues/13871 + // The following method provides ShardLock that is not maintained by NodeEnvironment. + // As part of https://github.com/opensearch-project/OpenSearch/issues/13075, we want to move away from keeping 2 + // store instances. + ShardLock remoteStoreLock = new ShardLock(shardId) { + @Override + protected void closeInternal() { + // Do nothing for shard lock on remote store + } + }; + remoteStore = new Store(shardId, this.indexSettings, remoteDirectory, remoteStoreLock, Store.OnClose.EMPTY, path); } else { // Disallow shards with remote store based settings to be created on non-remote store enabled nodes // Even though we have `RemoteStoreMigrationAllocationDecider` in place to prevent something like this from happening at the @@ -625,7 +639,6 @@ public synchronized IndexShard createShard( } else { directory = directoryFactory.newDirectory(this.indexSettings, path); } - store = new Store( shardId, this.indexSettings, From 1d20abe7c0221a894c15e519140bf92f6492382a Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 5 Jul 2024 11:59:40 -0400 Subject: [PATCH 16/20] Allow system index warning in OpenSearchRestTestCase.refreshAllIndices (#14635) * Allow system index warning Signed-off-by: Craig Perkins * Add to CHANGELOG Signed-off-by: Craig Perkins * Address code review comments Signed-off-by: Craig Perkins --------- Signed-off-by: Craig Perkins Signed-off-by: Kaushal Kumar --- CHANGELOG.md | 1 + .../opensearch/test/rest/OpenSearchRestTestCase.java | 12 ++++++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 892ab8ac49e68..2677047abdbf7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Make the class CommunityIdProcessor final ([#14448](https://github.com/opensearch-project/OpenSearch/pull/14448)) - Allow @InternalApi annotation on classes not meant to be constructed outside of the OpenSearch core ([#14575](https://github.com/opensearch-project/OpenSearch/pull/14575)) - Add @InternalApi annotation to japicmp exclusions ([#14597](https://github.com/opensearch-project/OpenSearch/pull/14597)) +- Allow system index warning in OpenSearchRestTestCase.refreshAllIndices ([#14635](https://github.com/opensearch-project/OpenSearch/pull/14635)) ### Deprecated diff --git a/test/framework/src/main/java/org/opensearch/test/rest/OpenSearchRestTestCase.java b/test/framework/src/main/java/org/opensearch/test/rest/OpenSearchRestTestCase.java index b7c31685bafa6..8c612d258f183 100644 --- a/test/framework/src/main/java/org/opensearch/test/rest/OpenSearchRestTestCase.java +++ b/test/framework/src/main/java/org/opensearch/test/rest/OpenSearchRestTestCase.java @@ -708,11 +708,15 @@ protected void refreshAllIndices() throws IOException { requestOptions.setWarningsHandler(warnings -> { if (warnings.isEmpty()) { return false; - } else if (warnings.size() > 1) { - return true; - } else { - return warnings.get(0).startsWith("this request accesses system indices:") == false; } + boolean allSystemIndexWarnings = true; + for (String warning : warnings) { + if (!warning.startsWith("this request accesses system indices:")) { + allSystemIndexWarnings = false; + break; + } + } + return !allSystemIndexWarnings; }); refreshRequest.setOptions(requestOptions); client().performRequest(refreshRequest); From fe1983e776d9fc7e48dccbfab9d5b7265362cce3 Mon Sep 17 00:00:00 2001 From: Bharathwaj G Date: Mon, 8 Jul 2024 19:31:58 +0530 Subject: [PATCH 17/20] Star tree codec changes (#14514) --------- Signed-off-by: Bharathwaj G Signed-off-by: Kaushal Kumar --- .../opensearch/index/codec/CodecService.java | 16 ++- .../codec/composite/Composite99Codec.java | 57 +++++++++ .../composite/Composite99DocValuesFormat.java | 64 ++++++++++ .../composite/Composite99DocValuesReader.java | 89 ++++++++++++++ .../composite/Composite99DocValuesWriter.java | 114 ++++++++++++++++++ .../composite/CompositeCodecFactory.java | 42 +++++++ .../codec/composite/CompositeIndexReader.java | 34 ++++++ .../codec/composite/CompositeIndexValues.java | 21 ++++ .../datacube/startree/StarTreeValues.java | 35 ++++++ .../datacube/startree/package-info.java | 12 ++ .../index/codec/composite/package-info.java | 12 ++ .../mapper/CompositeMappedFieldType.java | 3 + .../index/mapper/MapperService.java | 9 ++ .../services/org.apache.lucene.codecs.Codec | 1 + .../opensearch/index/codec/CodecTests.java | 47 ++++++++ .../StarTreeDocValuesFormatTests.java | 110 +++++++++++++++++ 16 files changed, 662 insertions(+), 4 deletions(-) create mode 100644 server/src/main/java/org/opensearch/index/codec/composite/Composite99Codec.java create mode 100644 server/src/main/java/org/opensearch/index/codec/composite/Composite99DocValuesFormat.java create mode 100644 server/src/main/java/org/opensearch/index/codec/composite/Composite99DocValuesReader.java create mode 100644 server/src/main/java/org/opensearch/index/codec/composite/Composite99DocValuesWriter.java create mode 100644 server/src/main/java/org/opensearch/index/codec/composite/CompositeCodecFactory.java create mode 100644 server/src/main/java/org/opensearch/index/codec/composite/CompositeIndexReader.java create mode 100644 server/src/main/java/org/opensearch/index/codec/composite/CompositeIndexValues.java create mode 100644 server/src/main/java/org/opensearch/index/codec/composite/datacube/startree/StarTreeValues.java create mode 100644 server/src/main/java/org/opensearch/index/codec/composite/datacube/startree/package-info.java create mode 100644 server/src/main/java/org/opensearch/index/codec/composite/package-info.java create mode 100644 server/src/main/resources/META-INF/services/org.apache.lucene.codecs.Codec create mode 100644 server/src/test/java/org/opensearch/index/codec/composite/datacube/startree/StarTreeDocValuesFormatTests.java diff --git a/server/src/main/java/org/opensearch/index/codec/CodecService.java b/server/src/main/java/org/opensearch/index/codec/CodecService.java index 67f38536a0d11..59fafdf1ba74e 100644 --- a/server/src/main/java/org/opensearch/index/codec/CodecService.java +++ b/server/src/main/java/org/opensearch/index/codec/CodecService.java @@ -39,6 +39,7 @@ import org.opensearch.common.Nullable; import org.opensearch.common.collect.MapBuilder; import org.opensearch.index.IndexSettings; +import org.opensearch.index.codec.composite.CompositeCodecFactory; import org.opensearch.index.mapper.MapperService; import java.util.Map; @@ -63,6 +64,7 @@ public class CodecService { * the raw unfiltered lucene default. useful for testing */ public static final String LUCENE_DEFAULT_CODEC = "lucene_default"; + private final CompositeCodecFactory compositeCodecFactory = new CompositeCodecFactory(); public CodecService(@Nullable MapperService mapperService, IndexSettings indexSettings, Logger logger) { final MapBuilder codecs = MapBuilder.newMapBuilder(); @@ -73,10 +75,16 @@ public CodecService(@Nullable MapperService mapperService, IndexSettings indexSe codecs.put(BEST_COMPRESSION_CODEC, new Lucene99Codec(Mode.BEST_COMPRESSION)); codecs.put(ZLIB, new Lucene99Codec(Mode.BEST_COMPRESSION)); } else { - codecs.put(DEFAULT_CODEC, new PerFieldMappingPostingFormatCodec(Mode.BEST_SPEED, mapperService, logger)); - codecs.put(LZ4, new PerFieldMappingPostingFormatCodec(Mode.BEST_SPEED, mapperService, logger)); - codecs.put(BEST_COMPRESSION_CODEC, new PerFieldMappingPostingFormatCodec(Mode.BEST_COMPRESSION, mapperService, logger)); - codecs.put(ZLIB, new PerFieldMappingPostingFormatCodec(Mode.BEST_COMPRESSION, mapperService, logger)); + // CompositeCodec still delegates to PerFieldMappingPostingFormatCodec + // We can still support all the compression codecs when composite index is present + if (mapperService.isCompositeIndexPresent()) { + codecs.putAll(compositeCodecFactory.getCompositeIndexCodecs(mapperService, logger)); + } else { + codecs.put(DEFAULT_CODEC, new PerFieldMappingPostingFormatCodec(Mode.BEST_SPEED, mapperService, logger)); + codecs.put(LZ4, new PerFieldMappingPostingFormatCodec(Mode.BEST_SPEED, mapperService, logger)); + codecs.put(BEST_COMPRESSION_CODEC, new PerFieldMappingPostingFormatCodec(Mode.BEST_COMPRESSION, mapperService, logger)); + codecs.put(ZLIB, new PerFieldMappingPostingFormatCodec(Mode.BEST_COMPRESSION, mapperService, logger)); + } } codecs.put(LUCENE_DEFAULT_CODEC, Codec.getDefault()); for (String codec : Codec.availableCodecs()) { diff --git a/server/src/main/java/org/opensearch/index/codec/composite/Composite99Codec.java b/server/src/main/java/org/opensearch/index/codec/composite/Composite99Codec.java new file mode 100644 index 0000000000000..de04944e67cd2 --- /dev/null +++ b/server/src/main/java/org/opensearch/index/codec/composite/Composite99Codec.java @@ -0,0 +1,57 @@ +/* + * 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.codec.composite; + +import org.apache.logging.log4j.Logger; +import org.apache.lucene.codecs.Codec; +import org.apache.lucene.codecs.DocValuesFormat; +import org.apache.lucene.codecs.FilterCodec; +import org.apache.lucene.codecs.lucene99.Lucene99Codec; +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.index.codec.PerFieldMappingPostingFormatCodec; +import org.opensearch.index.mapper.MapperService; + +/** + * Extends the Codec to support new file formats for composite indices eg: star tree index + * based on the mappings. + * + * @opensearch.experimental + */ +@ExperimentalApi +public class Composite99Codec extends FilterCodec { + public static final String COMPOSITE_INDEX_CODEC_NAME = "Composite99Codec"; + private final MapperService mapperService; + + // needed for SPI - this is used in reader path + public Composite99Codec() { + this(COMPOSITE_INDEX_CODEC_NAME, new Lucene99Codec(), null); + } + + public Composite99Codec(Lucene99Codec.Mode compressionMode, MapperService mapperService, Logger logger) { + this(COMPOSITE_INDEX_CODEC_NAME, new PerFieldMappingPostingFormatCodec(compressionMode, mapperService, logger), mapperService); + } + + /** + * Sole constructor. When subclassing this codec, create a no-arg ctor and pass the delegate codec and a unique name to + * this ctor. + * + * @param name name of the codec + * @param delegate codec delegate + * @param mapperService mapper service instance + */ + protected Composite99Codec(String name, Codec delegate, MapperService mapperService) { + super(name, delegate); + this.mapperService = mapperService; + } + + @Override + public DocValuesFormat docValuesFormat() { + return new Composite99DocValuesFormat(mapperService); + } +} diff --git a/server/src/main/java/org/opensearch/index/codec/composite/Composite99DocValuesFormat.java b/server/src/main/java/org/opensearch/index/codec/composite/Composite99DocValuesFormat.java new file mode 100644 index 0000000000000..216ed4f68f333 --- /dev/null +++ b/server/src/main/java/org/opensearch/index/codec/composite/Composite99DocValuesFormat.java @@ -0,0 +1,64 @@ +/* + * 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.codec.composite; + +import org.apache.lucene.codecs.DocValuesConsumer; +import org.apache.lucene.codecs.DocValuesFormat; +import org.apache.lucene.codecs.DocValuesProducer; +import org.apache.lucene.codecs.lucene90.Lucene90DocValuesFormat; +import org.apache.lucene.index.SegmentReadState; +import org.apache.lucene.index.SegmentWriteState; +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.index.mapper.MapperService; + +import java.io.IOException; + +/** + * DocValues format to handle composite indices + * + * @opensearch.experimental + */ +@ExperimentalApi +public class Composite99DocValuesFormat extends DocValuesFormat { + /** + * Creates a new docvalues format. + * + *

The provided name will be written into the index segment in some configurations (such as + * when using {@code PerFieldDocValuesFormat}): in such configurations, for the segment to be read + * this class should be registered with Java's SPI mechanism (registered in META-INF/ of your jar + * file, etc). + */ + private final DocValuesFormat delegate; + private final MapperService mapperService; + + // needed for SPI + public Composite99DocValuesFormat() { + this(new Lucene90DocValuesFormat(), null); + } + + public Composite99DocValuesFormat(MapperService mapperService) { + this(new Lucene90DocValuesFormat(), mapperService); + } + + public Composite99DocValuesFormat(DocValuesFormat delegate, MapperService mapperService) { + super(delegate.getName()); + this.delegate = delegate; + this.mapperService = mapperService; + } + + @Override + public DocValuesConsumer fieldsConsumer(SegmentWriteState state) throws IOException { + return new Composite99DocValuesWriter(delegate.fieldsConsumer(state), state, mapperService); + } + + @Override + public DocValuesProducer fieldsProducer(SegmentReadState state) throws IOException { + return new Composite99DocValuesReader(delegate.fieldsProducer(state), state); + } +} diff --git a/server/src/main/java/org/opensearch/index/codec/composite/Composite99DocValuesReader.java b/server/src/main/java/org/opensearch/index/codec/composite/Composite99DocValuesReader.java new file mode 100644 index 0000000000000..82c844088cfd4 --- /dev/null +++ b/server/src/main/java/org/opensearch/index/codec/composite/Composite99DocValuesReader.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.index.codec.composite; + +import org.apache.lucene.codecs.DocValuesProducer; +import org.apache.lucene.index.BinaryDocValues; +import org.apache.lucene.index.FieldInfo; +import org.apache.lucene.index.NumericDocValues; +import org.apache.lucene.index.SegmentReadState; +import org.apache.lucene.index.SortedDocValues; +import org.apache.lucene.index.SortedNumericDocValues; +import org.apache.lucene.index.SortedSetDocValues; +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.index.mapper.CompositeMappedFieldType; + +import java.io.IOException; +import java.util.List; + +/** + * Reader for star tree index and star tree doc values from the segments + * + * @opensearch.experimental + */ +@ExperimentalApi +public class Composite99DocValuesReader extends DocValuesProducer implements CompositeIndexReader { + private DocValuesProducer delegate; + + public Composite99DocValuesReader(DocValuesProducer producer, SegmentReadState state) throws IOException { + this.delegate = producer; + // TODO : read star tree files + } + + @Override + public NumericDocValues getNumeric(FieldInfo field) throws IOException { + return delegate.getNumeric(field); + } + + @Override + public BinaryDocValues getBinary(FieldInfo field) throws IOException { + return delegate.getBinary(field); + } + + @Override + public SortedDocValues getSorted(FieldInfo field) throws IOException { + return delegate.getSorted(field); + } + + @Override + public SortedNumericDocValues getSortedNumeric(FieldInfo field) throws IOException { + return delegate.getSortedNumeric(field); + } + + @Override + public SortedSetDocValues getSortedSet(FieldInfo field) throws IOException { + return delegate.getSortedSet(field); + } + + @Override + public void checkIntegrity() throws IOException { + delegate.checkIntegrity(); + // Todo : check integrity of composite index related [star tree] files + } + + @Override + public void close() throws IOException { + delegate.close(); + // Todo: close composite index related files [star tree] files + } + + @Override + public List getCompositeIndexFields() { + // todo : read from file formats and get the field names. + throw new UnsupportedOperationException(); + + } + + @Override + public CompositeIndexValues getCompositeIndexValues(String field, CompositeMappedFieldType.CompositeFieldType fieldType) + throws IOException { + // TODO : read compositeIndexValues [starTreeValues] from star tree files + throw new UnsupportedOperationException(); + } +} diff --git a/server/src/main/java/org/opensearch/index/codec/composite/Composite99DocValuesWriter.java b/server/src/main/java/org/opensearch/index/codec/composite/Composite99DocValuesWriter.java new file mode 100644 index 0000000000000..75bbf78dbdad2 --- /dev/null +++ b/server/src/main/java/org/opensearch/index/codec/composite/Composite99DocValuesWriter.java @@ -0,0 +1,114 @@ +/* + * 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.codec.composite; + +import org.apache.lucene.codecs.DocValuesConsumer; +import org.apache.lucene.codecs.DocValuesProducer; +import org.apache.lucene.index.FieldInfo; +import org.apache.lucene.index.MergeState; +import org.apache.lucene.index.SegmentWriteState; +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.index.mapper.CompositeMappedFieldType; +import org.opensearch.index.mapper.MapperService; +import org.opensearch.index.mapper.StarTreeMapper; + +import java.io.IOException; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; + +/** + * This class write the star tree index and star tree doc values + * based on the doc values structures of the original index + * + * @opensearch.experimental + */ +@ExperimentalApi +public class Composite99DocValuesWriter extends DocValuesConsumer { + private final DocValuesConsumer delegate; + private final SegmentWriteState state; + private final MapperService mapperService; + AtomicReference mergeState = new AtomicReference<>(); + private final Set compositeMappedFieldTypes; + private final Set compositeFieldSet; + + private final Map fieldProducerMap = new HashMap<>(); + + public Composite99DocValuesWriter(DocValuesConsumer delegate, SegmentWriteState segmentWriteState, MapperService mapperService) { + + this.delegate = delegate; + this.state = segmentWriteState; + this.mapperService = mapperService; + this.compositeMappedFieldTypes = mapperService.getCompositeFieldTypes(); + compositeFieldSet = new HashSet<>(); + for (CompositeMappedFieldType type : compositeMappedFieldTypes) { + compositeFieldSet.addAll(type.fields()); + } + } + + @Override + public void addNumericField(FieldInfo field, DocValuesProducer valuesProducer) throws IOException { + delegate.addNumericField(field, valuesProducer); + } + + @Override + public void addBinaryField(FieldInfo field, DocValuesProducer valuesProducer) throws IOException { + delegate.addBinaryField(field, valuesProducer); + } + + @Override + public void addSortedField(FieldInfo field, DocValuesProducer valuesProducer) throws IOException { + delegate.addSortedField(field, valuesProducer); + } + + @Override + public void addSortedNumericField(FieldInfo field, DocValuesProducer valuesProducer) throws IOException { + delegate.addSortedNumericField(field, valuesProducer); + // Perform this only during flush flow + if (mergeState.get() == null) { + createCompositeIndicesIfPossible(valuesProducer, field); + } + } + + @Override + public void addSortedSetField(FieldInfo field, DocValuesProducer valuesProducer) throws IOException { + delegate.addSortedSetField(field, valuesProducer); + } + + @Override + public void close() throws IOException { + delegate.close(); + } + + private void createCompositeIndicesIfPossible(DocValuesProducer valuesProducer, FieldInfo field) throws IOException { + if (compositeFieldSet.isEmpty()) return; + if (compositeFieldSet.contains(field.name)) { + fieldProducerMap.put(field.name, valuesProducer); + compositeFieldSet.remove(field.name); + } + // we have all the required fields to build composite fields + if (compositeFieldSet.isEmpty()) { + for (CompositeMappedFieldType mappedType : compositeMappedFieldTypes) { + if (mappedType instanceof StarTreeMapper.StarTreeFieldType) { + // TODO : Call StarTree builder + } + } + } + } + + @Override + public void merge(MergeState mergeState) throws IOException { + this.mergeState.compareAndSet(null, mergeState); + super.merge(mergeState); + // TODO : handle merge star tree + // mergeStarTreeFields(mergeState); + } +} diff --git a/server/src/main/java/org/opensearch/index/codec/composite/CompositeCodecFactory.java b/server/src/main/java/org/opensearch/index/codec/composite/CompositeCodecFactory.java new file mode 100644 index 0000000000000..3acedc6a27d7f --- /dev/null +++ b/server/src/main/java/org/opensearch/index/codec/composite/CompositeCodecFactory.java @@ -0,0 +1,42 @@ +/* + * 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.codec.composite; + +import org.apache.logging.log4j.Logger; +import org.apache.lucene.codecs.Codec; +import org.apache.lucene.codecs.lucene99.Lucene99Codec; +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.index.mapper.MapperService; + +import java.util.HashMap; +import java.util.Map; + +import static org.opensearch.index.codec.CodecService.BEST_COMPRESSION_CODEC; +import static org.opensearch.index.codec.CodecService.DEFAULT_CODEC; +import static org.opensearch.index.codec.CodecService.LZ4; +import static org.opensearch.index.codec.CodecService.ZLIB; + +/** + * Factory class to return the latest composite codec for all the modes + * + * @opensearch.experimental + */ +@ExperimentalApi +public class CompositeCodecFactory { + public CompositeCodecFactory() {} + + public Map getCompositeIndexCodecs(MapperService mapperService, Logger logger) { + Map codecs = new HashMap<>(); + codecs.put(DEFAULT_CODEC, new Composite99Codec(Lucene99Codec.Mode.BEST_SPEED, mapperService, logger)); + codecs.put(LZ4, new Composite99Codec(Lucene99Codec.Mode.BEST_SPEED, mapperService, logger)); + codecs.put(BEST_COMPRESSION_CODEC, new Composite99Codec(Lucene99Codec.Mode.BEST_COMPRESSION, mapperService, logger)); + codecs.put(ZLIB, new Composite99Codec(Lucene99Codec.Mode.BEST_COMPRESSION, mapperService, logger)); + return codecs; + } +} diff --git a/server/src/main/java/org/opensearch/index/codec/composite/CompositeIndexReader.java b/server/src/main/java/org/opensearch/index/codec/composite/CompositeIndexReader.java new file mode 100644 index 0000000000000..d02438b75377d --- /dev/null +++ b/server/src/main/java/org/opensearch/index/codec/composite/CompositeIndexReader.java @@ -0,0 +1,34 @@ +/* + * 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.codec.composite; + +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.index.mapper.CompositeMappedFieldType; + +import java.io.IOException; +import java.util.List; + +/** + * Interface that abstracts the functionality to read composite index structures from the segment + * + * @opensearch.experimental + */ +@ExperimentalApi +public interface CompositeIndexReader { + /** + * Get list of composite index fields from the segment + * + */ + List getCompositeIndexFields(); + + /** + * Get composite index values based on the field name and the field type + */ + CompositeIndexValues getCompositeIndexValues(String field, CompositeMappedFieldType.CompositeFieldType fieldType) throws IOException; +} diff --git a/server/src/main/java/org/opensearch/index/codec/composite/CompositeIndexValues.java b/server/src/main/java/org/opensearch/index/codec/composite/CompositeIndexValues.java new file mode 100644 index 0000000000000..f8848aceab343 --- /dev/null +++ b/server/src/main/java/org/opensearch/index/codec/composite/CompositeIndexValues.java @@ -0,0 +1,21 @@ +/* + * 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.codec.composite; + +import org.opensearch.common.annotation.ExperimentalApi; + +/** + * Interface for composite index values + * + * @opensearch.experimental + */ +@ExperimentalApi +public interface CompositeIndexValues { + CompositeIndexValues getValues(); +} diff --git a/server/src/main/java/org/opensearch/index/codec/composite/datacube/startree/StarTreeValues.java b/server/src/main/java/org/opensearch/index/codec/composite/datacube/startree/StarTreeValues.java new file mode 100644 index 0000000000000..2a5b96ce2620a --- /dev/null +++ b/server/src/main/java/org/opensearch/index/codec/composite/datacube/startree/StarTreeValues.java @@ -0,0 +1,35 @@ +/* + * 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.codec.composite.datacube.startree; + +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.index.codec.composite.CompositeIndexValues; + +import java.util.List; + +/** + * Concrete class that holds the star tree associated values from the segment + * + * @opensearch.experimental + */ +@ExperimentalApi +public class StarTreeValues implements CompositeIndexValues { + private final List dimensionsOrder; + + // TODO : come up with full set of vales such as dimensions and metrics doc values + star tree + public StarTreeValues(List dimensionsOrder) { + super(); + this.dimensionsOrder = List.copyOf(dimensionsOrder); + } + + @Override + public CompositeIndexValues getValues() { + return this; + } +} diff --git a/server/src/main/java/org/opensearch/index/codec/composite/datacube/startree/package-info.java b/server/src/main/java/org/opensearch/index/codec/composite/datacube/startree/package-info.java new file mode 100644 index 0000000000000..67808ad51289a --- /dev/null +++ b/server/src/main/java/org/opensearch/index/codec/composite/datacube/startree/package-info.java @@ -0,0 +1,12 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** + * classes responsible for handling all star tree structures and operations as part of codec + */ +package org.opensearch.index.codec.composite.datacube.startree; diff --git a/server/src/main/java/org/opensearch/index/codec/composite/package-info.java b/server/src/main/java/org/opensearch/index/codec/composite/package-info.java new file mode 100644 index 0000000000000..5d15e99c00975 --- /dev/null +++ b/server/src/main/java/org/opensearch/index/codec/composite/package-info.java @@ -0,0 +1,12 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** + * classes responsible for handling all composite index codecs and operations + */ +package org.opensearch.index.codec.composite; diff --git a/server/src/main/java/org/opensearch/index/mapper/CompositeMappedFieldType.java b/server/src/main/java/org/opensearch/index/mapper/CompositeMappedFieldType.java index f52ce29a86dd2..e067e70621304 100644 --- a/server/src/main/java/org/opensearch/index/mapper/CompositeMappedFieldType.java +++ b/server/src/main/java/org/opensearch/index/mapper/CompositeMappedFieldType.java @@ -45,7 +45,10 @@ public CompositeMappedFieldType(String name, List fields, CompositeField /** * Supported composite field types + * + * @opensearch.experimental */ + @ExperimentalApi public enum CompositeFieldType { STAR_TREE("star_tree"); diff --git a/server/src/main/java/org/opensearch/index/mapper/MapperService.java b/server/src/main/java/org/opensearch/index/mapper/MapperService.java index c2e7411a3b47a..530a3092a5aa7 100644 --- a/server/src/main/java/org/opensearch/index/mapper/MapperService.java +++ b/server/src/main/java/org/opensearch/index/mapper/MapperService.java @@ -226,6 +226,8 @@ public enum MergeReason { private final BooleanSupplier idFieldDataEnabled; + private volatile Set compositeMappedFieldTypes; + public MapperService( IndexSettings indexSettings, IndexAnalyzers indexAnalyzers, @@ -542,6 +544,9 @@ private synchronized Map internalMerge(DocumentMapper ma } assert results.values().stream().allMatch(this::assertSerialization); + + // initialize composite fields post merge + this.compositeMappedFieldTypes = getCompositeFieldTypesFromMapper(); return results; } @@ -655,6 +660,10 @@ public boolean isCompositeIndexPresent() { } public Set getCompositeFieldTypes() { + return compositeMappedFieldTypes; + } + + private Set getCompositeFieldTypesFromMapper() { Set compositeMappedFieldTypes = new HashSet<>(); if (this.mapper == null) { return Collections.emptySet(); diff --git a/server/src/main/resources/META-INF/services/org.apache.lucene.codecs.Codec b/server/src/main/resources/META-INF/services/org.apache.lucene.codecs.Codec new file mode 100644 index 0000000000000..e030a813373c1 --- /dev/null +++ b/server/src/main/resources/META-INF/services/org.apache.lucene.codecs.Codec @@ -0,0 +1 @@ +org.opensearch.index.codec.composite.Composite99Codec diff --git a/server/src/test/java/org/opensearch/index/codec/CodecTests.java b/server/src/test/java/org/opensearch/index/codec/CodecTests.java index b31edd79411d0..7146b7dc51753 100644 --- a/server/src/test/java/org/opensearch/index/codec/CodecTests.java +++ b/server/src/test/java/org/opensearch/index/codec/CodecTests.java @@ -48,6 +48,7 @@ import org.opensearch.env.Environment; import org.opensearch.index.IndexSettings; import org.opensearch.index.analysis.IndexAnalyzers; +import org.opensearch.index.codec.composite.Composite99Codec; import org.opensearch.index.engine.EngineConfig; import org.opensearch.index.mapper.MapperService; import org.opensearch.index.similarity.SimilarityService; @@ -59,6 +60,8 @@ import java.io.IOException; import java.util.Collections; +import org.mockito.Mockito; + import static org.opensearch.index.engine.EngineConfig.INDEX_CODEC_COMPRESSION_LEVEL_SETTING; import static org.hamcrest.Matchers.instanceOf; @@ -76,23 +79,52 @@ public void testDefault() throws Exception { assertStoredFieldsCompressionEquals(Lucene99Codec.Mode.BEST_SPEED, codec); } + public void testDefaultWithCompositeIndex() throws Exception { + Codec codec = createCodecService(false, true).codec("default"); + assertStoredFieldsCompressionEquals(Lucene99Codec.Mode.BEST_SPEED, codec); + assert codec instanceof Composite99Codec; + } + public void testBestCompression() throws Exception { Codec codec = createCodecService(false).codec("best_compression"); assertStoredFieldsCompressionEquals(Lucene99Codec.Mode.BEST_COMPRESSION, codec); } + public void testBestCompressionWithCompositeIndex() throws Exception { + Codec codec = createCodecService(false, true).codec("best_compression"); + assertStoredFieldsCompressionEquals(Lucene99Codec.Mode.BEST_COMPRESSION, codec); + assert codec instanceof Composite99Codec; + } + public void testLZ4() throws Exception { Codec codec = createCodecService(false).codec("lz4"); assertStoredFieldsCompressionEquals(Lucene99Codec.Mode.BEST_SPEED, codec); assert codec instanceof PerFieldMappingPostingFormatCodec; } + public void testLZ4WithCompositeIndex() throws Exception { + Codec codec = createCodecService(false, true).codec("lz4"); + assertStoredFieldsCompressionEquals(Lucene99Codec.Mode.BEST_SPEED, codec); + assert codec instanceof Composite99Codec; + } + public void testZlib() throws Exception { Codec codec = createCodecService(false).codec("zlib"); assertStoredFieldsCompressionEquals(Lucene99Codec.Mode.BEST_COMPRESSION, codec); assert codec instanceof PerFieldMappingPostingFormatCodec; } + public void testZlibWithCompositeIndex() throws Exception { + Codec codec = createCodecService(false, true).codec("zlib"); + assertStoredFieldsCompressionEquals(Lucene99Codec.Mode.BEST_COMPRESSION, codec); + assert codec instanceof Composite99Codec; + } + + public void testResolveDefaultCodecsWithCompositeIndex() throws Exception { + CodecService codecService = createCodecService(false, true); + assertThat(codecService.codec("default"), instanceOf(Composite99Codec.class)); + } + public void testBestCompressionWithCompressionLevel() { final Settings settings = Settings.builder() .put(INDEX_CODEC_COMPRESSION_LEVEL_SETTING.getKey(), randomIntBetween(1, 6)) @@ -150,10 +182,17 @@ private void assertStoredFieldsCompressionEquals(Lucene99Codec.Mode expected, Co } private CodecService createCodecService(boolean isMapperServiceNull) throws IOException { + return createCodecService(isMapperServiceNull, false); + } + + private CodecService createCodecService(boolean isMapperServiceNull, boolean isCompositeIndexPresent) throws IOException { Settings nodeSettings = Settings.builder().put(Environment.PATH_HOME_SETTING.getKey(), createTempDir()).build(); if (isMapperServiceNull) { return new CodecService(null, IndexSettingsModule.newIndexSettings("_na", nodeSettings), LogManager.getLogger("test")); } + if (isCompositeIndexPresent) { + return buildCodecServiceWithCompositeIndex(nodeSettings); + } return buildCodecService(nodeSettings); } @@ -176,6 +215,14 @@ private CodecService buildCodecService(Settings nodeSettings) throws IOException return new CodecService(service, indexSettings, LogManager.getLogger("test")); } + private CodecService buildCodecServiceWithCompositeIndex(Settings nodeSettings) throws IOException { + + IndexSettings indexSettings = IndexSettingsModule.newIndexSettings("_na", nodeSettings); + MapperService service = Mockito.mock(MapperService.class); + Mockito.when(service.isCompositeIndexPresent()).thenReturn(true); + return new CodecService(service, indexSettings, LogManager.getLogger("test")); + } + private SegmentReader getSegmentReader(Codec codec) throws IOException { Directory dir = newDirectory(); IndexWriterConfig iwc = newIndexWriterConfig(null); diff --git a/server/src/test/java/org/opensearch/index/codec/composite/datacube/startree/StarTreeDocValuesFormatTests.java b/server/src/test/java/org/opensearch/index/codec/composite/datacube/startree/StarTreeDocValuesFormatTests.java new file mode 100644 index 0000000000000..6c6d26656e4de --- /dev/null +++ b/server/src/test/java/org/opensearch/index/codec/composite/datacube/startree/StarTreeDocValuesFormatTests.java @@ -0,0 +1,110 @@ +/* + * 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.codec.composite.datacube.startree; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.lucene.codecs.Codec; +import org.apache.lucene.codecs.lucene99.Lucene99Codec; +import org.apache.lucene.document.Document; +import org.apache.lucene.document.SortedNumericDocValuesField; +import org.apache.lucene.index.IndexWriterConfig; +import org.apache.lucene.store.Directory; +import org.apache.lucene.tests.index.BaseDocValuesFormatTestCase; +import org.apache.lucene.tests.index.RandomIndexWriter; +import org.apache.lucene.tests.util.LuceneTestCase; +import org.opensearch.common.Rounding; +import org.opensearch.index.codec.composite.Composite99Codec; +import org.opensearch.index.compositeindex.datacube.DateDimension; +import org.opensearch.index.compositeindex.datacube.Dimension; +import org.opensearch.index.compositeindex.datacube.Metric; +import org.opensearch.index.compositeindex.datacube.MetricStat; +import org.opensearch.index.compositeindex.datacube.NumericDimension; +import org.opensearch.index.compositeindex.datacube.startree.StarTreeField; +import org.opensearch.index.compositeindex.datacube.startree.StarTreeFieldConfiguration; +import org.opensearch.index.mapper.MapperService; +import org.opensearch.index.mapper.StarTreeMapper; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Set; + +import org.mockito.Mockito; + +/** + * Star tree doc values Lucene tests + */ +@LuceneTestCase.SuppressSysoutChecks(bugUrl = "we log a lot on purpose") +public class StarTreeDocValuesFormatTests extends BaseDocValuesFormatTestCase { + @Override + protected Codec getCodec() { + MapperService service = Mockito.mock(MapperService.class); + Mockito.when(service.getCompositeFieldTypes()).thenReturn(Set.of(getStarTreeFieldType())); + final Logger testLogger = LogManager.getLogger(StarTreeDocValuesFormatTests.class); + return new Composite99Codec(Lucene99Codec.Mode.BEST_SPEED, service, testLogger); + } + + private StarTreeMapper.StarTreeFieldType getStarTreeFieldType() { + List m1 = new ArrayList<>(); + m1.add(MetricStat.MAX); + Metric metric = new Metric("sndv", m1); + List d1CalendarIntervals = new ArrayList<>(); + d1CalendarIntervals.add(Rounding.DateTimeUnit.HOUR_OF_DAY); + StarTreeField starTreeField = getStarTreeField(d1CalendarIntervals, metric); + + return new StarTreeMapper.StarTreeFieldType("star_tree", starTreeField); + } + + private static StarTreeField getStarTreeField(List d1CalendarIntervals, Metric metric1) { + DateDimension d1 = new DateDimension("field", d1CalendarIntervals); + NumericDimension d2 = new NumericDimension("dv"); + + List metrics = List.of(metric1); + List dims = List.of(d1, d2); + StarTreeFieldConfiguration config = new StarTreeFieldConfiguration( + 100, + Collections.emptySet(), + StarTreeFieldConfiguration.StarTreeBuildMode.OFF_HEAP + ); + + return new StarTreeField("starTree", dims, metrics, config); + } + + public void testStarTreeDocValues() throws IOException { + Directory directory = newDirectory(); + IndexWriterConfig conf = newIndexWriterConfig(null); + conf.setMergePolicy(newLogMergePolicy()); + RandomIndexWriter iw = new RandomIndexWriter(random(), directory, conf); + Document doc = new Document(); + doc.add(new SortedNumericDocValuesField("sndv", 1)); + doc.add(new SortedNumericDocValuesField("dv", 1)); + doc.add(new SortedNumericDocValuesField("field", 1)); + iw.addDocument(doc); + doc.add(new SortedNumericDocValuesField("sndv", 1)); + doc.add(new SortedNumericDocValuesField("dv", 1)); + doc.add(new SortedNumericDocValuesField("field", 1)); + iw.addDocument(doc); + iw.forceMerge(1); + doc.add(new SortedNumericDocValuesField("sndv", 2)); + doc.add(new SortedNumericDocValuesField("dv", 2)); + doc.add(new SortedNumericDocValuesField("field", 2)); + iw.addDocument(doc); + doc.add(new SortedNumericDocValuesField("sndv", 2)); + doc.add(new SortedNumericDocValuesField("dv", 2)); + doc.add(new SortedNumericDocValuesField("field", 2)); + iw.addDocument(doc); + iw.forceMerge(1); + iw.close(); + + // TODO : validate star tree structures that got created + directory.close(); + } +} From ba851fbf3d1f1d82b08d318cd0b0e2fded29c10c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 8 Jul 2024 10:19:29 -0400 Subject: [PATCH 18/20] Bump com.github.spullara.mustache.java:compiler from 0.9.13 to 0.9.14 in /modules/lang-mustache (#14672) * Bump com.github.spullara.mustache.java:compiler Bumps [com.github.spullara.mustache.java:compiler](https://github.com/spullara/mustache.java) from 0.9.13 to 0.9.14. - [Commits](https://github.com/spullara/mustache.java/compare/mustache.java-0.9.13...mustache.java-0.9.14) --- updated-dependencies: - dependency-name: com.github.spullara.mustache.java:compiler dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] * Updating SHAs 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] Signed-off-by: Kaushal Kumar --- CHANGELOG.md | 1 + modules/lang-mustache/build.gradle | 2 +- modules/lang-mustache/licenses/compiler-0.9.13.jar.sha1 | 1 - modules/lang-mustache/licenses/compiler-0.9.14.jar.sha1 | 1 + 4 files changed, 3 insertions(+), 2 deletions(-) delete mode 100644 modules/lang-mustache/licenses/compiler-0.9.13.jar.sha1 create mode 100644 modules/lang-mustache/licenses/compiler-0.9.14.jar.sha1 diff --git a/CHANGELOG.md b/CHANGELOG.md index 2677047abdbf7..a5d1c1433af71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Bump `azure-identity` from 1.11.4 to 1.13.0, Bump `msal4j` from 1.14.3 to 1.15.1, Bump `msal4j-persistence-extension` from 1.2.0 to 1.3.0 ([#14506](https://github.com/opensearch-project/OpenSearch/pull/14506)) - Bump `com.azure:azure-storage-common` from 12.21.2 to 12.25.1 ([#14517](https://github.com/opensearch-project/OpenSearch/pull/14517)) - Bump `com.microsoft.azure:msal4j` from 1.15.1 to 1.16.0 ([#14610](https://github.com/opensearch-project/OpenSearch/pull/14610)) +- Bump `com.github.spullara.mustache.java:compiler` from 0.9.13 to 0.9.14 ([#14672](https://github.com/opensearch-project/OpenSearch/pull/14672)) ### Changed - [Tiered Caching] Move query recomputation logic outside write lock ([#14187](https://github.com/opensearch-project/OpenSearch/pull/14187)) diff --git a/modules/lang-mustache/build.gradle b/modules/lang-mustache/build.gradle index bcf5c07ea8c64..a836124f94b41 100644 --- a/modules/lang-mustache/build.gradle +++ b/modules/lang-mustache/build.gradle @@ -38,7 +38,7 @@ opensearchplugin { } dependencies { - api "com.github.spullara.mustache.java:compiler:0.9.13" + api "com.github.spullara.mustache.java:compiler:0.9.14" } restResources { diff --git a/modules/lang-mustache/licenses/compiler-0.9.13.jar.sha1 b/modules/lang-mustache/licenses/compiler-0.9.13.jar.sha1 deleted file mode 100644 index 70d53aac260eb..0000000000000 --- a/modules/lang-mustache/licenses/compiler-0.9.13.jar.sha1 +++ /dev/null @@ -1 +0,0 @@ -60666500a7dce7a5d3e17c09b46ea6f037192bd5 \ No newline at end of file diff --git a/modules/lang-mustache/licenses/compiler-0.9.14.jar.sha1 b/modules/lang-mustache/licenses/compiler-0.9.14.jar.sha1 new file mode 100644 index 0000000000000..29069ac90817a --- /dev/null +++ b/modules/lang-mustache/licenses/compiler-0.9.14.jar.sha1 @@ -0,0 +1 @@ +e6df8b5aabb80d6eb6d8fef312a56d66b7659ba6 \ No newline at end of file From 3925aac87e5c8036c43b3e63189141c6a1c455c9 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 8 Jul 2024 11:29:48 -0400 Subject: [PATCH 19/20] Bump net.minidev:accessors-smart from 2.5.0 to 2.5.1 in /plugins/repository-azure (#14673) * Bump net.minidev:accessors-smart in /plugins/repository-azure Bumps [net.minidev:accessors-smart](https://github.com/netplex/json-smart-v2) from 2.5.0 to 2.5.1. - [Release notes](https://github.com/netplex/json-smart-v2/releases) - [Commits](https://github.com/netplex/json-smart-v2/compare/2.5.0...2.5.1) --- updated-dependencies: - dependency-name: net.minidev:accessors-smart dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] * Updating SHAs 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] Signed-off-by: Kaushal Kumar --- CHANGELOG.md | 1 + plugins/repository-azure/build.gradle | 2 +- .../repository-azure/licenses/accessors-smart-2.5.0.jar.sha1 | 1 - .../repository-azure/licenses/accessors-smart-2.5.1.jar.sha1 | 1 + 4 files changed, 3 insertions(+), 2 deletions(-) delete mode 100644 plugins/repository-azure/licenses/accessors-smart-2.5.0.jar.sha1 create mode 100644 plugins/repository-azure/licenses/accessors-smart-2.5.1.jar.sha1 diff --git a/CHANGELOG.md b/CHANGELOG.md index a5d1c1433af71..5d1d96dc91f68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Bump `com.azure:azure-storage-common` from 12.21.2 to 12.25.1 ([#14517](https://github.com/opensearch-project/OpenSearch/pull/14517)) - Bump `com.microsoft.azure:msal4j` from 1.15.1 to 1.16.0 ([#14610](https://github.com/opensearch-project/OpenSearch/pull/14610)) - Bump `com.github.spullara.mustache.java:compiler` from 0.9.13 to 0.9.14 ([#14672](https://github.com/opensearch-project/OpenSearch/pull/14672)) +- Bump `net.minidev:accessors-smart` from 2.5.0 to 2.5.1 ([#14673](https://github.com/opensearch-project/OpenSearch/pull/14673)) ### Changed - [Tiered Caching] Move query recomputation logic outside write lock ([#14187](https://github.com/opensearch-project/OpenSearch/pull/14187)) diff --git a/plugins/repository-azure/build.gradle b/plugins/repository-azure/build.gradle index 13b711019ff2a..0f822a02e05d8 100644 --- a/plugins/repository-azure/build.gradle +++ b/plugins/repository-azure/build.gradle @@ -69,7 +69,7 @@ dependencies { // Both msal4j:1.14.3 and oauth2-oidc-sdk:11.9.1 has compile dependency on different versions of json-smart, // selected the higher version which is 2.5.0 api 'net.minidev:json-smart:2.5.0' - api 'net.minidev:accessors-smart:2.5.0' + api 'net.minidev:accessors-smart:2.5.1' api "org.ow2.asm:asm:${versions.asm}" // End of transitive dependencies for azure-identity api "io.projectreactor.netty:reactor-netty-core:${versions.reactor_netty}" diff --git a/plugins/repository-azure/licenses/accessors-smart-2.5.0.jar.sha1 b/plugins/repository-azure/licenses/accessors-smart-2.5.0.jar.sha1 deleted file mode 100644 index 1578c94fcdc7b..0000000000000 --- a/plugins/repository-azure/licenses/accessors-smart-2.5.0.jar.sha1 +++ /dev/null @@ -1 +0,0 @@ -aca011492dfe9c26f4e0659028a4fe0970829dd8 \ No newline at end of file diff --git a/plugins/repository-azure/licenses/accessors-smart-2.5.1.jar.sha1 b/plugins/repository-azure/licenses/accessors-smart-2.5.1.jar.sha1 new file mode 100644 index 0000000000000..8f7452437323d --- /dev/null +++ b/plugins/repository-azure/licenses/accessors-smart-2.5.1.jar.sha1 @@ -0,0 +1 @@ +19b820261eb2e7de7d5bde11d1c06e4501dd7e5f \ No newline at end of file From 7b21688e4f432f9e09498869a19efd9831ab544a Mon Sep 17 00:00:00 2001 From: Kaushal Kumar Date: Thu, 11 Jul 2024 14:55:48 -0700 Subject: [PATCH 20/20] move query group thread context propagator out of ThreadContext Signed-off-by: Kaushal Kumar --- .../org/opensearch/common/util/concurrent/ThreadContext.java | 5 +---- 1 file changed, 1 insertion(+), 4 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 3925c84ac7261..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 @@ -47,7 +47,6 @@ import org.opensearch.http.HttpTransportSettings; import org.opensearch.tasks.Task; import org.opensearch.tasks.TaskThreadContextStatePropagator; -import org.opensearch.wlm.QueryGroupThreadContextStatePropagator; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -129,9 +128,7 @@ public ThreadContext(Settings settings) { this.threadLocal = ThreadLocal.withInitial(() -> DEFAULT_CONTEXT); this.maxWarningHeaderCount = SETTING_HTTP_MAX_WARNING_HEADER_COUNT.get(settings); this.maxWarningHeaderSize = SETTING_HTTP_MAX_WARNING_HEADER_SIZE.get(settings).getBytes(); - this.propagators = new CopyOnWriteArrayList<>( - List.of(new TaskThreadContextStatePropagator(), new QueryGroupThreadContextStatePropagator()) - ); + this.propagators = new CopyOnWriteArrayList<>(List.of(new TaskThreadContextStatePropagator())); } public void registerThreadContextStatePropagator(final ThreadContextStatePropagator propagator) {