Merge branch 'main' into index-template-create-pr

Signed-off-by: Mohit Godwani <[email protected]>
mgodwan · Jul 24, 2024 · 98858a0 · 98858a0
2 parents 0c163a3 + 7673a77
commit 98858a0
Show file tree

Hide file tree

Showing 73 changed files with 5,771 additions and 1,682 deletions.
diff --git a/.github/workflows/add-performance-comment.yml b/.github/workflows/add-performance-comment.yml
@@ -6,7 +6,10 @@ on:
 
 jobs:
   add-comment:
-    if: github.event.label.name == 'Performance'
+    if: |
+      github.event.label.name == 'Performance' ||
+      github.event.label.name == 'Search:Performance' ||
+      github.event.label.name == 'Indexing:Performance'
     runs-on: ubuntu-latest
     permissions:
       pull-requests: write

diff --git a/.github/workflows/benchmark-pull-request.yml b/.github/workflows/benchmark-pull-request.yml
@@ -77,18 +77,6 @@ jobs:
         run: |
           echo "Invalid comment format detected. Failing the workflow."
           exit 1
-      - id: get_approvers
-        run: |
-          echo "approvers=$(cat .github/CODEOWNERS | grep '^\*'  | tr -d '* ' | sed 's/@/,/g' | sed 's/,//1')" >> $GITHUB_OUTPUT
-      - uses: trstringer/manual-approval@v1
-        if: (!contains(steps.get_approvers.outputs.approvers, github.event.comment.user.login))
-        with:
-          secret: ${{ github.TOKEN }}
-          approvers: ${{ steps.get_approvers.outputs.approvers }}
-          minimum-approvals: 1
-          issue-title: 'Request to approve/deny benchmark run for PR #${{ env.PR_NUMBER }}'
-          issue-body: "Please approve or deny the benchmark run for PR #${{ env.PR_NUMBER }}"
-          exclude-workflow-initiator-as-approver: false
       - name: Get PR Details
         id: get_pr
         uses: actions/github-script@v7
@@ -106,21 +94,33 @@ jobs:
 
             return {
               "headRepoFullName": pull_request.head.repo.full_name,
-              "headRef": pull_request.head.ref
+              "headRefSha": pull_request.head.sha
             };
       - name: Set pr details env vars
         run: |
           echo '${{ steps.get_pr.outputs.result }}' | jq -r '.headRepoFullName'
-          echo '${{ steps.get_pr.outputs.result }}' | jq -r '.headRef'
+          echo '${{ steps.get_pr.outputs.result }}' | jq -r '.headRefSha'
           headRepo=$(echo '${{ steps.get_pr.outputs.result }}' | jq -r '.headRepoFullName')
-          headRef=$(echo '${{ steps.get_pr.outputs.result }}' | jq -r '.headRef')
+          headRefSha=$(echo '${{ steps.get_pr.outputs.result }}' | jq -r '.headRefSha')
           echo "prHeadRepo=$headRepo" >> $GITHUB_ENV
-          echo "prHeadRef=$headRef" >> $GITHUB_ENV
+          echo "prHeadRefSha=$headRefSha" >> $GITHUB_ENV
+      - id: get_approvers
+        run: |
+          echo "approvers=$(cat .github/CODEOWNERS | grep '^\*'  | tr -d '* ' | sed 's/@/,/g' | sed 's/,//1')" >> $GITHUB_OUTPUT
+      - uses: trstringer/manual-approval@v1
+        if: (!contains(steps.get_approvers.outputs.approvers, github.event.comment.user.login))
+        with:
+          secret: ${{ github.TOKEN }}
+          approvers: ${{ steps.get_approvers.outputs.approvers }}
+          minimum-approvals: 1
+          issue-title: 'Request to approve/deny benchmark run for PR #${{ env.PR_NUMBER }}'
+          issue-body: "Please approve or deny the benchmark run for PR #${{ env.PR_NUMBER }}"
+          exclude-workflow-initiator-as-approver: false
       - name: Checkout PR Repo
         uses: actions/checkout@v4
         with:
           repository: ${{ env.prHeadRepo }}
