From f5c897cf1db3d413c6ccd9bcad1284a7873a1556 Mon Sep 17 00:00:00 2001 From: David Zane <38449481+dzane17@users.noreply.github.com> Date: Thu, 5 Sep 2024 11:59:24 -0700 Subject: [PATCH 1/5] Adding WithFieldName interface for QueryBuilders with fieldName (#15705) Signed-off-by: David Zane Signed-off-by: Ankit Jain Co-authored-by: Ankit Jain --- CHANGELOG.md | 1 + .../index/query/CorrelationQueryBuilder.java | 4 +++- .../query/AbstractGeometryQueryBuilder.java | 5 ++++- .../index/query/BaseTermQueryBuilder.java | 3 ++- .../index/query/CommonTermsQueryBuilder.java | 3 ++- .../query/DistanceFeatureQueryBuilder.java | 5 +++-- .../index/query/ExistsQueryBuilder.java | 3 ++- .../query/FieldMaskingSpanQueryBuilder.java | 6 +++++- .../query/GeoBoundingBoxQueryBuilder.java | 3 ++- .../index/query/GeoDistanceQueryBuilder.java | 3 ++- .../index/query/GeoPolygonQueryBuilder.java | 3 ++- .../query/MatchBoolPrefixQueryBuilder.java | 3 ++- .../query/MatchPhrasePrefixQueryBuilder.java | 3 ++- .../index/query/MatchPhraseQueryBuilder.java | 3 ++- .../index/query/MatchQueryBuilder.java | 3 ++- .../index/query/MultiTermQueryBuilder.java | 7 +------ .../index/query/SpanNearQueryBuilder.java | 3 ++- .../index/query/TermsQueryBuilder.java | 3 ++- .../opensearch/index/query/WithFieldName.java | 21 +++++++++++++++++++ 19 files changed, 62 insertions(+), 23 deletions(-) create mode 100644 server/src/main/java/org/opensearch/index/query/WithFieldName.java diff --git a/CHANGELOG.md b/CHANGELOG.md index a7f3161daff23..06a749df1b3a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add canRemain method to TargetPoolAllocationDecider to move shards from local to remote pool for hot to warm tiering ([#15010](https://github.com/opensearch-project/OpenSearch/pull/15010)) - ClusterManagerTaskThrottler Improvements ([#15508](https://github.com/opensearch-project/OpenSearch/pull/15508)) - Reset DiscoveryNodes in all transport node actions request ([#15131](https://github.com/opensearch-project/OpenSearch/pull/15131)) +- Adding WithFieldName interface for QueryBuilders with fieldName ([#15705](https://github.com/opensearch-project/OpenSearch/pull/15705)) - Relax the join validation for Remote State publication ([#15471](https://github.com/opensearch-project/OpenSearch/pull/15471)) - MultiTermQueries in keyword fields now default to `indexed` approach and gated behind cluster setting ([#15637](https://github.com/opensearch-project/OpenSearch/pull/15637)) - Making _cat/allocation API use indexLevelStats ([#15292](https://github.com/opensearch-project/OpenSearch/pull/15292)) diff --git a/plugins/events-correlation-engine/src/main/java/org/opensearch/plugin/correlation/core/index/query/CorrelationQueryBuilder.java b/plugins/events-correlation-engine/src/main/java/org/opensearch/plugin/correlation/core/index/query/CorrelationQueryBuilder.java index 806ac0389b5f3..e95b68e855cca 100644 --- a/plugins/events-correlation-engine/src/main/java/org/opensearch/plugin/correlation/core/index/query/CorrelationQueryBuilder.java +++ b/plugins/events-correlation-engine/src/main/java/org/opensearch/plugin/correlation/core/index/query/CorrelationQueryBuilder.java @@ -23,6 +23,7 @@ import org.opensearch.index.query.AbstractQueryBuilder; import org.opensearch.index.query.QueryBuilder; import org.opensearch.index.query.QueryShardContext; +import org.opensearch.index.query.WithFieldName; import org.opensearch.plugin.correlation.core.index.mapper.VectorFieldMapper; import java.io.IOException; @@ -36,7 +37,7 @@ * * @opensearch.internal */ -public class CorrelationQueryBuilder extends AbstractQueryBuilder { +public class CorrelationQueryBuilder extends AbstractQueryBuilder implements WithFieldName { private static final Logger log = LogManager.getLogger(CorrelationQueryBuilder.class); protected static final ParseField VECTOR_FIELD = new ParseField("vector"); @@ -205,6 +206,7 @@ public void setFieldName(String fieldName) { * get field name * @return field name */ + @Override public String fieldName() { return fieldName; } diff --git a/server/src/main/java/org/opensearch/index/query/AbstractGeometryQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/AbstractGeometryQueryBuilder.java index 9fb857e33bfee..3823b524df6fe 100644 --- a/server/src/main/java/org/opensearch/index/query/AbstractGeometryQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/AbstractGeometryQueryBuilder.java @@ -67,7 +67,9 @@ * * @opensearch.internal */ -public abstract class AbstractGeometryQueryBuilder> extends AbstractQueryBuilder { +public abstract class AbstractGeometryQueryBuilder> extends AbstractQueryBuilder + implements + WithFieldName { public static final String DEFAULT_SHAPE_INDEX_NAME = "shapes"; public static final String DEFAULT_SHAPE_FIELD_NAME = "shape"; @@ -218,6 +220,7 @@ protected void doWriteTo(StreamOutput out) throws IOException { /** * @return the name of the field that will be queried */ + @Override public String fieldName() { return fieldName; } diff --git a/server/src/main/java/org/opensearch/index/query/BaseTermQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/BaseTermQueryBuilder.java index c4d9437a60c75..deb36a26e4c86 100644 --- a/server/src/main/java/org/opensearch/index/query/BaseTermQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/BaseTermQueryBuilder.java @@ -47,7 +47,7 @@ * * @opensearch.internal */ -public abstract class BaseTermQueryBuilder> extends AbstractQueryBuilder { +public abstract class BaseTermQueryBuilder> extends AbstractQueryBuilder implements WithFieldName { public static final ParseField VALUE_FIELD = new ParseField("value"); @@ -153,6 +153,7 @@ protected void doWriteTo(StreamOutput out) throws IOException { } /** Returns the field name used in this query. */ + @Override public String fieldName() { return this.fieldName; } diff --git a/server/src/main/java/org/opensearch/index/query/CommonTermsQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/CommonTermsQueryBuilder.java index 652cae86da0dc..24b10851cbe10 100644 --- a/server/src/main/java/org/opensearch/index/query/CommonTermsQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/CommonTermsQueryBuilder.java @@ -67,7 +67,7 @@ * @opensearch.internal */ @Deprecated -public class CommonTermsQueryBuilder extends AbstractQueryBuilder { +public class CommonTermsQueryBuilder extends AbstractQueryBuilder implements WithFieldName { public static final String COMMON_TERMS_QUERY_DEPRECATION_MSG = "[match] query which can efficiently " + "skip blocks of documents if the total number of hits is not tracked"; @@ -152,6 +152,7 @@ protected void doWriteTo(StreamOutput out) throws IOException { out.writeFloat(cutoffFrequency); } + @Override public String fieldName() { return this.fieldName; } diff --git a/server/src/main/java/org/opensearch/index/query/DistanceFeatureQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/DistanceFeatureQueryBuilder.java index 1d9f0479c6b17..e4f3d8556158a 100644 --- a/server/src/main/java/org/opensearch/index/query/DistanceFeatureQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/DistanceFeatureQueryBuilder.java @@ -57,7 +57,7 @@ * * @opensearch.internal */ -public class DistanceFeatureQueryBuilder extends AbstractQueryBuilder { +public class DistanceFeatureQueryBuilder extends AbstractQueryBuilder implements WithFieldName { public static final String NAME = "distance_feature"; private static final ParseField FIELD_FIELD = new ParseField("field"); @@ -136,7 +136,8 @@ protected Query doToQuery(QueryShardContext context) throws IOException { return fieldType.distanceFeatureQuery(origin.origin(), pivot, 1.0f, context); } - String fieldName() { + @Override + public String fieldName() { return field; } diff --git a/server/src/main/java/org/opensearch/index/query/ExistsQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/ExistsQueryBuilder.java index 6ae40fe1b1e64..f6b24e98f0f71 100644 --- a/server/src/main/java/org/opensearch/index/query/ExistsQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/ExistsQueryBuilder.java @@ -59,7 +59,7 @@ * * @opensearch.internal */ -public class ExistsQueryBuilder extends AbstractQueryBuilder { +public class ExistsQueryBuilder extends AbstractQueryBuilder implements WithFieldName { public static final String NAME = "exists"; public static final ParseField FIELD_FIELD = new ParseField("field"); @@ -89,6 +89,7 @@ protected void doWriteTo(StreamOutput out) throws IOException { /** * @return the field name that has to exist for this query to match */ + @Override public String fieldName() { return this.fieldName; } diff --git a/server/src/main/java/org/opensearch/index/query/FieldMaskingSpanQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/FieldMaskingSpanQueryBuilder.java index 4e73d87b07b7a..7846098b55071 100644 --- a/server/src/main/java/org/opensearch/index/query/FieldMaskingSpanQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/FieldMaskingSpanQueryBuilder.java @@ -53,7 +53,10 @@ * * @opensearch.internal */ -public class FieldMaskingSpanQueryBuilder extends AbstractQueryBuilder implements SpanQueryBuilder { +public class FieldMaskingSpanQueryBuilder extends AbstractQueryBuilder + implements + SpanQueryBuilder, + WithFieldName { public static final String NAME = "span_field_masking"; public static final ParseField SPAN_FIELD_MASKING_FIELD = new ParseField(NAME, "field_masking_span"); @@ -100,6 +103,7 @@ protected void doWriteTo(StreamOutput out) throws IOException { /** * @return the field name for this query */ + @Override public String fieldName() { return this.fieldName; } diff --git a/server/src/main/java/org/opensearch/index/query/GeoBoundingBoxQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/GeoBoundingBoxQueryBuilder.java index 1fade8601e2a6..52e96159cf06e 100644 --- a/server/src/main/java/org/opensearch/index/query/GeoBoundingBoxQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/GeoBoundingBoxQueryBuilder.java @@ -66,7 +66,7 @@ * * @opensearch.internal * */ -public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder { +public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder implements WithFieldName { public static final String NAME = "geo_bounding_box"; /** Default type for executing this query (memory as of this writing). */ @@ -263,6 +263,7 @@ public GeoExecType type() { } /** Returns the name of the field to base the bounding box computation on. */ + @Override public String fieldName() { return this.fieldName; } diff --git a/server/src/main/java/org/opensearch/index/query/GeoDistanceQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/GeoDistanceQueryBuilder.java index 8d126f19a204c..79377ba01701f 100644 --- a/server/src/main/java/org/opensearch/index/query/GeoDistanceQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/GeoDistanceQueryBuilder.java @@ -63,7 +63,7 @@ * * @opensearch.internal */ -public class GeoDistanceQueryBuilder extends AbstractQueryBuilder { +public class GeoDistanceQueryBuilder extends AbstractQueryBuilder implements WithFieldName { public static final String NAME = "geo_distance"; /** Default for distance unit computation. */ @@ -129,6 +129,7 @@ protected void doWriteTo(StreamOutput out) throws IOException { } /** Name of the field this query is operating on. */ + @Override public String fieldName() { return this.fieldName; } diff --git a/server/src/main/java/org/opensearch/index/query/GeoPolygonQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/GeoPolygonQueryBuilder.java index 47eafa3893384..e120f04ed9351 100644 --- a/server/src/main/java/org/opensearch/index/query/GeoPolygonQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/GeoPolygonQueryBuilder.java @@ -61,7 +61,7 @@ * * @opensearch.internal */ -public class GeoPolygonQueryBuilder extends AbstractQueryBuilder { +public class GeoPolygonQueryBuilder extends AbstractQueryBuilder implements WithFieldName { public static final String NAME = "geo_polygon"; /** @@ -131,6 +131,7 @@ protected void doWriteTo(StreamOutput out) throws IOException { out.writeBoolean(ignoreUnmapped); } + @Override public String fieldName() { return fieldName; } diff --git a/server/src/main/java/org/opensearch/index/query/MatchBoolPrefixQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/MatchBoolPrefixQueryBuilder.java index 7ceb17203e837..2c2c2de943e2f 100644 --- a/server/src/main/java/org/opensearch/index/query/MatchBoolPrefixQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/MatchBoolPrefixQueryBuilder.java @@ -61,7 +61,7 @@ * * @opensearch.internal */ -public class MatchBoolPrefixQueryBuilder extends AbstractQueryBuilder { +public class MatchBoolPrefixQueryBuilder extends AbstractQueryBuilder implements WithFieldName { public static final String NAME = "match_bool_prefix"; @@ -127,6 +127,7 @@ protected void doWriteTo(StreamOutput out) throws IOException { } /** Returns the field name used in this query. */ + @Override public String fieldName() { return this.fieldName; } diff --git a/server/src/main/java/org/opensearch/index/query/MatchPhrasePrefixQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/MatchPhrasePrefixQueryBuilder.java index d61a5957627ea..3337a31658ca1 100644 --- a/server/src/main/java/org/opensearch/index/query/MatchPhrasePrefixQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/MatchPhrasePrefixQueryBuilder.java @@ -52,7 +52,7 @@ * * @opensearch.internal */ -public class MatchPhrasePrefixQueryBuilder extends AbstractQueryBuilder { +public class MatchPhrasePrefixQueryBuilder extends AbstractQueryBuilder implements WithFieldName { public static final String NAME = "match_phrase_prefix"; public static final ParseField MAX_EXPANSIONS_FIELD = new ParseField("max_expansions"); public static final ParseField ZERO_TERMS_QUERY_FIELD = new ParseField("zero_terms_query"); @@ -104,6 +104,7 @@ protected void doWriteTo(StreamOutput out) throws IOException { } /** Returns the field name used in this query. */ + @Override public String fieldName() { return this.fieldName; } diff --git a/server/src/main/java/org/opensearch/index/query/MatchPhraseQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/MatchPhraseQueryBuilder.java index 6cdf6c6600304..97e03d53d38f1 100644 --- a/server/src/main/java/org/opensearch/index/query/MatchPhraseQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/MatchPhraseQueryBuilder.java @@ -52,7 +52,7 @@ * * @opensearch.internal */ -public class MatchPhraseQueryBuilder extends AbstractQueryBuilder { +public class MatchPhraseQueryBuilder extends AbstractQueryBuilder implements WithFieldName { public static final String NAME = "match_phrase"; public static final ParseField SLOP_FIELD = new ParseField("slop"); public static final ParseField ZERO_TERMS_QUERY_FIELD = new ParseField("zero_terms_query"); @@ -100,6 +100,7 @@ protected void doWriteTo(StreamOutput out) throws IOException { } /** Returns the field name used in this query. */ + @Override public String fieldName() { return this.fieldName; } diff --git a/server/src/main/java/org/opensearch/index/query/MatchQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/MatchQueryBuilder.java index 5e9e6a3660e76..593077d18951e 100644 --- a/server/src/main/java/org/opensearch/index/query/MatchQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/MatchQueryBuilder.java @@ -56,7 +56,7 @@ * * @opensearch.internal */ -public class MatchQueryBuilder extends AbstractQueryBuilder { +public class MatchQueryBuilder extends AbstractQueryBuilder implements WithFieldName { private static final String CUTOFF_FREQUENCY_DEPRECATION_MSG = "you can omit this option, " + "the [match] query can skip block of documents efficiently if the total number of hits is not tracked"; @@ -171,6 +171,7 @@ protected void doWriteTo(StreamOutput out) throws IOException { } /** Returns the field name used in this query. */ + @Override public String fieldName() { return this.fieldName; } diff --git a/server/src/main/java/org/opensearch/index/query/MultiTermQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/MultiTermQueryBuilder.java index b8e88da4741bb..f854df79f1ee8 100644 --- a/server/src/main/java/org/opensearch/index/query/MultiTermQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/MultiTermQueryBuilder.java @@ -36,9 +36,4 @@ * * @opensearch.internal */ -public interface MultiTermQueryBuilder extends QueryBuilder { - /** - * Get the field name for this query. - */ - String fieldName(); -} +public interface MultiTermQueryBuilder extends QueryBuilder, WithFieldName {} diff --git a/server/src/main/java/org/opensearch/index/query/SpanNearQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/SpanNearQueryBuilder.java index 30a1c29c29126..179673f500a92 100644 --- a/server/src/main/java/org/opensearch/index/query/SpanNearQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/SpanNearQueryBuilder.java @@ -322,7 +322,7 @@ public void visit(QueryBuilderVisitor visitor) { * * @opensearch.internal */ - public static class SpanGapQueryBuilder implements SpanQueryBuilder { + public static class SpanGapQueryBuilder implements SpanQueryBuilder, WithFieldName { public static final String NAME = "span_gap"; /** Name of field to match against. */ @@ -358,6 +358,7 @@ public SpanGapQueryBuilder(StreamInput in) throws IOException { /** * @return fieldName The name of the field */ + @Override public String fieldName() { return fieldName; } diff --git a/server/src/main/java/org/opensearch/index/query/TermsQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/TermsQueryBuilder.java index 4b92d6a1f5460..dbd141e00f81c 100644 --- a/server/src/main/java/org/opensearch/index/query/TermsQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/TermsQueryBuilder.java @@ -79,7 +79,7 @@ * * @opensearch.internal */ -public class TermsQueryBuilder extends AbstractQueryBuilder { +public class TermsQueryBuilder extends AbstractQueryBuilder implements WithFieldName { public static final String NAME = "terms"; private final String fieldName; @@ -269,6 +269,7 @@ protected void doWriteTo(StreamOutput out) throws IOException { } } + @Override public String fieldName() { return this.fieldName; } diff --git a/server/src/main/java/org/opensearch/index/query/WithFieldName.java b/server/src/main/java/org/opensearch/index/query/WithFieldName.java new file mode 100644 index 0000000000000..adfd789ac3707 --- /dev/null +++ b/server/src/main/java/org/opensearch/index/query/WithFieldName.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.query; + +/** + * Interface for classes with a fieldName method + * + * @opensearch.internal + */ +public interface WithFieldName { + /** + * Get the field name for this query. + */ + String fieldName(); +} From 8f34ce5ca3849687d509205bfe4378e1183eb6fb Mon Sep 17 00:00:00 2001 From: Finn Date: Thu, 5 Sep 2024 12:01:43 -0700 Subject: [PATCH 2/5] Mitigation for remote snapshot filecache overflow (#15077) TransferManager fails BlobFetchRequest on full cache Signed-off-by: Finn Carroll --- CHANGELOG.md | 1 + .../store/remote/utils/TransferManager.java | 13 +++++++ .../remote/utils/TransferManagerTestCase.java | 39 ++++++++----------- 3 files changed, 30 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 06a749df1b3a7..409e152ac60af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -112,6 +112,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fix unchecked cast in dynamic action map getter ([#15394](https://github.com/opensearch-project/OpenSearch/pull/15394)) - Fix null values indexed as "null" strings in flat_object field ([#14069](https://github.com/opensearch-project/OpenSearch/pull/14069)) - Fix terms query on wildcard field returns nothing ([#15607](https://github.com/opensearch-project/OpenSearch/pull/15607)) +- Fix remote snapshot file_cache exceeding capacity ([#15077](https://github.com/opensearch-project/OpenSearch/pull/15077)) ### Security diff --git a/server/src/main/java/org/opensearch/index/store/remote/utils/TransferManager.java b/server/src/main/java/org/opensearch/index/store/remote/utils/TransferManager.java index f07c4832d982c..94c25202ac90c 100644 --- a/server/src/main/java/org/opensearch/index/store/remote/utils/TransferManager.java +++ b/server/src/main/java/org/opensearch/index/store/remote/utils/TransferManager.java @@ -95,6 +95,19 @@ public IndexInput fetchBlob(BlobFetchRequest blobFetchRequest) throws IOExceptio @SuppressWarnings("removal") private static FileCachedIndexInput createIndexInput(FileCache fileCache, StreamReader streamReader, BlobFetchRequest request) { try { + // This local file cache is ref counted and may not strictly enforce configured capacity. + // If we find available capacity is exceeded, deny further BlobFetchRequests. + if (fileCache.capacity() < fileCache.usage().usage()) { + fileCache.prune(); + throw new IOException( + "Local file cache capacity (" + + fileCache.capacity() + + ") exceeded (" + + fileCache.usage().usage() + + ") - BlobFetchRequest failed: " + + request.getFilePath() + ); + } if (Files.exists(request.getFilePath()) == false) { logger.trace("Fetching from Remote in createIndexInput of Transfer Manager"); try ( diff --git a/server/src/test/java/org/opensearch/index/store/remote/utils/TransferManagerTestCase.java b/server/src/test/java/org/opensearch/index/store/remote/utils/TransferManagerTestCase.java index 810a4c336fdf7..1eae5119ab462 100644 --- a/server/src/test/java/org/opensearch/index/store/remote/utils/TransferManagerTestCase.java +++ b/server/src/test/java/org/opensearch/index/store/remote/utils/TransferManagerTestCase.java @@ -99,7 +99,7 @@ public void testConcurrentAccess() throws Exception { } } - public void testFetchBlobWithConcurrentCacheEvictions() throws Exception { + public void testFetchBlobWithConcurrentCacheEvictions() { // Submit 256 tasks to an executor with 16 threads that will each randomly // request one of eight blobs. Given that the cache can only hold two // blobs this will lead to a huge amount of contention and thrashing. @@ -114,41 +114,34 @@ public void testFetchBlobWithConcurrentCacheEvictions() throws Exception { try (IndexInput indexInput = fetchBlobWithName(blobname)) { assertIndexInputIsFunctional(indexInput); } + } catch (IOException ignored) { // fetchBlobWithName may fail due to fixed capacity } catch (Exception e) { throw new AssertionError(e); } })); } // Wait for all threads to complete - for (Future future : futures) { - future.get(10, TimeUnit.SECONDS); + try { + for (Future future : futures) { + future.get(10, TimeUnit.SECONDS); + } + } catch (java.util.concurrent.ExecutionException ignored) { // Index input may be null + } catch (Exception e) { + throw new AssertionError(e); } + } finally { assertTrue(terminate(testRunner)); } MatcherAssert.assertThat("Expected many evictions to happen", fileCache.stats().evictionCount(), greaterThan(0L)); } - public void testUsageExceedsCapacity() throws Exception { - // Fetch resources that exceed the configured capacity of the cache and assert that the - // returned IndexInputs are still functional. - try (IndexInput i1 = fetchBlobWithName("1"); IndexInput i2 = fetchBlobWithName("2"); IndexInput i3 = fetchBlobWithName("3")) { - assertIndexInputIsFunctional(i1); - assertIndexInputIsFunctional(i2); - assertIndexInputIsFunctional(i3); - MatcherAssert.assertThat(fileCache.usage().activeUsage(), equalTo((long) EIGHT_MB * 3)); - MatcherAssert.assertThat(fileCache.usage().usage(), equalTo((long) EIGHT_MB * 3)); - } - MatcherAssert.assertThat(fileCache.usage().activeUsage(), equalTo(0L)); - MatcherAssert.assertThat(fileCache.usage().usage(), equalTo((long) EIGHT_MB * 3)); - // Fetch another resource which will trigger an eviction - try (IndexInput i1 = fetchBlobWithName("1")) { - assertIndexInputIsFunctional(i1); - MatcherAssert.assertThat(fileCache.usage().activeUsage(), equalTo((long) EIGHT_MB)); - MatcherAssert.assertThat(fileCache.usage().usage(), equalTo((long) EIGHT_MB)); - } - MatcherAssert.assertThat(fileCache.usage().activeUsage(), equalTo(0L)); - MatcherAssert.assertThat(fileCache.usage().usage(), equalTo((long) EIGHT_MB)); + public void testOverflowDisabled() throws Exception { + initializeTransferManager(); + IndexInput i1 = fetchBlobWithName("1"); + IndexInput i2 = fetchBlobWithName("2"); + + assertThrows(IOException.class, () -> { IndexInput i3 = fetchBlobWithName("3"); }); } public void testDownloadFails() throws Exception { From 375dda3e07b2c37450c4cb19b6722e04a1ce64f5 Mon Sep 17 00:00:00 2001 From: Sachin Kale Date: Fri, 6 Sep 2024 00:41:54 +0530 Subject: [PATCH 3/5] Mute flaky test RemoteFsTimestampAwareTranslogTests.testSimpleOperationsUpload (#15732) Signed-off-by: Sachin Kale Co-authored-by: Sachin Kale --- .../index/translog/RemoteFsTimestampAwareTranslogTests.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/test/java/org/opensearch/index/translog/RemoteFsTimestampAwareTranslogTests.java b/server/src/test/java/org/opensearch/index/translog/RemoteFsTimestampAwareTranslogTests.java index c510a6475147d..4c9da7e95dfa7 100644 --- a/server/src/test/java/org/opensearch/index/translog/RemoteFsTimestampAwareTranslogTests.java +++ b/server/src/test/java/org/opensearch/index/translog/RemoteFsTimestampAwareTranslogTests.java @@ -310,6 +310,7 @@ public void testIndexDeletionWithNoPinnedTimestampButRecentFiles() throws Except } @Override + @AwaitsFix(bugUrl = "https://github.com/opensearch-project/OpenSearch/issues/15731") public void testSimpleOperationsUpload() throws Exception { ArrayList ops = new ArrayList<>(); From fe61e4f299d4b832dc232cb223ec946585177f52 Mon Sep 17 00:00:00 2001 From: Andriy Redko Date: Thu, 5 Sep 2024 16:42:45 -0400 Subject: [PATCH 4/5] Fixing MacOS 13 assemble workflows (#15747) Signed-off-by: Andriy Redko --- .github/workflows/assemble.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/assemble.yml b/.github/workflows/assemble.yml index 294627622a136..b3838b8e5ae97 100644 --- a/.github/workflows/assemble.yml +++ b/.github/workflows/assemble.yml @@ -30,8 +30,11 @@ jobs: - name: Setup docker (missing on MacOS) id: setup_docker if: runner.os == 'macos' + continue-on-error: true run: | - exit 0; + brew install docker colima coreutils + gtimeout 15m colima start + shell: bash - name: Run Gradle (assemble) if: runner.os == 'macos' && steps.setup_docker.outcome != 'success' run: | @@ -45,4 +48,4 @@ jobs: - name: Run Gradle (assemble) if: runner.os == 'macos' && steps.setup_docker.outcome == 'success' run: | - exit 0; + ./gradlew assemble --parallel --no-build-cache -PDISABLE_BUILD_CACHE -Druntime.java=${{ matrix.java }} From 3fdbd8b7308149d5432f8d04b8040e15a934c906 Mon Sep 17 00:00:00 2001 From: Shivansh Arora Date: Fri, 6 Sep 2024 07:25:02 +0530 Subject: [PATCH 5/5] Make Remote Publication a static setting (#15478) * Make Remote Publication a static setting Signed-off-by: Shivansh Arora --- CHANGELOG.md | 1 + .../RemoteClusterStateCleanupManagerIT.java | 4 ++-- .../remote/RemoteRoutingTableServiceIT.java | 4 ++-- .../remote/RemoteStatePublicationIT.java | 18 ++++++------------ .../coordination/CoordinationState.java | 7 +++---- .../common/settings/ClusterSettings.java | 1 + .../common/settings/FeatureFlagSettings.java | 1 - .../opensearch/common/util/FeatureFlags.java | 12 ------------ .../remote/RemoteClusterStateService.java | 16 +++++++++++++--- .../coordination/CoordinationStateTests.java | 4 ++-- .../RemoteRoutingTableServiceFactoryTests.java | 6 ++---- .../remote/RemoteRoutingTableServiceTests.java | 6 ++---- .../remote/RemoteClusterStateServiceTests.java | 18 ++++++------------ 13 files changed, 40 insertions(+), 58 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 409e152ac60af..26358d269a2ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Reset DiscoveryNodes in all transport node actions request ([#15131](https://github.com/opensearch-project/OpenSearch/pull/15131)) - Adding WithFieldName interface for QueryBuilders with fieldName ([#15705](https://github.com/opensearch-project/OpenSearch/pull/15705)) - Relax the join validation for Remote State publication ([#15471](https://github.com/opensearch-project/OpenSearch/pull/15471)) +- Static RemotePublication setting added, removed experimental feature flag ([#15478](https://github.com/opensearch-project/OpenSearch/pull/15478)) - MultiTermQueries in keyword fields now default to `indexed` approach and gated behind cluster setting ([#15637](https://github.com/opensearch-project/OpenSearch/pull/15637)) - Making _cat/allocation API use indexLevelStats ([#15292](https://github.com/opensearch-project/OpenSearch/pull/15292)) diff --git a/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteClusterStateCleanupManagerIT.java b/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteClusterStateCleanupManagerIT.java index 47ec3f25bcd64..cf17a58d937de 100644 --- a/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteClusterStateCleanupManagerIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteClusterStateCleanupManagerIT.java @@ -35,12 +35,12 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; -import static org.opensearch.common.util.FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL; import static org.opensearch.gateway.remote.RemoteClusterStateCleanupManager.CLUSTER_STATE_CLEANUP_INTERVAL_DEFAULT; import static org.opensearch.gateway.remote.RemoteClusterStateCleanupManager.REMOTE_CLUSTER_STATE_CLEANUP_INTERVAL_SETTING; import static org.opensearch.gateway.remote.RemoteClusterStateCleanupManager.RETAINED_MANIFESTS; import static org.opensearch.gateway.remote.RemoteClusterStateCleanupManager.SKIP_CLEANUP_STATE_CHANGES; import static org.opensearch.gateway.remote.RemoteClusterStateService.REMOTE_CLUSTER_STATE_ENABLED_SETTING; +import static org.opensearch.gateway.remote.RemoteClusterStateService.REMOTE_PUBLICATION_SETTING_KEY; import static org.opensearch.gateway.remote.RemoteUploadStats.REMOTE_UPLOAD; import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTable.INDEX_ROUTING_TABLE; import static org.opensearch.indices.IndicesService.CLUSTER_DEFAULT_INDEX_REFRESH_INTERVAL_SETTING; @@ -189,7 +189,7 @@ public void testRemoteCleanupDeleteStaleIndexRoutingFiles() throws Exception { RemoteStoreEnums.PathType.HASHED_PREFIX.toString() ) .put("node.attr." + REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, REMOTE_ROUTING_TABLE_REPO) - .put(REMOTE_PUBLICATION_EXPERIMENTAL, true); + .put(REMOTE_PUBLICATION_SETTING_KEY, true); int shardCount = randomIntBetween(1, 2); int replicaCount = 1; diff --git a/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteRoutingTableServiceIT.java b/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteRoutingTableServiceIT.java index 0a8c13adb034f..d143cbd7c3450 100644 --- a/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteRoutingTableServiceIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteRoutingTableServiceIT.java @@ -37,8 +37,8 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; -import static org.opensearch.common.util.FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL; import static org.opensearch.gateway.remote.RemoteClusterStateService.REMOTE_CLUSTER_STATE_ENABLED_SETTING; +import static org.opensearch.gateway.remote.RemoteClusterStateService.REMOTE_PUBLICATION_SETTING_KEY; import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTable.INDEX_ROUTING_TABLE; import static org.opensearch.indices.IndicesService.CLUSTER_DEFAULT_INDEX_REFRESH_INTERVAL_SETTING; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY; @@ -66,7 +66,7 @@ protected Settings nodeSettings(int nodeOrdinal) { RemoteStoreEnums.PathType.HASHED_PREFIX.toString() ) .put("node.attr." + REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, REMOTE_ROUTING_TABLE_REPO) - .put(REMOTE_PUBLICATION_EXPERIMENTAL, true) + .put(REMOTE_PUBLICATION_SETTING_KEY, true) .put( RemoteClusterStateService.REMOTE_CLUSTER_STATE_CHECKSUM_VALIDATION_MODE_SETTING.getKey(), RemoteClusterStateService.RemoteClusterStateValidationMode.FAILURE diff --git a/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteStatePublicationIT.java b/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteStatePublicationIT.java index 0778782c5bec5..7e7d3060ec7a9 100644 --- a/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteStatePublicationIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteStatePublicationIT.java @@ -49,10 +49,10 @@ import static org.opensearch.action.admin.cluster.node.info.NodesInfoRequest.Metric.SETTINGS; import static org.opensearch.action.admin.cluster.node.stats.NodesStatsRequest.Metric.DISCOVERY; import static org.opensearch.cluster.metadata.Metadata.isGlobalStateEquals; -import static org.opensearch.common.util.FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL; -import static org.opensearch.common.util.FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL_SETTING; import static org.opensearch.gateway.remote.RemoteClusterStateAttributesManager.DISCOVERY_NODES; import static org.opensearch.gateway.remote.RemoteClusterStateService.REMOTE_CLUSTER_STATE_ENABLED_SETTING; +import static org.opensearch.gateway.remote.RemoteClusterStateService.REMOTE_PUBLICATION_SETTING; +import static org.opensearch.gateway.remote.RemoteClusterStateService.REMOTE_PUBLICATION_SETTING_KEY; import static org.opensearch.gateway.remote.RemoteClusterStateUtils.DELIMITER; import static org.opensearch.gateway.remote.model.RemoteClusterBlocks.CLUSTER_BLOCKS; import static org.opensearch.gateway.remote.model.RemoteCoordinationMetadata.COORDINATION_METADATA; @@ -87,11 +87,6 @@ public void setup() { hasRemoteRoutingCharPrefix = randomBoolean(); } - @Override - protected Settings featureFlagSettings() { - return Settings.builder().put(super.featureFlagSettings()).put(REMOTE_PUBLICATION_EXPERIMENTAL, isRemotePublicationEnabled).build(); - } - @Override protected Settings nodeSettings(int nodeOrdinal) { String routingTableRepoName = "remote-routing-repo"; @@ -109,6 +104,7 @@ protected Settings nodeSettings(int nodeOrdinal) { return Settings.builder() .put(super.nodeSettings(nodeOrdinal)) .put(REMOTE_CLUSTER_STATE_ENABLED_SETTING.getKey(), isRemoteStateEnabled) + .put(REMOTE_PUBLICATION_SETTING_KEY, isRemotePublicationEnabled) .put("node.attr." + REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, routingTableRepoName) .put(routingTableRepoTypeAttributeKey, ReloadableFsRepository.TYPE) .put(routingTableRepoSettingsAttributeKeyPrefix + "location", segmentRepoPath) @@ -248,7 +244,7 @@ public void testRemotePublicationDisabledByRollingRestart() throws Exception { @Override public Settings onNodeStopped(String nodeName) { restartedMasters.add(nodeName); - return Settings.builder().put(REMOTE_PUBLICATION_EXPERIMENTAL, false).build(); + return Settings.builder().put(REMOTE_PUBLICATION_SETTING_KEY, false).build(); } @Override @@ -287,9 +283,7 @@ public void doAfterNodes(int n, Client client) { .addMetric(SETTINGS.metricName()) .get(); // if masterRestarted is true Publication Setting should be false, and vice versa - assertTrue( - REMOTE_PUBLICATION_EXPERIMENTAL_SETTING.get(nodesInfoResponse.getNodes().get(0).getSettings()) != activeCMRestarted - ); + assertTrue(REMOTE_PUBLICATION_SETTING.get(nodesInfoResponse.getNodes().get(0).getSettings()) != activeCMRestarted); followingCMs.forEach(node -> { PersistedStateRegistry registry = internalCluster().getInstance(PersistedStateRegistry.class, node); @@ -336,7 +330,7 @@ public void doAfterNodes(int n, Client client) { .addMetric(SETTINGS.metricName()) .get(); // if masterRestarted is true Publication Setting should be false, and vice versa - assertFalse(REMOTE_PUBLICATION_EXPERIMENTAL_SETTING.get(nodesInfoResponse.getNodes().get(0).getSettings())); + assertFalse(REMOTE_PUBLICATION_SETTING.get(nodesInfoResponse.getNodes().get(0).getSettings())); followingCMs.forEach(node -> { PersistedStateRegistry registry = internalCluster().getInstance(PersistedStateRegistry.class, node); diff --git a/server/src/main/java/org/opensearch/cluster/coordination/CoordinationState.java b/server/src/main/java/org/opensearch/cluster/coordination/CoordinationState.java index c7820c2c9a365..9c883175e3ee0 100644 --- a/server/src/main/java/org/opensearch/cluster/coordination/CoordinationState.java +++ b/server/src/main/java/org/opensearch/cluster/coordination/CoordinationState.java @@ -39,7 +39,6 @@ import org.opensearch.cluster.metadata.Metadata; import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.common.util.io.IOUtils; import java.io.Closeable; @@ -53,7 +52,7 @@ import java.util.Set; import static org.opensearch.cluster.coordination.Coordinator.ZEN1_BWC_TERM; -import static org.opensearch.common.util.FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL; +import static org.opensearch.gateway.remote.RemoteClusterStateService.REMOTE_PUBLICATION_SETTING; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteStoreClusterStateEnabled; /** @@ -81,7 +80,7 @@ public class CoordinationState { private VotingConfiguration lastPublishedConfiguration; private VoteCollection publishVotes; private final boolean isRemoteStateEnabled; - private final boolean isRemotePublicationEnabled; + private boolean isRemotePublicationEnabled; public CoordinationState( DiscoveryNode localNode, @@ -106,7 +105,7 @@ public CoordinationState( this.publishVotes = new VoteCollection(); this.isRemoteStateEnabled = isRemoteStoreClusterStateEnabled(settings); this.isRemotePublicationEnabled = isRemoteStateEnabled - && FeatureFlags.isEnabled(REMOTE_PUBLICATION_EXPERIMENTAL) + && REMOTE_PUBLICATION_SETTING.get(settings) && localNode.isRemoteStatePublicationEnabled(); } diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index e35aa42563d4d..09832e2b41b6d 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -734,6 +734,7 @@ public void apply(Settings value, Settings current, Settings previous) { // Remote cluster state settings RemoteClusterStateCleanupManager.REMOTE_CLUSTER_STATE_CLEANUP_INTERVAL_SETTING, RemoteClusterStateService.REMOTE_CLUSTER_STATE_ENABLED_SETTING, + RemoteClusterStateService.REMOTE_PUBLICATION_SETTING, INDEX_METADATA_UPLOAD_TIMEOUT_SETTING, GLOBAL_METADATA_UPLOAD_TIMEOUT_SETTING, METADATA_MANIFEST_UPLOAD_TIMEOUT_SETTING, diff --git a/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java b/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java index 9c7684923d06c..c8d00f65bda10 100644 --- a/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java @@ -37,7 +37,6 @@ protected FeatureFlagSettings( FeatureFlags.TIERED_REMOTE_INDEX_SETTING, FeatureFlags.REMOTE_STORE_MIGRATION_EXPERIMENTAL_SETTING, FeatureFlags.PLUGGABLE_CACHE_SETTING, - FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL_SETTING, FeatureFlags.STAR_TREE_INDEX_SETTING, FeatureFlags.APPLICATION_BASED_CONFIGURATION_TEMPLATES_SETTING, FeatureFlags.READER_WRITER_SPLIT_EXPERIMENTAL_SETTING diff --git a/server/src/main/java/org/opensearch/common/util/FeatureFlags.java b/server/src/main/java/org/opensearch/common/util/FeatureFlags.java index 0fd5edde2b94c..49ecbb0a7069d 100644 --- a/server/src/main/java/org/opensearch/common/util/FeatureFlags.java +++ b/server/src/main/java/org/opensearch/common/util/FeatureFlags.java @@ -67,11 +67,6 @@ public class FeatureFlags { */ public static final String PLUGGABLE_CACHE = "opensearch.experimental.feature.pluggable.caching.enabled"; - /** - * Gates the functionality of remote routing table. - */ - public static final String REMOTE_PUBLICATION_EXPERIMENTAL = "opensearch.experimental.feature.remote_store.publication.enabled"; - /** * Gates the functionality of background task execution. */ @@ -101,12 +96,6 @@ public class FeatureFlags { public static final Setting PLUGGABLE_CACHE_SETTING = Setting.boolSetting(PLUGGABLE_CACHE, false, Property.NodeScope); - public static final Setting REMOTE_PUBLICATION_EXPERIMENTAL_SETTING = Setting.boolSetting( - REMOTE_PUBLICATION_EXPERIMENTAL, - false, - Property.NodeScope - ); - public static final Setting READER_WRITER_SPLIT_EXPERIMENTAL_SETTING = Setting.boolSetting( READER_WRITER_SPLIT_EXPERIMENTAL, false, @@ -148,7 +137,6 @@ public class FeatureFlags { DATETIME_FORMATTER_CACHING_SETTING, TIERED_REMOTE_INDEX_SETTING, PLUGGABLE_CACHE_SETTING, - REMOTE_PUBLICATION_EXPERIMENTAL_SETTING, STAR_TREE_INDEX_SETTING, APPLICATION_BASED_CONFIGURATION_TEMPLATES_SETTING, READER_WRITER_SPLIT_EXPERIMENTAL_SETTING 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 fe34b68702c41..3425550a9f548 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -40,7 +40,6 @@ import org.opensearch.common.settings.Setting.Property; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.common.util.io.IOUtils; import org.opensearch.core.action.ActionListener; import org.opensearch.core.common.io.stream.NamedWriteableRegistry; @@ -92,7 +91,6 @@ import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; import static org.opensearch.cluster.ClusterState.CUSTOM_VALUE_SERIALIZER; -import static org.opensearch.common.util.FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL; import static org.opensearch.gateway.PersistedClusterStateService.SLOW_WRITE_LOGGING_THRESHOLD; import static org.opensearch.gateway.remote.ClusterMetadataManifest.CODEC_V2; import static org.opensearch.gateway.remote.ClusterMetadataManifest.CODEC_V3; @@ -123,6 +121,18 @@ public class RemoteClusterStateService implements Closeable { private static final Logger logger = LogManager.getLogger(RemoteClusterStateService.class); + /** + * Gates the functionality of remote publication. + */ + public static final String REMOTE_PUBLICATION_SETTING_KEY = "cluster.remote_store.publication.enabled"; + + public static final Setting REMOTE_PUBLICATION_SETTING = Setting.boolSetting( + REMOTE_PUBLICATION_SETTING_KEY, + false, + Property.NodeScope, + Property.Final + ); + /** * Used to specify if cluster state metadata should be published to remote store */ @@ -260,7 +270,7 @@ public RemoteClusterStateService( this.remoteStateStats = new RemotePersistenceStats(); this.namedWriteableRegistry = namedWriteableRegistry; this.indexMetadataUploadListeners = indexMetadataUploadListeners; - this.isPublicationEnabled = FeatureFlags.isEnabled(REMOTE_PUBLICATION_EXPERIMENTAL) + this.isPublicationEnabled = REMOTE_PUBLICATION_SETTING.get(settings) && RemoteStoreNodeAttribute.isRemoteStoreClusterStateEnabled(settings) && RemoteStoreNodeAttribute.isRemoteRoutingTableEnabled(settings); this.remotePathPrefix = CLUSTER_REMOTE_STORE_STATE_PATH_PREFIX.get(settings); diff --git a/server/src/test/java/org/opensearch/cluster/coordination/CoordinationStateTests.java b/server/src/test/java/org/opensearch/cluster/coordination/CoordinationStateTests.java index ee9a2951ec541..d003b54adcccc 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/CoordinationStateTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/CoordinationStateTests.java @@ -66,9 +66,9 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.emptySet; -import static org.opensearch.common.util.FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL; import static org.opensearch.gateway.remote.ClusterMetadataManifest.MANIFEST_CURRENT_CODEC_VERSION; import static org.opensearch.gateway.remote.RemoteClusterStateService.REMOTE_CLUSTER_STATE_ENABLED_SETTING; +import static org.opensearch.gateway.remote.RemoteClusterStateService.REMOTE_PUBLICATION_SETTING_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY; 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; @@ -1010,7 +1010,7 @@ public void testIsRemotePublicationEnabled_WithInconsistentSettings() { // create settings with remote state disabled but publication enabled Settings settings = Settings.builder() .put(REMOTE_CLUSTER_STATE_ENABLED_SETTING.getKey(), false) - .put(REMOTE_PUBLICATION_EXPERIMENTAL, true) + .put(REMOTE_PUBLICATION_SETTING_KEY, true) .build(); CoordinationState coordinationState = createCoordinationState(psr1, node1, settings); assertFalse(coordinationState.isRemotePublicationEnabled()); diff --git a/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceFactoryTests.java b/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceFactoryTests.java index 86f4b9502d6ab..683942fd34a37 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceFactoryTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceFactoryTests.java @@ -10,7 +10,6 @@ import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.repositories.RepositoriesService; import org.opensearch.repositories.fs.FsRepository; import org.opensearch.test.OpenSearchTestCase; @@ -20,7 +19,7 @@ import java.util.function.Supplier; -import static org.opensearch.common.util.FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL; +import static org.opensearch.gateway.remote.RemoteClusterStateService.REMOTE_PUBLICATION_SETTING_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY; public class RemoteRoutingTableServiceFactoryTests extends OpenSearchTestCase { @@ -50,9 +49,8 @@ public void testGetServiceWhenRemoteRoutingEnabled() { Settings settings = Settings.builder() .put("node.attr." + REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, "routing_repository") .put(FsRepository.REPOSITORIES_COMPRESS_SETTING.getKey(), false) + .put(REMOTE_PUBLICATION_SETTING_KEY, "true") .build(); - Settings nodeSettings = Settings.builder().put(REMOTE_PUBLICATION_EXPERIMENTAL, "true").build(); - FeatureFlags.initializeFeatureFlags(nodeSettings); RemoteRoutingTableService service = RemoteRoutingTableServiceFactory.getService( repositoriesService, settings, diff --git a/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java b/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java index 5061de9161ab4..63501f878d55d 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java @@ -28,7 +28,6 @@ import org.opensearch.common.compress.DeflateCompressor; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.common.util.TestCapturingListener; import org.opensearch.core.action.ActionListener; import org.opensearch.core.compress.Compressor; @@ -63,8 +62,8 @@ import org.mockito.Mockito; -import static org.opensearch.common.util.FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL; import static org.opensearch.gateway.remote.ClusterMetadataManifestTests.randomUploadedIndexMetadataList; +import static org.opensearch.gateway.remote.RemoteClusterStateService.REMOTE_PUBLICATION_SETTING_KEY; import static org.opensearch.gateway.remote.RemoteClusterStateServiceTests.generateClusterStateWithOneIndex; import static org.opensearch.gateway.remote.RemoteClusterStateUtils.CLUSTER_STATE_PATH_TOKEN; import static org.opensearch.gateway.remote.RemoteClusterStateUtils.DELIMITER; @@ -114,6 +113,7 @@ public void setup() { Settings settings = Settings.builder() .put("node.attr." + REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, "routing_repository") + .put(REMOTE_PUBLICATION_SETTING_KEY, "true") .build(); clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); clusterService = mock(ClusterService.class); @@ -126,8 +126,6 @@ public void setup() { when(repositoriesService.repository("routing_repository")).thenReturn(blobStoreRepository); when(blobStoreRepository.blobStore()).thenReturn(blobStore); when(blobStore.blobContainer(any())).thenReturn(blobContainer); - Settings nodeSettings = Settings.builder().put(REMOTE_PUBLICATION_EXPERIMENTAL, "true").build(); - FeatureFlags.initializeFeatureFlags(nodeSettings); compressor = new NoneCompressor(); basePath = BlobPath.cleanPath().add("base-path"); when(blobStoreRepository.basePath()).thenReturn(basePath); 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 21b88e5bd66b9..608cc2e12b055 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -107,12 +107,12 @@ import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; import static java.util.stream.Collectors.toList; -import static org.opensearch.common.util.FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL; import static org.opensearch.gateway.remote.ClusterMetadataManifest.CODEC_V1; import static org.opensearch.gateway.remote.ClusterMetadataManifest.CODEC_V2; import static org.opensearch.gateway.remote.ClusterMetadataManifest.MANIFEST_CURRENT_CODEC_VERSION; import static org.opensearch.gateway.remote.RemoteClusterStateAttributesManager.CLUSTER_BLOCKS; import static org.opensearch.gateway.remote.RemoteClusterStateAttributesManager.CLUSTER_STATE_ATTRIBUTE; +import static org.opensearch.gateway.remote.RemoteClusterStateService.REMOTE_PUBLICATION_SETTING_KEY; import static org.opensearch.gateway.remote.RemoteClusterStateTestUtils.CustomMetadata1; import static org.opensearch.gateway.remote.RemoteClusterStateTestUtils.CustomMetadata2; import static org.opensearch.gateway.remote.RemoteClusterStateTestUtils.CustomMetadata3; @@ -273,8 +273,6 @@ public void teardown() throws Exception { super.tearDown(); remoteClusterStateService.close(); publicationEnabled = false; - Settings nodeSettings = Settings.builder().build(); - FeatureFlags.initializeFeatureFlags(nodeSettings); threadPool.shutdown(); } @@ -371,8 +369,7 @@ public void testWriteFullMetadataSuccess() throws IOException { public void testWriteFullMetadataSuccessPublicationEnabled() throws IOException { // TODO Make the publication flag parameterized publicationEnabled = true; - Settings nodeSettings = Settings.builder().put(REMOTE_PUBLICATION_EXPERIMENTAL, publicationEnabled).build(); - FeatureFlags.initializeFeatureFlags(nodeSettings); + settings = Settings.builder().put(settings).put(REMOTE_PUBLICATION_SETTING_KEY, publicationEnabled).build(); remoteClusterStateService = new RemoteClusterStateService( "test-node-id", repositoriesServiceSupplier, @@ -749,8 +746,7 @@ public void testWriteIncrementalMetadataSuccess() throws IOException { public void testWriteIncrementalMetadataSuccessWhenPublicationEnabled() throws IOException { publicationEnabled = true; - Settings nodeSettings = Settings.builder().put(REMOTE_PUBLICATION_EXPERIMENTAL, publicationEnabled).build(); - FeatureFlags.initializeFeatureFlags(nodeSettings); + settings = Settings.builder().put(settings).put(REMOTE_PUBLICATION_SETTING_KEY, true).build(); remoteClusterStateService = new RemoteClusterStateService( "test-node-id", repositoriesServiceSupplier, @@ -2658,7 +2654,7 @@ public void testRemoteRoutingTableInitializedWhenEnabled() { .build(); clusterSettings.applySettings(newSettings); - Settings nodeSettings = Settings.builder().put(REMOTE_PUBLICATION_EXPERIMENTAL, "true").build(); + Settings nodeSettings = Settings.builder().put(REMOTE_PUBLICATION_SETTING_KEY, "true").build(); FeatureFlags.initializeFeatureFlags(nodeSettings); remoteClusterStateService = new RemoteClusterStateService( @@ -2935,11 +2931,10 @@ private void initializeRoutingTable() { .put("node.attr." + REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, "routing_repository") .put("node.attr." + REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY, "remote_store_repository") .put(RemoteClusterStateService.REMOTE_CLUSTER_STATE_ENABLED_SETTING.getKey(), true) + .put(REMOTE_PUBLICATION_SETTING_KEY, "true") .build(); clusterSettings.applySettings(newSettings); - Settings nodeSettings = Settings.builder().put(REMOTE_PUBLICATION_EXPERIMENTAL, "true").build(); - FeatureFlags.initializeFeatureFlags(nodeSettings); remoteClusterStateService = new RemoteClusterStateService( "test-node-id", repositoriesServiceSupplier, @@ -2966,11 +2961,10 @@ private void initializeWithChecksumEnabled(RemoteClusterStateService.RemoteClust .put("node.attr." + REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY, "remote_store_repository") .put(RemoteClusterStateService.REMOTE_CLUSTER_STATE_ENABLED_SETTING.getKey(), true) .put(RemoteClusterStateService.REMOTE_CLUSTER_STATE_CHECKSUM_VALIDATION_MODE_SETTING.getKey(), mode.name()) + .put(REMOTE_PUBLICATION_SETTING_KEY, true) .build(); clusterSettings.applySettings(newSettings); - Settings nodeSettings = Settings.builder().put(REMOTE_PUBLICATION_EXPERIMENTAL, "true").build(); - FeatureFlags.initializeFeatureFlags(nodeSettings); remoteClusterStateService = new RemoteClusterStateService( "test-node-id", repositoriesServiceSupplier,