Merge branch 'main' into fix_uploaded_metadata

Signed-off-by: Sooraj Sinha <81695996+soosinha@users.noreply.github.com>
opensearch-project · Oct 23, 2024 · bba8f04 · bba8f04
2 parents e9288b4 + 66f0110
commit bba8f04
Show file tree

Hide file tree

Showing 12 changed files with 133 additions and 24 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -85,6 +85,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Streaming bulk request hangs ([#16158](https://github.com/opensearch-project/OpenSearch/pull/16158))
 - Fix warnings from SLF4J on startup when repository-s3 is installed ([#16194](https://github.com/opensearch-project/OpenSearch/pull/16194))
 - Fix protobuf-java leak through client library dependencies ([#16254](https://github.com/opensearch-project/OpenSearch/pull/16254))
+- Fix get index settings API doesn't show `number_of_routing_shards` setting when it was explicitly set ([#16294](https://github.com/opensearch-project/OpenSearch/pull/16294))
 - Fix multi-search with template doesn't return status code ([#16265](https://github.com/opensearch-project/OpenSearch/pull/16265))
 - [Streaming Indexing] Fix intermittent 'The bulk request must be terminated by a newline [\n]' failures [#16337](https://github.com/opensearch-project/OpenSearch/pull/16337))
 - Fix wrong default value when setting `index.number_of_routing_shards` to null on index creation ([#16331](https://github.com/opensearch-project/OpenSearch/pull/16331))
@@ -96,6 +97,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Fix typo super->sb in method toString() of RemoteStoreNodeAttribute ([#15362](https://github.com/opensearch-project/OpenSearch/pull/15362))
 - [Workload Management] Fixing Create/Update QueryGroup TransportActions to execute from non-cluster manager nodes ([16422](https://github.com/opensearch-project/OpenSearch/pull/16422))
 - Revert changes to upload remote state manifest using minimum codec version([#16403](https://github.com/opensearch-project/OpenSearch/pull/16403))
+- Fix flaky test in `testApproximateRangeWithSizeOverDefault` by adjusting totalHits assertion logic ([#16434](https://github.com/opensearch-project/OpenSearch/pull/16434#pullrequestreview-2386999409))
 
 ### Security
 

diff --git a/...sport-netty4/src/main/java/org/opensearch/transport/netty4/ssl/SecureNetty4Transport.java b/...sport-netty4/src/main/java/org/opensearch/transport/netty4/ssl/SecureNetty4Transport.java
@@ -146,6 +146,7 @@ protected void initChannel(Channel ch) throws Exception {
                 .map(SecureTransportSettingsProvider.SecureTransportParameters::dualModeEnabled)
                 .orElse(false);
             if (dualModeEnabled) {
+                logger.info("SSL Dual mode enabled, using port unification handler");
                 final ChannelHandler portUnificationHandler = new DualModeSslHandler(
                     settings,
                     secureTransportSettingsProvider,

diff --git a/qa/rolling-upgrade/src/test/java/org/opensearch/upgrades/ClusterStatsIT.java b/qa/rolling-upgrade/src/test/java/org/opensearch/upgrades/ClusterStatsIT.java
@@ -26,8 +26,8 @@ public class ClusterStatsIT extends AbstractRollingTestCase {
     public void testClusterStats() throws IOException {
         Response response = client().performRequest(new Request("GET", "/_cluster/stats"));
         validateClusterStatsWithFilterResponse(response, nodeStatsMetrics, indicesStatsMetrics);
-        if (AbstractRollingTestCase.UPGRADE_FROM_VERSION.onOrAfter(Version.V_3_0_0) || (
-            CLUSTER_TYPE == ClusterType.UPGRADED && Version.CURRENT.onOrAfter(Version.V_3_0_0))) {
+        if (AbstractRollingTestCase.UPGRADE_FROM_VERSION.onOrAfter(Version.V_2_18_0) || (
+            CLUSTER_TYPE == ClusterType.UPGRADED && Version.CURRENT.onOrAfter(Version.V_2_18_0))) {
             response = client().performRequest(new Request("GET", "/_cluster/stats/os/nodes/_all"));
             validateClusterStatsWithFilterResponse(response, List.of("os"), Collections.emptyList());
             response = client().performRequest(new Request("GET", "/_cluster/stats/indices/mappings/nodes/_all"));

diff --git a/...rc/main/resources/rest-api-spec/test/indices.get_settings/40_number_of_routing_shards.yml b/...rc/main/resources/rest-api-spec/test/indices.get_settings/40_number_of_routing_shards.yml
@@ -0,0 +1,41 @@
+---
+setup:
+  - do:
+      indices.create:
+        body:
+          settings:
+            index:
+              number_of_routing_shards: 4
+              number_of_shards: 2
+              number_of_replicas: 1
+        index: test-index
+
+  - do:
+      indices.create:
+        body:
+          settings:
+            index:
+              number_of_shards: 2
+              number_of_replicas: 1
+        index: test-index1
+
+---
+Test retrieval of number_routing_shards settings:
+  - skip:
+      version: " - 2.99.99"
+      reason: "introduced in 3.0.0" # TODO: change it to 2.18.0 after backport to 2.x branch
+  - do:
+      indices.get_settings:
+        flat_settings: true
+        index: test-index
+  # show `index.number_of_routing_shards` if it was explicitly set when creating
+  - match:
+      test-index.settings.index\.number_of_routing_shards: "4"
+
+  - do:
+      indices.get_settings:
+        flat_settings: true
+        index: test-index1
+  # do not show `index.number_of_routing_shards` if it was not explicitly set when creating
+  - match:
+      test-index1.settings.index\.number_of_routing_shards: null
diff --git a/...er/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java b/...er/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java
@@ -108,6 +108,12 @@ public void onResponse(ClusterStateResponse clusterStateResponse) {
                                 : paginationStrategy.getRequestedEntities()
                         );
                         catShardsResponse.setPageToken(Objects.isNull(paginationStrategy) ? null : paginationStrategy.getResponseToken());
+                        // For paginated queries, if strategy outputs no shards to be returned, avoid fetching IndicesStats.
+                        if (shouldSkipIndicesStatsRequest(paginationStrategy)) {
+                            catShardsResponse.setIndicesStatsResponse(IndicesStatsResponse.getEmptyResponse());
+                            cancellableListener.onResponse(catShardsResponse);
+                            return;
+                        }
                         IndicesStatsRequest indicesStatsRequest = new IndicesStatsRequest();
                         indicesStatsRequest.setShouldCancelOnTimeout(true);
                         indicesStatsRequest.all();
@@ -159,4 +165,8 @@ private void validateRequestLimit(
             }
         }
     }
+
+    private boolean shouldSkipIndicesStatsRequest(ShardPaginationStrategy paginationStrategy) {
+        return Objects.nonNull(paginationStrategy) && paginationStrategy.getRequestedEntities().isEmpty();
+    }
 }
diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/stats/ClusterStatsRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/stats/ClusterStatsRequest.java
@@ -59,7 +59,7 @@ public ClusterStatsRequest(StreamInput in) throws IOException {
         if (in.getVersion().onOrAfter(Version.V_2_16_0)) {
             useAggregatedNodeLevelResponses = in.readOptionalBoolean();
         }
-        if (in.getVersion().onOrAfter(Version.V_3_0_0)) {
+        if (in.getVersion().onOrAfter(Version.V_2_18_0)) {
             computeAllMetrics = in.readOptionalBoolean();
             final long longMetricsFlags = in.readLong();
             for (Metric metric : Metric.values()) {
@@ -135,7 +135,7 @@ public void writeTo(StreamOutput out) throws IOException {
         if (out.getVersion().onOrAfter(Version.V_2_16_0)) {
             out.writeOptionalBoolean(useAggregatedNodeLevelResponses);
         }
-        if (out.getVersion().onOrAfter(Version.V_3_0_0)) {
+        if (out.getVersion().onOrAfter(Version.V_2_18_0)) {
             out.writeOptionalBoolean(computeAllMetrics);
             long longMetricFlags = 0;
             for (Metric metric : requestedMetrics) {
@@ -154,7 +154,7 @@ public void writeTo(StreamOutput out) throws IOException {
      * An enumeration of the "core" sections of metrics that may be requested
      * from the cluster stats endpoint.
      */
-    @PublicApi(since = "3.0.0")
+    @PublicApi(since = "2.18.0")
     public enum Metric {
         OS("os", 0),
         JVM("jvm", 1),
@@ -192,7 +192,7 @@ public int getIndex() {
      *
      * When no value is provided for param index_metric, default filter is set to _all.
      */
-    @PublicApi(since = "3.0.0")
+    @PublicApi(since = "2.18.0")
     public enum IndexMetric {
         // Metrics computed from ShardStats
         SHARDS("shards", 0),

diff --git a/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsResponse.java b/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsResponse.java
@@ -45,6 +45,7 @@
 import org.opensearch.core.xcontent.XContentBuilder;
 
 import java.io.IOException;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -230,4 +231,8 @@ static final class Fields {
     public String toString() {
         return Strings.toString(MediaTypeRegistry.JSON, this, true, false);
     }
+
+    public static IndicesStatsResponse getEmptyResponse() {
+        return new IndicesStatsResponse(new ShardStats[0], 0, 0, 0, Collections.emptyList());
+    }
 }
diff --git a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java
@@ -300,7 +300,8 @@ public Iterator<Setting<?>> settings() {
             }
 
         },
-        Property.IndexScope
+        Property.IndexScope,
+        Property.NotCopyableOnResize
     );
 
     /**

diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java
@@ -626,14 +626,9 @@ IndexMetadata buildAndValidateTemporaryIndexMetadata(
         final boolean isHiddenAfterTemplates = IndexMetadata.INDEX_HIDDEN_SETTING.get(aggregatedIndexSettings);
         final boolean isSystem = validateDotIndex(request.index(), isHiddenAfterTemplates);
 
-        // remove the setting it's temporary and is only relevant once we create the index
-        final Settings.Builder settingsBuilder = Settings.builder().put(aggregatedIndexSettings);
-        settingsBuilder.remove(IndexMetadata.INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING.getKey());
-        final Settings indexSettings = settingsBuilder.build();
-
         final IndexMetadata.Builder tmpImdBuilder = IndexMetadata.builder(request.index());
         tmpImdBuilder.setRoutingNumShards(routingNumShards);
-        tmpImdBuilder.settings(indexSettings);
+        tmpImdBuilder.settings(aggregatedIndexSettings);
         tmpImdBuilder.system(isSystem);
         addRemoteStoreCustomMetadata(tmpImdBuilder, true);
 

diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestIndicesAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestIndicesAction.java
@@ -212,13 +212,19 @@ public void onResponse(ClusterStateResponse clusterStateResponse) {
                                     groupedListener.onResponse(getSettingsResponse);
                                     groupedListener.onResponse(clusterStateResponse);
 
-                                    sendIndicesStatsRequest(
-                                        indicesToBeQueried,
-                                        subRequestIndicesOptions,
-                                        includeUnloadedSegments,
-                                        client,
-                                        ActionListener.wrap(groupedListener::onResponse, groupedListener::onFailure)
-                                    );
+                                    // For paginated queries, if strategy outputs no indices to be returned,
+                                    // avoid fetching indices stats.
+                                    if (shouldSkipIndicesStatsRequest(paginationStrategy)) {
+                                        groupedListener.onResponse(IndicesStatsResponse.getEmptyResponse());
+                                    } else {
+                                        sendIndicesStatsRequest(
+                                            indicesToBeQueried,
+                                            subRequestIndicesOptions,
+                                            includeUnloadedSegments,
+                                            client,
+                                            ActionListener.wrap(groupedListener::onResponse, groupedListener::onFailure)
+                                        );
+                                    }
 
                                     sendClusterHealthRequest(
                                         indicesToBeQueried,
@@ -1093,4 +1099,8 @@ public Tuple<String, Settings> next() {
         };
     }
 
+    private boolean shouldSkipIndicesStatsRequest(IndexPaginationStrategy paginationStrategy) {
+        return Objects.nonNull(paginationStrategy) && paginationStrategy.getRequestedEntities().isEmpty();
+    }
+
 }
diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java
@@ -136,6 +136,7 @@
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.singleton;
 import static java.util.Collections.singletonList;
+import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING;
 import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING;
 import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING;
 import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_READ_ONLY_BLOCK;
@@ -1821,6 +1822,42 @@ private void validateRemoteCustomData(Map<String, String> customData, String exp
         assertEquals(expectedValue, customData.get(expectedKey));
     }
 
+    public void testNumberOfRoutingShardsShowsInIndexSettings() {
+        withTemporaryClusterService(((clusterService, threadPool) -> {
+            MetadataCreateIndexService checkerService = new MetadataCreateIndexService(
+                Settings.EMPTY,
+                clusterService,
+                indicesServices,
+                null,
+                null,
+                createTestShardLimitService(randomIntBetween(1, 1000), false, clusterService),
+                null,
+                null,
+                threadPool,
+                null,
+                new SystemIndices(Collections.emptyMap()),
+                false,
+                new AwarenessReplicaBalance(Settings.EMPTY, clusterService.getClusterSettings()),
+                DefaultRemoteStoreSettings.INSTANCE,
+                repositoriesServiceSupplier
+            );
+            final int routingNumberOfShards = 4;
+            Settings indexSettings = Settings.builder()
+                .put("index.version.created", Version.CURRENT)
+                .put(INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), 2)
+                .put(INDEX_NUMBER_OF_REPLICAS_SETTING.getKey(), 0)
+                .put(INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING.getKey(), routingNumberOfShards)
+                .build();
+            CreateIndexClusterStateUpdateRequest request = new CreateIndexClusterStateUpdateRequest("create index", "test", "test");
+            IndexMetadata indexMetadata = checkerService.buildAndValidateTemporaryIndexMetadata(
+                indexSettings,
+                request,
+                routingNumberOfShards
+            );
+            assertEquals(INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING.get(indexMetadata.getSettings()).intValue(), routingNumberOfShards);
+        }));
+    }
+
     public void testGetIndexNumberOfRoutingShardsWithNullSourceIndex() {
         Settings indexSettings = Settings.builder()
             .put("index.version.created", Version.CURRENT)

diff --git a/server/src/test/java/org/opensearch/search/approximate/ApproximatePointRangeQueryTests.java b/server/src/test/java/org/opensearch/search/approximate/ApproximatePointRangeQueryTests.java
@@ -21,6 +21,7 @@
 import org.apache.lucene.search.SortField;
 import org.apache.lucene.search.TopDocs;
 import org.apache.lucene.search.TotalHits;
+import org.apache.lucene.search.TotalHits.Relation;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.tests.index.RandomIndexWriter;
 import org.opensearch.search.internal.SearchContext;
@@ -175,6 +176,7 @@ public void testApproximateRangeWithSizeOverDefault() throws IOException {
                     try {
                         long lower = 0;
                         long upper = 12000;
+                        long maxHits = 12001;
                         Query approximateQuery = new ApproximatePointRangeQuery(
                             "point",
                             pack(lower).bytes,
@@ -188,7 +190,13 @@ protected String toString(int dimension, byte[] value) {
                         };
                         IndexSearcher searcher = new IndexSearcher(reader);
                         TopDocs topDocs = searcher.search(approximateQuery, 11000);
-                        assertEquals(topDocs.totalHits, new TotalHits(11000, TotalHits.Relation.EQUAL_TO));
+
+                        if (topDocs.totalHits.relation == Relation.EQUAL_TO) {
+                            assertEquals(topDocs.totalHits.value, 11000);
+                        } else {
+                            assertTrue(11000 <= topDocs.totalHits.value);
+                            assertTrue(maxHits >= topDocs.totalHits.value);
+                        }
                     } catch (IOException e) {
                         throw new RuntimeException(e);
                     }
@@ -226,7 +234,7 @@ protected String toString(int dimension, byte[] value) {
                             }
                         };
                         Query query = LongPoint.newRangeQuery("point", lower, upper);
-                        ;
+
                         IndexSearcher searcher = new IndexSearcher(reader);
                         TopDocs topDocs = searcher.search(approximateQuery, 10);
                         TopDocs topDocs1 = searcher.search(query, 10);
@@ -235,7 +243,6 @@ protected String toString(int dimension, byte[] value) {
                         assertNotEquals(topDocs.totalHits, topDocs1.totalHits);
                         assertEquals(topDocs.totalHits, new TotalHits(10, TotalHits.Relation.EQUAL_TO));
                         assertEquals(topDocs1.totalHits, new TotalHits(101, TotalHits.Relation.EQUAL_TO));
-
                     } catch (IOException e) {
                         throw new RuntimeException(e);
                     }
@@ -278,7 +285,7 @@ protected String toString(int dimension, byte[] value) {
                             }
                         };
                         Query query = LongPoint.newRangeQuery("point", lower, upper);
-                        ;
+
                         IndexSearcher searcher = new IndexSearcher(reader);
                         Sort sort = new Sort(new SortField("point", SortField.Type.LONG));
                         TopDocs topDocs = searcher.search(approximateQuery, 10, sort);