From df3d79c68df5d327a87b6d1445b3d967b994290a Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Tue, 16 Apr 2024 13:33:17 -0500 Subject: [PATCH] =?UTF-8?q?Revert=20"Add=20faster=20scaling=20composite=20?= =?UTF-8?q?hash=20value=20encoding=20for=20remote=20path=20(#13=E2=80=A6"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 3d1d5e7a9e707e02ace1b79d30110cc3d31a336c. --- .../remotestore/RemoteRestoreSnapshotIT.java | 16 +- .../common/settings/ClusterSettings.java | 3 +- .../index/remote/RemoteStoreEnums.java | 19 +- .../RemoteStorePathStrategyResolver.java | 14 +- .../index/remote/RemoteStoreUtils.java | 36 +-- .../opensearch/indices/IndicesService.java | 20 +- .../MetadataCreateIndexServiceTests.java | 4 +- .../index/remote/RemoteStoreEnumsTests.java | 244 ++---------------- .../RemoteStorePathStrategyResolverTests.java | 103 +------- .../index/remote/RemoteStoreUtilsTests.java | 116 +-------- ...oteStoreShardShallowCopySnapshotTests.java | 220 +--------------- .../RemoteSegmentStoreDirectoryTests.java | 2 +- .../test/OpenSearchIntegTestCase.java | 4 +- 13 files changed, 69 insertions(+), 732 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java index 95b7d4381da18..d34a5f4edbaec 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java @@ -59,7 +59,7 @@ import static org.opensearch.index.remote.RemoteStoreEnums.DataCategory.TRANSLOG; import static org.opensearch.index.remote.RemoteStoreEnums.DataType.DATA; import static org.opensearch.index.remote.RemoteStoreEnums.DataType.METADATA; -import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING; +import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_STORE_PATH_PREFIX_TYPE_SETTING; import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; @@ -229,7 +229,7 @@ public void testRemoteStoreCustomDataOnIndexCreationAndRestore() { client(clusterManagerNode).admin() .cluster() .prepareUpdateSettings() - .setTransientSettings(Settings.builder().put(CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING.getKey(), PathType.FIXED)) + .setTransientSettings(Settings.builder().put(CLUSTER_REMOTE_STORE_PATH_PREFIX_TYPE_SETTING.getKey(), PathType.FIXED)) .get(); createRepository(snapshotRepoName, "fs", getRepositorySettings(absolutePath1, true)); Client client = client(); @@ -260,7 +260,7 @@ public void testRemoteStoreCustomDataOnIndexCreationAndRestore() { client(clusterManagerNode).admin() .cluster() .prepareUpdateSettings() - .setTransientSettings(Settings.builder().put(CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING.getKey(), PathType.HASHED_PREFIX)) + .setTransientSettings(Settings.builder().put(CLUSTER_REMOTE_STORE_PATH_PREFIX_TYPE_SETTING.getKey(), PathType.HASHED_PREFIX)) .get(); restoreSnapshotResponse = client.admin() @@ -272,13 +272,13 @@ public void testRemoteStoreCustomDataOnIndexCreationAndRestore() { .get(); assertEquals(RestStatus.ACCEPTED, restoreSnapshotResponse.status()); ensureGreen(restoredIndexName1version2); - validatePathType(restoredIndexName1version2, PathType.HASHED_PREFIX, PathHashAlgorithm.FNV_1A_COMPOSITE_1); + validatePathType(restoredIndexName1version2, PathType.HASHED_PREFIX, PathHashAlgorithm.FNV_1A); - // Create index with cluster setting cluster.remote_store.index.path.type as hashed_prefix. + // Create index with cluster setting cluster.remote_store.index.path.prefix.type as hashed_prefix. indexSettings = getIndexSettings(1, 0).build(); createIndex(indexName2, indexSettings); ensureGreen(indexName2); - validatePathType(indexName2, PathType.HASHED_PREFIX, PathHashAlgorithm.FNV_1A_COMPOSITE_1); + validatePathType(indexName2, PathType.HASHED_PREFIX, PathHashAlgorithm.FNV_1A); // Validating that custom data has not changed for indexes which were created before the cluster setting got updated validatePathType(indexName1, PathType.FIXED); @@ -294,7 +294,7 @@ public void testRemoteStoreCustomDataOnIndexCreationAndRestore() { client(clusterManagerNode).admin() .cluster() .prepareUpdateSettings() - .setTransientSettings(Settings.builder().put(CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING.getKey(), PathType.FIXED)) + .setTransientSettings(Settings.builder().put(CLUSTER_REMOTE_STORE_PATH_PREFIX_TYPE_SETTING.getKey(), PathType.FIXED)) .get(); // Close index 2 @@ -309,7 +309,7 @@ public void testRemoteStoreCustomDataOnIndexCreationAndRestore() { ensureGreen(indexName2); // Validating that custom data has not changed for testindex2 which was created before the cluster setting got updated - validatePathType(indexName2, PathType.HASHED_PREFIX, PathHashAlgorithm.FNV_1A_COMPOSITE_1); + validatePathType(indexName2, PathType.HASHED_PREFIX, PathHashAlgorithm.FNV_1A); } private void validatePathType(String index, PathType pathType) { diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index 2904d49c224d7..fd352b33e87fa 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -713,8 +713,7 @@ public void apply(Settings value, Settings current, Settings previous) { RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING, IndicesService.CLUSTER_REMOTE_INDEX_RESTRICT_ASYNC_DURABILITY_SETTING, IndicesService.CLUSTER_INDEX_RESTRICT_REPLICATION_TYPE_SETTING, - IndicesService.CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING, - IndicesService.CLUSTER_REMOTE_STORE_PATH_HASH_ALGORITHM_SETTING, + IndicesService.CLUSTER_REMOTE_STORE_PATH_PREFIX_TYPE_SETTING, // Admission Control Settings AdmissionControlSettings.ADMISSION_CONTROL_TRANSPORT_LAYER_MODE, diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteStoreEnums.java b/server/src/main/java/org/opensearch/index/remote/RemoteStoreEnums.java index 9acf390c6b707..b51abf19fc000 100644 --- a/server/src/main/java/org/opensearch/index/remote/RemoteStoreEnums.java +++ b/server/src/main/java/org/opensearch/index/remote/RemoteStoreEnums.java @@ -23,8 +23,6 @@ import static java.util.Collections.unmodifiableMap; import static org.opensearch.index.remote.RemoteStoreEnums.DataType.DATA; import static org.opensearch.index.remote.RemoteStoreEnums.DataType.METADATA; -import static org.opensearch.index.remote.RemoteStoreUtils.longToCompositeBase64AndBinaryEncoding; -import static org.opensearch.index.remote.RemoteStoreUtils.longToUrlBase64; /** * This class contains the different enums related to remote store like data categories and types, path types @@ -218,26 +216,13 @@ public static PathType parseString(String pathType) { @PublicApi(since = "2.14.0") public enum PathHashAlgorithm { - FNV_1A_BASE64(0) { + FNV_1A(0) { @Override String hash(PathInput pathInput) { String input = pathInput.indexUUID() + pathInput.shardId() + pathInput.dataCategory().getName() + pathInput.dataType() .getName(); long hash = FNV1a.hash64(input); - return longToUrlBase64(hash); - } - }, - /** - * This hash algorithm will generate a hash value which will use 1st 6 bits to create bas64 character and next 14 - * bits to create binary string. - */ - FNV_1A_COMPOSITE_1(1) { - @Override - String hash(PathInput pathInput) { - String input = pathInput.indexUUID() + pathInput.shardId() + pathInput.dataCategory().getName() + pathInput.dataType() - .getName(); - long hash = FNV1a.hash64(input); - return longToCompositeBase64AndBinaryEncoding(hash, 20); + return RemoteStoreUtils.longToUrlBase64(hash); } }; diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteStorePathStrategyResolver.java b/server/src/main/java/org/opensearch/index/remote/RemoteStorePathStrategyResolver.java index f6925bcbcc92d..5b067115df781 100644 --- a/server/src/main/java/org/opensearch/index/remote/RemoteStorePathStrategyResolver.java +++ b/server/src/main/java/org/opensearch/index/remote/RemoteStorePathStrategyResolver.java @@ -25,16 +25,12 @@ public class RemoteStorePathStrategyResolver { private volatile PathType type; - private volatile PathHashAlgorithm hashAlgorithm; - private final Supplier minNodeVersionSupplier; public RemoteStorePathStrategyResolver(ClusterSettings clusterSettings, Supplier minNodeVersionSupplier) { this.minNodeVersionSupplier = minNodeVersionSupplier; - type = clusterSettings.get(IndicesService.CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING); - hashAlgorithm = clusterSettings.get(IndicesService.CLUSTER_REMOTE_STORE_PATH_HASH_ALGORITHM_SETTING); - clusterSettings.addSettingsUpdateConsumer(IndicesService.CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING, this::setType); - clusterSettings.addSettingsUpdateConsumer(IndicesService.CLUSTER_REMOTE_STORE_PATH_HASH_ALGORITHM_SETTING, this::setHashAlgorithm); + type = clusterSettings.get(IndicesService.CLUSTER_REMOTE_STORE_PATH_PREFIX_TYPE_SETTING); + clusterSettings.addSettingsUpdateConsumer(IndicesService.CLUSTER_REMOTE_STORE_PATH_PREFIX_TYPE_SETTING, this::setType); } public RemoteStorePathStrategy get() { @@ -43,15 +39,11 @@ public RemoteStorePathStrategy get() { // Min node version check ensures that we are enabling the new prefix type only when all the nodes understand it. pathType = Version.CURRENT.compareTo(minNodeVersionSupplier.get()) <= 0 ? type : PathType.FIXED; // If the path type is fixed, hash algorithm is not applicable. - pathHashAlgorithm = pathType == PathType.FIXED ? null : hashAlgorithm; + pathHashAlgorithm = pathType == PathType.FIXED ? null : PathHashAlgorithm.FNV_1A; return new RemoteStorePathStrategy(pathType, pathHashAlgorithm); } private void setType(PathType type) { this.type = type; } - - private void setHashAlgorithm(PathHashAlgorithm hashAlgorithm) { - this.hashAlgorithm = hashAlgorithm; - } } diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java b/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java index 4d1d98334c3c4..7d0743e70b6cb 100644 --- a/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java +++ b/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java @@ -15,7 +15,6 @@ import java.util.Base64; import java.util.HashMap; import java.util.List; -import java.util.Locale; import java.util.Map; import java.util.function.Function; @@ -27,16 +26,10 @@ public class RemoteStoreUtils { public static final int LONG_MAX_LENGTH = String.valueOf(Long.MAX_VALUE).length(); - /** - * URL safe base 64 character set. This must not be changed as this is used in deriving the base64 equivalent of binary. - */ - static final char[] URL_BASE64_CHARSET = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_".toCharArray(); - /** * This method subtracts given numbers from Long.MAX_VALUE and returns a string representation of the result. * The resultant string is guaranteed to be of the same length that of Long.MAX_VALUE. If shorter, we add left padding * of 0s to the string. - * * @param num number to get the inverted long string for * @return String value of Long.MAX_VALUE - num */ @@ -53,7 +46,6 @@ public static String invertLong(long num) { /** * This method converts the given string into long and subtracts it from Long.MAX_VALUE - * * @param str long in string format to be inverted * @return long value of the invert result */ @@ -67,7 +59,6 @@ public static long invertLong(String str) { /** * Extracts the segment name from the provided segment file name - * * @param filename Segment file name to parse * @return Name of the segment that the segment file belongs to */ @@ -88,9 +79,10 @@ public static String getSegmentName(String filename) { } /** + * * @param mdFiles List of segment/translog metadata files - * @param fn Function to extract PrimaryTerm_Generation and Node Id from metadata file name . - * fn returns null if node id is not part of the file name + * @param fn Function to extract PrimaryTerm_Generation and Node Id from metadata file name . + * fn returns null if node id is not part of the file name */ public static void verifyNoMultipleWriters(List mdFiles, Function> fn) { Map nodesByPrimaryTermAndGen = new HashMap<>(); @@ -124,26 +116,4 @@ static String longToUrlBase64(long value) { String base64Str = Base64.getUrlEncoder().encodeToString(hashBytes); return base64Str.substring(0, base64Str.length() - 1); } - - static long urlBase64ToLong(String base64Str) { - byte[] hashBytes = Base64.getUrlDecoder().decode(base64Str); - return ByteBuffer.wrap(hashBytes).getLong(); - } - - /** - * Converts an input hash which occupies 64 bits of memory into a composite encoded string. The string will have 2 parts - - * 1. Base 64 string and 2. Binary String. We will use the first 6 bits for creating the base 64 string. - * For the second part, the rest of the bits (of length {@code len}-6) will be used as is in string form. - */ - static String longToCompositeBase64AndBinaryEncoding(long value, int len) { - if (len < 7 || len > 64) { - throw new IllegalArgumentException("In longToCompositeBase64AndBinaryEncoding, len must be between 7 and 64 (both inclusive)"); - } - String binaryEncoding = String.format(Locale.ROOT, "%64s", Long.toBinaryString(value)).replace(' ', '0'); - String base64Part = binaryEncoding.substring(0, 6); - String binaryPart = binaryEncoding.substring(6, len); - int base64DecimalValue = Integer.valueOf(base64Part, 2); - assert base64DecimalValue >= 0 && base64DecimalValue < 64; - return URL_BASE64_CHARSET[base64DecimalValue] + binaryPart; - } } diff --git a/server/src/main/java/org/opensearch/indices/IndicesService.java b/server/src/main/java/org/opensearch/indices/IndicesService.java index df473a94a863e..7e2ea5a77cbfa 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesService.java +++ b/server/src/main/java/org/opensearch/indices/IndicesService.java @@ -124,7 +124,6 @@ import org.opensearch.index.query.QueryRewriteContext; import org.opensearch.index.recovery.RecoveryStats; import org.opensearch.index.refresh.RefreshStats; -import org.opensearch.index.remote.RemoteStoreEnums.PathHashAlgorithm; import org.opensearch.index.remote.RemoteStoreEnums.PathType; import org.opensearch.index.remote.RemoteStoreStatsTrackerFactory; import org.opensearch.index.search.stats.SearchStats; @@ -308,30 +307,17 @@ public class IndicesService extends AbstractLifecycleComponent ); /** - * This setting is used to set the remote store blob store path type strategy. This setting is effective only for + * This setting is used to set the remote store blob store path prefix strategy. This setting is effective only for * remote store enabled cluster. */ - public static final Setting CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING = new Setting<>( - "cluster.remote_store.index.path.type", + public static final Setting CLUSTER_REMOTE_STORE_PATH_PREFIX_TYPE_SETTING = new Setting<>( + "cluster.remote_store.index.path.prefix.type", PathType.FIXED.toString(), PathType::parseString, Property.NodeScope, Property.Dynamic ); - /** - * This setting is used to set the remote store blob store path hash algorithm strategy. This setting is effective only for - * remote store enabled cluster. This setting will come to effect if the {@link #CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING} - * is either {@code HASHED_PREFIX} or {@code HASHED_INFIX}. - */ - public static final Setting CLUSTER_REMOTE_STORE_PATH_HASH_ALGORITHM_SETTING = new Setting<>( - "cluster.remote_store.index.path.hash_algorithm", - PathHashAlgorithm.FNV_1A_COMPOSITE_1.toString(), - PathHashAlgorithm::parseString, - Property.NodeScope, - Property.Dynamic - ); - /** * The node's settings. */ diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java index 1a9321a755fef..d3086de6ec89e 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java @@ -1711,7 +1711,7 @@ public void testRemoteCustomData() { validateRemoteCustomData( indexMetadata.getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY), PathHashAlgorithm.NAME, - PathHashAlgorithm.FNV_1A_COMPOSITE_1.name() + PathHashAlgorithm.FNV_1A.name() ); } @@ -1720,7 +1720,7 @@ private IndexMetadata testRemoteCustomData(boolean remoteStoreEnabled, PathType if (remoteStoreEnabled) { settingsBuilder.put(NODE_ATTRIBUTES.getKey() + REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY, "test"); } - settingsBuilder.put(IndicesService.CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING.getKey(), pathType.toString()); + settingsBuilder.put(IndicesService.CLUSTER_REMOTE_STORE_PATH_PREFIX_TYPE_SETTING.getKey(), pathType.toString()); Settings settings = settingsBuilder.build(); ClusterService clusterService = mock(ClusterService.class); diff --git a/server/src/test/java/org/opensearch/index/remote/RemoteStoreEnumsTests.java b/server/src/test/java/org/opensearch/index/remote/RemoteStoreEnumsTests.java index 575b397382f24..fe5635063f783 100644 --- a/server/src/test/java/org/opensearch/index/remote/RemoteStoreEnumsTests.java +++ b/server/src/test/java/org/opensearch/index/remote/RemoteStoreEnumsTests.java @@ -25,8 +25,7 @@ import static org.opensearch.index.remote.RemoteStoreEnums.DataType.DATA; import static org.opensearch.index.remote.RemoteStoreEnums.DataType.LOCK_FILES; import static org.opensearch.index.remote.RemoteStoreEnums.DataType.METADATA; -import static org.opensearch.index.remote.RemoteStoreEnums.PathHashAlgorithm.FNV_1A_BASE64; -import static org.opensearch.index.remote.RemoteStoreEnums.PathHashAlgorithm.FNV_1A_COMPOSITE_1; +import static org.opensearch.index.remote.RemoteStoreEnums.PathHashAlgorithm.FNV_1A; import static org.opensearch.index.remote.RemoteStoreEnums.PathType.FIXED; import static org.opensearch.index.remote.RemoteStoreEnums.PathType.HASHED_INFIX; import static org.opensearch.index.remote.RemoteStoreEnums.PathType.HASHED_PREFIX; @@ -162,10 +161,10 @@ public void testGeneratePathForHashedPrefixType() { .dataCategory(dataCategory) .dataType(dataType) .build(); - BlobPath result = HASHED_PREFIX.path(pathInput, FNV_1A_BASE64); + BlobPath result = HASHED_PREFIX.path(pathInput, FNV_1A); assertTrue( result.buildAsString() - .startsWith(String.join(SEPARATOR, FNV_1A_BASE64.hash(pathInput), basePath, dataCategory.getName(), dataType.getName())) + .startsWith(String.join(SEPARATOR, FNV_1A.hash(pathInput), basePath, dataCategory.getName(), dataType.getName())) ); // assert with exact value for known base path @@ -179,7 +178,7 @@ public void testGeneratePathForHashedPrefixType() { .dataCategory(dataCategory) .dataType(dataType) .build(); - result = HASHED_PREFIX.path(pathInput, FNV_1A_BASE64); + result = HASHED_PREFIX.path(pathInput, FNV_1A); assertEquals("DgSI70IciXs/xjsdhj/ddjsha/yudy7sd/32hdhua7/89jdij/k2ijhe877d7yuhx7/10/translog/data/", result.buildAsString()); // Translog Metadata @@ -191,10 +190,10 @@ public void testGeneratePathForHashedPrefixType() { .dataCategory(dataCategory) .dataType(dataType) .build(); - result = HASHED_PREFIX.path(pathInput, FNV_1A_BASE64); + result = HASHED_PREFIX.path(pathInput, FNV_1A); assertTrue( result.buildAsString() - .startsWith(String.join(SEPARATOR, FNV_1A_BASE64.hash(pathInput), basePath, dataCategory.getName(), dataType.getName())) + .startsWith(String.join(SEPARATOR, FNV_1A.hash(pathInput), basePath, dataCategory.getName(), dataType.getName())) ); // assert with exact value for known base path @@ -205,7 +204,7 @@ public void testGeneratePathForHashedPrefixType() { .dataCategory(dataCategory) .dataType(dataType) .build(); - result = HASHED_PREFIX.path(pathInput, FNV_1A_BASE64); + result = HASHED_PREFIX.path(pathInput, FNV_1A); assertEquals("oKU5SjILiy4/xjsdhj/ddjsha/yudy7sd/32hdhua7/89jdij/k2ijhe877d7yuhx7/10/translog/metadata/", result.buildAsString()); // Translog Lock files - This is a negative case where the assertion will trip. @@ -239,10 +238,10 @@ public void testGeneratePathForHashedPrefixType() { .dataCategory(dataCategory) .dataType(dataType) .build(); - result = HASHED_PREFIX.path(pathInput, FNV_1A_BASE64); + result = HASHED_PREFIX.path(pathInput, FNV_1A); assertTrue( result.buildAsString() - .startsWith(String.join(SEPARATOR, FNV_1A_BASE64.hash(pathInput), basePath, dataCategory.getName(), dataType.getName())) + .startsWith(String.join(SEPARATOR, FNV_1A.hash(pathInput), basePath, dataCategory.getName(), dataType.getName())) ); // assert with exact value for known base path @@ -253,7 +252,7 @@ public void testGeneratePathForHashedPrefixType() { .dataCategory(dataCategory) .dataType(dataType) .build(); - result = HASHED_PREFIX.path(pathInput, FNV_1A_BASE64); + result = HASHED_PREFIX.path(pathInput, FNV_1A); assertEquals("AUBRfCIuWdk/xjsdhj/ddjsha/yudy7sd/32hdhua7/89jdij/k2ijhe877d7yuhx7/10/segments/data/", result.buildAsString()); // Segment Metadata @@ -265,10 +264,10 @@ public void testGeneratePathForHashedPrefixType() { .dataCategory(dataCategory) .dataType(dataType) .build(); - result = HASHED_PREFIX.path(pathInput, FNV_1A_BASE64); + result = HASHED_PREFIX.path(pathInput, FNV_1A); assertTrue( result.buildAsString() - .startsWith(String.join(SEPARATOR, FNV_1A_BASE64.hash(pathInput), basePath, dataCategory.getName(), dataType.getName())) + .startsWith(String.join(SEPARATOR, FNV_1A.hash(pathInput), basePath, dataCategory.getName(), dataType.getName())) ); // assert with exact value for known base path @@ -279,7 +278,7 @@ public void testGeneratePathForHashedPrefixType() { .dataCategory(dataCategory) .dataType(dataType) .build(); - result = HASHED_PREFIX.path(pathInput, FNV_1A_BASE64); + result = HASHED_PREFIX.path(pathInput, FNV_1A); assertEquals("erwR-G735Uw/xjsdhj/ddjsha/yudy7sd/32hdhua7/89jdij/k2ijhe877d7yuhx7/10/segments/metadata/", result.buildAsString()); // Segment Lockfiles @@ -291,10 +290,10 @@ public void testGeneratePathForHashedPrefixType() { .dataCategory(dataCategory) .dataType(dataType) .build(); - result = HASHED_PREFIX.path(pathInput, FNV_1A_BASE64); + result = HASHED_PREFIX.path(pathInput, FNV_1A); assertTrue( result.buildAsString() - .startsWith(String.join(SEPARATOR, FNV_1A_BASE64.hash(pathInput), basePath, dataCategory.getName(), dataType.getName())) + .startsWith(String.join(SEPARATOR, FNV_1A.hash(pathInput), basePath, dataCategory.getName(), dataType.getName())) ); // assert with exact value for known base path @@ -305,197 +304,10 @@ public void testGeneratePathForHashedPrefixType() { .dataCategory(dataCategory) .dataType(dataType) .build(); - result = HASHED_PREFIX.path(pathInput, FNV_1A_BASE64); + result = HASHED_PREFIX.path(pathInput, FNV_1A); assertEquals("KeYDIk0mJXI/xjsdhj/ddjsha/yudy7sd/32hdhua7/89jdij/k2ijhe877d7yuhx7/10/segments/lock_files/", result.buildAsString()); } - public void testGeneratePathForHashedPrefixTypeAndFNVCompositeHashAlgorithm() { - BlobPath blobPath = new BlobPath(); - List pathList = getPathList(); - for (String path : pathList) { - blobPath = blobPath.add(path); - } - - String indexUUID = randomAlphaOfLength(10); - String shardId = String.valueOf(randomInt(100)); - DataCategory dataCategory = TRANSLOG; - DataType dataType = DATA; - - String basePath = getPath(pathList) + indexUUID + SEPARATOR + shardId; - // Translog Data - PathInput pathInput = PathInput.builder() - .basePath(blobPath) - .indexUUID(indexUUID) - .shardId(shardId) - .dataCategory(dataCategory) - .dataType(dataType) - .build(); - BlobPath result = HASHED_PREFIX.path(pathInput, FNV_1A_COMPOSITE_1); - assertTrue( - result.buildAsString() - .startsWith( - String.join(SEPARATOR, FNV_1A_COMPOSITE_1.hash(pathInput), basePath, dataCategory.getName(), dataType.getName()) - ) - ); - - // assert with exact value for known base path - BlobPath fixedBlobPath = BlobPath.cleanPath().add("xjsdhj").add("ddjsha").add("yudy7sd").add("32hdhua7").add("89jdij"); - String fixedIndexUUID = "k2ijhe877d7yuhx7"; - String fixedShardId = "10"; - pathInput = PathInput.builder() - .basePath(fixedBlobPath) - .indexUUID(fixedIndexUUID) - .shardId(fixedShardId) - .dataCategory(dataCategory) - .dataType(dataType) - .build(); - result = HASHED_PREFIX.path(pathInput, FNV_1A_COMPOSITE_1); - assertEquals("D10000001001000/xjsdhj/ddjsha/yudy7sd/32hdhua7/89jdij/k2ijhe877d7yuhx7/10/translog/data/", result.buildAsString()); - - // Translog Metadata - dataType = METADATA; - pathInput = PathInput.builder() - .basePath(blobPath) - .indexUUID(indexUUID) - .shardId(shardId) - .dataCategory(dataCategory) - .dataType(dataType) - .build(); - result = HASHED_PREFIX.path(pathInput, FNV_1A_COMPOSITE_1); - assertTrue( - result.buildAsString() - .startsWith( - String.join(SEPARATOR, FNV_1A_COMPOSITE_1.hash(pathInput), basePath, dataCategory.getName(), dataType.getName()) - ) - ); - - // assert with exact value for known base path - pathInput = PathInput.builder() - .basePath(fixedBlobPath) - .indexUUID(fixedIndexUUID) - .shardId(fixedShardId) - .dataCategory(dataCategory) - .dataType(dataType) - .build(); - result = HASHED_PREFIX.path(pathInput, FNV_1A_COMPOSITE_1); - assertEquals( - "o00101001010011/xjsdhj/ddjsha/yudy7sd/32hdhua7/89jdij/k2ijhe877d7yuhx7/10/translog/metadata/", - result.buildAsString() - ); - - // Translog Lock files - This is a negative case where the assertion will trip. - dataType = LOCK_FILES; - PathInput finalPathInput = PathInput.builder() - .basePath(blobPath) - .indexUUID(indexUUID) - .shardId(shardId) - .dataCategory(dataCategory) - .dataType(dataType) - .build(); - assertThrows(AssertionError.class, () -> HASHED_PREFIX.path(finalPathInput, null)); - - // assert with exact value for known base path - pathInput = PathInput.builder() - .basePath(fixedBlobPath) - .indexUUID(fixedIndexUUID) - .shardId(fixedShardId) - .dataCategory(dataCategory) - .dataType(dataType) - .build(); - assertThrows(AssertionError.class, () -> HASHED_PREFIX.path(finalPathInput, null)); - - // Segment Data - dataCategory = SEGMENTS; - dataType = DATA; - pathInput = PathInput.builder() - .basePath(blobPath) - .indexUUID(indexUUID) - .shardId(shardId) - .dataCategory(dataCategory) - .dataType(dataType) - .build(); - result = HASHED_PREFIX.path(pathInput, FNV_1A_COMPOSITE_1); - assertTrue( - result.buildAsString() - .startsWith( - String.join(SEPARATOR, FNV_1A_COMPOSITE_1.hash(pathInput), basePath, dataCategory.getName(), dataType.getName()) - ) - ); - - // assert with exact value for known base path - pathInput = PathInput.builder() - .basePath(fixedBlobPath) - .indexUUID(fixedIndexUUID) - .shardId(fixedShardId) - .dataCategory(dataCategory) - .dataType(dataType) - .build(); - result = HASHED_PREFIX.path(pathInput, FNV_1A_COMPOSITE_1); - assertEquals("A01010000000101/xjsdhj/ddjsha/yudy7sd/32hdhua7/89jdij/k2ijhe877d7yuhx7/10/segments/data/", result.buildAsString()); - - // Segment Metadata - dataType = METADATA; - pathInput = PathInput.builder() - .basePath(blobPath) - .indexUUID(indexUUID) - .shardId(shardId) - .dataCategory(dataCategory) - .dataType(dataType) - .build(); - result = HASHED_PREFIX.path(pathInput, FNV_1A_COMPOSITE_1); - assertTrue( - result.buildAsString() - .startsWith( - String.join(SEPARATOR, FNV_1A_COMPOSITE_1.hash(pathInput), basePath, dataCategory.getName(), dataType.getName()) - ) - ); - - // assert with exact value for known base path - pathInput = PathInput.builder() - .basePath(fixedBlobPath) - .indexUUID(fixedIndexUUID) - .shardId(fixedShardId) - .dataCategory(dataCategory) - .dataType(dataType) - .build(); - result = HASHED_PREFIX.path(pathInput, FNV_1A_COMPOSITE_1); - assertEquals( - "e10101111000001/xjsdhj/ddjsha/yudy7sd/32hdhua7/89jdij/k2ijhe877d7yuhx7/10/segments/metadata/", - result.buildAsString() - ); - - // Segment Lockfiles - dataType = LOCK_FILES; - pathInput = PathInput.builder() - .basePath(blobPath) - .indexUUID(indexUUID) - .shardId(shardId) - .dataCategory(dataCategory) - .dataType(dataType) - .build(); - result = HASHED_PREFIX.path(pathInput, FNV_1A_COMPOSITE_1); - assertTrue( - result.buildAsString() - .startsWith( - String.join(SEPARATOR, FNV_1A_COMPOSITE_1.hash(pathInput), basePath, dataCategory.getName(), dataType.getName()) - ) - ); - - // assert with exact value for known base path - pathInput = PathInput.builder() - .basePath(fixedBlobPath) - .indexUUID(fixedIndexUUID) - .shardId(fixedShardId) - .dataCategory(dataCategory) - .dataType(dataType) - .build(); - result = HASHED_PREFIX.path(pathInput, FNV_1A_COMPOSITE_1); - assertEquals( - "K01111001100000/xjsdhj/ddjsha/yudy7sd/32hdhua7/89jdij/k2ijhe877d7yuhx7/10/segments/lock_files/", - result.buildAsString() - ); - } - public void testGeneratePathForHashedInfixType() { BlobPath blobPath = new BlobPath(); List pathList = getPathList(); @@ -518,7 +330,7 @@ public void testGeneratePathForHashedInfixType() { .dataCategory(dataCategory) .dataType(dataType) .build(); - BlobPath result = HASHED_INFIX.path(pathInput, FNV_1A_BASE64); + BlobPath result = HASHED_INFIX.path(pathInput, FNV_1A); String expected = derivePath(basePath, pathInput); String actual = result.buildAsString(); assertTrue(new ParameterizedMessage("expected={} actual={}", expected, actual).getFormattedMessage(), actual.startsWith(expected)); @@ -534,7 +346,7 @@ public void testGeneratePathForHashedInfixType() { .dataCategory(dataCategory) .dataType(dataType) .build(); - result = HASHED_INFIX.path(pathInput, FNV_1A_BASE64); + result = HASHED_INFIX.path(pathInput, FNV_1A); expected = "xjsdhj/ddjsha/yudy7sd/32hdhua7/89jdij/DgSI70IciXs/k2ijhe877d7yuhx7/10/translog/data/"; actual = result.buildAsString(); assertTrue(new ParameterizedMessage("expected={} actual={}", expected, actual).getFormattedMessage(), actual.startsWith(expected)); @@ -549,7 +361,7 @@ public void testGeneratePathForHashedInfixType() { .dataType(dataType) .build(); - result = HASHED_INFIX.path(pathInput, FNV_1A_BASE64); + result = HASHED_INFIX.path(pathInput, FNV_1A); expected = derivePath(basePath, pathInput); actual = result.buildAsString(); assertTrue(new ParameterizedMessage("expected={} actual={}", expected, actual).getFormattedMessage(), actual.startsWith(expected)); @@ -562,7 +374,7 @@ public void testGeneratePathForHashedInfixType() { .dataCategory(dataCategory) .dataType(dataType) .build(); - result = HASHED_INFIX.path(pathInput, FNV_1A_BASE64); + result = HASHED_INFIX.path(pathInput, FNV_1A); expected = "xjsdhj/ddjsha/yudy7sd/32hdhua7/89jdij/oKU5SjILiy4/k2ijhe877d7yuhx7/10/translog/metadata/"; actual = result.buildAsString(); assertTrue(new ParameterizedMessage("expected={} actual={}", expected, actual).getFormattedMessage(), actual.startsWith(expected)); @@ -598,7 +410,7 @@ public void testGeneratePathForHashedInfixType() { .dataCategory(dataCategory) .dataType(dataType) .build(); - result = HASHED_INFIX.path(pathInput, FNV_1A_BASE64); + result = HASHED_INFIX.path(pathInput, FNV_1A); expected = derivePath(basePath, pathInput); actual = result.buildAsString(); assertTrue(new ParameterizedMessage("expected={} actual={}", expected, actual).getFormattedMessage(), actual.startsWith(expected)); @@ -611,7 +423,7 @@ public void testGeneratePathForHashedInfixType() { .dataCategory(dataCategory) .dataType(dataType) .build(); - result = HASHED_INFIX.path(pathInput, FNV_1A_BASE64); + result = HASHED_INFIX.path(pathInput, FNV_1A); expected = "xjsdhj/ddjsha/yudy7sd/32hdhua7/89jdij/AUBRfCIuWdk/k2ijhe877d7yuhx7/10/segments/data/"; actual = result.buildAsString(); assertTrue(new ParameterizedMessage("expected={} actual={}", expected, actual).getFormattedMessage(), actual.startsWith(expected)); @@ -625,7 +437,7 @@ public void testGeneratePathForHashedInfixType() { .dataCategory(dataCategory) .dataType(dataType) .build(); - result = HASHED_INFIX.path(pathInput, FNV_1A_BASE64); + result = HASHED_INFIX.path(pathInput, FNV_1A); expected = derivePath(basePath, pathInput); actual = result.buildAsString(); assertTrue(new ParameterizedMessage("expected={} actual={}", expected, actual).getFormattedMessage(), actual.startsWith(expected)); @@ -638,7 +450,7 @@ public void testGeneratePathForHashedInfixType() { .dataCategory(dataCategory) .dataType(dataType) .build(); - result = HASHED_INFIX.path(pathInput, FNV_1A_BASE64); + result = HASHED_INFIX.path(pathInput, FNV_1A); expected = "xjsdhj/ddjsha/yudy7sd/32hdhua7/89jdij/erwR-G735Uw/k2ijhe877d7yuhx7/10/segments/metadata/"; actual = result.buildAsString(); assertTrue(new ParameterizedMessage("expected={} actual={}", expected, actual).getFormattedMessage(), actual.startsWith(expected)); @@ -652,7 +464,7 @@ public void testGeneratePathForHashedInfixType() { .dataCategory(dataCategory) .dataType(dataType) .build(); - result = HASHED_INFIX.path(pathInput, FNV_1A_BASE64); + result = HASHED_INFIX.path(pathInput, FNV_1A); expected = derivePath(basePath, pathInput); actual = result.buildAsString(); assertTrue(new ParameterizedMessage("expected={} actual={}", expected, actual).getFormattedMessage(), actual.startsWith(expected)); @@ -665,7 +477,7 @@ public void testGeneratePathForHashedInfixType() { .dataCategory(dataCategory) .dataType(dataType) .build(); - result = HASHED_INFIX.path(pathInput, FNV_1A_BASE64); + result = HASHED_INFIX.path(pathInput, FNV_1A); expected = "xjsdhj/ddjsha/yudy7sd/32hdhua7/89jdij/KeYDIk0mJXI/k2ijhe877d7yuhx7/10/segments/lock_files/"; actual = result.buildAsString(); assertTrue(new ParameterizedMessage("expected={} actual={}", expected, actual).getFormattedMessage(), actual.startsWith(expected)); @@ -675,7 +487,7 @@ private String derivePath(String basePath, PathInput pathInput) { return "".equals(basePath) ? String.join( SEPARATOR, - FNV_1A_BASE64.hash(pathInput), + FNV_1A.hash(pathInput), pathInput.indexUUID(), pathInput.shardId(), pathInput.dataCategory().getName(), @@ -684,7 +496,7 @@ private String derivePath(String basePath, PathInput pathInput) { : String.join( SEPARATOR, basePath, - FNV_1A_BASE64.hash(pathInput), + FNV_1A.hash(pathInput), pathInput.indexUUID(), pathInput.shardId(), pathInput.dataCategory().getName(), diff --git a/server/src/test/java/org/opensearch/index/remote/RemoteStorePathStrategyResolverTests.java b/server/src/test/java/org/opensearch/index/remote/RemoteStorePathStrategyResolverTests.java index 4aa0d11601a05..9d4b41f5c395f 100644 --- a/server/src/test/java/org/opensearch/index/remote/RemoteStorePathStrategyResolverTests.java +++ b/server/src/test/java/org/opensearch/index/remote/RemoteStorePathStrategyResolverTests.java @@ -11,17 +11,17 @@ import org.opensearch.Version; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; -import org.opensearch.index.remote.RemoteStoreEnums.PathHashAlgorithm; import org.opensearch.index.remote.RemoteStoreEnums.PathType; import org.opensearch.test.OpenSearchTestCase; -import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_STORE_PATH_HASH_ALGORITHM_SETTING; -import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING; +import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_STORE_PATH_PREFIX_TYPE_SETTING; public class RemoteStorePathStrategyResolverTests extends OpenSearchTestCase { public void testGetMinVersionOlder() { - Settings settings = Settings.builder().put(CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING.getKey(), randomFrom(PathType.values())).build(); + Settings settings = Settings.builder() + .put(CLUSTER_REMOTE_STORE_PATH_PREFIX_TYPE_SETTING.getKey(), randomFrom(PathType.values())) + .build(); ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); RemoteStorePathStrategyResolver resolver = new RemoteStorePathStrategyResolver(clusterSettings, () -> Version.V_2_13_0); assertEquals(PathType.FIXED, resolver.get().getType()); @@ -30,7 +30,7 @@ public void testGetMinVersionOlder() { public void testGetMinVersionNewer() { PathType pathType = randomFrom(PathType.values()); - Settings settings = Settings.builder().put(CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING.getKey(), pathType).build(); + Settings settings = Settings.builder().put(CLUSTER_REMOTE_STORE_PATH_PREFIX_TYPE_SETTING.getKey(), pathType).build(); ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); RemoteStorePathStrategyResolver resolver = new RemoteStorePathStrategyResolver(clusterSettings, () -> Version.CURRENT); assertEquals(pathType, resolver.get().getType()); @@ -39,100 +39,7 @@ public void testGetMinVersionNewer() { } else { assertNull(resolver.get().getHashAlgorithm()); } - } - - public void testGetStrategy() { - // FIXED type - Settings settings = Settings.builder().put(CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING.getKey(), PathType.FIXED).build(); - ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - RemoteStorePathStrategyResolver resolver = new RemoteStorePathStrategyResolver(clusterSettings, () -> Version.CURRENT); - assertEquals(PathType.FIXED, resolver.get().getType()); - - // FIXED type with hash algorithm - settings = Settings.builder() - .put(CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING.getKey(), PathType.FIXED) - .put(CLUSTER_REMOTE_STORE_PATH_HASH_ALGORITHM_SETTING.getKey(), randomFrom(PathHashAlgorithm.values())) - .build(); - clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - resolver = new RemoteStorePathStrategyResolver(clusterSettings, () -> Version.CURRENT); - assertEquals(PathType.FIXED, resolver.get().getType()); - - // HASHED_PREFIX type with FNV_1A_COMPOSITE - settings = Settings.builder().put(CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING.getKey(), PathType.HASHED_PREFIX).build(); - clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - resolver = new RemoteStorePathStrategyResolver(clusterSettings, () -> Version.CURRENT); - assertEquals(PathType.HASHED_PREFIX, resolver.get().getType()); - assertEquals(PathHashAlgorithm.FNV_1A_COMPOSITE_1, resolver.get().getHashAlgorithm()); - - // HASHED_PREFIX type with FNV_1A_COMPOSITE - settings = Settings.builder().put(CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING.getKey(), PathType.HASHED_PREFIX).build(); - clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - resolver = new RemoteStorePathStrategyResolver(clusterSettings, () -> Version.CURRENT); - assertEquals(PathType.HASHED_PREFIX, resolver.get().getType()); - assertEquals(PathHashAlgorithm.FNV_1A_COMPOSITE_1, resolver.get().getHashAlgorithm()); - - // HASHED_PREFIX type with FNV_1A_BASE64 - settings = Settings.builder() - .put(CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING.getKey(), PathType.HASHED_PREFIX) - .put(CLUSTER_REMOTE_STORE_PATH_HASH_ALGORITHM_SETTING.getKey(), PathHashAlgorithm.FNV_1A_BASE64) - .build(); - clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - resolver = new RemoteStorePathStrategyResolver(clusterSettings, () -> Version.CURRENT); - assertEquals(PathType.HASHED_PREFIX, resolver.get().getType()); - assertEquals(PathHashAlgorithm.FNV_1A_BASE64, resolver.get().getHashAlgorithm()); - // HASHED_PREFIX type with FNV_1A_BASE64 - settings = Settings.builder() - .put(CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING.getKey(), PathType.HASHED_PREFIX) - .put(CLUSTER_REMOTE_STORE_PATH_HASH_ALGORITHM_SETTING.getKey(), PathHashAlgorithm.FNV_1A_BASE64) - .build(); - clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - resolver = new RemoteStorePathStrategyResolver(clusterSettings, () -> Version.CURRENT); - assertEquals(PathType.HASHED_PREFIX, resolver.get().getType()); - assertEquals(PathHashAlgorithm.FNV_1A_BASE64, resolver.get().getHashAlgorithm()); } - public void testGetStrategyWithDynamicUpdate() { - - // Default value - Settings settings = Settings.builder().build(); - ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - RemoteStorePathStrategyResolver resolver = new RemoteStorePathStrategyResolver(clusterSettings, () -> Version.CURRENT); - assertEquals(PathType.FIXED, resolver.get().getType()); - assertNull(resolver.get().getHashAlgorithm()); - - // Set HASHED_PREFIX with default hash algorithm - clusterSettings.applySettings( - Settings.builder().put(CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING.getKey(), PathType.HASHED_PREFIX).build() - ); - assertEquals(PathType.HASHED_PREFIX, resolver.get().getType()); - assertEquals(PathHashAlgorithm.FNV_1A_COMPOSITE_1, resolver.get().getHashAlgorithm()); - - // Set HASHED_PREFIX with FNV_1A_BASE64 hash algorithm - clusterSettings.applySettings( - Settings.builder() - .put(CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING.getKey(), PathType.HASHED_PREFIX) - .put(CLUSTER_REMOTE_STORE_PATH_HASH_ALGORITHM_SETTING.getKey(), PathHashAlgorithm.FNV_1A_BASE64) - .build() - ); - assertEquals(PathType.HASHED_PREFIX, resolver.get().getType()); - assertEquals(PathHashAlgorithm.FNV_1A_BASE64, resolver.get().getHashAlgorithm()); - - // Set HASHED_INFIX with default hash algorithm - clusterSettings.applySettings( - Settings.builder().put(CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING.getKey(), PathType.HASHED_INFIX).build() - ); - assertEquals(PathType.HASHED_INFIX, resolver.get().getType()); - assertEquals(PathHashAlgorithm.FNV_1A_COMPOSITE_1, resolver.get().getHashAlgorithm()); - - // Set HASHED_INFIX with FNV_1A_BASE64 hash algorithm - clusterSettings.applySettings( - Settings.builder() - .put(CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING.getKey(), PathType.HASHED_INFIX) - .put(CLUSTER_REMOTE_STORE_PATH_HASH_ALGORITHM_SETTING.getKey(), PathHashAlgorithm.FNV_1A_BASE64) - .build() - ); - assertEquals(PathType.HASHED_INFIX, resolver.get().getType()); - assertEquals(PathHashAlgorithm.FNV_1A_BASE64, resolver.get().getHashAlgorithm()); - } } diff --git a/server/src/test/java/org/opensearch/index/remote/RemoteStoreUtilsTests.java b/server/src/test/java/org/opensearch/index/remote/RemoteStoreUtilsTests.java index 4d3e633848975..34074861f2764 100644 --- a/server/src/test/java/org/opensearch/index/remote/RemoteStoreUtilsTests.java +++ b/server/src/test/java/org/opensearch/index/remote/RemoteStoreUtilsTests.java @@ -14,19 +14,13 @@ import org.opensearch.index.translog.transfer.TranslogTransferMetadata; import org.opensearch.test.OpenSearchTestCase; -import java.math.BigInteger; import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.stream.Collectors; -import static org.opensearch.index.remote.RemoteStoreUtils.URL_BASE64_CHARSET; -import static org.opensearch.index.remote.RemoteStoreUtils.longToCompositeBase64AndBinaryEncoding; import static org.opensearch.index.remote.RemoteStoreUtils.longToUrlBase64; -import static org.opensearch.index.remote.RemoteStoreUtils.urlBase64ToLong; import static org.opensearch.index.remote.RemoteStoreUtils.verifyNoMultipleWriters; import static org.opensearch.index.store.RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX; import static org.opensearch.index.store.RemoteSegmentStoreDirectory.MetadataFilenameUtils.SEPARATOR; @@ -34,16 +28,6 @@ public class RemoteStoreUtilsTests extends OpenSearchTestCase { - private static Map BASE64_CHARSET_IDX_MAP; - - static { - Map charToIndexMap = new HashMap<>(); - for (int i = 0; i < URL_BASE64_CHARSET.length; i++) { - charToIndexMap.put(URL_BASE64_CHARSET[i], i); - } - BASE64_CHARSET_IDX_MAP = Collections.unmodifiableMap(charToIndexMap); - } - private final String metadataFilename = RemoteSegmentStoreDirectory.MetadataFilenameUtils.getMetadataFilename( 12, 23, @@ -221,106 +205,8 @@ public void testLongToBase64() { "6kv3yZNv9kY" ); for (Map.Entry entry : longToExpectedBase64String.entrySet()) { - String base64Str = longToUrlBase64(entry.getKey()); - assertEquals(entry.getValue(), base64Str); + assertEquals(entry.getValue(), longToUrlBase64(entry.getKey())); assertEquals(11, entry.getValue().length()); - assertEquals((long) entry.getKey(), urlBase64ToLong(base64Str)); - } - - int iters = randomInt(100); - for (int i = 0; i < iters; i++) { - long value = randomLong(); - String base64Str = longToUrlBase64(value); - assertEquals(value, urlBase64ToLong(base64Str)); } } - - public void testLongToCompositeUrlBase64AndBinaryEncodingUsing20Bits() { - Map longToExpectedBase64String = Map.of( - -5537941589147079860L, - "s11001001010100", - -5878421770170594047L, - "r10011010111010", - -5147010836697060622L, - "u00100100100010", - 937096430362711837L, - "D01000000010011", - 8422273604115462710L, - "d00111000011110", - -2528761975013221124L, - "300111010000000", - -5512387536280560513L, - "s11100000000001", - -5749656451579835857L, - "s00001101010001", - 5569654857969679538L, - "T01010010110110", - -1563884000447039930L, - "610010010111111" - ); - for (Map.Entry entry : longToExpectedBase64String.entrySet()) { - String base64Str = RemoteStoreUtils.longToCompositeBase64AndBinaryEncoding(entry.getKey(), 20); - assertEquals(entry.getValue(), base64Str); - assertEquals(15, entry.getValue().length()); - assertEquals(longToUrlBase64(entry.getKey()).charAt(0), base64Str.charAt(0)); - } - - int iters = randomInt(1000); - for (int i = 0; i < iters; i++) { - long value = randomLong(); - assertEquals(RemoteStoreUtils.longToCompositeBase64AndBinaryEncoding(value, 20).charAt(0), longToUrlBase64(value).charAt(0)); - } - } - - public void testLongToCompositeUrlBase64AndBinaryEncoding() { - Map longToExpectedBase64String = Map.of( - -5537941589147079860L, - "s1100100101010001110111011101001000000001101010101101001100", - -5878421770170594047L, - "r1001101011101001101000101110010101000011110000110100000001", - -5147010836697060622L, - "u0010010010001001001110100111111111100101011110101011110010", - 937096430362711837L, - "D0100000001001111000011110100001100000011100101011100011101", - 8422273604115462710L, - "d0011100001111011010011100001000110011100110111101000110110", - -2528761975013221124L, - "30011101000000010000110000110110101110100100101110011111100", - -5512387536280560513L, - "s1110000000000100001011110111011011101101001101110001111111", - -5749656451579835857L, - "s0000110101000111011110101110010111000011010000101000101111", - 5569654857969679538L, - "T0101001011011000111001010110000010110011111011110010110010", - -1563884000447039930L, - "61001001011111101111100100110010011011011111111011001000110" - ); - for (Map.Entry entry : longToExpectedBase64String.entrySet()) { - Long hashValue = entry.getKey(); - String expectedCompositeEncoding = entry.getValue(); - String actualCompositeEncoding = longToCompositeBase64AndBinaryEncoding(hashValue, 64); - assertEquals(expectedCompositeEncoding, actualCompositeEncoding); - assertEquals(59, expectedCompositeEncoding.length()); - assertEquals(longToUrlBase64(entry.getKey()).charAt(0), actualCompositeEncoding.charAt(0)); - assertEquals(RemoteStoreUtils.longToCompositeBase64AndBinaryEncoding(hashValue, 20), actualCompositeEncoding.substring(0, 15)); - - Long computedHashValue = compositeUrlBase64BinaryEncodingToLong(actualCompositeEncoding); - assertEquals(hashValue, computedHashValue); - } - - int iters = randomInt(1000); - for (int i = 0; i < iters; i++) { - long value = randomLong(); - String compositeEncoding = longToCompositeBase64AndBinaryEncoding(value, 64); - assertEquals(value, compositeUrlBase64BinaryEncodingToLong(compositeEncoding)); - } - } - - static long compositeUrlBase64BinaryEncodingToLong(String encodedValue) { - char ch = encodedValue.charAt(0); - int base64BitsIntValue = BASE64_CHARSET_IDX_MAP.get(ch); - String base64PartBinary = Integer.toBinaryString(base64BitsIntValue); - String binaryString = base64PartBinary + encodedValue.substring(1); - return new BigInteger(binaryString, 2).longValue(); - } } diff --git a/server/src/test/java/org/opensearch/index/snapshots/blobstore/RemoteStoreShardShallowCopySnapshotTests.java b/server/src/test/java/org/opensearch/index/snapshots/blobstore/RemoteStoreShardShallowCopySnapshotTests.java index e81eef67d6704..e3259a3097278 100644 --- a/server/src/test/java/org/opensearch/index/snapshots/blobstore/RemoteStoreShardShallowCopySnapshotTests.java +++ b/server/src/test/java/org/opensearch/index/snapshots/blobstore/RemoteStoreShardShallowCopySnapshotTests.java @@ -104,7 +104,7 @@ public void testToXContent() throws IOException { + "\"test-repo-basepath\",\"file_names\":[\"file1\",\"file2\",\"file3\",\"file4\",\"file5\"],\"path_type\":0}"; assert Objects.equals(actual, expectedXContent) : "xContent is " + actual; - // Case 3 - with just hashed prefix type and FNV_1A_BASE64 hash algorithm + // Case 3 - with just hashed prefix type and hash algorithm shardShallowCopySnapshot = new RemoteStoreShardShallowCopySnapshot( snapshot, indexVersion, @@ -119,7 +119,7 @@ public void testToXContent() throws IOException { repositoryBasePath, fileNames, PathType.HASHED_PREFIX, - PathHashAlgorithm.FNV_1A_BASE64 + PathHashAlgorithm.FNV_1A ); try (XContentBuilder builder = MediaTypeRegistry.JSON.contentBuilder()) { builder.startObject(); @@ -134,99 +134,6 @@ public void testToXContent() throws IOException { + "\"test-repo-basepath\",\"file_names\":[\"file1\",\"file2\",\"file3\",\"file4\",\"file5\"],\"path_type\":1" + ",\"path_hash_algorithm\":0}"; assert Objects.equals(actual, expectedXContent) : "xContent is " + actual; - - // Case 4 - with just hashed prefix type and FNV_1A_COMPOSITE hash algorithm - shardShallowCopySnapshot = new RemoteStoreShardShallowCopySnapshot( - snapshot, - indexVersion, - primaryTerm, - commitGeneration, - startTime, - time, - totalFileCount, - totalSize, - indexUUID, - remoteStoreRepository, - repositoryBasePath, - fileNames, - PathType.HASHED_PREFIX, - PathHashAlgorithm.FNV_1A_COMPOSITE_1 - ); - try (XContentBuilder builder = MediaTypeRegistry.JSON.contentBuilder()) { - builder.startObject(); - shardShallowCopySnapshot.toXContent(builder, ToXContent.EMPTY_PARAMS); - builder.endObject(); - actual = builder.toString(); - } - - expectedXContent = "{\"version\":\"2\",\"name\":\"test-snapshot\",\"index_version\":1,\"start_time\":123,\"time\":123," - + "\"number_of_files\":5,\"total_size\":5,\"index_uuid\":\"syzhajds-ashdlfj\",\"remote_store_repository\":" - + "\"test-rs-repository\",\"commit_generation\":5,\"primary_term\":3,\"remote_store_repository_base_path\":" - + "\"test-repo-basepath\",\"file_names\":[\"file1\",\"file2\",\"file3\",\"file4\",\"file5\"],\"path_type\":1" - + ",\"path_hash_algorithm\":1}"; - assert Objects.equals(actual, expectedXContent) : "xContent is " + actual; - - // Case 5 - with just hashed infix type and FNV_1A_BASE64 hash algorithm - shardShallowCopySnapshot = new RemoteStoreShardShallowCopySnapshot( - snapshot, - indexVersion, - primaryTerm, - commitGeneration, - startTime, - time, - totalFileCount, - totalSize, - indexUUID, - remoteStoreRepository, - repositoryBasePath, - fileNames, - PathType.HASHED_INFIX, - PathHashAlgorithm.FNV_1A_BASE64 - ); - try (XContentBuilder builder = MediaTypeRegistry.JSON.contentBuilder()) { - builder.startObject(); - shardShallowCopySnapshot.toXContent(builder, ToXContent.EMPTY_PARAMS); - builder.endObject(); - actual = builder.toString(); - } - - expectedXContent = "{\"version\":\"2\",\"name\":\"test-snapshot\",\"index_version\":1,\"start_time\":123,\"time\":123," - + "\"number_of_files\":5,\"total_size\":5,\"index_uuid\":\"syzhajds-ashdlfj\",\"remote_store_repository\":" - + "\"test-rs-repository\",\"commit_generation\":5,\"primary_term\":3,\"remote_store_repository_base_path\":" - + "\"test-repo-basepath\",\"file_names\":[\"file1\",\"file2\",\"file3\",\"file4\",\"file5\"],\"path_type\":2" - + ",\"path_hash_algorithm\":0}"; - assert Objects.equals(actual, expectedXContent) : "xContent is " + actual; - - // Case 6 - with just hashed infix type and FNV_1A_COMPOSITE hash algorithm - shardShallowCopySnapshot = new RemoteStoreShardShallowCopySnapshot( - snapshot, - indexVersion, - primaryTerm, - commitGeneration, - startTime, - time, - totalFileCount, - totalSize, - indexUUID, - remoteStoreRepository, - repositoryBasePath, - fileNames, - PathType.HASHED_INFIX, - PathHashAlgorithm.FNV_1A_COMPOSITE_1 - ); - try (XContentBuilder builder = MediaTypeRegistry.JSON.contentBuilder()) { - builder.startObject(); - shardShallowCopySnapshot.toXContent(builder, ToXContent.EMPTY_PARAMS); - builder.endObject(); - actual = builder.toString(); - } - - expectedXContent = "{\"version\":\"2\",\"name\":\"test-snapshot\",\"index_version\":1,\"start_time\":123,\"time\":123," - + "\"number_of_files\":5,\"total_size\":5,\"index_uuid\":\"syzhajds-ashdlfj\",\"remote_store_repository\":" - + "\"test-rs-repository\",\"commit_generation\":5,\"primary_term\":3,\"remote_store_repository_base_path\":" - + "\"test-repo-basepath\",\"file_names\":[\"file1\",\"file2\",\"file3\",\"file4\",\"file5\"],\"path_type\":2" - + ",\"path_hash_algorithm\":1}"; - assert Objects.equals(actual, expectedXContent) : "xContent is " + actual; } public void testFromXContent() throws IOException { @@ -316,88 +223,7 @@ public void testFromXContent() throws IOException { repositoryBasePath, fileNames, PathType.HASHED_PREFIX, - PathHashAlgorithm.FNV_1A_BASE64 - ); - try (XContentParser parser = createParser(JsonXContent.jsonXContent, xContent)) { - RemoteStoreShardShallowCopySnapshot actualShardShallowCopySnapshot = RemoteStoreShardShallowCopySnapshot.fromXContent(parser); - assert Objects.equals(expectedShardShallowCopySnapshot, actualShardShallowCopySnapshot); - } - - // with pathType=PathType.HASHED_PREFIX and pathHashAlgorithm=PathHashAlgorithm.FNV_1A_COMPOSITE - xContent = "{\"version\":\"2\",\"name\":\"test-snapshot\",\"index_version\":1,\"start_time\":123,\"time\":123," - + "\"number_of_files\":5,\"total_size\":5,\"index_uuid\":\"syzhajds-ashdlfj\",\"remote_store_repository\":" - + "\"test-rs-repository\",\"commit_generation\":5,\"primary_term\":3,\"remote_store_repository_base_path\":" - + "\"test-repo-basepath\",\"file_names\":[\"file1\",\"file2\",\"file3\",\"file4\",\"file5\"],\"path_type\":1,\"path_hash_algorithm\":1}"; - expectedShardShallowCopySnapshot = new RemoteStoreShardShallowCopySnapshot( - "2", - snapshot, - indexVersion, - primaryTerm, - commitGeneration, - startTime, - time, - totalFileCount, - totalSize, - indexUUID, - remoteStoreRepository, - repositoryBasePath, - fileNames, - PathType.HASHED_PREFIX, - PathHashAlgorithm.FNV_1A_COMPOSITE_1 - ); - try (XContentParser parser = createParser(JsonXContent.jsonXContent, xContent)) { - RemoteStoreShardShallowCopySnapshot actualShardShallowCopySnapshot = RemoteStoreShardShallowCopySnapshot.fromXContent(parser); - assert Objects.equals(expectedShardShallowCopySnapshot, actualShardShallowCopySnapshot); - } - - // with pathType=PathType.HASHED_INFIX and pathHashAlgorithm=PathHashAlgorithm.FNV_1A - xContent = "{\"version\":\"2\",\"name\":\"test-snapshot\",\"index_version\":1,\"start_time\":123,\"time\":123," - + "\"number_of_files\":5,\"total_size\":5,\"index_uuid\":\"syzhajds-ashdlfj\",\"remote_store_repository\":" - + "\"test-rs-repository\",\"commit_generation\":5,\"primary_term\":3,\"remote_store_repository_base_path\":" - + "\"test-repo-basepath\",\"file_names\":[\"file1\",\"file2\",\"file3\",\"file4\",\"file5\"],\"path_type\":2,\"path_hash_algorithm\":0}"; - expectedShardShallowCopySnapshot = new RemoteStoreShardShallowCopySnapshot( - "2", - snapshot, - indexVersion, - primaryTerm, - commitGeneration, - startTime, - time, - totalFileCount, - totalSize, - indexUUID, - remoteStoreRepository, - repositoryBasePath, - fileNames, - PathType.HASHED_INFIX, - PathHashAlgorithm.FNV_1A_BASE64 - ); - try (XContentParser parser = createParser(JsonXContent.jsonXContent, xContent)) { - RemoteStoreShardShallowCopySnapshot actualShardShallowCopySnapshot = RemoteStoreShardShallowCopySnapshot.fromXContent(parser); - assert Objects.equals(expectedShardShallowCopySnapshot, actualShardShallowCopySnapshot); - } - - // with pathType=PathType.HASHED_INFIX and pathHashAlgorithm=PathHashAlgorithm.FNV_1A_COMPOSITE - xContent = "{\"version\":\"2\",\"name\":\"test-snapshot\",\"index_version\":1,\"start_time\":123,\"time\":123," - + "\"number_of_files\":5,\"total_size\":5,\"index_uuid\":\"syzhajds-ashdlfj\",\"remote_store_repository\":" - + "\"test-rs-repository\",\"commit_generation\":5,\"primary_term\":3,\"remote_store_repository_base_path\":" - + "\"test-repo-basepath\",\"file_names\":[\"file1\",\"file2\",\"file3\",\"file4\",\"file5\"],\"path_type\":2,\"path_hash_algorithm\":1}"; - expectedShardShallowCopySnapshot = new RemoteStoreShardShallowCopySnapshot( - "2", - snapshot, - indexVersion, - primaryTerm, - commitGeneration, - startTime, - time, - totalFileCount, - totalSize, - indexUUID, - remoteStoreRepository, - repositoryBasePath, - fileNames, - PathType.HASHED_INFIX, - PathHashAlgorithm.FNV_1A_COMPOSITE_1 + PathHashAlgorithm.FNV_1A ); try (XContentParser parser = createParser(JsonXContent.jsonXContent, xContent)) { RemoteStoreShardShallowCopySnapshot actualShardShallowCopySnapshot = RemoteStoreShardShallowCopySnapshot.fromXContent(parser); @@ -406,7 +232,7 @@ public void testFromXContent() throws IOException { } public void testFromXContentInvalid() throws IOException { - final int iters = 18; + final int iters = 14; for (int iter = 0; iter < iters; iter++) { String snapshot = "test-snapshot"; long indexVersion = 1; @@ -470,47 +296,21 @@ public void testFromXContentInvalid() throws IOException { break; case 10: version = "1"; - pathHashAlgorithm = PathHashAlgorithm.FNV_1A_BASE64; - failure = "Invalid combination of pathType=null pathHashAlgorithm=FNV_1A_BASE64 for version=1"; + pathHashAlgorithm = PathHashAlgorithm.FNV_1A; + failure = "Invalid combination of pathType=null pathHashAlgorithm=FNV_1A for version=1"; break; case 11: version = "2"; pathType = PathType.FIXED; - pathHashAlgorithm = PathHashAlgorithm.FNV_1A_BASE64; - failure = "Invalid combination of pathType=FIXED pathHashAlgorithm=FNV_1A_BASE64 for version=2"; + pathHashAlgorithm = PathHashAlgorithm.FNV_1A; + failure = "Invalid combination of pathType=FIXED pathHashAlgorithm=FNV_1A for version=2"; break; case 12: - version = "1"; - pathHashAlgorithm = PathHashAlgorithm.FNV_1A_COMPOSITE_1; - failure = "Invalid combination of pathType=null pathHashAlgorithm=FNV_1A_COMPOSITE_1 for version=1"; - break; - case 13: - version = "2"; - pathType = PathType.FIXED; - pathHashAlgorithm = PathHashAlgorithm.FNV_1A_COMPOSITE_1; - failure = "Invalid combination of pathType=FIXED pathHashAlgorithm=FNV_1A_COMPOSITE_1 for version=2"; - break; - case 14: - version = "2"; - pathType = PathType.HASHED_PREFIX; - pathHashAlgorithm = PathHashAlgorithm.FNV_1A_BASE64; - break; - case 15: version = "2"; pathType = PathType.HASHED_PREFIX; - pathHashAlgorithm = PathHashAlgorithm.FNV_1A_COMPOSITE_1; + pathHashAlgorithm = PathHashAlgorithm.FNV_1A; break; - case 16: - version = "2"; - pathType = PathType.HASHED_INFIX; - pathHashAlgorithm = PathHashAlgorithm.FNV_1A_BASE64; - break; - case 17: - version = "2"; - pathType = PathType.HASHED_INFIX; - pathHashAlgorithm = PathHashAlgorithm.FNV_1A_COMPOSITE_1; - break; - case 18: + case 13: break; default: fail("shouldn't be here"); diff --git a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java index b1e2028d761f0..44ddd2de9d007 100644 --- a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java +++ b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java @@ -706,7 +706,7 @@ public void testCleanupAsync() throws Exception { ShardId shardId = new ShardId(Index.UNKNOWN_INDEX_NAME, indexUUID, Integer.parseInt("0")); RemoteStorePathStrategy pathStrategy = randomFrom( new RemoteStorePathStrategy(PathType.FIXED), - new RemoteStorePathStrategy(randomFrom(PathType.HASHED_INFIX, PathType.HASHED_PREFIX), randomFrom(PathHashAlgorithm.values())) + new RemoteStorePathStrategy(PathType.HASHED_PREFIX, PathHashAlgorithm.FNV_1A) ); RemoteSegmentStoreDirectory.remoteDirectoryCleanup( 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 c8d44efd8076a..c26c3f8d21380 100644 --- a/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java +++ b/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java @@ -211,7 +211,7 @@ import static org.opensearch.index.IndexSettings.INDEX_DOC_ID_FUZZY_SET_FALSE_POSITIVE_PROBABILITY_SETTING; import static org.opensearch.index.IndexSettings.INDEX_SOFT_DELETES_RETENTION_LEASE_PERIOD_SETTING; import static org.opensearch.index.query.QueryBuilders.matchAllQuery; -import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING; +import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_STORE_PATH_PREFIX_TYPE_SETTING; import static org.opensearch.indices.IndicesService.CLUSTER_REPLICATION_TYPE_SETTING; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_REPOSITORY_SETTINGS_ATTRIBUTE_KEY_PREFIX; @@ -2619,7 +2619,7 @@ private static Settings buildRemoteStoreNodeAttributes( settings.put(segmentRepoSettingsAttributeKeyPrefix + "compress", randomBoolean()) .put(segmentRepoSettingsAttributeKeyPrefix + "chunk_size", 200, ByteSizeUnit.BYTES); } - settings.put(CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING.getKey(), randomFrom(PathType.values())); + settings.put(CLUSTER_REMOTE_STORE_PATH_PREFIX_TYPE_SETTING.getKey(), randomFrom(PathType.values())); return settings.build(); }