From 2ef85afa574e95356746f19503ea93207553335c Mon Sep 17 00:00:00 2001 From: Ashish Singh Date: Wed, 3 Apr 2024 14:31:27 +0530 Subject: [PATCH] Update Shallow Snapshot flows to support remote path type & hash algo Signed-off-by: Ashish Singh --- .../index/remote/RemoteStoreEnums.java | 63 +++++++++++++++- .../opensearch/index/shard/StoreRecovery.java | 5 +- .../RemoteStoreShardShallowCopySnapshot.java | 71 +++++++++++++------ .../blobstore/BlobStoreRepository.java | 15 ++-- ...oteStoreShardShallowCopySnapshotTests.java | 15 ++-- 5 files changed, 129 insertions(+), 40 deletions(-) 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 4e557d8c24431..74d40061c33e4 100644 --- a/server/src/main/java/org/opensearch/index/remote/RemoteStoreEnums.java +++ b/server/src/main/java/org/opensearch/index/remote/RemoteStoreEnums.java @@ -13,9 +13,12 @@ import org.opensearch.common.hash.FNV1a; import org.opensearch.index.remote.RemoteStorePathStrategy.PathInput; +import java.util.HashMap; import java.util.Locale; +import java.util.Map; import java.util.Set; +import static java.util.Collections.unmodifiableMap; import static org.opensearch.index.remote.RemoteStoreEnums.DataType.DATA; import static org.opensearch.index.remote.RemoteStoreEnums.DataType.METADATA; @@ -78,7 +81,7 @@ public String getName() { */ @PublicApi(since = "2.14.0") public enum PathType { - FIXED { + FIXED(0) { @Override public BlobPath generatePath(PathInput pathInput, PathHashAlgorithm hashAlgorithm) { // Hash algorithm is not used in FIXED path type @@ -94,7 +97,7 @@ boolean requiresHashAlgorithm() { return false; } }, - HASHED_PREFIX { + HASHED_PREFIX(1) { @Override public BlobPath generatePath(PathInput pathInput, PathHashAlgorithm hashAlgorithm) { // TODO - We need to implement this, keeping the same path as Fixed for sake of multiple tests that can fail otherwise. @@ -112,6 +115,33 @@ boolean requiresHashAlgorithm() { } }; + private final int code; + + PathType(int code) { + this.code = code; + } + + public int getCode() { + return code; + } + + private static final Map CODE_TO_ENUM; + static { + PathType[] values = values(); + Map codeToStatus = new HashMap<>(values.length); + for (PathType value : values) { + codeToStatus.put(value.code, value); + } + CODE_TO_ENUM = unmodifiableMap(codeToStatus); + } + + /** + * Turn a status code into a {@link PathType}. + */ + public static PathType fromCode(int code) { + return CODE_TO_ENUM.get(code); + } + /** * This method generates the path for the given path input which constitutes multiple fields and characteristics * of the data. @@ -158,7 +188,7 @@ public static PathType parseString(String pathType) { @PublicApi(since = "2.14.0") public enum PathHashAlgorithm { - FNV_1A { + FNV_1A(0) { @Override long hash(PathInput pathInput) { String input = pathInput.indexUUID() + pathInput.shardId() + pathInput.dataCategory().getName() + pathInput.dataType() @@ -167,6 +197,33 @@ long hash(PathInput pathInput) { } }; + private final int code; + + PathHashAlgorithm(int code) { + this.code = code; + } + + public int getCode() { + return code; + } + + private static final Map CODE_TO_ENUM; + static { + PathHashAlgorithm[] values = values(); + Map codeToStatus = new HashMap<>(values.length); + for (PathHashAlgorithm value : values) { + codeToStatus.put(value.code, value); + } + CODE_TO_ENUM = unmodifiableMap(codeToStatus); + } + + /** + * Turn a status code into a {@link PathHashAlgorithm}. + */ + public static PathHashAlgorithm fromCode(int code) { + return CODE_TO_ENUM.get(code); + } + abstract long hash(PathInput pathInput); public static PathHashAlgorithm parseString(String pathHashAlgorithm) { diff --git a/server/src/main/java/org/opensearch/index/shard/StoreRecovery.java b/server/src/main/java/org/opensearch/index/shard/StoreRecovery.java index c74ab5e24a980..f5e342d28fde1 100644 --- a/server/src/main/java/org/opensearch/index/shard/StoreRecovery.java +++ b/server/src/main/java/org/opensearch/index/shard/StoreRecovery.java @@ -58,8 +58,6 @@ import org.opensearch.index.engine.Engine; import org.opensearch.index.engine.EngineException; import org.opensearch.index.mapper.MapperService; -import org.opensearch.index.remote.RemoteStoreEnums.PathType; -import org.opensearch.index.remote.RemoteStorePathStrategy; import org.opensearch.index.seqno.SequenceNumbers; import org.opensearch.index.snapshots.IndexShardRestoreFailedException; import org.opensearch.index.snapshots.blobstore.RemoteStoreShardShallowCopySnapshot; @@ -412,8 +410,7 @@ void recoverFromSnapshotAndRemoteStore( remoteStoreRepository, indexUUID, shardId, - new RemoteStorePathStrategy(PathType.FIXED) - // TODO - The path type needs to be obtained from RemoteStoreShardShallowCopySnapshot + shallowCopyShardMetadata.getRemoteStorePathStrategy() ); sourceRemoteDirectory.initializeToSpecificCommit( primaryTerm, diff --git a/server/src/main/java/org/opensearch/index/snapshots/blobstore/RemoteStoreShardShallowCopySnapshot.java b/server/src/main/java/org/opensearch/index/snapshots/blobstore/RemoteStoreShardShallowCopySnapshot.java index d54e9686ab951..f156d983a4d3a 100644 --- a/server/src/main/java/org/opensearch/index/snapshots/blobstore/RemoteStoreShardShallowCopySnapshot.java +++ b/server/src/main/java/org/opensearch/index/snapshots/blobstore/RemoteStoreShardShallowCopySnapshot.java @@ -14,11 +14,15 @@ import org.opensearch.core.xcontent.ToXContentFragment; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.index.remote.RemoteStoreEnums.PathHashAlgorithm; +import org.opensearch.index.remote.RemoteStoreEnums.PathType; +import org.opensearch.index.remote.RemoteStorePathStrategy; import org.opensearch.index.snapshots.IndexShardSnapshotStatus; import java.io.IOException; import java.util.ArrayList; import java.util.List; +import java.util.Objects; /** * Remote Store based Shard snapshot metadata @@ -41,8 +45,10 @@ public class RemoteStoreShardShallowCopySnapshot implements ToXContentFragment, private final String repositoryBasePath; private final String indexUUID; private final List fileNames; + private final PathType pathType; + private final PathHashAlgorithm pathHashAlgorithm; - static final String DEFAULT_VERSION = "1"; + static final String DEFAULT_VERSION = "2"; static final String NAME = "name"; static final String VERSION = "version"; static final String INDEX_VERSION = "index_version"; @@ -61,6 +67,8 @@ public class RemoteStoreShardShallowCopySnapshot implements ToXContentFragment, static final String TOTAL_FILE_COUNT = "number_of_files"; static final String TOTAL_SIZE = "total_size"; + static final String PATH_TYPE = "path_type"; + static final String PATH_HASH_ALGORITHM = "path_hash_algorithm"; private static final ParseField PARSE_NAME = new ParseField(NAME); private static final ParseField PARSE_VERSION = new ParseField(VERSION); @@ -75,6 +83,8 @@ public class RemoteStoreShardShallowCopySnapshot implements ToXContentFragment, private static final ParseField PARSE_REMOTE_STORE_REPOSITORY = new ParseField(REMOTE_STORE_REPOSITORY); private static final ParseField PARSE_REPOSITORY_BASE_PATH = new ParseField(REPOSITORY_BASE_PATH); private static final ParseField PARSE_FILE_NAMES = new ParseField(FILE_NAMES); + private static final ParseField PARSE_PATH_TYPE = new ParseField(PATH_TYPE); + private static final ParseField PARSE_PATH_HASH_ALGORITHM = new ParseField(PATH_HASH_ALGORITHM); /** * Serializes shard snapshot metadata info into JSON @@ -101,6 +111,8 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.value(fileName); } builder.endArray(); + builder.field(PATH_TYPE, pathType.getCode()); + builder.field(PATH_HASH_ALGORITHM, pathHashAlgorithm.getCode()); return builder; } @@ -116,31 +128,27 @@ public RemoteStoreShardShallowCopySnapshot( String indexUUID, String remoteStoreRepository, String repositoryBasePath, - List fileNames + List fileNames, + PathType pathType, + PathHashAlgorithm pathHashAlgorithm ) { - this.version = DEFAULT_VERSION; - verifyParameters( - version, + this( + DEFAULT_VERSION, snapshot, indexVersion, primaryTerm, commitGeneration, + startTime, + time, + totalFileCount, + totalSize, indexUUID, remoteStoreRepository, - repositoryBasePath + repositoryBasePath, + fileNames, + pathType, + pathHashAlgorithm ); - this.snapshot = snapshot; - this.indexVersion = indexVersion; - this.primaryTerm = primaryTerm; - this.commitGeneration = commitGeneration; - this.startTime = startTime; - this.time = time; - this.totalFileCount = totalFileCount; - this.totalSize = totalSize; - this.indexUUID = indexUUID; - this.remoteStoreRepository = remoteStoreRepository; - this.repositoryBasePath = repositoryBasePath; - this.fileNames = fileNames; } private RemoteStoreShardShallowCopySnapshot( @@ -156,7 +164,9 @@ private RemoteStoreShardShallowCopySnapshot( String indexUUID, String remoteStoreRepository, String repositoryBasePath, - List fileNames + List fileNames, + PathType pathType, + PathHashAlgorithm pathHashAlgorithm ) { verifyParameters( version, @@ -181,6 +191,8 @@ private RemoteStoreShardShallowCopySnapshot( this.remoteStoreRepository = remoteStoreRepository; this.repositoryBasePath = repositoryBasePath; this.fileNames = fileNames; + this.pathType = pathType; + this.pathHashAlgorithm = pathHashAlgorithm; } /** @@ -203,6 +215,8 @@ public static RemoteStoreShardShallowCopySnapshot fromXContent(XContentParser pa long primaryTerm = -1; long commitGeneration = -1; List fileNames = new ArrayList<>(); + PathType pathType = null; + PathHashAlgorithm pathHashAlgorithm = null; if (parser.currentToken() == null) { // fresh parser? move to the first token parser.nextToken(); @@ -237,6 +251,10 @@ public static RemoteStoreShardShallowCopySnapshot fromXContent(XContentParser pa remoteStoreRepository = parser.text(); } else if (PARSE_REPOSITORY_BASE_PATH.match(currentFieldName, parser.getDeprecationHandler())) { repositoryBasePath = parser.text(); + } else if (PARSE_PATH_TYPE.match(currentFieldName, parser.getDeprecationHandler())) { + pathType = PathType.fromCode(parser.intValue()); + } else if (PARSE_PATH_HASH_ALGORITHM.match(currentFieldName, parser.getDeprecationHandler())) { + pathHashAlgorithm = PathHashAlgorithm.fromCode(parser.intValue()); } else { throw new OpenSearchParseException("unknown parameter [{}]", currentFieldName); } @@ -266,7 +284,9 @@ public static RemoteStoreShardShallowCopySnapshot fromXContent(XContentParser pa indexUUID, remoteStoreRepository, repositoryBasePath, - fileNames + fileNames, + pathType, + pathHashAlgorithm ); } @@ -433,7 +453,9 @@ public RemoteStoreShardShallowCopySnapshot asClone(String targetSnapshotName, lo indexUUID, remoteStoreRepository, repositoryBasePath, - fileNames + fileNames, + pathType, + pathHashAlgorithm ); } @@ -449,4 +471,11 @@ public IndexShardSnapshotStatus getIndexShardSnapshotStatus() { null ); // Not adding a real generation here as it doesn't matter to callers } + + public RemoteStorePathStrategy getRemoteStorePathStrategy() { + if (Objects.nonNull(pathType) && Objects.nonNull(pathHashAlgorithm)) { + return new RemoteStorePathStrategy(pathType, pathHashAlgorithm); + } + return new RemoteStorePathStrategy(PathType.FIXED); + } } 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 5aab02993db34..ce2ffd8bf3fb4 100644 --- a/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java @@ -108,7 +108,6 @@ import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.core.xcontent.XContentParser; import org.opensearch.index.mapper.MapperService; -import org.opensearch.index.remote.RemoteStoreEnums.PathType; import org.opensearch.index.remote.RemoteStorePathStrategy; import org.opensearch.index.snapshots.IndexShardRestoreFailedException; import org.opensearch.index.snapshots.IndexShardSnapshotStatus; @@ -672,8 +671,7 @@ public void cloneRemoteStoreIndexShardSnapshot( remoteStoreRepository, indexUUID, String.valueOf(shardId.shardId()), - new RemoteStorePathStrategy(PathType.FIXED) - // TODO - The path type needs to be obtained from RemoteStoreShardShallowCopySnapshot + remStoreBasedShardMetadata.getRemoteStorePathStrategy() ); remoteStoreMetadataLockManger.cloneLock( FileLockInfo.getLockInfoBuilder().withAcquirerId(source.getUUID()).build(), @@ -1154,8 +1152,7 @@ protected void releaseRemoteStoreLockAndCleanup( remoteStoreRepoForIndex, indexUUID, shardId, - new RemoteStorePathStrategy(PathType.FIXED) - // TODO - The path type needs to be obtained from RemoteStoreShardShallowCopySnapshot + remoteStoreShardShallowCopySnapshot.getRemoteStorePathStrategy() ); remoteStoreMetadataLockManager.release(FileLockInfo.getLockInfoBuilder().withAcquirerId(shallowSnapshotUUID).build()); logger.debug("Successfully released lock for shard {} of index with uuid {}", shardId, indexUUID); @@ -1178,8 +1175,7 @@ protected void releaseRemoteStoreLockAndCleanup( indexUUID, new ShardId(Index.UNKNOWN_INDEX_NAME, indexUUID, Integer.parseInt(shardId)), ThreadPool.Names.REMOTE_PURGE, - new RemoteStorePathStrategy(PathType.FIXED) - // TODO - The path type needs to be obtained from RemoteStoreShardShallowCopySnapshot + remoteStoreShardShallowCopySnapshot.getRemoteStorePathStrategy() ); } } @@ -2694,6 +2690,7 @@ public void snapshotRemoteStoreIndexShard( // now create and write the commit point logger.trace("[{}] [{}] writing shard snapshot file", shardId, snapshotId); try { + RemoteStorePathStrategy pathStrategy = store.indexSettings().getRemoteStorePathStrategy(); REMOTE_STORE_SHARD_SHALLOW_COPY_SNAPSHOT_FORMAT.write( new RemoteStoreShardShallowCopySnapshot( snapshotId.getName(), @@ -2707,7 +2704,9 @@ public void snapshotRemoteStoreIndexShard( store.indexSettings().getUUID(), store.indexSettings().getRemoteStoreRepository(), this.basePath().toString(), - fileNames + fileNames, + pathStrategy.getType(), + pathStrategy.getHashAlgorithm() ), shardContainer, snapshotId.getUUID(), 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 38c4bb781ce06..eaad0e48345ed 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 @@ -14,6 +14,8 @@ import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.index.remote.RemoteStoreEnums.PathHashAlgorithm; +import org.opensearch.index.remote.RemoteStoreEnums.PathType; import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; @@ -52,7 +54,9 @@ public void testToXContent() throws IOException { indexUUID, remoteStoreRepository, repositoryBasePath, - fileNames + fileNames, + PathType.FIXED, + PathHashAlgorithm.FNV_1A ); String actual; try (XContentBuilder builder = MediaTypeRegistry.JSON.contentBuilder()) { @@ -61,10 +65,11 @@ public void testToXContent() throws IOException { builder.endObject(); actual = builder.toString(); } - String expectedXContent = "{\"version\":\"1\",\"name\":\"test-snapshot\",\"index_version\":1,\"start_time\":123,\"time\":123," + String 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\"]}"; + + "\"test-repo-basepath\",\"file_names\":[\"file1\",\"file2\",\"file3\",\"file4\",\"file5\"],\"path_type\":" + + "0,\"path_hash_algorithm\":0}"; assert Objects.equals(actual, expectedXContent) : "xContent is " + actual; } @@ -94,7 +99,9 @@ public void testFromXContent() throws IOException { indexUUID, remoteStoreRepository, repositoryBasePath, - fileNames + fileNames, + PathType.FIXED, + PathHashAlgorithm.FNV_1A ); String xContent = "{\"version\":\"1\",\"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\":"