-          ref: ${{ env.prHeadRef }}
+          ref: ${{ env.prHeadRefSha }}
           token: ${{ secrets.GITHUB_TOKEN }}
       - name: Setup Java
         uses: actions/setup-java@v1

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -20,6 +20,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Add matchesPluginSystemIndexPattern to SystemIndexRegistry ([#14750](https://github.com/opensearch-project/OpenSearch/pull/14750))
 - Add Plugin interface for loading application based configuration templates (([#14659](https://github.com/opensearch-project/OpenSearch/issues/14659)))
 - Refactor remote-routing-table service inline with remote state interfaces([#14668](https://github.com/opensearch-project/OpenSearch/pull/14668))
+- Add shard-diff path to diff manifest to reduce number of read calls remote store (([#14684](https://github.com/opensearch-project/OpenSearch/pull/14684)))
 - Add SortResponseProcessor to Search Pipelines (([#14785](https://github.com/opensearch-project/OpenSearch/issues/14785)))
 - Add prefix mode verification setting for repository verification (([#14790](https://github.com/opensearch-project/OpenSearch/pull/14790)))
 - Add SplitResponseProcessor to Search Pipelines (([#14800](https://github.com/opensearch-project/OpenSearch/issues/14800)))
@@ -29,6 +30,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Add persian_stem filter (([#14847](https://github.com/opensearch-project/OpenSearch/pull/14847)))
 - Create listener to refresh search thread resource usage ([#14832](https://github.com/opensearch-project/OpenSearch/pull/14832))
 - Add rest, transport layer changes for hot to warm tiering - dedicated setup (([#13980](https://github.com/opensearch-project/OpenSearch/pull/13980))
+- Optimize Cluster Stats Indices to precomute node level stats ([#14426](https://github.com/opensearch-project/OpenSearch/pull/14426))
 - Add logic to create index templates (v2) using context field ([#14811](https://github.com/opensearch-project/OpenSearch/pull/14811))
 
 ### Dependencies
@@ -86,6 +88,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Fix create or update alias API doesn't throw exception for unsupported parameters ([#14719](https://github.com/opensearch-project/OpenSearch/pull/14719))
 - Refactoring FilterPath.parse by using an iterative approach ([#14200](https://github.com/opensearch-project/OpenSearch/pull/14200))
 - Refactoring Grok.validatePatternBank by using an iterative approach ([#14206](https://github.com/opensearch-project/OpenSearch/pull/14206))
+- Fix NPE when creating index with index.number_of_replicas set to null ([#14812](https://github.com/opensearch-project/OpenSearch/pull/14812))
 - Update help output for _cat ([#14722](https://github.com/opensearch-project/OpenSearch/pull/14722))
 - Fix bulk upsert ignores the default_pipeline and final_pipeline when auto-created index matches the index template ([#12891](https://github.com/opensearch-project/OpenSearch/pull/12891))
 - Fix NPE in ReplicaShardAllocator ([#14385](https://github.com/opensearch-project/OpenSearch/pull/14385))

diff --git a/...rc/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java b/...rc/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java
@@ -88,7 +88,11 @@ public void testNodeCounts() {
         Map<String, Integer> expectedCounts = getExpectedCounts(1, 1, 1, 1, 1, 0, 0);
         int numNodes = randomIntBetween(1, 5);
 
-        ClusterStatsResponse response = client().admin().cluster().prepareClusterStats().get();
+        ClusterStatsResponse response = client().admin()
+            .cluster()
+            .prepareClusterStats()
+            .useAggregatedNodeLevelResponses(randomBoolean())
+            .get();
         assertCounts(response.getNodesStats().getCounts(), total, expectedCounts);
 
         for (int i = 0; i < numNodes; i++) {
@@ -153,7 +157,11 @@ public void testNodeCountsWithDeprecatedMasterRole() throws ExecutionException,
         Map<String, Integer> expectedCounts = getExpectedCounts(0, 1, 1, 0, 0, 0, 0);
 
         Client client = client();
-        ClusterStatsResponse response = client.admin().cluster().prepareClusterStats().get();
+        ClusterStatsResponse response = client.admin()
+            .cluster()
+            .prepareClusterStats()
+            .useAggregatedNodeLevelResponses(randomBoolean())
+            .get();
         assertCounts(response.getNodesStats().getCounts(), total, expectedCounts);
 
         Set<String> expectedRoles = Set.of(DiscoveryNodeRole.MASTER_ROLE.roleName());
@@ -176,15 +184,60 @@ private void assertShardStats(ClusterStatsIndices.ShardStats stats, int indices,
         assertThat(stats.getReplication(), Matchers.equalTo(replicationFactor));
     }
 
-    public void testIndicesShardStats() throws ExecutionException, InterruptedException {
+    public void testIndicesShardStatsWithoutNodeLevelAggregations() {
+        internalCluster().startNode();
+        ensureGreen();
+        ClusterStatsResponse response = client().admin().cluster().prepareClusterStats().useAggregatedNodeLevelResponses(false).get();
+        assertThat(response.getStatus(), Matchers.equalTo(ClusterHealthStatus.GREEN));
+
+        prepareCreate("test1").setSettings(Settings.builder().put("number_of_shards", 2).put("number_of_replicas", 1)).get();
+
+        response = client().admin().cluster().prepareClusterStats().useAggregatedNodeLevelResponses(false).get();
+        assertThat(response.getStatus(), Matchers.equalTo(ClusterHealthStatus.YELLOW));
+        assertThat(response.indicesStats.getDocs().getCount(), Matchers.equalTo(0L));
+        assertThat(response.indicesStats.getIndexCount(), Matchers.equalTo(1));
+        assertShardStats(response.getIndicesStats().getShards(), 1, 2, 2, 0.0);
+
+        // add another node, replicas should get assigned
+        internalCluster().startNode();
+        ensureGreen();
+        index("test1", "type", "1", "f", "f");
+        refresh(); // make the doc visible
+        response = client().admin().cluster().prepareClusterStats().useAggregatedNodeLevelResponses(false).get();
+        assertThat(response.getStatus(), Matchers.equalTo(ClusterHealthStatus.GREEN));
+        assertThat(response.indicesStats.getDocs().getCount(), Matchers.equalTo(1L));
+        assertShardStats(response.getIndicesStats().getShards(), 1, 4, 2, 1.0);
+
+        prepareCreate("test2").setSettings(Settings.builder().put("number_of_shards", 3).put("number_of_replicas", 0)).get();
+        ensureGreen();
+        response = client().admin().cluster().prepareClusterStats().useAggregatedNodeLevelResponses(false).get();
+        assertThat(response.getStatus(), Matchers.equalTo(ClusterHealthStatus.GREEN));
+        assertThat(response.indicesStats.getIndexCount(), Matchers.equalTo(2));
+        assertShardStats(response.getIndicesStats().getShards(), 2, 7, 5, 2.0 / 5);
+
+        assertThat(response.getIndicesStats().getShards().getAvgIndexPrimaryShards(), Matchers.equalTo(2.5));
+        assertThat(response.getIndicesStats().getShards().getMinIndexPrimaryShards(), Matchers.equalTo(2));
+        assertThat(response.getIndicesStats().getShards().getMaxIndexPrimaryShards(), Matchers.equalTo(3));
+
+        assertThat(response.getIndicesStats().getShards().getAvgIndexShards(), Matchers.equalTo(3.5));
+        assertThat(response.getIndicesStats().getShards().getMinIndexShards(), Matchers.equalTo(3));
+        assertThat(response.getIndicesStats().getShards().getMaxIndexShards(), Matchers.equalTo(4));
+
+        assertThat(response.getIndicesStats().getShards().getAvgIndexReplication(), Matchers.equalTo(0.5));
+        assertThat(response.getIndicesStats().getShards().getMinIndexReplication(), Matchers.equalTo(0.0));
+        assertThat(response.getIndicesStats().getShards().getMaxIndexReplication(), Matchers.equalTo(1.0));
+
+    }
+
+    public void testIndicesShardStatsWithNodeLevelAggregations() {
         internalCluster().startNode();
         ensureGreen();
-        ClusterStatsResponse response = client().admin().cluster().prepareClusterStats().get();
+        ClusterStatsResponse response = client().admin().cluster().prepareClusterStats().useAggregatedNodeLevelResponses(true).get();
         assertThat(response.getStatus(), Matchers.equalTo(ClusterHealthStatus.GREEN));
 
         prepareCreate("test1").setSettings(Settings.builder().put("number_of_shards", 2).put("number_of_replicas", 1)).get();
 
-        response = client().admin().cluster().prepareClusterStats().get();
+        response = client().admin().cluster().prepareClusterStats().useAggregatedNodeLevelResponses(true).get();
         assertThat(response.getStatus(), Matchers.equalTo(ClusterHealthStatus.YELLOW));
         assertThat(response.indicesStats.getDocs().getCount(), Matchers.equalTo(0L));
         assertThat(response.indicesStats.getIndexCount(), Matchers.equalTo(1));
@@ -195,14 +248,14 @@ public void testIndicesShardStats() throws ExecutionException, InterruptedExcept
         ensureGreen();
         index("test1", "type", "1", "f", "f");
         refresh(); // make the doc visible
-        response = client().admin().cluster().prepareClusterStats().get();
+        response = client().admin().cluster().prepareClusterStats().useAggregatedNodeLevelResponses(true).get();
         assertThat(response.getStatus(), Matchers.equalTo(ClusterHealthStatus.GREEN));
         assertThat(response.indicesStats.getDocs().getCount(), Matchers.equalTo(1L));
         assertShardStats(response.getIndicesStats().getShards(), 1, 4, 2, 1.0);
 
         prepareCreate("test2").setSettings(Settings.builder().put("number_of_shards", 3).put("number_of_replicas", 0)).get();
         ensureGreen();
-        response = client().admin().cluster().prepareClusterStats().get();
+        response = client().admin().cluster().prepareClusterStats().useAggregatedNodeLevelResponses(true).get();
         assertThat(response.getStatus(), Matchers.equalTo(ClusterHealthStatus.GREEN));
         assertThat(response.indicesStats.getIndexCount(), Matchers.equalTo(2));
         assertShardStats(response.getIndicesStats().getShards(), 2, 7, 5, 2.0 / 5);
@@ -225,7 +278,11 @@ public void testValuesSmokeScreen() throws IOException, ExecutionException, Inte
         internalCluster().startNodes(randomIntBetween(1, 3));
         index("test1", "type", "1", "f", "f");
 
-        ClusterStatsResponse response = client().admin().cluster().prepareClusterStats().get();
+        ClusterStatsResponse response = client().admin()
+            .cluster()
+            .prepareClusterStats()
+            .useAggregatedNodeLevelResponses(randomBoolean())
+            .get();
         String msg = response.toString();
         assertThat(msg, response.getTimestamp(), Matchers.greaterThan(946681200000L)); // 1 Jan 2000
         assertThat(msg, response.indicesStats.getStore().getSizeInBytes(), Matchers.greaterThan(0L));
@@ -265,13 +322,21 @@ public void testAllocatedProcessors() throws Exception {
         internalCluster().startNode(Settings.builder().put(OpenSearchExecutors.NODE_PROCESSORS_SETTING.getKey(), 7).build());
         waitForNodes(1);
 
-        ClusterStatsResponse response = client().admin().cluster().prepareClusterStats().get();
+        ClusterStatsResponse response = client().admin()
+            .cluster()
+            .prepareClusterStats()
+            .useAggregatedNodeLevelResponses(randomBoolean())
+            .get();
         assertThat(response.getNodesStats().getOs().getAllocatedProcessors(), equalTo(7));
     }
 
     public void testClusterStatusWhenStateNotRecovered() throws Exception {
         internalCluster().startClusterManagerOnlyNode(Settings.builder().put("gateway.recover_after_nodes", 2).build());
-        ClusterStatsResponse response = client().admin().cluster().prepareClusterStats().get();
+        ClusterStatsResponse response = client().admin()
+            .cluster()
+            .prepareClusterStats()
+            .useAggregatedNodeLevelResponses(randomBoolean())
+            .get();
         assertThat(response.getStatus(), equalTo(ClusterHealthStatus.RED));
 
         if (randomBoolean()) {
@@ -281,14 +346,18 @@ public void testClusterStatusWhenStateNotRecovered() throws Exception {
         }
         // wait for the cluster status to settle
         ensureGreen();
-        response = client().admin().cluster().prepareClusterStats().get();
+        response = client().admin().cluster().prepareClusterStats().useAggregatedNodeLevelResponses(randomBoolean()).get();
         assertThat(response.getStatus(), equalTo(ClusterHealthStatus.GREEN));
     }
 
     public void testFieldTypes() {
         internalCluster().startNode();
         ensureGreen();
-        ClusterStatsResponse response = client().admin().cluster().prepareClusterStats().get();
+        ClusterStatsResponse response = client().admin()
+            .cluster()
+            .prepareClusterStats()
+            .useAggregatedNodeLevelResponses(randomBoolean())
+            .get();
         assertThat(response.getStatus(), Matchers.equalTo(ClusterHealthStatus.GREEN));
         assertTrue(response.getIndicesStats().getMappings().getFieldTypeStats().isEmpty());
 
@@ -301,7 +370,7 @@ public void testFieldTypes() {
                     + "\"eggplant\":{\"type\":\"integer\"}}}}}"
             )
             .get();
-        response = client().admin().cluster().prepareClusterStats().get();
+        response = client().admin().cluster().prepareClusterStats().useAggregatedNodeLevelResponses(randomBoolean()).get();
         assertThat(response.getIndicesStats().getMappings().getFieldTypeStats().size(), equalTo(3));
         Set<IndexFeatureStats> stats = response.getIndicesStats().getMappings().getFieldTypeStats();
         for (IndexFeatureStats stat : stats) {
@@ -329,7 +398,11 @@ public void testNodeRolesWithMasterLegacySettings() throws ExecutionException, I
         Map<String, Integer> expectedCounts = getExpectedCounts(0, 1, 1, 0, 1, 0, 0);
 
         Client client = client();
-        ClusterStatsResponse clusterStatsResponse = client.admin().cluster().prepareClusterStats().get();
+        ClusterStatsResponse clusterStatsResponse = client.admin()
+            .cluster()
+            .prepareClusterStats()
+            .useAggregatedNodeLevelResponses(randomBoolean())
+            .get();
         assertCounts(clusterStatsResponse.getNodesStats().getCounts(), total, expectedCounts);
 
         Set<String> expectedRoles = Set.of(
@@ -359,7 +432,11 @@ public void testNodeRolesWithClusterManagerRole() throws ExecutionException, Int
         Map<String, Integer> expectedCounts = getExpectedCounts(0, 1, 1, 0, 1, 0, 0);
 
         Client client = client();
-        ClusterStatsResponse clusterStatsResponse = client.admin().cluster().prepareClusterStats().get();
+        ClusterStatsResponse clusterStatsResponse = client.admin()
+            .cluster()
+            .prepareClusterStats()
+            .useAggregatedNodeLevelResponses(randomBoolean())
+            .get();
         assertCounts(clusterStatsResponse.getNodesStats().getCounts(), total, expectedCounts);
 
         Set<String> expectedRoles = Set.of(
@@ -383,7 +460,11 @@ public void testNodeRolesWithSeedDataNodeLegacySettings() throws ExecutionExcept
         Map<String, Integer> expectedRoleCounts = getExpectedCounts(1, 1, 1, 0, 1, 0, 0);
 
         Client client = client();
-        ClusterStatsResponse clusterStatsResponse = client.admin().cluster().prepareClusterStats().get();
+        ClusterStatsResponse clusterStatsResponse = client.admin()
+            .cluster()
+            .prepareClusterStats()
+            .useAggregatedNodeLevelResponses(randomBoolean())
+            .get();
         assertCounts(clusterStatsResponse.getNodesStats().getCounts(), total, expectedRoleCounts);
 
         Set<String> expectedRoles = Set.of(
@@ -410,7 +491,11 @@ public void testNodeRolesWithDataNodeLegacySettings() throws ExecutionException,
         Map<String, Integer> expectedRoleCounts = getExpectedCounts(1, 1, 1, 0, 1, 0, 0);
 
         Client client = client();
-        ClusterStatsResponse clusterStatsResponse = client.admin().cluster().prepareClusterStats().get();
+        ClusterStatsResponse clusterStatsResponse = client.admin()
+            .cluster()
+            .prepareClusterStats()
+            .useAggregatedNodeLevelResponses(randomBoolean())
+            .get();
         assertCounts(clusterStatsResponse.getNodesStats().getCounts(), total, expectedRoleCounts);
 
         Set<Set<String>> expectedNodesRoles = Set.of(