From 798cf4bc57c95209e652642689c9c6bb32b17aef Mon Sep 17 00:00:00 2001 From: Ashish Singh Date: Sat, 31 Aug 2024 19:35:06 +0530 Subject: [PATCH] Fix tests Signed-off-by: Ashish Singh --- .../repositories/RepositoryBlocksIT.java | 3 +- .../RemoteStoreRepositoryRegistrationIT.java | 3 +- .../CorruptedBlobStoreRepositoryIT.java | 26 ++++++++++++---- .../DedicatedClusterSnapshotRestoreIT.java | 6 ++-- .../snapshots/MultiClusterRepoAccessIT.java | 3 +- .../opensearch/snapshots/RepositoriesIT.java | 6 ++-- .../blobstore/BlobStoreRepository.java | 8 ++--- .../snapshots/SnapshotShardPaths.java | 4 +-- .../blobstore/BlobStoreRepositoryTests.java | 2 +- .../test/OpenSearchIntegTestCase.java | 30 +++++++++++++------ 10 files changed, 62 insertions(+), 29 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/repositories/RepositoryBlocksIT.java b/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/repositories/RepositoryBlocksIT.java index dc1ace7c43e40..6c0a156eb6752 100644 --- a/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/repositories/RepositoryBlocksIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/repositories/RepositoryBlocksIT.java @@ -63,7 +63,8 @@ public void testPutRepositoryWithBlocks() { "fs", false, settings, - null + null, + false ), Metadata.CLUSTER_READ_ONLY_BLOCK ); diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreRepositoryRegistrationIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreRepositoryRegistrationIT.java index 5e8761d837195..4cbafde6417af 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreRepositoryRegistrationIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreRepositoryRegistrationIT.java @@ -137,7 +137,8 @@ public void testMultiNodeClusterRandomNodeRecoverNetworkIsolationPostNonRestrict repositoryMetadata.type(), true, updatedSettings, - null + null, + false ).get(); ensureStableCluster(3, nodesInOneSide.stream().findAny().get()); diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/CorruptedBlobStoreRepositoryIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/CorruptedBlobStoreRepositoryIT.java index a9d4242ec1bbf..95cc9eca92355 100644 --- a/server/src/internalClusterTest/java/org/opensearch/snapshots/CorruptedBlobStoreRepositoryIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/snapshots/CorruptedBlobStoreRepositoryIT.java @@ -40,9 +40,13 @@ import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.metadata.Metadata; import org.opensearch.cluster.metadata.RepositoriesMetadata; +import org.opensearch.common.blobstore.BlobPath; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.io.IOUtils; import org.opensearch.core.common.unit.ByteSizeUnit; +import org.opensearch.index.remote.RemoteStoreEnums.PathHashAlgorithm; +import org.opensearch.index.remote.RemoteStoreEnums.PathType; +import org.opensearch.index.remote.RemoteStorePathStrategy; import org.opensearch.repositories.IndexId; import org.opensearch.repositories.RepositoriesService; import org.opensearch.repositories.Repository; @@ -58,6 +62,7 @@ import java.util.Map; import java.util.stream.Stream; +import static org.opensearch.index.remote.RemoteStoreEnums.PathHashAlgorithm.FNV_1A_COMPOSITE_1; import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked; import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertRequestBuilderThrows; import static org.hamcrest.Matchers.containsString; @@ -358,9 +363,7 @@ public void testSnapshotWithCorruptedShardIndexFile() throws Exception { assertThat(indexIds.size(), equalTo(1)); final IndexId corruptedIndex = indexIds.get(indexName); - final Path shardIndexFile = repo.resolve("indices") - .resolve(corruptedIndex.getId()) - .resolve("0") + final Path shardIndexFile = repo.resolve(resolvePath(corruptedIndex, "0")) .resolve("index-" + repositoryData.shardGenerations().getShardGen(corruptedIndex, 0)); logger.info("--> truncating shard index file [{}]", shardIndexFile); @@ -435,7 +438,7 @@ public void testDeleteSnapshotWithMissingIndexAndShardMetadata() throws Exceptio logger.info("--> delete index metadata and shard metadata"); for (String index : indices) { - Path shardZero = indicesPath.resolve(indexIds.get(index).getId()).resolve("0"); + Path shardZero = repo.resolve(resolvePath(indexIds.get(index), "0")); if (randomBoolean()) { Files.delete( shardZero.resolve("index-" + getRepositoryData("test-repo").shardGenerations().getShardGen(indexIds.get(index), 0)) @@ -628,10 +631,9 @@ public void testSnapshotWithMissingShardLevelIndexFile() throws Exception { clusterAdmin().prepareCreateSnapshot("test-repo", "test-snap-1").setWaitForCompletion(true).setIndices("test-idx-*").get(); logger.info("--> deleting shard level index file"); - final Path indicesPath = repo.resolve("indices"); for (IndexId indexId : getRepositoryData("test-repo").getIndices().values()) { final Path shardGen; - try (Stream shardFiles = Files.list(indicesPath.resolve(indexId.getId()).resolve("0"))) { + try (Stream shardFiles = Files.list(repo.resolve(resolvePath(indexId, "0")))) { shardGen = shardFiles.filter(file -> file.getFileName().toString().startsWith(BlobStoreRepository.INDEX_FILE_PREFIX)) .findFirst() .orElseThrow(() -> new AssertionError("Failed to find shard index blob")); @@ -681,4 +683,16 @@ private void assertRepositoryBlocked(Client client, String repo, String existing containsString("Could not read repository data because the contents of the repository do not match its expected state.") ); } + + private static String resolvePath(IndexId indexId, String shardId) { + PathType pathType = PathType.fromCode(indexId.getShardPathType()); + RemoteStorePathStrategy.SnapshotShardPathInput shardPathInput = new RemoteStorePathStrategy.SnapshotShardPathInput.Builder() + .basePath(BlobPath.cleanPath()) + .indexUUID(indexId.getId()) + .shardId(shardId) + .build(); + PathHashAlgorithm pathHashAlgorithm = pathType != PathType.FIXED ? FNV_1A_COMPOSITE_1 : null; + BlobPath blobPath = pathType.path(shardPathInput, pathHashAlgorithm); + return blobPath.buildAsString(); + } } diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/DedicatedClusterSnapshotRestoreIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/DedicatedClusterSnapshotRestoreIT.java index 79e3ea325503e..686853c42aa03 100644 --- a/server/src/internalClusterTest/java/org/opensearch/snapshots/DedicatedClusterSnapshotRestoreIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/snapshots/DedicatedClusterSnapshotRestoreIT.java @@ -766,7 +766,8 @@ public void testRegistrationFailure() { "mock", false, Settings.builder().put("location", randomRepoPath()), - null + null, + false ).get(); } logger.info("--> make sure that properly setup repository can be registered on all nodes"); @@ -776,7 +777,8 @@ public void testRegistrationFailure() { "fs", true, Settings.builder().put("location", randomRepoPath()), - null + null, + false ).get(); } diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/MultiClusterRepoAccessIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/MultiClusterRepoAccessIT.java index 50a611a3df9c6..c96d4a2f079ee 100644 --- a/server/src/internalClusterTest/java/org/opensearch/snapshots/MultiClusterRepoAccessIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/snapshots/MultiClusterRepoAccessIT.java @@ -121,7 +121,8 @@ public void testConcurrentDeleteFromOtherCluster() throws InterruptedException { "fs", true, Settings.builder().put("location", repoPath), - null + null, + false ).get(); createIndexWithRandomDocs("test-idx-1", randomIntBetween(1, 100)); diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/RepositoriesIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/RepositoriesIT.java index 0d32290fadd86..2c399f59e4ee3 100644 --- a/server/src/internalClusterTest/java/org/opensearch/snapshots/RepositoriesIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/snapshots/RepositoriesIT.java @@ -269,7 +269,8 @@ public void testRepositoryVerification() throws Exception { "mock", true, Settings.builder().put(settings), - null + null, + false ), RepositoryVerificationException.class ); @@ -282,7 +283,8 @@ public void testRepositoryVerification() throws Exception { "mock", true, Settings.builder().put(readonlySettings), - null + null, + false ), RepositoryVerificationException.class ); diff --git a/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java index c8597e2de08af..316c0eec8cae9 100644 --- a/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java @@ -279,7 +279,7 @@ public abstract class BlobStoreRepository extends AbstractLifecycleComponent imp public static final Setting SHARD_PATH_TYPE = new Setting<>( "shard_path_type", - PathType.HASHED_PREFIX.toString(), + PathType.FIXED.toString(), PathType::parseString ); @@ -1896,7 +1896,7 @@ private List findMatchingShardPaths(String indexId, Map findHighestGenerationShardPaths(List matchingShardPaths) { return matchingShardPaths.stream() - .map(s -> s.split(SnapshotShardPaths.DELIMITER)) + .map(s -> s.split("\\" + SnapshotShardPaths.DELIMITER)) .sorted((a, b) -> Integer.parseInt(b[2]) - Integer.parseInt(a[2])) .map(parts -> String.join(SnapshotShardPaths.DELIMITER, parts)) .findFirst(); @@ -2117,11 +2117,11 @@ public void finalizeSnapshot( */ private void cleanupRedundantSnapshotShardPaths(Set updatedShardPathsIndexIds) { Set updatedIndexIds = updatedShardPathsIndexIds.stream() - .map(s -> s.split(SnapshotShardPaths.DELIMITER)[0]) + .map(s -> s.split("\\" + SnapshotShardPaths.DELIMITER)[0]) .collect(Collectors.toSet()); Set indexIdShardPaths = getSnapshotShardPaths().keySet(); List staleShardPaths = indexIdShardPaths.stream().filter(s -> updatedShardPathsIndexIds.contains(s) == false).filter(s -> { - String indexId = s.split(SnapshotShardPaths.DELIMITER)[0]; + String indexId = s.split("\\" + SnapshotShardPaths.DELIMITER)[0]; return updatedIndexIds.contains(indexId); }).collect(Collectors.toList()); try { diff --git a/server/src/main/java/org/opensearch/snapshots/SnapshotShardPaths.java b/server/src/main/java/org/opensearch/snapshots/SnapshotShardPaths.java index 40757b679c32a..88af14e2232f9 100644 --- a/server/src/main/java/org/opensearch/snapshots/SnapshotShardPaths.java +++ b/server/src/main/java/org/opensearch/snapshots/SnapshotShardPaths.java @@ -27,7 +27,7 @@ public class SnapshotShardPaths implements ToXContent { public static final String DIR = "snapshot_shard_paths"; - public static final String DELIMITER = "#"; + public static final String DELIMITER = "."; public static final String FILE_NAME_FORMAT = "%s"; @@ -96,7 +96,7 @@ public static SnapshotShardPaths fromXContent(XContentParser ignored) { * @throws IllegalArgumentException if the shard path format is invalid or cannot be parsed. */ public static ShardInfo parseShardPath(String shardPath) { - String[] parts = shardPath.split(SnapshotShardPaths.DELIMITER); + String[] parts = shardPath.split("\\" + SnapshotShardPaths.DELIMITER); if (parts.length != 5) { throw new IllegalArgumentException("Invalid shard path format: " + shardPath); } diff --git a/server/src/test/java/org/opensearch/repositories/blobstore/BlobStoreRepositoryTests.java b/server/src/test/java/org/opensearch/repositories/blobstore/BlobStoreRepositoryTests.java index 8e03e231c7269..63257a5575970 100644 --- a/server/src/test/java/org/opensearch/repositories/blobstore/BlobStoreRepositoryTests.java +++ b/server/src/test/java/org/opensearch/repositories/blobstore/BlobStoreRepositoryTests.java @@ -435,7 +435,7 @@ public void testWriteAndReadShardPaths() throws Exception { assertNotNull("IndexId should not be null", indexId); assertEquals("Index ID should match", shardInfo.getIndexId().getId(), indexId.getId()); assertEquals("Shard path type should match", shardInfo.getIndexId().getShardPathType(), indexId.getShardPathType()); - String[] parts = shardPathBlobName.split(SnapshotShardPaths.DELIMITER); + String[] parts = shardPathBlobName.split("\\" + SnapshotShardPaths.DELIMITER); assertEquals( "Path hash algorithm should be FNV_1A_COMPOSITE_1", RemoteStoreEnums.PathHashAlgorithm.FNV_1A_COMPOSITE_1, diff --git a/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java b/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java index d56242a102f9f..de7dde5e9c6e9 100644 --- a/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java +++ b/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java @@ -2581,7 +2581,7 @@ protected void updateRepository(String repoName, String type, Settings.Builder s } public static void putRepository(ClusterAdminClient adminClient, String repoName, String type, Settings.Builder settings) { - assertAcked(putRepositoryRequestBuilder(adminClient, repoName, type, true, settings, null)); + assertAcked(putRepositoryRequestBuilder(adminClient, repoName, type, true, settings, null, false)); } public static void putRepository( @@ -2591,7 +2591,7 @@ public static void putRepository( String timeout, Settings.Builder settings ) { - assertAcked(putRepositoryRequestBuilder(adminClient, repoName, type, true, settings, timeout)); + assertAcked(putRepositoryRequestBuilder(adminClient, repoName, type, true, settings, timeout, false)); } public static void putRepository( @@ -2601,7 +2601,17 @@ public static void putRepository( boolean verify, Settings.Builder settings ) { - assertAcked(putRepositoryRequestBuilder(adminClient, repoName, type, verify, settings, null)); + assertAcked(putRepositoryRequestBuilder(adminClient, repoName, type, verify, settings, null, false)); + } + + public static void putRepositoryWithNoSettingOverrides( + ClusterAdminClient adminClient, + String repoName, + String type, + boolean verify, + Settings.Builder settings + ) { + assertAcked(putRepositoryRequestBuilder(adminClient, repoName, type, verify, settings, null, true)); } public static void putRepository( @@ -2611,7 +2621,7 @@ public static void putRepository( Settings.Builder settings, ActionListener listener ) { - putRepositoryRequestBuilder(adminClient, repoName, type, true, settings, null).execute(listener); + putRepositoryRequestBuilder(adminClient, repoName, type, true, settings, null, false).execute(listener); } public static PutRepositoryRequestBuilder putRepositoryRequestBuilder( @@ -2620,15 +2630,17 @@ public static PutRepositoryRequestBuilder putRepositoryRequestBuilder( String type, boolean verify, Settings.Builder settings, - String timeout + String timeout, + boolean finalSettings ) { - PutRepositoryRequestBuilder builder = adminClient.preparePutRepository(repoName) - .setType(type) - .setVerify(verify) - .setSettings(settings); + PutRepositoryRequestBuilder builder = adminClient.preparePutRepository(repoName).setType(type).setVerify(verify); if (timeout != null) { builder.setTimeout(timeout); } + if (finalSettings == false) { + settings.put(BlobStoreRepository.SHARD_PATH_TYPE.getKey(), randomFrom(PathType.values())); + } + builder.setSettings(settings); return builder; }