Fix length calculation for block based fetching

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>
tlfeng · Nov 3, 2022 · b5af206 · b5af206
1 parent 22dd0fd
commit b5af206
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 3 deletions.
diff --git a/...src/test/java/org/opensearch/example/customsettings/ExampleCustomSettingsConfigTests.java b/...src/test/java/org/opensearch/example/customsettings/ExampleCustomSettingsConfigTests.java
@@ -54,6 +54,6 @@ public void testValidatedSetting() {
             IllegalArgumentException.class,
             () -> VALIDATED_SETTING.get(Settings.builder().put("custom.validated", "it's forbidden").build())
         );
-        assertEquals("Setting must not contain [forbidden]", exception.getMessage());
+        assertEquals("Setting must not contain [forbidden]", exception.getCause().getMessage());
     }
 }
diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java
@@ -22,6 +22,7 @@
 import org.opensearch.common.collect.Map;
 import org.opensearch.common.io.PathUtils;
 import org.opensearch.common.settings.Settings;
+import org.opensearch.common.unit.ByteSizeUnit;
 import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.index.Index;
 import org.opensearch.index.IndexNotFoundException;
@@ -60,6 +61,65 @@ protected Settings.Builder randomRepositorySettings() {
         return settings;
     }
 
+    private Settings.Builder chunkedRepositorySettings() {
+        final Settings.Builder settings = Settings.builder();
+        settings.put("location", randomRepoPath()).put("compress", randomBoolean());
+        settings.put("chunk_size", 2 << 13, ByteSizeUnit.BYTES);
+        return settings;
+    }
+
+    public void testCreateSearchableSnapshotWithChunks() throws Exception {
+        final int numReplicasIndex = randomIntBetween(1, 4);
+        final String indexName = "test-idx";
+        final String restoredIndexName = indexName + "-copy";
+        final Client client = client();
+
+        Settings.Builder repositorySettings = chunkedRepositorySettings();
+
+        internalCluster().ensureAtLeastNumDataNodes(numReplicasIndex + 1);
+        createIndex(
+            indexName,
+            Settings.builder()
+                .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, Integer.toString(numReplicasIndex))
+                .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, "1")
+                .build()
+        );
+        ensureGreen();
+        indexRandomDocs(indexName, 1000);
+
+        createRepository("test-repo", "fs", repositorySettings);
+        logger.info("--> snapshot");
+        final CreateSnapshotResponse createSnapshotResponse = client.admin()
+            .cluster()
+            .prepareCreateSnapshot("test-repo", "test-snap")
+            .setWaitForCompletion(true)
+            .setIndices(indexName)
+            .get();
+        MatcherAssert.assertThat(createSnapshotResponse.getSnapshotInfo().successfulShards(), greaterThan(0));
+        MatcherAssert.assertThat(
+            createSnapshotResponse.getSnapshotInfo().successfulShards(),
+            equalTo(createSnapshotResponse.getSnapshotInfo().totalShards())
+        );
+
+        assertTrue(client.admin().indices().prepareDelete(indexName).get().isAcknowledged());
+
+        internalCluster().ensureAtLeastNumSearchNodes(numReplicasIndex + 1);
+        logger.info("--> restore indices as 'remote_snapshot'");
+        client.admin()
+            .cluster()
+            .prepareRestoreSnapshot("test-repo", "test-snap")
+            .setRenamePattern("(.+)")
+            .setRenameReplacement("$1-copy")
+            .setStorageType(RestoreSnapshotRequest.StorageType.REMOTE_SNAPSHOT)
+            .setWaitForCompletion(true)
+            .execute()
+            .actionGet();
+        ensureGreen();
+
+        assertDocCount(restoredIndexName, 1000L);
+
+    }
+
     public void testCreateSearchableSnapshot() throws Exception {
         final int numReplicasIndex1 = randomIntBetween(1, 4);
         final int numReplicasIndex2 = randomIntBetween(0, 2);

diff --git a/...src/main/java/org/opensearch/index/store/remote/file/OnDemandBlockSnapshotIndexInput.java b/...src/main/java/org/opensearch/index/store/remote/file/OnDemandBlockSnapshotIndexInput.java
@@ -137,11 +137,11 @@ protected IndexInput fetchBlock(int blockId) throws IOException {
         final long partStart = part * partSize;
 
         final long position = blockStart - partStart;
-        final long offset = blockEnd - blockStart - partStart;
+        final long length = blockEnd - blockStart;
 
         BlobFetchRequest blobFetchRequest = BlobFetchRequest.builder()
             .position(position)
-            .length(offset)
+            .length(length)
             .blobName(fileInfo.partName(part))
             .directory(directory)
             .fileName(blockFileName)

diff --git a/server/src/main/java/org/opensearch/index/store/remote/utils/BlobFetchRequest.java b/server/src/main/java/org/opensearch/index/store/remote/utils/BlobFetchRequest.java
@@ -107,6 +107,9 @@ public Builder position(long position) {
         }
 
         public Builder length(long length) {
+            if (length <= 0) {
+                throw new IllegalArgumentException("Length for blob fetch request needs to be non-negative");
+            }
             this.length = length;
             return this;
         }