Merge branch 'main' into issue_14377

Signed-off-by: kkewwei <kkewwei@163.com>
kkewwei · Jun 22, 2024 · 5e7ab2a · 5e7ab2a
2 parents ba4b35f + f8e8865
commit 5e7ab2a
Show file tree

Hide file tree

Showing 37 changed files with 1,446 additions and 471 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -33,10 +33,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 ### Removed
 
 ### Fixed
+- Fix bug in SBP cancellation logic ([#13259](https://github.com/opensearch-project/OpenSearch/pull/13474))
 - Fix handling of Short and Byte data types in ScriptProcessor ingest pipeline ([#14379](https://github.com/opensearch-project/OpenSearch/issues/14379))
 - Switch to iterative version of WKT format parser ([#14086](https://github.com/opensearch-project/OpenSearch/pull/14086))
 - Fix the computed max shards of cluster to avoid int overflow ([#14155](https://github.com/opensearch-project/OpenSearch/pull/14155))
 - Fixed rest-high-level client searchTemplate & mtermVectors endpoints to have a leading slash ([#14465](https://github.com/opensearch-project/OpenSearch/pull/14465))
+- Write shard level metadata blob when snapshotting searchable snapshot indexes ([#13190](https://github.com/opensearch-project/OpenSearch/pull/13190))
+- Fix aggs result of NestedAggregator with sub NestedAggregator ([#13324](https://github.com/opensearch-project/OpenSearch/pull/13324))
+- Add ListPitInfo::getKeepAlive() getter ([#14495](https://github.com/opensearch-project/OpenSearch/pull/14495))
 - Fix FuzzyQuery in keyword field will use IndexOrDocValuesQuery when both of index and doc_value are true ([#14378](https://github.com/opensearch-project/OpenSearch/pull/14378))
 
 ### Security

diff --git a/TESTING.md b/TESTING.md
@@ -91,21 +91,23 @@ This will instruct all JVMs (including any that run cli tools such as creating t
 
 ## Test case filtering
 
--   `tests.class` is a class-filtering shell-like glob pattern
--   `tests.method` is a method-filtering glob pattern.
+To be able to run a single test you need to specify the module where you're running the tests from.
+
+Example: `./gradlew server:test --tests "*.ReplicaShardBatchAllocatorTests.testNoAsyncFetchData"`
 
 Run a single test case (variants)
 
-    ./gradlew test -Dtests.class=org.opensearch.package.ClassName
-    ./gradlew test "-Dtests.class=*.ClassName"
+    ./gradlew module:test --tests org.opensearch.package.ClassName
+    ./gradlew module:test --tests org.opensearch.package.ClassName.testName
+    ./gradlew module:test --tests "*.ClassName"
 
 Run all tests in a package and its sub-packages
 
-    ./gradlew test "-Dtests.class=org.opensearch.package.*"
+    ./gradlew module:test --tests "org.opensearch.package.*"
 
 Run any test methods that contain *esi* (e.g.: .r*esi*ze.)
 
-    ./gradlew test "-Dtests.method=*esi*"
+    ./gradlew module:test --tests "*esi*"
 
 Run all tests that are waiting for a bugfix (disabled by default)
 

diff --git a/...src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationIT.java b/...src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationIT.java
@@ -1405,7 +1405,7 @@ public void testPitCreatedOnReplica() throws Exception {
             .setPointInTime(new PointInTimeBuilder(pitResponse.getId()).setKeepAlive(TimeValue.timeValueDays(1)))
             .setRequestCache(false)
             .get();
-        PitTestsUtil.assertUsingGetAllPits(client(replica), pitResponse.getId(), pitResponse.getCreationTime());
+        PitTestsUtil.assertUsingGetAllPits(client(replica), pitResponse.getId(), pitResponse.getCreationTime(), TimeValue.timeValueDays(1));
         assertSegments(false, INDEX_NAME, 1, client(replica), pitResponse.getId());
 
         List<String> currentFiles = List.of(replicaShard.store().directory().listAll());

diff --git a/server/src/internalClusterTest/java/org/opensearch/search/pit/PitMultiNodeIT.java b/server/src/internalClusterTest/java/org/opensearch/search/pit/PitMultiNodeIT.java
@@ -104,7 +104,7 @@ public void testPit() throws Exception {
         assertEquals(2, searchResponse.getSuccessfulShards());
         assertEquals(2, searchResponse.getTotalShards());
         validatePitStats("index", 2, 2);
-        PitTestsUtil.assertUsingGetAllPits(client(), pitResponse.getId(), pitResponse.getCreationTime());
+        PitTestsUtil.assertUsingGetAllPits(client(), pitResponse.getId(), pitResponse.getCreationTime(), TimeValue.timeValueDays(1));
         assertSegments(false, client(), pitResponse.getId());
     }
 
@@ -131,7 +131,12 @@ public void testCreatePitWhileNodeDropWithAllowPartialCreationTrue() throws Exce
             public Settings onNodeStopped(String nodeName) throws Exception {
                 ActionFuture<CreatePitResponse> execute = client().execute(CreatePitAction.INSTANCE, request);
                 CreatePitResponse pitResponse = execute.get();
-                PitTestsUtil.assertUsingGetAllPits(client(), pitResponse.getId(), pitResponse.getCreationTime());
+                PitTestsUtil.assertUsingGetAllPits(
+                    client(),
+                    pitResponse.getId(),
+                    pitResponse.getCreationTime(),
+                    TimeValue.timeValueDays(1)
+                );
                 assertSegments(false, "index", 1, client(), pitResponse.getId());
                 assertEquals(1, pitResponse.getSuccessfulShards());
                 assertEquals(2, pitResponse.getTotalShards());
@@ -164,7 +169,12 @@ public Settings onNodeStopped(String nodeName) throws Exception {
                 assertEquals(0, searchResponse.getSkippedShards());
                 assertEquals(2, searchResponse.getTotalShards());
                 validatePitStats("index", 1, 1);
-                PitTestsUtil.assertUsingGetAllPits(client(), pitResponse.getId(), pitResponse.getCreationTime());
+                PitTestsUtil.assertUsingGetAllPits(
+                    client(),
+                    pitResponse.getId(),
+                    pitResponse.getCreationTime(),
+                    TimeValue.timeValueDays(1)
+                );
                 return super.onNodeStopped(nodeName);
             }
         });

diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java
@@ -17,6 +17,7 @@
 import org.opensearch.action.admin.cluster.snapshots.delete.DeleteSnapshotRequest;
 import org.opensearch.action.admin.cluster.snapshots.get.GetSnapshotsResponse;
 import org.opensearch.action.admin.cluster.snapshots.restore.RestoreSnapshotRequest;
+import org.opensearch.action.admin.cluster.snapshots.status.SnapshotsStatusResponse;
 import org.opensearch.action.admin.indices.settings.get.GetSettingsRequest;
 import org.opensearch.action.admin.indices.settings.get.GetSettingsResponse;
 import org.opensearch.action.admin.indices.settings.put.UpdateSettingsRequestBuilder;
@@ -53,11 +54,13 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
+import java.util.stream.IntStream;
 import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
 
@@ -132,21 +135,24 @@ public void testCreateSearchableSnapshot() throws Exception {
 
     public void testSnapshottingSearchableSnapshots() throws Exception {
         final String repoName = "test-repo";
+        final String initSnapName = "initial-snapshot";
         final String indexName = "test-idx";
+        final String repeatSnapNamePrefix = "test-repeated-snap-";
+        final String repeatIndexNamePrefix = indexName + "-copy-";
         final Client client = client();
 
         // create an index, add data, snapshot it, then delete it
         internalCluster().ensureAtLeastNumDataNodes(1);
         createIndexWithDocsAndEnsureGreen(0, 100, indexName);
         createRepositoryWithSettings(null, repoName);
-        takeSnapshot(client, "initial-snapshot", repoName, indexName);
+        takeSnapshot(client, initSnapName, repoName, indexName);
         deleteIndicesAndEnsureGreen(client, indexName);
 
         // restore the index as a searchable snapshot
         internalCluster().ensureAtLeastNumSearchNodes(1);
         client.admin()
             .cluster()
-            .prepareRestoreSnapshot(repoName, "initial-snapshot")
+            .prepareRestoreSnapshot(repoName, initSnapName)
             .setRenamePattern("(.+)")
             .setRenameReplacement("$1-copy-0")
             .setStorageType(RestoreSnapshotRequest.StorageType.REMOTE_SNAPSHOT)
@@ -159,7 +165,7 @@ public void testSnapshottingSearchableSnapshots() throws Exception {
 
         // Test that the searchable snapshot index can continue to be snapshotted and restored
         for (int i = 0; i < 4; i++) {
-            final String repeatedSnapshotName = "test-repeated-snap-" + i;
+            final String repeatedSnapshotName = repeatSnapNamePrefix + i;
             takeSnapshot(client, repeatedSnapshotName, repoName);
             deleteIndicesAndEnsureGreen(client, "_all");
             client.admin()
@@ -181,21 +187,34 @@ public void testSnapshottingSearchableSnapshots() throws Exception {
         final Map<String, List<String>> snapshotInfoMap = response.getSnapshots()
             .stream()
             .collect(Collectors.toMap(s -> s.snapshotId().getName(), SnapshotInfo::indices));
-        assertEquals(
-            Map.of(
-                "initial-snapshot",
-                List.of("test-idx"),
-                "test-repeated-snap-0",
-                List.of("test-idx-copy-0"),
-                "test-repeated-snap-1",
-                List.of("test-idx-copy-1"),
-                "test-repeated-snap-2",
-                List.of("test-idx-copy-2"),
-                "test-repeated-snap-3",
-                List.of("test-idx-copy-3")
-            ),
-            snapshotInfoMap
-        );
+        final Map<String, List<String>> expect = new HashMap<>();
+        expect.put(initSnapName, List.of(indexName));
+        IntStream.range(0, 4).forEach(i -> expect.put(repeatSnapNamePrefix + i, List.of(repeatIndexNamePrefix + i)));
+        assertEquals(expect, snapshotInfoMap);
+
+        String[] snapNames = new String[5];
+        IntStream.range(0, 4).forEach(i -> snapNames[i] = repeatSnapNamePrefix + i);
+        snapNames[4] = initSnapName;
+        SnapshotsStatusResponse snapshotsStatusResponse = client.admin()
+            .cluster()
+            .prepareSnapshotStatus(repoName)
+            .addSnapshots(snapNames)
+            .execute()
+            .actionGet();
+        snapshotsStatusResponse.getSnapshots().forEach(s -> {
+            String snapName = s.getSnapshot().getSnapshotId().getName();
+            assertEquals(1, s.getIndices().size());
+            assertEquals(1, s.getShards().size());
+            if (snapName.equals("initial-snapshot")) {
+                assertNotNull(s.getIndices().get("test-idx"));
+                assertTrue(s.getShards().get(0).getStats().getTotalFileCount() > 0);
+            } else {
+                assertTrue(snapName.startsWith(repeatSnapNamePrefix));
+                assertEquals(1, s.getIndices().size());
+                assertNotNull(s.getIndices().get(repeatIndexNamePrefix + snapName.substring(repeatSnapNamePrefix.length())));
+                assertEquals(0L, s.getShards().get(0).getStats().getTotalFileCount());
+            }
+        });
     }
 
     /**

diff --git a/server/src/main/java/org/opensearch/action/search/ListPitInfo.java b/server/src/main/java/org/opensearch/action/search/ListPitInfo.java
@@ -53,6 +53,10 @@ public long getCreationTime() {
         return creationTime;
     }
 
+    public long getKeepAlive() {
+        return keepAlive;
+    }
+
     @Override
     public void writeTo(StreamOutput out) throws IOException {
         out.writeString(pitId);

diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java
@@ -1160,7 +1160,8 @@ private ClusterState readClusterStateInParallel(
         for (Map.Entry<String, UploadedMetadataAttribute> entry : clusterStateCustomToRead.entrySet()) {
             asyncMetadataReadActions.add(
                 remoteClusterStateAttributesManager.getAsyncMetadataReadAction(
-                    CLUSTER_STATE_CUSTOM,
+                    // pass component name as cluster-state-custom--<custom_name>, so that we can interpret it later
+                    String.join(CUSTOM_DELIMITER, CLUSTER_STATE_CUSTOM, entry.getKey()),
                     new RemoteClusterStateCustoms(
                         entry.getValue().getUploadedFilename(),
                         entry.getValue().getAttributeName(),

diff --git a/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java
@@ -2801,9 +2801,12 @@ public void snapshotShard(
             long indexIncrementalSize = 0;
             long indexTotalFileSize = 0;
             final BlockingQueue<BlobStoreIndexShardSnapshot.FileInfo> filesToSnapshot = new LinkedBlockingQueue<>();
-            // If we did not find a set of files that is equal to the current commit we determine the files to upload by comparing files
-            // in the commit with files already in the repository
-            if (filesFromSegmentInfos == null) {
+            if (store.indexSettings().isRemoteSnapshot()) {
+                // If the source of the data is another remote snapshot (i.e. searchable snapshot) then no need to snapshot the shard
+                indexCommitPointFiles = List.of();
+            } else if (filesFromSegmentInfos == null) {
+                // If we did not find a set of files that is equal to the current commit we determine the files to upload by comparing files
+                // in the commit with files already in the repository
                 indexCommitPointFiles = new ArrayList<>();
                 final Collection<String> fileNames;
                 final Store.MetadataSnapshot metadataFromStore;

diff --git a/server/src/main/java/org/opensearch/search/ResourceType.java b/server/src/main/java/org/opensearch/search/ResourceType.java
@@ -0,0 +1,41 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+package org.opensearch.search;
+
+/**
+ * Enum to hold the resource type
+ */
+public enum ResourceType {
+    CPU("cpu"),
+    JVM("jvm");
+
+    private final String name;
+
+    ResourceType(String name) {
+        this.name = name;
+    }
+
+    /**
+     * The string match here is case-sensitive
+     * @param s name matching the resource type name
+     * @return a {@link ResourceType}
+     */
+    public static ResourceType fromName(String s) {
+        for (ResourceType resourceType : values()) {
+            if (resourceType.getName().equals(s)) {
+                return resourceType;
+            }
+        }
+        throw new IllegalArgumentException("Unknown resource type: [" + s + "]");
+    }
+
+    private String getName() {
+        return name;
+    }
+}