From 5038b26c7efcb2499a2ce89bdd393e4fe6710458 Mon Sep 17 00:00:00 2001 From: Ashish Singh Date: Wed, 10 Apr 2024 23:58:11 +0530 Subject: [PATCH 01/15] Upload remote paths during index creation or full cluster upload Signed-off-by: Ashish Singh --- .../remote/RemoteClusterStateService.java | 297 +++++++++++++++--- .../index/remote/RemoteIndexPath.java | 138 ++++++++ .../blobstore/ChecksumBlobStoreFormat.java | 92 +++++- .../index/remote/RemoteIndexPathTests.java | 54 ++++ 4 files changed, 537 insertions(+), 44 deletions(-) create mode 100644 server/src/main/java/org/opensearch/index/remote/RemoteIndexPath.java create mode 100644 server/src/test/java/org/opensearch/index/remote/RemoteIndexPathTests.java diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java index c892b475d71da..4d885e014931c 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -26,10 +26,16 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.io.IOUtils; +import org.opensearch.common.xcontent.XContentType; import org.opensearch.core.action.ActionListener; import org.opensearch.core.index.Index; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.gateway.remote.ClusterMetadataManifest.UploadedIndexMetadata; +import org.opensearch.index.remote.RemoteIndexPath; +import org.opensearch.index.remote.RemoteStoreEnums.DataCategory; +import org.opensearch.index.remote.RemoteStoreEnums.DataType; +import org.opensearch.index.remote.RemoteStoreEnums.PathHashAlgorithm; +import org.opensearch.index.remote.RemoteStoreEnums.PathType; import org.opensearch.index.remote.RemoteStoreUtils; import org.opensearch.index.translog.transfer.BlobStoreTransferService; import org.opensearch.node.Node; @@ -64,6 +70,9 @@ import java.util.stream.Collectors; import static org.opensearch.gateway.PersistedClusterStateService.SLOW_WRITE_LOGGING_THRESHOLD; +import static org.opensearch.index.remote.RemoteIndexPath.SEGMENT_PATH; +import static org.opensearch.index.remote.RemoteIndexPath.TRANSLOG_PATH; +import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteDataAttributePresent; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteStoreClusterStateEnabled; /** @@ -122,6 +131,12 @@ public class RemoteClusterStateService implements Closeable { Metadata::fromXContent ); + public static final ChecksumBlobStoreFormat REMOTE_INDEX_PATH_FORMAT = new ChecksumBlobStoreFormat<>( + "remote-index-path", + RemoteIndexPath.FILE_NAME_FORMAT, + RemoteIndexPath::fromXContent + ); + /** * Manifest format compatible with older codec v0, where codec version was missing. */ @@ -163,6 +178,11 @@ public class RemoteClusterStateService implements Closeable { private BlobStoreRepository blobStoreRepository; private BlobStoreTransferService blobStoreTransferService; private volatile TimeValue slowWriteLoggingThreshold; + private BlobStoreRepository translogRepository; + private BlobStoreTransferService translogTransferService; + private BlobStoreRepository segmentRepository; + private BlobStoreTransferService segmentsTransferService; + private final boolean isRemoteDataAttributePresent; private volatile TimeValue indexMetadataUploadTimeout; private volatile TimeValue globalMetadataUploadTimeout; @@ -206,6 +226,7 @@ public RemoteClusterStateService( clusterSettings.addSettingsUpdateConsumer(GLOBAL_METADATA_UPLOAD_TIMEOUT_SETTING, this::setGlobalMetadataUploadTimeout); clusterSettings.addSettingsUpdateConsumer(METADATA_MANIFEST_UPLOAD_TIMEOUT_SETTING, this::setMetadataManifestUploadTimeout); this.remoteStateStats = new RemotePersistenceStats(); + this.isRemoteDataAttributePresent = isRemoteDataAttributePresent(settings); } private BlobStoreTransferService getBlobStoreTransferService() { @@ -215,6 +236,20 @@ private BlobStoreTransferService getBlobStoreTransferService() { return blobStoreTransferService; } + private BlobStoreTransferService getTranslogTransferService() { + if (translogTransferService == null) { + translogTransferService = new BlobStoreTransferService(translogRepository.blobStore(), threadpool); + } + return translogTransferService; + } + + private BlobStoreTransferService getSegmentsTransferService() { + if (segmentsTransferService == null) { + segmentsTransferService = new BlobStoreTransferService(segmentRepository.blobStore(), threadpool); + } + return segmentsTransferService; + } + /** * This method uploads entire cluster state metadata to the configured blob store. For now only index metadata upload is supported. This method should be * invoked by the elected cluster manager when the remote cluster state is enabled. @@ -236,7 +271,8 @@ public ClusterMetadataManifest writeFullMetadata(ClusterState clusterState, Stri // any validations before/after upload ? final List allUploadedIndexMetadata = writeIndexMetadataParallel( clusterState, - new ArrayList<>(clusterState.metadata().indices().values()) + new ArrayList<>(clusterState.metadata().indices().values()), + previousClusterUUID ); final ClusterMetadataManifest manifest = uploadManifest( clusterState, @@ -313,7 +349,7 @@ public ClusterMetadataManifest writeIncrementalMetadata( .collect(Collectors.toMap(UploadedIndexMetadata::getIndexName, Function.identity())); List toUpload = new ArrayList<>(); - + final Map indexNamePreviousVersionMap = new HashMap<>(previousStateIndexMetadataVersionByName); for (final IndexMetadata indexMetadata : clusterState.metadata().indices().values()) { final Long previousVersion = previousStateIndexMetadataVersionByName.get(indexMetadata.getIndex().getName()); if (previousVersion == null || indexMetadata.getVersion() != previousVersion) { @@ -331,7 +367,11 @@ public ClusterMetadataManifest writeIncrementalMetadata( previousStateIndexMetadataVersionByName.remove(indexMetadata.getIndex().getName()); } - List uploadedIndexMetadataList = writeIndexMetadataParallel(clusterState, toUpload); + List uploadedIndexMetadataList = writeIndexMetadataParallel( + clusterState, + toUpload, + indexNamePreviousVersionMap + ); uploadedIndexMetadataList.forEach( uploadedIndexMetadata -> allUploadedIndexMetadata.put(uploadedIndexMetadata.getIndexName(), uploadedIndexMetadata) ); @@ -439,33 +479,18 @@ private String writeGlobalMetadata(ClusterState clusterState) throws IOException * @param toUpload list of IndexMetadata to upload * @return {@code List} list of IndexMetadata uploaded to remote */ - private List writeIndexMetadataParallel(ClusterState clusterState, List toUpload) - throws IOException { - List exceptionList = Collections.synchronizedList(new ArrayList<>(toUpload.size())); - final CountDownLatch latch = new CountDownLatch(toUpload.size()); + private List writeIndexMetadataParallel( + ClusterState clusterState, + List toUpload, + List toUploadIndexPath + ) throws IOException { + boolean isTranslogSegmentRepoSame = isTranslogSegmentRepoSame(); + int latchCount = toUpload.size() + (isTranslogSegmentRepoSame ? toUploadIndexPath.size() : 2 * toUploadIndexPath.size()); + List exceptionList = Collections.synchronizedList(new ArrayList<>(latchCount)); + final CountDownLatch latch = new CountDownLatch(latchCount); List result = new ArrayList<>(toUpload.size()); - - LatchedActionListener latchedActionListener = new LatchedActionListener<>( - ActionListener.wrap((UploadedIndexMetadata uploadedIndexMetadata) -> { - logger.trace( - String.format(Locale.ROOT, "IndexMetadata uploaded successfully for %s", uploadedIndexMetadata.getIndexName()) - ); - result.add(uploadedIndexMetadata); - }, ex -> { - assert ex instanceof RemoteStateTransferException; - logger.error( - () -> new ParameterizedMessage("Exception during transfer of IndexMetadata to Remote {}", ex.getMessage()), - ex - ); - exceptionList.add(ex); - }), - latch - ); - - for (IndexMetadata indexMetadata : toUpload) { - // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/index/ftqsCnn9TgOX/metadata_4_1690947200 - writeIndexMetadataAsync(clusterState, indexMetadata, latchedActionListener); - } + uploadIndexMetadataAsync(clusterState, result, toUpload, latch, exceptionList); + uploadIndexPathAsync(toUploadIndexPath, latch, isTranslogSegmentRepoSame, exceptionList); try { if (latch.await(getIndexMetadataUploadTimeout().millis(), TimeUnit.MILLISECONDS) == false) { @@ -506,6 +531,190 @@ private List writeIndexMetadataParallel(ClusterState clus return result; } + private void uploadIndexPathAsync( + List toUploadIndexPath, + CountDownLatch latch, + boolean isTranslogSegmentRepoSame, + List exceptionList + ) throws IOException { + for (IndexMetadata indexMetadata : toUploadIndexPath) { + writeIndexPathAsync(indexMetadata, latch, isTranslogSegmentRepoSame, exceptionList); + } + } + + private void writeIndexPathAsync( + IndexMetadata idxMD, + CountDownLatch latch, + boolean isTranslogSegmentRepoSame, + List exceptionList + ) throws IOException { + Map remoteCustomData = idxMD.getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY); + PathType pathType = PathType.valueOf(remoteCustomData.get(PathType.NAME)); + PathHashAlgorithm hashAlgorithm = PathHashAlgorithm.valueOf(remoteCustomData.get(PathHashAlgorithm.NAME)); + String indexUUID = idxMD.getIndexUUID(); + int shardCount = idxMD.getNumberOfShards(); + BlobPath translogBasePath = translogRepository.basePath(); + BlobContainer translogBlobContainer = translogRepository.blobStore().blobContainer(translogBasePath.add(RemoteIndexPath.DIR)); + + if (isTranslogSegmentRepoSame) { + // If the repositories are same, then we need to upload a single file containing paths for both translog and segments. + Map> pathCreationMap = new HashMap<>(); + pathCreationMap.putAll(TRANSLOG_PATH); + pathCreationMap.putAll(SEGMENT_PATH); + REMOTE_INDEX_PATH_FORMAT.writeAsyncWithUrgentPriority( + new RemoteIndexPath(indexUUID, shardCount, translogBasePath, pathType, hashAlgorithm, pathCreationMap), + translogBlobContainer, + indexUUID, + translogRepository.getCompressor(), + getUploadPathLatchedActionListener(idxMD, latch, exceptionList, pathCreationMap), + FORMAT_PARAMS, + true, + XContentType.JSON + ); + } else { + // If the repositories are different, then we need to upload one file per segment and translog containing their individual + // paths. + REMOTE_INDEX_PATH_FORMAT.writeAsyncWithUrgentPriority( + new RemoteIndexPath(indexUUID, shardCount, translogBasePath, pathType, hashAlgorithm, TRANSLOG_PATH), + translogBlobContainer, + indexUUID, + translogRepository.getCompressor(), + getUploadPathLatchedActionListener(idxMD, latch, exceptionList, TRANSLOG_PATH), + FORMAT_PARAMS, + true, + XContentType.JSON + ); + + BlobPath segmentBasePath = segmentRepository.basePath(); + BlobContainer segmentBlobContainer = segmentRepository.blobStore().blobContainer(segmentBasePath.add(RemoteIndexPath.DIR)); + REMOTE_INDEX_PATH_FORMAT.writeAsyncWithUrgentPriority( + new RemoteIndexPath(indexUUID, shardCount, segmentBasePath, pathType, hashAlgorithm, SEGMENT_PATH), + segmentBlobContainer, + indexUUID, + segmentRepository.getCompressor(), + getUploadPathLatchedActionListener(idxMD, latch, exceptionList, SEGMENT_PATH), + FORMAT_PARAMS, + true, + XContentType.JSON + ); + } + } + + private LatchedActionListener getUploadPathLatchedActionListener( + IndexMetadata indexMetadata, + CountDownLatch latch, + List exceptionList, + Map> pathCreationMap + ) { + return new LatchedActionListener<>( + ActionListener.wrap( + resp -> logger.trace( + new ParameterizedMessage("Index path uploaded for {} indexMetadata={}", pathCreationMap, indexMetadata) + ), + ex -> { + logger.error( + new ParameterizedMessage( + "Exception during Index path upload for {} indexMetadata={}", + pathCreationMap, + indexMetadata + ), + ex + ); + exceptionList.add(ex); + } + ), + latch + ); + } + + private void uploadIndexMetadataAsync( + ClusterState clusterState, + List result, + List toUpload, + CountDownLatch latch, + List exceptionList + ) throws IOException { + LatchedActionListener indexMetadataLatchedActionListener = new LatchedActionListener<>( + ActionListener.wrap((UploadedIndexMetadata uploadedIndexMetadata) -> { + logger.trace( + String.format(Locale.ROOT, "IndexMetadata uploaded successfully for %s", uploadedIndexMetadata.getIndexName()) + ); + result.add(uploadedIndexMetadata); + }, ex -> { + assert ex instanceof RemoteStateTransferException; + logger.error( + () -> new ParameterizedMessage("Exception during transfer of IndexMetadata to Remote {}", ex.getMessage()), + ex + ); + exceptionList.add(ex); + }), + latch + ); + + for (IndexMetadata indexMetadata : toUpload) { + // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/index/ftqsCnn9TgOX/metadata_4_1690947200 + writeIndexMetadataAsync(clusterState, indexMetadata, indexMetadataLatchedActionListener); + } + } + + private boolean isTranslogSegmentRepoSame() { + String translogRepoName = settings.get( + Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY + ); + String segmentRepoName = settings.get( + Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY + ); + return Objects.equals(translogRepoName, segmentRepoName); + } + + private List writeIndexMetadataParallel( + ClusterState clusterState, + List toUpload, + String previousClusterUUID + ) throws IOException { + List toUploadIndexPath = Collections.emptyList(); + if (ClusterState.UNKNOWN_UUID.equals(previousClusterUUID)) { + toUploadIndexPath = toUpload; + } + return writeIndexMetadataParallel(clusterState, toUpload, toUploadIndexPath); + } + + private List writeIndexMetadataParallel( + ClusterState clusterState, + List toUpload, + Map indexNamePreviousVersionMap + ) throws IOException { + List toUploadIndexPath = Collections.emptyList(); + if (isRemoteDataAttributePresent) { + toUploadIndexPath = toUpload.stream() + /* If the previous state's index metadata version is null, then this is index creation */ + .filter(indexMetadata -> Objects.isNull(indexNamePreviousVersionMap.get(indexMetadata.getIndex().getName()))) + /* Checks the condition if the Index path needs to be uploaded or not */ + .filter(this::uploadIndexPathFile) + .collect(Collectors.toList()); + } + return writeIndexMetadataParallel(clusterState, toUpload, toUploadIndexPath); + } + + /** + * This method checks if the index metadata has attributes that calls for uploading the index path for remote store + * uploads. It checks if the remote store path type is {@code HASHED_PREFIX} and returns true if so. + */ + private boolean uploadIndexPathFile(IndexMetadata indexMetadata) { + assert isRemoteDataAttributePresent : "Remote data attributes is expected to be present"; + // A cluster will have remote custom metadata only if the cluster is remote store enabled from data side. + Map remoteCustomData = indexMetadata.getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY); + if (Objects.isNull(remoteCustomData) || remoteCustomData.isEmpty()) { + return false; + } + String pathTypeStr = remoteCustomData.get(PathType.NAME); + if (Objects.isNull(pathTypeStr)) { + return false; + } + // We need to upload the path only if the path type for an index is hashed_prefix + return PathType.HASHED_PREFIX == PathType.parseString(pathTypeStr); + } + /** * Allows async Upload of IndexMetadata to remote * @@ -574,13 +783,32 @@ public void close() throws IOException { public void start() { assert isRemoteStoreClusterStateEnabled(settings) == true : "Remote cluster state is not enabled"; - final String remoteStoreRepo = settings.get( - Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY + blobStoreRepository = (BlobStoreRepository) validateAndGetRepository( + Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY, + "Cluster State" ); - assert remoteStoreRepo != null : "Remote Cluster State repository is not configured"; - final Repository repository = repositoriesService.get().repository(remoteStoreRepo); + + if (isRemoteDataAttributePresent == false) { + // If remote store data attributes are not present than we skip this. + return; + } + translogRepository = (BlobStoreRepository) validateAndGetRepository( + Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY, + "Translog" + ); + segmentRepository = (BlobStoreRepository) validateAndGetRepository( + Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY, + "Translog" + ); + + } + + private Repository validateAndGetRepository(String repoSetting, String repoName) { + final String repo = settings.get(repoSetting); + assert repo != null : "Remote " + repoName + " repository is not configured"; + final Repository repository = repositoriesService.get().repository(repo); assert repository instanceof BlobStoreRepository : "Repository should be instance of BlobStoreRepository"; - blobStoreRepository = (BlobStoreRepository) repository; + return repository; } private ClusterMetadataManifest uploadManifest( @@ -825,7 +1053,6 @@ private IndexMetadata getIndexMetadata(String clusterName, String clusterUUID, U * @return {@link IndexMetadata} */ public ClusterState getLatestClusterState(String clusterName, String clusterUUID) { - start(); Optional clusterMetadataManifest = getLatestClusterMetadataManifest(clusterName, clusterUUID); if (clusterMetadataManifest.isEmpty()) { throw new IllegalStateException( diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteIndexPath.java b/server/src/main/java/org/opensearch/index/remote/RemoteIndexPath.java new file mode 100644 index 0000000000000..83448df427c84 --- /dev/null +++ b/server/src/main/java/org/opensearch/index/remote/RemoteIndexPath.java @@ -0,0 +1,138 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.index.remote; + +import org.apache.logging.log4j.message.ParameterizedMessage; +import org.opensearch.common.blobstore.BlobPath; +import org.opensearch.core.xcontent.ToXContentFragment; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.index.remote.RemoteStoreEnums.DataCategory; +import org.opensearch.index.remote.RemoteStoreEnums.DataType; +import org.opensearch.index.remote.RemoteStoreEnums.PathHashAlgorithm; +import org.opensearch.index.remote.RemoteStoreEnums.PathType; +import org.opensearch.index.remote.RemoteStorePathStrategy.PathInput; + +import java.io.IOException; +import java.util.List; +import java.util.Map; +import java.util.Objects; + +import static org.opensearch.index.remote.RemoteStoreEnums.DataCategory.SEGMENTS; +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.index.remote.RemoteStorePathStrategy.isCompatible; + +/** + * Remote index path information. + * + * @opensearch.internal + */ +public class RemoteIndexPath implements ToXContentFragment { + + public static final Map> TRANSLOG_PATH = Map.of(TRANSLOG, List.of(DATA, METADATA)); + public static final Map> SEGMENT_PATH = Map.of(SEGMENTS, List.of(DataType.values())); + private static final String DEFAULT_VERSION = "1"; + public static final String DIR = "remote-index-path"; + public static final String FILE_NAME_FORMAT = "remote_path_%s"; + static final String KEY_VERSION = "version"; + static final String KEY_INDEX_UUID = "index_uuid"; + static final String KEY_SHARD_COUNT = "shard_count"; + static final String KEY_PATHS = "paths"; + private final String indexUUID; + private final int shardCount; + private final Iterable basePath; + private final PathType pathType; + private final PathHashAlgorithm pathHashAlgorithm; + + /** + * This keeps the map of paths that would be present in the content of the index path file. For eg - It is possible + * that segment and translog repository can be different. For this use case, we have either segment or translog as the + * key, and list of data, metadata, and lock_files (only for segment) as the value. + */ + private final Map> pathCreationMap; + + public RemoteIndexPath( + String indexUUID, + int shardCount, + Iterable basePath, + PathType pathType, + PathHashAlgorithm pathHashAlgorithm, + Map> pathCreationMap + ) { + if (Objects.isNull(pathCreationMap) + || Objects.isNull(pathType) + || isCompatible(pathType, pathHashAlgorithm) == false + || shardCount < 1 + || Objects.isNull(basePath) + || pathCreationMap.isEmpty() + || pathCreationMap.keySet().stream().anyMatch(k -> pathCreationMap.get(k).isEmpty())) { + ParameterizedMessage parameterizedMessage = new ParameterizedMessage( + "Invalid input in RemoteIndexPath constructor indexUUID={} shardCount={} basePath={} pathType={}" + + " pathHashAlgorithm={} pathCreationMap={}", + indexUUID, + shardCount, + basePath, + pathType, + pathHashAlgorithm, + pathCreationMap + ); + throw new IllegalArgumentException(parameterizedMessage.getFormattedMessage()); + } + boolean validMap = pathCreationMap.keySet() + .stream() + .allMatch(k -> pathCreationMap.get(k).stream().allMatch(k::isSupportedDataType)); + if (validMap == false) { + throw new IllegalArgumentException( + new ParameterizedMessage("pathCreationMap={} is having illegal combination of category and type", pathCreationMap) + .getFormattedMessage() + ); + } + this.indexUUID = indexUUID; + this.shardCount = shardCount; + this.basePath = basePath; + this.pathType = pathType; + this.pathHashAlgorithm = pathHashAlgorithm; + this.pathCreationMap = pathCreationMap; + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.field(KEY_VERSION, DEFAULT_VERSION); + builder.field(KEY_INDEX_UUID, indexUUID); + builder.field(KEY_SHARD_COUNT, shardCount); + builder.field(PathType.NAME, pathType.name()); + if (Objects.nonNull(pathHashAlgorithm)) { + builder.field(PathHashAlgorithm.NAME, pathHashAlgorithm.name()); + } + builder.startArray(KEY_PATHS); + for (Map.Entry> entry : pathCreationMap.entrySet()) { + DataCategory dataCategory = entry.getKey(); + for (DataType type : entry.getValue()) { + for (int shardNo = 0; shardNo < shardCount; shardNo++) { + PathInput pathInput = PathInput.builder() + .basePath(new BlobPath().add(basePath)) + .indexUUID(indexUUID) + .shardId(Integer.toString(shardNo)) + .dataCategory(dataCategory) + .dataType(type) + .build(); + builder.value(pathType.path(pathInput, pathHashAlgorithm).buildAsString()); + } + } + } + builder.endArray(); + return builder; + } + + public static RemoteIndexPath fromXContent(XContentParser ignored) throws IOException { + throw new UnsupportedOperationException("RemoteIndexPath.fromXContent() is not supported"); + } +} diff --git a/server/src/main/java/org/opensearch/repositories/blobstore/ChecksumBlobStoreFormat.java b/server/src/main/java/org/opensearch/repositories/blobstore/ChecksumBlobStoreFormat.java index 3e6052a5ef820..1be096dd92577 100644 --- a/server/src/main/java/org/opensearch/repositories/blobstore/ChecksumBlobStoreFormat.java +++ b/server/src/main/java/org/opensearch/repositories/blobstore/ChecksumBlobStoreFormat.java @@ -57,6 +57,7 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.core.compress.Compressor; +import org.opensearch.core.xcontent.MediaType; import org.opensearch.core.xcontent.MediaTypeRegistry; import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.core.xcontent.ToXContent; @@ -190,9 +191,32 @@ public void write( final String name, final Compressor compressor, final ToXContent.Params params + ) throws IOException { + write(obj, blobContainer, name, compressor, params, false, XContentType.SMILE); + } + + /** + * Writes blob with resolving the blob name using {@link #blobName} method. + *

+ * The blob will optionally by compressed. + * + * @param obj object to be serialized + * @param blobContainer blob container + * @param name blob name + * @param compressor whether to use compression + * @param params ToXContent params + */ + public void write( + final T obj, + final BlobContainer blobContainer, + final String name, + final Compressor compressor, + final ToXContent.Params params, + boolean skipHeaderFooter, + MediaType mediaType ) throws IOException { final String blobName = blobName(name); - final BytesReference bytes = serialize(obj, blobName, compressor, params); + final BytesReference bytes = serialize(obj, blobName, compressor, params, skipHeaderFooter, mediaType); blobContainer.writeBlob(blobName, bytes.streamInput(), bytes.length(), false); } @@ -208,7 +232,17 @@ public void writeAsync( final ToXContent.Params params ) throws IOException { // use NORMAL priority by default - this.writeAsyncWithPriority(obj, blobContainer, name, compressor, WritePriority.NORMAL, listener, params); + this.writeAsyncWithPriority( + obj, + blobContainer, + name, + compressor, + WritePriority.NORMAL, + listener, + params, + false, + XContentType.SMILE + ); } /** @@ -226,7 +260,30 @@ public void writeAsyncWithUrgentPriority( ActionListener listener, final ToXContent.Params params ) throws IOException { - this.writeAsyncWithPriority(obj, blobContainer, name, compressor, WritePriority.URGENT, listener, params); + this.writeAsyncWithPriority( + obj, + blobContainer, + name, + compressor, + WritePriority.URGENT, + listener, + params, + false, + XContentType.SMILE + ); + } + + public void writeAsyncWithUrgentPriority( + final T obj, + final BlobContainer blobContainer, + final String name, + final Compressor compressor, + ActionListener listener, + final ToXContent.Params params, + boolean skipHeaderFooter, + MediaType type + ) throws IOException { + this.writeAsyncWithPriority(obj, blobContainer, name, compressor, WritePriority.URGENT, listener, params, skipHeaderFooter, type); } /** @@ -248,15 +305,17 @@ private void writeAsyncWithPriority( final Compressor compressor, final WritePriority priority, ActionListener listener, - final ToXContent.Params params + final ToXContent.Params params, + boolean skipHeaderFooter, + MediaType mediaType ) throws IOException { if (blobContainer instanceof AsyncMultiStreamBlobContainer == false) { - write(obj, blobContainer, name, compressor, params); + write(obj, blobContainer, name, compressor, params, skipHeaderFooter, mediaType); listener.onResponse(null); return; } final String blobName = blobName(name); - final BytesReference bytes = serialize(obj, blobName, compressor, params); + final BytesReference bytes = serialize(obj, blobName, compressor, params, skipHeaderFooter, mediaType); final String resourceDescription = "ChecksumBlobStoreFormat.writeAsyncWithPriority(blob=\"" + blobName + "\")"; try (IndexInput input = new ByteArrayIndexInput(resourceDescription, BytesReference.toBytes(bytes))) { long expectedChecksum; @@ -290,6 +349,17 @@ private void writeAsyncWithPriority( public BytesReference serialize(final T obj, final String blobName, final Compressor compressor, final ToXContent.Params params) throws IOException { + return serialize(obj, blobName, compressor, params, false, XContentType.SMILE); + } + + public BytesReference serialize( + final T obj, + final String blobName, + final Compressor compressor, + final ToXContent.Params params, + boolean skipHeaderFooter, + MediaType type + ) throws IOException { try (BytesStreamOutput outputStream = new BytesStreamOutput()) { try ( OutputStreamIndexOutput indexOutput = new OutputStreamIndexOutput( @@ -299,7 +369,9 @@ public BytesReference serialize(final T obj, final String blobName, final Compre BUFFER_SIZE ) ) { - CodecUtil.writeHeader(indexOutput, codec, VERSION); + if (skipHeaderFooter == false) { + CodecUtil.writeHeader(indexOutput, codec, VERSION); + } try (OutputStream indexOutputOutputStream = new IndexOutputOutputStream(indexOutput) { @Override public void close() throws IOException { @@ -308,7 +380,7 @@ public void close() throws IOException { } }; XContentBuilder builder = MediaTypeRegistry.contentBuilder( - XContentType.SMILE, + type, compressor.threadLocalOutputStream(indexOutputOutputStream) ) ) { @@ -316,7 +388,9 @@ public void close() throws IOException { obj.toXContent(builder, params); builder.endObject(); } - CodecUtil.writeFooter(indexOutput); + if (skipHeaderFooter == false) { + CodecUtil.writeFooter(indexOutput); + } } return outputStream.bytes(); } diff --git a/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathTests.java b/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathTests.java new file mode 100644 index 0000000000000..42e42bca8e55f --- /dev/null +++ b/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathTests.java @@ -0,0 +1,54 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.index.remote; + +import org.opensearch.common.blobstore.BlobPath; +import org.opensearch.core.xcontent.MediaTypeRegistry; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.index.remote.RemoteStoreEnums.PathHashAlgorithm; +import org.opensearch.index.remote.RemoteStoreEnums.PathType; +import org.opensearch.test.OpenSearchTestCase; + +import java.io.IOException; +import java.util.List; +import java.util.Map; + +import static org.opensearch.index.remote.RemoteStoreEnums.DataCategory.SEGMENTS; +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; + +public class RemoteIndexPathTests extends OpenSearchTestCase { + + public void testToXContent() throws IOException { + RemoteIndexPath indexPath = new RemoteIndexPath( + "test", + 5, + new BlobPath().add("dsd").add("hello"), + PathType.HASHED_PREFIX, + PathHashAlgorithm.FNV_1A, + Map.of(SEGMENTS, List.of(DATA, METADATA, LOCK_FILES)) + ); + XContentBuilder xContentBuilder = MediaTypeRegistry.contentBuilder(MediaTypeRegistry.JSON); + xContentBuilder.startObject(); + xContentBuilder = indexPath.toXContent(xContentBuilder, ToXContent.EMPTY_PARAMS); + xContentBuilder.endObject(); + String expected = "{\"version\":\"1\",\"index_uuid\":\"test\",\"shard_count\":5,\"path_type\":\"HASHED_PREFIX\"" + + ",\"path_hash_algorithm\":\"FNV_1A\",\"paths\":[\"7w7J_t5dXgk/dsd/hello/test/0/segments/data/\"," + + "\"d2tIsVGsh9I/dsd/hello/test/1/segments/data/\",\"jDj7aact8oc/dsd/hello/test/2/segments/data/\"," + + "\"Ea34xvbcE4g/dsd/hello/test/3/segments/data/\",\"EaMx7uStzM0/dsd/hello/test/4/segments/data/\"," + + "\"WR3ZZ6pAH9w/dsd/hello/test/0/segments/metadata/\",\"OmAyQrYTH9c/dsd/hello/test/1/segments/metadata/\"," + + "\"9GFNLB8iu2I/dsd/hello/test/2/segments/metadata/\",\"t4sqq-4QAiU/dsd/hello/test/3/segments/metadata/\"," + + "\"m5IHfRLxuTg/dsd/hello/test/4/segments/metadata/\",\"1l_zRI9-N0I/dsd/hello/test/0/segments/lock_files/\"," + + "\"BJxu8oivE1k/dsd/hello/test/1/segments/lock_files/\",\"mkME0l_DbDA/dsd/hello/test/2/segments/lock_files/\"," + + "\"W7LqEuhiyTc/dsd/hello/test/3/segments/lock_files/\",\"Ls5bj0LgiKY/dsd/hello/test/4/segments/lock_files/\"]}"; + assertEquals(expected, xContentBuilder.toString()); + } +} From b733297533d16ec06eae5eddadec945032b9f102 Mon Sep 17 00:00:00 2001 From: Ashish Singh Date: Thu, 11 Apr 2024 12:31:24 +0530 Subject: [PATCH 02/15] Add UTs Signed-off-by: Ashish Singh --- .../index/remote/RemoteIndexPathTests.java | 120 +++++++++++++++--- 1 file changed, 101 insertions(+), 19 deletions(-) diff --git a/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathTests.java b/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathTests.java index 42e42bca8e55f..1ce05f204e5d0 100644 --- a/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathTests.java +++ b/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathTests.java @@ -17,38 +17,120 @@ import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; +import java.util.HashMap; import java.util.List; import java.util.Map; - -import static org.opensearch.index.remote.RemoteStoreEnums.DataCategory.SEGMENTS; -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 java.util.TreeMap; public class RemoteIndexPathTests extends OpenSearchTestCase { - public void testToXContent() throws IOException { + /** + * This checks that the remote path contains paths only for segment and data/metadata/lock_files combination. + */ + public void testToXContentWithSegmentRepo() throws IOException { + RemoteIndexPath indexPath = new RemoteIndexPath( + "djjsid73he8yd7usduh", + 2, + new BlobPath().add("djsd878ndjh").add("hcs87cj8"), + PathType.HASHED_PREFIX, + PathHashAlgorithm.FNV_1A, + RemoteIndexPath.SEGMENT_PATH + ); + XContentBuilder xContentBuilder = MediaTypeRegistry.contentBuilder(MediaTypeRegistry.JSON); + xContentBuilder.startObject(); + xContentBuilder = indexPath.toXContent(xContentBuilder, ToXContent.EMPTY_PARAMS); + xContentBuilder.endObject(); + String expected = "{\"version\":\"1\",\"index_uuid\":\"djjsid73he8yd7usduh\",\"shard_count\":2," + + "\"path_type\":\"HASHED_PREFIX\",\"path_hash_algorithm\":\"FNV_1A\",\"paths\":[" + + "\"9BmBinD5HYs/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/0/segments/data/\"," + + "\"ExCNOD8_5ew/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/1/segments/data/\"," + + "\"z8wtf0yr2l4/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/0/segments/metadata/\"," + + "\"VheHVwFlExE/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/1/segments/metadata/\"," + + "\"IgFKbsDeUpQ/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/0/segments/lock_files/\"," + + "\"pA3gy_GZtns/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/1/segments/lock_files/\"]}"; + assertEquals(expected, xContentBuilder.toString()); + } + + /** + * This checks that the remote path contains paths only for translog and data/metadata combination. + */ + public void testToXContentForTranslogRepoOnly() throws IOException { RemoteIndexPath indexPath = new RemoteIndexPath( - "test", - 5, - new BlobPath().add("dsd").add("hello"), + "djjsid73he8yd7usduh", + 2, + new BlobPath().add("djsd878ndjh").add("hcs87cj8"), PathType.HASHED_PREFIX, PathHashAlgorithm.FNV_1A, - Map.of(SEGMENTS, List.of(DATA, METADATA, LOCK_FILES)) + RemoteIndexPath.TRANSLOG_PATH ); XContentBuilder xContentBuilder = MediaTypeRegistry.contentBuilder(MediaTypeRegistry.JSON); xContentBuilder.startObject(); xContentBuilder = indexPath.toXContent(xContentBuilder, ToXContent.EMPTY_PARAMS); xContentBuilder.endObject(); - String expected = "{\"version\":\"1\",\"index_uuid\":\"test\",\"shard_count\":5,\"path_type\":\"HASHED_PREFIX\"" - + ",\"path_hash_algorithm\":\"FNV_1A\",\"paths\":[\"7w7J_t5dXgk/dsd/hello/test/0/segments/data/\"," - + "\"d2tIsVGsh9I/dsd/hello/test/1/segments/data/\",\"jDj7aact8oc/dsd/hello/test/2/segments/data/\"," - + "\"Ea34xvbcE4g/dsd/hello/test/3/segments/data/\",\"EaMx7uStzM0/dsd/hello/test/4/segments/data/\"," - + "\"WR3ZZ6pAH9w/dsd/hello/test/0/segments/metadata/\",\"OmAyQrYTH9c/dsd/hello/test/1/segments/metadata/\"," - + "\"9GFNLB8iu2I/dsd/hello/test/2/segments/metadata/\",\"t4sqq-4QAiU/dsd/hello/test/3/segments/metadata/\"," - + "\"m5IHfRLxuTg/dsd/hello/test/4/segments/metadata/\",\"1l_zRI9-N0I/dsd/hello/test/0/segments/lock_files/\"," - + "\"BJxu8oivE1k/dsd/hello/test/1/segments/lock_files/\",\"mkME0l_DbDA/dsd/hello/test/2/segments/lock_files/\"," - + "\"W7LqEuhiyTc/dsd/hello/test/3/segments/lock_files/\",\"Ls5bj0LgiKY/dsd/hello/test/4/segments/lock_files/\"]}"; + String expected = "{\"version\":\"1\",\"index_uuid\":\"djjsid73he8yd7usduh\",\"shard_count\":2," + + "\"path_type\":\"HASHED_PREFIX\",\"path_hash_algorithm\":\"FNV_1A\",\"paths\":[" + + "\"2EaVODaKBck/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/0/translog/data/\"," + + "\"dTS2VqEOUNo/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/1/translog/data/\"," + + "\"PVNKNGonmZw/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/0/translog/metadata/\"," + + "\"NXmt0Y6NjA8/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/1/translog/metadata/\"]}"; assertEquals(expected, xContentBuilder.toString()); } + + /** + * This checks that the remote path contains paths only for translog and data/metadata combination. + */ + public void testToXContentForBothRepos() throws IOException { + Map> pathCreationMap = new TreeMap<>(); + pathCreationMap.putAll(RemoteIndexPath.TRANSLOG_PATH); + pathCreationMap.putAll(RemoteIndexPath.SEGMENT_PATH); + RemoteIndexPath indexPath = new RemoteIndexPath( + "csbdqiu8a7sdnjdks", + 3, + new BlobPath().add("nxf9yv0").add("c3ejoi"), + PathType.HASHED_PREFIX, + PathHashAlgorithm.FNV_1A, + pathCreationMap + ); + XContentBuilder xContentBuilder = MediaTypeRegistry.contentBuilder(MediaTypeRegistry.JSON); + xContentBuilder.startObject(); + xContentBuilder = indexPath.toXContent(xContentBuilder, ToXContent.EMPTY_PARAMS); + xContentBuilder.endObject(); + String expected = "{\"version\":\"1\",\"index_uuid\":\"csbdqiu8a7sdnjdks\",\"shard_count\":3,\"path_type\":" + + "\"HASHED_PREFIX\",\"path_hash_algorithm\":\"FNV_1A\",\"paths\":[" + + "\"Cjo0F6kNjYk/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/0/segments/data/\"," + + "\"kpayyhxct1I/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/1/segments/data/\"," + + "\"p2RlgnHeIgc/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/2/segments/data/\"," + + "\"gkPIurBtB1w/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/0/segments/metadata/\"," + + "\"Y4YhlbxAB1c/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/1/segments/metadata/\"," + + "\"HYc8fyVPouI/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/2/segments/metadata/\"," + + "\"igzyZCz1ysI/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/0/segments/lock_files/\"," + + "\"uEluEiYmptk/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/1/segments/lock_files/\"," + + "\"TfAD8f06_7A/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/2/segments/lock_files/\"," + + "\"QqKEpasbEGs/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/0/translog/data/\"," + + "\"sNyoimoe1Bw/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/1/translog/data/\"," + + "\"d4YQtONfq50/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/2/translog/data/\"," + + "\"zLr4UXjK8T4/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/0/translog/metadata/\"," + + "\"_s8i7ZmlXGE/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/1/translog/metadata/\"," + + "\"tvtD3-k5ISg/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/2/translog/metadata/\"]}"; + assertEquals(expected, xContentBuilder.toString()); + } + + public void testRemoteIndexPathWithInvalidPathCreationMap() throws IOException { + IllegalArgumentException ex = assertThrows( + IllegalArgumentException.class, + () -> new RemoteIndexPath( + "djjsid73he8yd7usduh", + 2, + new BlobPath().add("djsd878ndjh").add("hcs87cj8"), + PathType.HASHED_PREFIX, + PathHashAlgorithm.FNV_1A, + new HashMap<>() + ) + ); + assertEquals( + "Invalid input in RemoteIndexPath constructor indexUUID=djjsid73he8yd7usduh shardCount=2 " + + "basePath=[djsd878ndjh][hcs87cj8] pathType=HASHED_PREFIX pathHashAlgorithm=FNV_1A pathCreationMap={}", + ex.getMessage() + ); + } } From 9c380a2d78ba83026535fe5656816caad30a26c9 Mon Sep 17 00:00:00 2001 From: Ashish Singh Date: Mon, 15 Apr 2024 15:11:38 +0530 Subject: [PATCH 03/15] Incoporate PR review feedback Signed-off-by: Ashish Singh --- ...reationPreIndexMetadataUploadListener.java | 46 ++++ .../remote/RemoteClusterStateService.java | 233 ++--------------- ...RemoteUploadPathIndexCreationListener.java | 234 ++++++++++++++++++ .../main/java/org/opensearch/node/Node.java | 14 +- .../GatewayMetaStatePersistedStateTests.java | 16 +- .../RemoteClusterStateServiceTests.java | 28 ++- 6 files changed, 355 insertions(+), 216 deletions(-) create mode 100644 server/src/main/java/org/opensearch/gateway/remote/IndexCreationPreIndexMetadataUploadListener.java create mode 100644 server/src/main/java/org/opensearch/index/remote/RemoteUploadPathIndexCreationListener.java diff --git a/server/src/main/java/org/opensearch/gateway/remote/IndexCreationPreIndexMetadataUploadListener.java b/server/src/main/java/org/opensearch/gateway/remote/IndexCreationPreIndexMetadataUploadListener.java new file mode 100644 index 0000000000000..8b8c0ca137079 --- /dev/null +++ b/server/src/main/java/org/opensearch/gateway/remote/IndexCreationPreIndexMetadataUploadListener.java @@ -0,0 +1,46 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.gateway.remote; + +import org.opensearch.cluster.metadata.IndexMetadata; + +import java.io.IOException; +import java.util.List; +import java.util.concurrent.CountDownLatch; + +/** + * Hook for running code that needs to be executed before the upload of index metadata during index creation or + * after enabling the remote cluster statement for the first time. The listener is intended to be run in parallel and + * async with the index metadata upload. + * + * @opensearch.internal + */ +public interface IndexCreationPreIndexMetadataUploadListener { + + /** + * This returns the additional count that needs to be added in the latch present in {@link RemoteClusterStateService} + * which is used to achieve parallelism and async nature of uploads for index metadata upload. The same latch is used + * for running pre index metadata upload listener. + * + * @param newIndexMetadataList list of index metadata of new indexes (or first time index metadata upload). + * @return latch count to be added by the caller. + */ + int latchCount(List newIndexMetadataList); + + /** + * This will run the pre index metadata upload listener using the {@code newIndexMetadataList}, {@code latch} and + * {@code exceptionList}. This method must execute the operation in parallel and async to ensure that the cluster state + * upload time remains the same. + * + * @param newIndexMetadataList list of index metadata of new indexes (or first time index metadata upload). + * @param latch this is used for counting down once the unit of work per index is done. + * @param exceptionList exception if any during run will be added here and used by the caller. + */ + void run(List newIndexMetadataList, CountDownLatch latch, List exceptionList) throws IOException; +} diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java index 4d885e014931c..c7790ab965ca1 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -26,16 +26,10 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.io.IOUtils; -import org.opensearch.common.xcontent.XContentType; import org.opensearch.core.action.ActionListener; import org.opensearch.core.index.Index; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.gateway.remote.ClusterMetadataManifest.UploadedIndexMetadata; -import org.opensearch.index.remote.RemoteIndexPath; -import org.opensearch.index.remote.RemoteStoreEnums.DataCategory; -import org.opensearch.index.remote.RemoteStoreEnums.DataType; -import org.opensearch.index.remote.RemoteStoreEnums.PathHashAlgorithm; -import org.opensearch.index.remote.RemoteStoreEnums.PathType; import org.opensearch.index.remote.RemoteStoreUtils; import org.opensearch.index.translog.transfer.BlobStoreTransferService; import org.opensearch.node.Node; @@ -70,9 +64,6 @@ import java.util.stream.Collectors; import static org.opensearch.gateway.PersistedClusterStateService.SLOW_WRITE_LOGGING_THRESHOLD; -import static org.opensearch.index.remote.RemoteIndexPath.SEGMENT_PATH; -import static org.opensearch.index.remote.RemoteIndexPath.TRANSLOG_PATH; -import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteDataAttributePresent; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteStoreClusterStateEnabled; /** @@ -131,12 +122,6 @@ public class RemoteClusterStateService implements Closeable { Metadata::fromXContent ); - public static final ChecksumBlobStoreFormat REMOTE_INDEX_PATH_FORMAT = new ChecksumBlobStoreFormat<>( - "remote-index-path", - RemoteIndexPath.FILE_NAME_FORMAT, - RemoteIndexPath::fromXContent - ); - /** * Manifest format compatible with older codec v0, where codec version was missing. */ @@ -175,14 +160,10 @@ public class RemoteClusterStateService implements Closeable { private final Settings settings; private final LongSupplier relativeTimeNanosSupplier; private final ThreadPool threadpool; + private final IndexCreationPreIndexMetadataUploadListener indexCreationListener; private BlobStoreRepository blobStoreRepository; private BlobStoreTransferService blobStoreTransferService; private volatile TimeValue slowWriteLoggingThreshold; - private BlobStoreRepository translogRepository; - private BlobStoreTransferService translogTransferService; - private BlobStoreRepository segmentRepository; - private BlobStoreTransferService segmentsTransferService; - private final boolean isRemoteDataAttributePresent; private volatile TimeValue indexMetadataUploadTimeout; private volatile TimeValue globalMetadataUploadTimeout; @@ -209,7 +190,8 @@ public RemoteClusterStateService( Settings settings, ClusterSettings clusterSettings, LongSupplier relativeTimeNanosSupplier, - ThreadPool threadPool + ThreadPool threadPool, + IndexCreationPreIndexMetadataUploadListener indexCreationListener ) { assert isRemoteStoreClusterStateEnabled(settings) : "Remote cluster state is not enabled"; this.nodeId = nodeId; @@ -226,7 +208,7 @@ public RemoteClusterStateService( clusterSettings.addSettingsUpdateConsumer(GLOBAL_METADATA_UPLOAD_TIMEOUT_SETTING, this::setGlobalMetadataUploadTimeout); clusterSettings.addSettingsUpdateConsumer(METADATA_MANIFEST_UPLOAD_TIMEOUT_SETTING, this::setMetadataManifestUploadTimeout); this.remoteStateStats = new RemotePersistenceStats(); - this.isRemoteDataAttributePresent = isRemoteDataAttributePresent(settings); + this.indexCreationListener = indexCreationListener; } private BlobStoreTransferService getBlobStoreTransferService() { @@ -236,20 +218,6 @@ private BlobStoreTransferService getBlobStoreTransferService() { return blobStoreTransferService; } - private BlobStoreTransferService getTranslogTransferService() { - if (translogTransferService == null) { - translogTransferService = new BlobStoreTransferService(translogRepository.blobStore(), threadpool); - } - return translogTransferService; - } - - private BlobStoreTransferService getSegmentsTransferService() { - if (segmentsTransferService == null) { - segmentsTransferService = new BlobStoreTransferService(segmentRepository.blobStore(), threadpool); - } - return segmentsTransferService; - } - /** * This method uploads entire cluster state metadata to the configured blob store. For now only index metadata upload is supported. This method should be * invoked by the elected cluster manager when the remote cluster state is enabled. @@ -482,15 +450,15 @@ private String writeGlobalMetadata(ClusterState clusterState) throws IOException private List writeIndexMetadataParallel( ClusterState clusterState, List toUpload, - List toUploadIndexPath + List newIndexMetadataList ) throws IOException { - boolean isTranslogSegmentRepoSame = isTranslogSegmentRepoSame(); - int latchCount = toUpload.size() + (isTranslogSegmentRepoSame ? toUploadIndexPath.size() : 2 * toUploadIndexPath.size()); + assert Objects.nonNull(indexCreationListener) : "indexCreationListener can not be null"; + int latchCount = toUpload.size() + indexCreationListener.latchCount(newIndexMetadataList); List exceptionList = Collections.synchronizedList(new ArrayList<>(latchCount)); final CountDownLatch latch = new CountDownLatch(latchCount); List result = new ArrayList<>(toUpload.size()); uploadIndexMetadataAsync(clusterState, result, toUpload, latch, exceptionList); - uploadIndexPathAsync(toUploadIndexPath, latch, isTranslogSegmentRepoSame, exceptionList); + indexCreationListener.run(newIndexMetadataList, latch, exceptionList); try { if (latch.await(getIndexMetadataUploadTimeout().millis(), TimeUnit.MILLISECONDS) == false) { @@ -531,102 +499,6 @@ private List writeIndexMetadataParallel( return result; } - private void uploadIndexPathAsync( - List toUploadIndexPath, - CountDownLatch latch, - boolean isTranslogSegmentRepoSame, - List exceptionList - ) throws IOException { - for (IndexMetadata indexMetadata : toUploadIndexPath) { - writeIndexPathAsync(indexMetadata, latch, isTranslogSegmentRepoSame, exceptionList); - } - } - - private void writeIndexPathAsync( - IndexMetadata idxMD, - CountDownLatch latch, - boolean isTranslogSegmentRepoSame, - List exceptionList - ) throws IOException { - Map remoteCustomData = idxMD.getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY); - PathType pathType = PathType.valueOf(remoteCustomData.get(PathType.NAME)); - PathHashAlgorithm hashAlgorithm = PathHashAlgorithm.valueOf(remoteCustomData.get(PathHashAlgorithm.NAME)); - String indexUUID = idxMD.getIndexUUID(); - int shardCount = idxMD.getNumberOfShards(); - BlobPath translogBasePath = translogRepository.basePath(); - BlobContainer translogBlobContainer = translogRepository.blobStore().blobContainer(translogBasePath.add(RemoteIndexPath.DIR)); - - if (isTranslogSegmentRepoSame) { - // If the repositories are same, then we need to upload a single file containing paths for both translog and segments. - Map> pathCreationMap = new HashMap<>(); - pathCreationMap.putAll(TRANSLOG_PATH); - pathCreationMap.putAll(SEGMENT_PATH); - REMOTE_INDEX_PATH_FORMAT.writeAsyncWithUrgentPriority( - new RemoteIndexPath(indexUUID, shardCount, translogBasePath, pathType, hashAlgorithm, pathCreationMap), - translogBlobContainer, - indexUUID, - translogRepository.getCompressor(), - getUploadPathLatchedActionListener(idxMD, latch, exceptionList, pathCreationMap), - FORMAT_PARAMS, - true, - XContentType.JSON - ); - } else { - // If the repositories are different, then we need to upload one file per segment and translog containing their individual - // paths. - REMOTE_INDEX_PATH_FORMAT.writeAsyncWithUrgentPriority( - new RemoteIndexPath(indexUUID, shardCount, translogBasePath, pathType, hashAlgorithm, TRANSLOG_PATH), - translogBlobContainer, - indexUUID, - translogRepository.getCompressor(), - getUploadPathLatchedActionListener(idxMD, latch, exceptionList, TRANSLOG_PATH), - FORMAT_PARAMS, - true, - XContentType.JSON - ); - - BlobPath segmentBasePath = segmentRepository.basePath(); - BlobContainer segmentBlobContainer = segmentRepository.blobStore().blobContainer(segmentBasePath.add(RemoteIndexPath.DIR)); - REMOTE_INDEX_PATH_FORMAT.writeAsyncWithUrgentPriority( - new RemoteIndexPath(indexUUID, shardCount, segmentBasePath, pathType, hashAlgorithm, SEGMENT_PATH), - segmentBlobContainer, - indexUUID, - segmentRepository.getCompressor(), - getUploadPathLatchedActionListener(idxMD, latch, exceptionList, SEGMENT_PATH), - FORMAT_PARAMS, - true, - XContentType.JSON - ); - } - } - - private LatchedActionListener getUploadPathLatchedActionListener( - IndexMetadata indexMetadata, - CountDownLatch latch, - List exceptionList, - Map> pathCreationMap - ) { - return new LatchedActionListener<>( - ActionListener.wrap( - resp -> logger.trace( - new ParameterizedMessage("Index path uploaded for {} indexMetadata={}", pathCreationMap, indexMetadata) - ), - ex -> { - logger.error( - new ParameterizedMessage( - "Exception during Index path upload for {} indexMetadata={}", - pathCreationMap, - indexMetadata - ), - ex - ); - exceptionList.add(ex); - } - ), - latch - ); - } - private void uploadIndexMetadataAsync( ClusterState clusterState, List result, @@ -657,26 +529,16 @@ private void uploadIndexMetadataAsync( } } - private boolean isTranslogSegmentRepoSame() { - String translogRepoName = settings.get( - Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY - ); - String segmentRepoName = settings.get( - Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY - ); - return Objects.equals(translogRepoName, segmentRepoName); - } - private List writeIndexMetadataParallel( ClusterState clusterState, List toUpload, String previousClusterUUID ) throws IOException { - List toUploadIndexPath = Collections.emptyList(); + List newIndexMetadaList = Collections.emptyList(); if (ClusterState.UNKNOWN_UUID.equals(previousClusterUUID)) { - toUploadIndexPath = toUpload; + newIndexMetadaList = toUpload; } - return writeIndexMetadataParallel(clusterState, toUpload, toUploadIndexPath); + return writeIndexMetadataParallel(clusterState, toUpload, newIndexMetadaList); } private List writeIndexMetadataParallel( @@ -684,35 +546,11 @@ private List writeIndexMetadataParallel( List toUpload, Map indexNamePreviousVersionMap ) throws IOException { - List toUploadIndexPath = Collections.emptyList(); - if (isRemoteDataAttributePresent) { - toUploadIndexPath = toUpload.stream() - /* If the previous state's index metadata version is null, then this is index creation */ - .filter(indexMetadata -> Objects.isNull(indexNamePreviousVersionMap.get(indexMetadata.getIndex().getName()))) - /* Checks the condition if the Index path needs to be uploaded or not */ - .filter(this::uploadIndexPathFile) - .collect(Collectors.toList()); - } - return writeIndexMetadataParallel(clusterState, toUpload, toUploadIndexPath); - } - - /** - * This method checks if the index metadata has attributes that calls for uploading the index path for remote store - * uploads. It checks if the remote store path type is {@code HASHED_PREFIX} and returns true if so. - */ - private boolean uploadIndexPathFile(IndexMetadata indexMetadata) { - assert isRemoteDataAttributePresent : "Remote data attributes is expected to be present"; - // A cluster will have remote custom metadata only if the cluster is remote store enabled from data side. - Map remoteCustomData = indexMetadata.getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY); - if (Objects.isNull(remoteCustomData) || remoteCustomData.isEmpty()) { - return false; - } - String pathTypeStr = remoteCustomData.get(PathType.NAME); - if (Objects.isNull(pathTypeStr)) { - return false; - } - // We need to upload the path only if the path type for an index is hashed_prefix - return PathType.HASHED_PREFIX == PathType.parseString(pathTypeStr); + List newIndexMetadataList = toUpload.stream() + /* If the previous state's index metadata version is null, then this is index creation */ + .filter(indexMetadata -> Objects.isNull(indexNamePreviousVersionMap.get(indexMetadata.getIndex().getName()))) + .collect(Collectors.toList()); + return writeIndexMetadataParallel(clusterState, toUpload, newIndexMetadataList); } /** @@ -783,32 +621,13 @@ public void close() throws IOException { public void start() { assert isRemoteStoreClusterStateEnabled(settings) == true : "Remote cluster state is not enabled"; - blobStoreRepository = (BlobStoreRepository) validateAndGetRepository( - Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY, - "Cluster State" - ); - - if (isRemoteDataAttributePresent == false) { - // If remote store data attributes are not present than we skip this. - return; - } - translogRepository = (BlobStoreRepository) validateAndGetRepository( - Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY, - "Translog" + final String remoteStoreRepo = settings.get( + Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY ); - segmentRepository = (BlobStoreRepository) validateAndGetRepository( - Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY, - "Translog" - ); - - } - - private Repository validateAndGetRepository(String repoSetting, String repoName) { - final String repo = settings.get(repoSetting); - assert repo != null : "Remote " + repoName + " repository is not configured"; - final Repository repository = repositoriesService.get().repository(repo); + assert remoteStoreRepo != null : "Remote Cluster State repository is not configured"; + final Repository repository = repositoriesService.get().repository(remoteStoreRepo); assert repository instanceof BlobStoreRepository : "Repository should be instance of BlobStoreRepository"; - return repository; + blobStoreRepository = (BlobStoreRepository) repository; } private ClusterMetadataManifest uploadManifest( @@ -887,16 +706,6 @@ private void writeMetadataManifest(String clusterName, String clusterUUID, Clust ); } - private String fetchPreviousClusterUUID(String clusterName, String clusterUUID) { - final Optional latestManifest = getLatestClusterMetadataManifest(clusterName, clusterUUID); - if (!latestManifest.isPresent()) { - final String previousClusterUUID = getLastKnownUUIDFromRemote(clusterName); - assert !clusterUUID.equals(previousClusterUUID) : "Last cluster UUID is same current cluster UUID"; - return previousClusterUUID; - } - return latestManifest.get().getPreviousClusterUUID(); - } - private BlobContainer indexMetadataContainer(String clusterName, String clusterUUID, String indexUUID) { // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/index/ftqsCnn9TgOX return blobStoreRepository.blobStore() diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteUploadPathIndexCreationListener.java b/server/src/main/java/org/opensearch/index/remote/RemoteUploadPathIndexCreationListener.java new file mode 100644 index 0000000000000..29868b1e0ecc5 --- /dev/null +++ b/server/src/main/java/org/opensearch/index/remote/RemoteUploadPathIndexCreationListener.java @@ -0,0 +1,234 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.index.remote; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.message.ParameterizedMessage; +import org.opensearch.action.LatchedActionListener; +import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.common.blobstore.BlobContainer; +import org.opensearch.common.blobstore.BlobPath; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.xcontent.XContentType; +import org.opensearch.core.action.ActionListener; +import org.opensearch.gateway.remote.IndexCreationPreIndexMetadataUploadListener; +import org.opensearch.gateway.remote.RemoteClusterStateService; +import org.opensearch.node.Node; +import org.opensearch.node.remotestore.RemoteStoreNodeAttribute; +import org.opensearch.repositories.RepositoriesService; +import org.opensearch.repositories.Repository; +import org.opensearch.repositories.blobstore.BlobStoreRepository; +import org.opensearch.repositories.blobstore.ChecksumBlobStoreFormat; + +import java.io.IOException; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.CountDownLatch; +import java.util.function.Supplier; +import java.util.stream.Collectors; + +import static org.opensearch.index.remote.RemoteIndexPath.SEGMENT_PATH; +import static org.opensearch.index.remote.RemoteIndexPath.TRANSLOG_PATH; +import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteDataAttributePresent; +import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteStoreClusterStateEnabled; + +/** + * Uploads the remote store path for all possible combinations of {@link org.opensearch.index.remote.RemoteStoreEnums.DataCategory} + * and {@link org.opensearch.index.remote.RemoteStoreEnums.DataType} for each shard of an index. + */ +public class RemoteUploadPathIndexCreationListener implements IndexCreationPreIndexMetadataUploadListener { + + public static final ChecksumBlobStoreFormat REMOTE_INDEX_PATH_FORMAT = new ChecksumBlobStoreFormat<>( + "remote-index-path", + RemoteIndexPath.FILE_NAME_FORMAT, + RemoteIndexPath::fromXContent + ); + + private static final Logger logger = LogManager.getLogger(RemoteUploadPathIndexCreationListener.class); + + private final Settings settings; + private final boolean isRemoteDataAttributePresent; + private final boolean isTranslogSegmentRepoSame; + private final Supplier repositoriesService; + + private BlobStoreRepository translogRepository; + private BlobStoreRepository segmentRepository; + + public RemoteUploadPathIndexCreationListener(Settings settings, Supplier repositoriesService) { + this.settings = settings; + this.repositoriesService = repositoriesService; + isRemoteDataAttributePresent = isRemoteDataAttributePresent(settings); + // If the remote data attributes are not present, then there is no effect of translog and segment being same or different or null. + isTranslogSegmentRepoSame = isTranslogSegmentRepoSame(); + } + + @Override + public int latchCount(List newIndexMetadataList) { + if (isRemoteDataAttributePresent == false) { + return 0; + } + int eligibleIndexCount = (int) newIndexMetadataList.stream().filter(this::uploadIndexPathFile).count(); + return isTranslogSegmentRepoSame ? eligibleIndexCount : 2 * eligibleIndexCount; + } + + @Override + public void run(List newIndexMetadataList, CountDownLatch latch, List exceptionList) throws IOException { + if (isRemoteDataAttributePresent == false) { + return; + } + List elibibleIndexMetadaList = newIndexMetadataList.stream() + .filter(this::uploadIndexPathFile) + .collect(Collectors.toList()); + if (isTranslogSegmentRepoSame) { + assert latchCount(newIndexMetadataList) == elibibleIndexMetadaList.size() + : "Latch count is not equal to elibibleIndexMetadaList's size for path upload"; + } else { + assert latchCount(newIndexMetadataList) == 2 * elibibleIndexMetadaList.size() + : "Latch count is not equal to (2 * elibibleIndexMetadaList's size) for path upload"; + } + for (IndexMetadata indexMetadata : elibibleIndexMetadaList) { + writeIndexPathAsync(indexMetadata, latch, exceptionList); + } + } + + private void writeIndexPathAsync(IndexMetadata idxMD, CountDownLatch latch, List exceptionList) throws IOException { + Map remoteCustomData = idxMD.getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY); + RemoteStoreEnums.PathType pathType = RemoteStoreEnums.PathType.valueOf(remoteCustomData.get(RemoteStoreEnums.PathType.NAME)); + RemoteStoreEnums.PathHashAlgorithm hashAlgorithm = RemoteStoreEnums.PathHashAlgorithm.valueOf( + remoteCustomData.get(RemoteStoreEnums.PathHashAlgorithm.NAME) + ); + String indexUUID = idxMD.getIndexUUID(); + int shardCount = idxMD.getNumberOfShards(); + BlobPath translogBasePath = translogRepository.basePath(); + BlobContainer translogBlobContainer = translogRepository.blobStore().blobContainer(translogBasePath.add(RemoteIndexPath.DIR)); + + if (isTranslogSegmentRepoSame) { + // If the repositories are same, then we need to upload a single file containing paths for both translog and segments. + Map> pathCreationMap = new HashMap<>(); + pathCreationMap.putAll(TRANSLOG_PATH); + pathCreationMap.putAll(SEGMENT_PATH); + REMOTE_INDEX_PATH_FORMAT.writeAsyncWithUrgentPriority( + new RemoteIndexPath(indexUUID, shardCount, translogBasePath, pathType, hashAlgorithm, pathCreationMap), + translogBlobContainer, + indexUUID, + translogRepository.getCompressor(), + getUploadPathLatchedActionListener(idxMD, latch, exceptionList, pathCreationMap), + RemoteClusterStateService.FORMAT_PARAMS, + true, + XContentType.JSON + ); + } else { + // If the repositories are different, then we need to upload one file per segment and translog containing their individual + // paths. + REMOTE_INDEX_PATH_FORMAT.writeAsyncWithUrgentPriority( + new RemoteIndexPath(indexUUID, shardCount, translogBasePath, pathType, hashAlgorithm, TRANSLOG_PATH), + translogBlobContainer, + indexUUID, + translogRepository.getCompressor(), + getUploadPathLatchedActionListener(idxMD, latch, exceptionList, TRANSLOG_PATH), + RemoteClusterStateService.FORMAT_PARAMS, + true, + XContentType.JSON + ); + + BlobPath segmentBasePath = segmentRepository.basePath(); + BlobContainer segmentBlobContainer = segmentRepository.blobStore().blobContainer(segmentBasePath.add(RemoteIndexPath.DIR)); + REMOTE_INDEX_PATH_FORMAT.writeAsyncWithUrgentPriority( + new RemoteIndexPath(indexUUID, shardCount, segmentBasePath, pathType, hashAlgorithm, SEGMENT_PATH), + segmentBlobContainer, + indexUUID, + segmentRepository.getCompressor(), + getUploadPathLatchedActionListener(idxMD, latch, exceptionList, SEGMENT_PATH), + RemoteClusterStateService.FORMAT_PARAMS, + true, + XContentType.JSON + ); + } + } + + private Repository validateAndGetRepository(String repoSetting) { + final String repo = settings.get(repoSetting); + assert repo != null : "Remote " + repoSetting + " repository is not configured"; + final Repository repository = repositoriesService.get().repository(repo); + assert repository instanceof BlobStoreRepository : "Repository should be instance of BlobStoreRepository"; + return repository; + } + + public void start() { + assert isRemoteStoreClusterStateEnabled(settings) == true : "Remote cluster state is not enabled"; + if (isRemoteDataAttributePresent == false) { + // If remote store data attributes are not present than we skip this. + return; + } + translogRepository = (BlobStoreRepository) validateAndGetRepository( + Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY + ); + segmentRepository = (BlobStoreRepository) validateAndGetRepository( + Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY + ); + } + + private boolean isTranslogSegmentRepoSame() { + String translogRepoName = settings.get( + Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY + ); + String segmentRepoName = settings.get( + Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY + ); + return Objects.equals(translogRepoName, segmentRepoName); + } + + private LatchedActionListener getUploadPathLatchedActionListener( + IndexMetadata indexMetadata, + CountDownLatch latch, + List exceptionList, + Map> pathCreationMap + ) { + return new LatchedActionListener<>( + ActionListener.wrap( + resp -> logger.trace( + new ParameterizedMessage("Index path uploaded for {} indexMetadata={}", pathCreationMap, indexMetadata) + ), + ex -> { + logger.error( + new ParameterizedMessage( + "Exception during Index path upload for {} indexMetadata={}", + pathCreationMap, + indexMetadata + ), + ex + ); + exceptionList.add(ex); + } + ), + latch + ); + } + + /** + * This method checks if the index metadata has attributes that calls for uploading the index path for remote store + * uploads. It checks if the remote store path type is {@code HASHED_PREFIX} and returns true if so. + */ + private boolean uploadIndexPathFile(IndexMetadata indexMetadata) { + // A cluster will have remote custom metadata only if the cluster is remote store enabled from data side. + Map remoteCustomData = indexMetadata.getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY); + if (Objects.isNull(remoteCustomData) || remoteCustomData.isEmpty()) { + return false; + } + String pathTypeStr = remoteCustomData.get(RemoteStoreEnums.PathType.NAME); + if (Objects.isNull(pathTypeStr)) { + return false; + } + // We need to upload the path only if the path type for an index is hashed_prefix + return RemoteStoreEnums.PathType.HASHED_PREFIX == RemoteStoreEnums.PathType.parseString(pathTypeStr); + } +} diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index a33fd71e21896..ef81575d63336 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -147,6 +147,7 @@ import org.opensearch.index.engine.EngineFactory; import org.opensearch.index.recovery.RemoteStoreRestoreService; import org.opensearch.index.remote.RemoteStoreStatsTrackerFactory; +import org.opensearch.index.remote.RemoteUploadPathIndexCreationListener; import org.opensearch.index.store.RemoteSegmentStoreDirectoryFactory; import org.opensearch.index.store.remote.filecache.FileCache; import org.opensearch.index.store.remote.filecache.FileCacheCleaner; @@ -726,17 +727,21 @@ protected Node( threadPool::relativeTimeInMillis ); final RemoteClusterStateService remoteClusterStateService; + final RemoteUploadPathIndexCreationListener indexCreationListener; if (isRemoteStoreClusterStateEnabled(settings)) { + indexCreationListener = new RemoteUploadPathIndexCreationListener(settings, repositoriesServiceReference::get); remoteClusterStateService = new RemoteClusterStateService( nodeEnvironment.nodeId(), repositoriesServiceReference::get, settings, clusterService.getClusterSettings(), threadPool::preciseRelativeTimeInNanos, - threadPool + threadPool, + indexCreationListener ); } else { remoteClusterStateService = null; + indexCreationListener = null; } // collect engine factory providers from plugins @@ -1313,6 +1318,7 @@ protected Node( b.bind(SearchRequestSlowLog.class).toInstance(searchRequestSlowLog); b.bind(MetricsRegistry.class).toInstance(metricsRegistry); b.bind(RemoteClusterStateService.class).toProvider(() -> remoteClusterStateService); + b.bind(RemoteUploadPathIndexCreationListener.class).toProvider(() -> indexCreationListener); b.bind(PersistedStateRegistry.class).toInstance(persistedStateRegistry); b.bind(SegmentReplicationStatsTracker.class).toInstance(segmentReplicationStatsTracker); b.bind(SearchRequestOperationsCompositeListenerFactory.class).toInstance(searchRequestOperationsCompositeListenerFactory); @@ -1462,6 +1468,12 @@ public Node start() throws NodeValidationException { if (remoteClusterStateService != null) { remoteClusterStateService.start(); } + final RemoteUploadPathIndexCreationListener indexCreationListener = injector.getInstance( + RemoteUploadPathIndexCreationListener.class + ); + if (indexCreationListener != null) { + indexCreationListener.start(); + } // Load (and maybe upgrade) the metadata stored on disk final GatewayMetaState gatewayMetaState = injector.getInstance(GatewayMetaState.class); gatewayMetaState.start( diff --git a/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java b/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java index 74bae7b5eb7cf..7ecbc19c507d8 100644 --- a/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java +++ b/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java @@ -67,6 +67,7 @@ import org.opensearch.gateway.GatewayMetaState.RemotePersistedState; import org.opensearch.gateway.PersistedClusterStateService.Writer; import org.opensearch.gateway.remote.ClusterMetadataManifest; +import org.opensearch.gateway.remote.IndexCreationPreIndexMetadataUploadListener; import org.opensearch.gateway.remote.RemoteClusterStateService; import org.opensearch.gateway.remote.RemotePersistenceStats; import org.opensearch.index.recovery.RemoteStoreRestoreService; @@ -88,6 +89,7 @@ import java.util.List; import java.util.Locale; import java.util.Optional; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Supplier; @@ -486,7 +488,19 @@ public void testDataOnlyNodePersistence() throws Exception { settings, new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), () -> 0L, - threadPool + threadPool, + new IndexCreationPreIndexMetadataUploadListener() { + @Override + public int latchCount(List newIndexMetadataList) { + return 0; + } + + @Override + public void run(List newIndexMetadataList, CountDownLatch latch, List exceptionList) + throws IOException { + + } + } ); } else { return null; diff --git a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java index 65477051cdb30..4f7e27f9f8ce2 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -154,7 +154,19 @@ public void setup() { settings, clusterSettings, () -> 0L, - threadPool + threadPool, + new IndexCreationPreIndexMetadataUploadListener() { + @Override + public int latchCount(List newIndexMetadataList) { + return 0; + } + + @Override + public void run(List newIndexMetadataList, CountDownLatch latch, List exceptionList) + throws IOException { + + } + } ); } @@ -181,7 +193,19 @@ public void testFailInitializationWhenRemoteStateDisabled() { settings, new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), () -> 0L, - threadPool + threadPool, + new IndexCreationPreIndexMetadataUploadListener() { + @Override + public int latchCount(List newIndexMetadataList) { + return 0; + } + + @Override + public void run(List newIndexMetadataList, CountDownLatch latch, List exceptionList) + throws IOException { + + } + } ) ); } From 4955705700c1c947f761f5910a3abf95e66af9eb Mon Sep 17 00:00:00 2001 From: Ashish Singh Date: Mon, 15 Apr 2024 15:49:34 +0530 Subject: [PATCH 04/15] Refactor existing tests Signed-off-by: Ashish Singh --- .../GatewayMetaStatePersistedStateTests.java | 33 +++++++------------ .../RemoteClusterStateServiceTests.java | 27 ++------------- 2 files changed, 14 insertions(+), 46 deletions(-) diff --git a/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java b/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java index 7ecbc19c507d8..6a831f636df6f 100644 --- a/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java +++ b/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java @@ -67,11 +67,11 @@ import org.opensearch.gateway.GatewayMetaState.RemotePersistedState; import org.opensearch.gateway.PersistedClusterStateService.Writer; import org.opensearch.gateway.remote.ClusterMetadataManifest; -import org.opensearch.gateway.remote.IndexCreationPreIndexMetadataUploadListener; import org.opensearch.gateway.remote.RemoteClusterStateService; import org.opensearch.gateway.remote.RemotePersistenceStats; import org.opensearch.index.recovery.RemoteStoreRestoreService; import org.opensearch.index.recovery.RemoteStoreRestoreService.RemoteRestoreResult; +import org.opensearch.index.remote.RemoteUploadPathIndexCreationListener; import org.opensearch.node.Node; import org.opensearch.repositories.RepositoriesService; import org.opensearch.repositories.fs.FsRepository; @@ -89,7 +89,6 @@ import java.util.List; import java.util.Locale; import java.util.Optional; -import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Supplier; @@ -475,32 +474,22 @@ public void testDataOnlyNodePersistence() throws Exception { ); Supplier remoteClusterStateServiceSupplier = () -> { if (isRemoteStoreClusterStateEnabled(settings)) { + Supplier repositoriesServiceSupplier = () -> new RepositoriesService( + settings, + clusterService, + transportService, + Collections.emptyMap(), + Collections.emptyMap(), + transportService.getThreadPool() + ); return new RemoteClusterStateService( nodeEnvironment.nodeId(), - () -> new RepositoriesService( - settings, - clusterService, - transportService, - Collections.emptyMap(), - Collections.emptyMap(), - transportService.getThreadPool() - ), + repositoriesServiceSupplier, settings, new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), () -> 0L, threadPool, - new IndexCreationPreIndexMetadataUploadListener() { - @Override - public int latchCount(List newIndexMetadataList) { - return 0; - } - - @Override - public void run(List newIndexMetadataList, CountDownLatch latch, List exceptionList) - throws IOException { - - } - } + new RemoteUploadPathIndexCreationListener(settings, repositoriesServiceSupplier) ); } else { return null; diff --git a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java index 4f7e27f9f8ce2..e760b915ed260 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -39,6 +39,7 @@ import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.gateway.remote.ClusterMetadataManifest.UploadedIndexMetadata; import org.opensearch.index.remote.RemoteStoreUtils; +import org.opensearch.index.remote.RemoteUploadPathIndexCreationListener; import org.opensearch.indices.IndicesModule; import org.opensearch.repositories.FilterRepository; import org.opensearch.repositories.RepositoriesService; @@ -155,18 +156,7 @@ public void setup() { clusterSettings, () -> 0L, threadPool, - new IndexCreationPreIndexMetadataUploadListener() { - @Override - public int latchCount(List newIndexMetadataList) { - return 0; - } - - @Override - public void run(List newIndexMetadataList, CountDownLatch latch, List exceptionList) - throws IOException { - - } - } + new RemoteUploadPathIndexCreationListener(settings, repositoriesServiceSupplier) ); } @@ -194,18 +184,7 @@ public void testFailInitializationWhenRemoteStateDisabled() { new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), () -> 0L, threadPool, - new IndexCreationPreIndexMetadataUploadListener() { - @Override - public int latchCount(List newIndexMetadataList) { - return 0; - } - - @Override - public void run(List newIndexMetadataList, CountDownLatch latch, List exceptionList) - throws IOException { - - } - } + new RemoteUploadPathIndexCreationListener(settings, repositoriesServiceSupplier) ) ); } From 19949692ab960e255499150cbdf4d886d3bebc1d Mon Sep 17 00:00:00 2001 From: Ashish Singh Date: Tue, 16 Apr 2024 15:55:26 +0530 Subject: [PATCH 05/15] Incoporate PR review feedback Signed-off-by: Ashish Singh --- ...reationPreIndexMetadataUploadListener.java | 46 ------ .../IndexMetadataUploadInterceptor.java | 34 +++++ .../remote/RemoteClusterStateService.java | 136 +++++++++--------- ...ener.java => RemoteIndexPathUploader.java} | 96 ++++++++----- .../main/java/org/opensearch/node/Node.java | 20 +-- .../blobstore/ChecksumBlobStoreFormat.java | 92 ++---------- .../GatewayMetaStatePersistedStateTests.java | 7 +- .../RemoteClusterStateServiceTests.java | 9 +- 8 files changed, 196 insertions(+), 244 deletions(-) delete mode 100644 server/src/main/java/org/opensearch/gateway/remote/IndexCreationPreIndexMetadataUploadListener.java create mode 100644 server/src/main/java/org/opensearch/gateway/remote/IndexMetadataUploadInterceptor.java rename server/src/main/java/org/opensearch/index/remote/{RemoteUploadPathIndexCreationListener.java => RemoteIndexPathUploader.java} (72%) diff --git a/server/src/main/java/org/opensearch/gateway/remote/IndexCreationPreIndexMetadataUploadListener.java b/server/src/main/java/org/opensearch/gateway/remote/IndexCreationPreIndexMetadataUploadListener.java deleted file mode 100644 index 8b8c0ca137079..0000000000000 --- a/server/src/main/java/org/opensearch/gateway/remote/IndexCreationPreIndexMetadataUploadListener.java +++ /dev/null @@ -1,46 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.gateway.remote; - -import org.opensearch.cluster.metadata.IndexMetadata; - -import java.io.IOException; -import java.util.List; -import java.util.concurrent.CountDownLatch; - -/** - * Hook for running code that needs to be executed before the upload of index metadata during index creation or - * after enabling the remote cluster statement for the first time. The listener is intended to be run in parallel and - * async with the index metadata upload. - * - * @opensearch.internal - */ -public interface IndexCreationPreIndexMetadataUploadListener { - - /** - * This returns the additional count that needs to be added in the latch present in {@link RemoteClusterStateService} - * which is used to achieve parallelism and async nature of uploads for index metadata upload. The same latch is used - * for running pre index metadata upload listener. - * - * @param newIndexMetadataList list of index metadata of new indexes (or first time index metadata upload). - * @return latch count to be added by the caller. - */ - int latchCount(List newIndexMetadataList); - - /** - * This will run the pre index metadata upload listener using the {@code newIndexMetadataList}, {@code latch} and - * {@code exceptionList}. This method must execute the operation in parallel and async to ensure that the cluster state - * upload time remains the same. - * - * @param newIndexMetadataList list of index metadata of new indexes (or first time index metadata upload). - * @param latch this is used for counting down once the unit of work per index is done. - * @param exceptionList exception if any during run will be added here and used by the caller. - */ - void run(List newIndexMetadataList, CountDownLatch latch, List exceptionList) throws IOException; -} diff --git a/server/src/main/java/org/opensearch/gateway/remote/IndexMetadataUploadInterceptor.java b/server/src/main/java/org/opensearch/gateway/remote/IndexMetadataUploadInterceptor.java new file mode 100644 index 0000000000000..709b61ef6efb2 --- /dev/null +++ b/server/src/main/java/org/opensearch/gateway/remote/IndexMetadataUploadInterceptor.java @@ -0,0 +1,34 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.gateway.remote; + +import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.core.action.ActionListener; + +import java.io.IOException; +import java.util.List; + +/** + * Hook for running code that needs to be executed before the upload of index metadata. Here we have introduced a hook + * for index creation (also triggerred after enabling the remote cluster statement for the first time). The Interceptor + * is intended to be run in parallel and async with the index metadata upload. + * + * @opensearch.internal + */ +public interface IndexMetadataUploadInterceptor { + + /** + * Intercepts the index metadata upload flow with input containing index metadata of new indexes (or first time upload). + * The caller is expected to trigger onSuccess or onFailure of the {@code ActionListener}. + * + * @param indexMetadataList list of index metadata of new indexes (or first time index metadata upload). + * @param actionListener listener to be invoked on success or failure. + */ + void interceptIndexCreation(List indexMetadataList, ActionListener actionListener) throws IOException; +} diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java index c7790ab965ca1..dbd2754cd3a1d 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -27,6 +27,7 @@ import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.io.IOUtils; import org.opensearch.core.action.ActionListener; +import org.opensearch.core.common.util.CollectionUtils; import org.opensearch.core.index.Index; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.gateway.remote.ClusterMetadataManifest.UploadedIndexMetadata; @@ -160,7 +161,7 @@ public class RemoteClusterStateService implements Closeable { private final Settings settings; private final LongSupplier relativeTimeNanosSupplier; private final ThreadPool threadpool; - private final IndexCreationPreIndexMetadataUploadListener indexCreationListener; + private final List indexMetadataUploadInterceptors; private BlobStoreRepository blobStoreRepository; private BlobStoreTransferService blobStoreTransferService; private volatile TimeValue slowWriteLoggingThreshold; @@ -191,7 +192,7 @@ public RemoteClusterStateService( ClusterSettings clusterSettings, LongSupplier relativeTimeNanosSupplier, ThreadPool threadPool, - IndexCreationPreIndexMetadataUploadListener indexCreationListener + List indexMetadataUploadInterceptors ) { assert isRemoteStoreClusterStateEnabled(settings) : "Remote cluster state is not enabled"; this.nodeId = nodeId; @@ -208,7 +209,7 @@ public RemoteClusterStateService( clusterSettings.addSettingsUpdateConsumer(GLOBAL_METADATA_UPLOAD_TIMEOUT_SETTING, this::setGlobalMetadataUploadTimeout); clusterSettings.addSettingsUpdateConsumer(METADATA_MANIFEST_UPLOAD_TIMEOUT_SETTING, this::setMetadataManifestUploadTimeout); this.remoteStateStats = new RemotePersistenceStats(); - this.indexCreationListener = indexCreationListener; + this.indexMetadataUploadInterceptors = indexMetadataUploadInterceptors; } private BlobStoreTransferService getBlobStoreTransferService() { @@ -236,11 +237,12 @@ public ClusterMetadataManifest writeFullMetadata(ClusterState clusterState, Stri // Write globalMetadata String globalMetadataFile = writeGlobalMetadata(clusterState); + List toUpload = new ArrayList<>(clusterState.metadata().indices().values()); // any validations before/after upload ? final List allUploadedIndexMetadata = writeIndexMetadataParallel( clusterState, - new ArrayList<>(clusterState.metadata().indices().values()), - previousClusterUUID + toUpload, + ClusterState.UNKNOWN_UUID.equals(previousClusterUUID) ? toUpload : Collections.emptyList() ); final ClusterMetadataManifest manifest = uploadManifest( clusterState, @@ -335,11 +337,11 @@ public ClusterMetadataManifest writeIncrementalMetadata( previousStateIndexMetadataVersionByName.remove(indexMetadata.getIndex().getName()); } - List uploadedIndexMetadataList = writeIndexMetadataParallel( - clusterState, - toUpload, - indexNamePreviousVersionMap - ); + List newIndexMetadataList = toUpload.stream() + /* If the previous state's index metadata version is null, then this is index creation */ + .filter(indexMetadata -> Objects.isNull(indexNamePreviousVersionMap.get(indexMetadata.getIndex().getName()))) + .collect(Collectors.toList()); + List uploadedIndexMetadataList = writeIndexMetadataParallel(clusterState, toUpload, newIndexMetadataList); uploadedIndexMetadataList.forEach( uploadedIndexMetadata -> allUploadedIndexMetadata.put(uploadedIndexMetadata.getIndexName(), uploadedIndexMetadata) ); @@ -452,13 +454,50 @@ private List writeIndexMetadataParallel( List toUpload, List newIndexMetadataList ) throws IOException { - assert Objects.nonNull(indexCreationListener) : "indexCreationListener can not be null"; - int latchCount = toUpload.size() + indexCreationListener.latchCount(newIndexMetadataList); + assert CollectionUtils.isEmpty(indexMetadataUploadInterceptors) == false : "indexMetadataUploadInterceptors can not be empty"; + int latchCount = toUpload.size() + indexMetadataUploadInterceptors.size(); List exceptionList = Collections.synchronizedList(new ArrayList<>(latchCount)); final CountDownLatch latch = new CountDownLatch(latchCount); List result = new ArrayList<>(toUpload.size()); - uploadIndexMetadataAsync(clusterState, result, toUpload, latch, exceptionList); - indexCreationListener.run(newIndexMetadataList, latch, exceptionList); + + LatchedActionListener latchedActionListener = new LatchedActionListener<>( + ActionListener.wrap((UploadedIndexMetadata uploadedIndexMetadata) -> { + logger.trace( + String.format(Locale.ROOT, "IndexMetadata uploaded successfully for %s", uploadedIndexMetadata.getIndexName()) + ); + result.add(uploadedIndexMetadata); + }, ex -> { + assert ex instanceof RemoteStateTransferException; + logger.error( + () -> new ParameterizedMessage("Exception during transfer of IndexMetadata to Remote {}", ex.getMessage()), + ex + ); + exceptionList.add(ex); + }), + latch + ); + + for (IndexMetadata indexMetadata : toUpload) { + // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/index/ftqsCnn9TgOX/metadata_4_1690947200 + writeIndexMetadataAsync(clusterState, indexMetadata, latchedActionListener); + } + + for (IndexMetadataUploadInterceptor interceptor : indexMetadataUploadInterceptors) { + // We are submitting the task for async execution to ensure that we are not blocking the cluster state upload + String interceptorName = interceptor.getClass().getSimpleName(); + threadpool.executor(ThreadPool.Names.GENERIC).execute(() -> { + try { + interceptor.interceptIndexCreation( + newIndexMetadataList, + getIndexMetadataUploadInterceptorListener(newIndexMetadataList, latch, exceptionList, interceptorName) + ); + } catch (IOException e) { + exceptionList.add( + new RemoteStateTransferException("Exception occurred while running interceptIndexCreation in " + interceptorName, e) + ); + } + }); + } try { if (latch.await(getIndexMetadataUploadTimeout().millis(), TimeUnit.MILLISECONDS) == false) { @@ -499,58 +538,27 @@ private List writeIndexMetadataParallel( return result; } - private void uploadIndexMetadataAsync( - ClusterState clusterState, - List result, - List toUpload, + private ActionListener getIndexMetadataUploadInterceptorListener( + List newIndexMetadataList, CountDownLatch latch, - List exceptionList - ) throws IOException { - LatchedActionListener indexMetadataLatchedActionListener = new LatchedActionListener<>( - ActionListener.wrap((UploadedIndexMetadata uploadedIndexMetadata) -> { - logger.trace( - String.format(Locale.ROOT, "IndexMetadata uploaded successfully for %s", uploadedIndexMetadata.getIndexName()) - ); - result.add(uploadedIndexMetadata); - }, ex -> { - assert ex instanceof RemoteStateTransferException; - logger.error( - () -> new ParameterizedMessage("Exception during transfer of IndexMetadata to Remote {}", ex.getMessage()), - ex - ); - exceptionList.add(ex); - }), + List exceptionList, + String interceptorName + ) { + return new LatchedActionListener<>( + ActionListener.wrap( + ignored -> logger.trace( + new ParameterizedMessage("{} : Intercepted {} successfully", interceptorName, newIndexMetadataList) + ), + ex -> { + logger.error( + new ParameterizedMessage("{} : Exception during interception of {}", interceptorName, newIndexMetadataList), + ex + ); + exceptionList.add(ex); + } + ), latch ); - - for (IndexMetadata indexMetadata : toUpload) { - // 123456789012_test-cluster/cluster-state/dsgYj10Nkso7/index/ftqsCnn9TgOX/metadata_4_1690947200 - writeIndexMetadataAsync(clusterState, indexMetadata, indexMetadataLatchedActionListener); - } - } - - private List writeIndexMetadataParallel( - ClusterState clusterState, - List toUpload, - String previousClusterUUID - ) throws IOException { - List newIndexMetadaList = Collections.emptyList(); - if (ClusterState.UNKNOWN_UUID.equals(previousClusterUUID)) { - newIndexMetadaList = toUpload; - } - return writeIndexMetadataParallel(clusterState, toUpload, newIndexMetadaList); - } - - private List writeIndexMetadataParallel( - ClusterState clusterState, - List toUpload, - Map indexNamePreviousVersionMap - ) throws IOException { - List newIndexMetadataList = toUpload.stream() - /* If the previous state's index metadata version is null, then this is index creation */ - .filter(indexMetadata -> Objects.isNull(indexNamePreviousVersionMap.get(indexMetadata.getIndex().getName()))) - .collect(Collectors.toList()); - return writeIndexMetadataParallel(clusterState, toUpload, newIndexMetadataList); } /** @@ -1177,7 +1185,7 @@ public void writeMetadataFailed() { /** * Exception for Remote state transfer. */ - static class RemoteStateTransferException extends RuntimeException { + public static class RemoteStateTransferException extends RuntimeException { public RemoteStateTransferException(String errorDesc) { super(errorDesc); diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteUploadPathIndexCreationListener.java b/server/src/main/java/org/opensearch/index/remote/RemoteIndexPathUploader.java similarity index 72% rename from server/src/main/java/org/opensearch/index/remote/RemoteUploadPathIndexCreationListener.java rename to server/src/main/java/org/opensearch/index/remote/RemoteIndexPathUploader.java index 29868b1e0ecc5..2d0fc8406f783 100644 --- a/server/src/main/java/org/opensearch/index/remote/RemoteUploadPathIndexCreationListener.java +++ b/server/src/main/java/org/opensearch/index/remote/RemoteIndexPathUploader.java @@ -15,11 +15,14 @@ import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.common.blobstore.BlobContainer; import org.opensearch.common.blobstore.BlobPath; +import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; -import org.opensearch.common.xcontent.XContentType; +import org.opensearch.common.unit.TimeValue; import org.opensearch.core.action.ActionListener; -import org.opensearch.gateway.remote.IndexCreationPreIndexMetadataUploadListener; +import org.opensearch.core.index.Index; +import org.opensearch.gateway.remote.IndexMetadataUploadInterceptor; import org.opensearch.gateway.remote.RemoteClusterStateService; +import org.opensearch.gateway.remote.RemoteClusterStateService.RemoteStateTransferException; import org.opensearch.node.Node; import org.opensearch.node.remotestore.RemoteStoreNodeAttribute; import org.opensearch.repositories.RepositoriesService; @@ -28,14 +31,19 @@ import org.opensearch.repositories.blobstore.ChecksumBlobStoreFormat; import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Objects; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import java.util.function.Supplier; import java.util.stream.Collectors; +import static org.opensearch.gateway.remote.RemoteClusterStateService.INDEX_METADATA_UPLOAD_TIMEOUT_SETTING; import static org.opensearch.index.remote.RemoteIndexPath.SEGMENT_PATH; import static org.opensearch.index.remote.RemoteIndexPath.TRANSLOG_PATH; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteDataAttributePresent; @@ -45,7 +53,7 @@ * Uploads the remote store path for all possible combinations of {@link org.opensearch.index.remote.RemoteStoreEnums.DataCategory} * and {@link org.opensearch.index.remote.RemoteStoreEnums.DataType} for each shard of an index. */ -public class RemoteUploadPathIndexCreationListener implements IndexCreationPreIndexMetadataUploadListener { +public class RemoteIndexPathUploader implements IndexMetadataUploadInterceptor { public static final ChecksumBlobStoreFormat REMOTE_INDEX_PATH_FORMAT = new ChecksumBlobStoreFormat<>( "remote-index-path", @@ -53,51 +61,71 @@ public class RemoteUploadPathIndexCreationListener implements IndexCreationPreIn RemoteIndexPath::fromXContent ); - private static final Logger logger = LogManager.getLogger(RemoteUploadPathIndexCreationListener.class); + private static final String TIMEOUT_EXCEPTION_MSG = "Timed out waiting while uploading remote index path file for indexes=%s"; + private static final String UPLOAD_EXCEPTION_MSG = "Exception occurred while uploading remote index paths for indexes=%s"; + + private static final Logger logger = LogManager.getLogger(RemoteIndexPathUploader.class); private final Settings settings; private final boolean isRemoteDataAttributePresent; private final boolean isTranslogSegmentRepoSame; private final Supplier repositoriesService; + private volatile TimeValue indexMetadataUploadTimeout; private BlobStoreRepository translogRepository; private BlobStoreRepository segmentRepository; - public RemoteUploadPathIndexCreationListener(Settings settings, Supplier repositoriesService) { + public RemoteIndexPathUploader(Settings settings, Supplier repositoriesService, ClusterSettings clusterSettings) { this.settings = settings; this.repositoriesService = repositoriesService; isRemoteDataAttributePresent = isRemoteDataAttributePresent(settings); // If the remote data attributes are not present, then there is no effect of translog and segment being same or different or null. isTranslogSegmentRepoSame = isTranslogSegmentRepoSame(); + indexMetadataUploadTimeout = clusterSettings.get(INDEX_METADATA_UPLOAD_TIMEOUT_SETTING); + clusterSettings.addSettingsUpdateConsumer(INDEX_METADATA_UPLOAD_TIMEOUT_SETTING, this::setIndexMetadataUploadTimeout); } @Override - public int latchCount(List newIndexMetadataList) { + public void interceptIndexCreation(List indexMetadataList, ActionListener actionListener) throws IOException { if (isRemoteDataAttributePresent == false) { - return 0; + actionListener.onResponse(null); + return; } - int eligibleIndexCount = (int) newIndexMetadataList.stream().filter(this::uploadIndexPathFile).count(); - return isTranslogSegmentRepoSame ? eligibleIndexCount : 2 * eligibleIndexCount; - } - @Override - public void run(List newIndexMetadataList, CountDownLatch latch, List exceptionList) throws IOException { - if (isRemoteDataAttributePresent == false) { - return; + List eligibleList = indexMetadataList.stream().filter(this::requiresPathUpload).collect(Collectors.toList()); + int latchCount = eligibleList.size() * (isTranslogSegmentRepoSame ? 1 : 2); + CountDownLatch latch = new CountDownLatch(latchCount); + List exceptionList = Collections.synchronizedList(new ArrayList<>(latchCount)); + for (IndexMetadata indexMetadata : eligibleList) { + writeIndexPathAsync(indexMetadata, latch, exceptionList); } - List elibibleIndexMetadaList = newIndexMetadataList.stream() - .filter(this::uploadIndexPathFile) - .collect(Collectors.toList()); - if (isTranslogSegmentRepoSame) { - assert latchCount(newIndexMetadataList) == elibibleIndexMetadaList.size() - : "Latch count is not equal to elibibleIndexMetadaList's size for path upload"; - } else { - assert latchCount(newIndexMetadataList) == 2 * elibibleIndexMetadaList.size() - : "Latch count is not equal to (2 * elibibleIndexMetadaList's size) for path upload"; + String indexNames = eligibleList.stream().map(IndexMetadata::getIndex).map(Index::toString).collect(Collectors.joining(",")); + + try { + if (latch.await(indexMetadataUploadTimeout.millis(), TimeUnit.MILLISECONDS) == false) { + RemoteStateTransferException ex = new RemoteStateTransferException( + String.format(Locale.ROOT, TIMEOUT_EXCEPTION_MSG, indexNames) + ); + exceptionList.forEach(ex::addSuppressed); + actionListener.onFailure(ex); + return; + } + } catch (InterruptedException exception) { + exceptionList.forEach(exception::addSuppressed); + RemoteStateTransferException ex = new RemoteStateTransferException( + String.format(Locale.ROOT, TIMEOUT_EXCEPTION_MSG, indexNames), + exception + ); + actionListener.onFailure(ex); } - for (IndexMetadata indexMetadata : elibibleIndexMetadaList) { - writeIndexPathAsync(indexMetadata, latch, exceptionList); + if (exceptionList.size() > 0) { + RemoteStateTransferException ex = new RemoteStateTransferException( + String.format(Locale.ROOT, UPLOAD_EXCEPTION_MSG, indexNames) + ); + exceptionList.forEach(ex::addSuppressed); + actionListener.onFailure(ex); } + actionListener.onResponse(null); } private void writeIndexPathAsync(IndexMetadata idxMD, CountDownLatch latch, List exceptionList) throws IOException { @@ -122,9 +150,7 @@ private void writeIndexPathAsync(IndexMetadata idxMD, CountDownLatch latch, List indexUUID, translogRepository.getCompressor(), getUploadPathLatchedActionListener(idxMD, latch, exceptionList, pathCreationMap), - RemoteClusterStateService.FORMAT_PARAMS, - true, - XContentType.JSON + RemoteClusterStateService.FORMAT_PARAMS ); } else { // If the repositories are different, then we need to upload one file per segment and translog containing their individual @@ -135,9 +161,7 @@ private void writeIndexPathAsync(IndexMetadata idxMD, CountDownLatch latch, List indexUUID, translogRepository.getCompressor(), getUploadPathLatchedActionListener(idxMD, latch, exceptionList, TRANSLOG_PATH), - RemoteClusterStateService.FORMAT_PARAMS, - true, - XContentType.JSON + RemoteClusterStateService.FORMAT_PARAMS ); BlobPath segmentBasePath = segmentRepository.basePath(); @@ -148,9 +172,7 @@ private void writeIndexPathAsync(IndexMetadata idxMD, CountDownLatch latch, List indexUUID, segmentRepository.getCompressor(), getUploadPathLatchedActionListener(idxMD, latch, exceptionList, SEGMENT_PATH), - RemoteClusterStateService.FORMAT_PARAMS, - true, - XContentType.JSON + RemoteClusterStateService.FORMAT_PARAMS ); } } @@ -218,7 +240,7 @@ private LatchedActionListener getUploadPathLatchedActionListener( * This method checks if the index metadata has attributes that calls for uploading the index path for remote store * uploads. It checks if the remote store path type is {@code HASHED_PREFIX} and returns true if so. */ - private boolean uploadIndexPathFile(IndexMetadata indexMetadata) { + private boolean requiresPathUpload(IndexMetadata indexMetadata) { // A cluster will have remote custom metadata only if the cluster is remote store enabled from data side. Map remoteCustomData = indexMetadata.getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY); if (Objects.isNull(remoteCustomData) || remoteCustomData.isEmpty()) { @@ -231,4 +253,8 @@ private boolean uploadIndexPathFile(IndexMetadata indexMetadata) { // We need to upload the path only if the path type for an index is hashed_prefix return RemoteStoreEnums.PathType.HASHED_PREFIX == RemoteStoreEnums.PathType.parseString(pathTypeStr); } + + private void setIndexMetadataUploadTimeout(TimeValue newIndexMetadataUploadTimeout) { + this.indexMetadataUploadTimeout = newIndexMetadataUploadTimeout; + } } diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index ef81575d63336..70357a707d6c0 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -146,8 +146,8 @@ import org.opensearch.index.analysis.AnalysisRegistry; import org.opensearch.index.engine.EngineFactory; import org.opensearch.index.recovery.RemoteStoreRestoreService; +import org.opensearch.index.remote.RemoteIndexPathUploader; import org.opensearch.index.remote.RemoteStoreStatsTrackerFactory; -import org.opensearch.index.remote.RemoteUploadPathIndexCreationListener; import org.opensearch.index.store.RemoteSegmentStoreDirectoryFactory; import org.opensearch.index.store.remote.filecache.FileCache; import org.opensearch.index.store.remote.filecache.FileCacheCleaner; @@ -727,9 +727,13 @@ protected Node( threadPool::relativeTimeInMillis ); final RemoteClusterStateService remoteClusterStateService; - final RemoteUploadPathIndexCreationListener indexCreationListener; + final RemoteIndexPathUploader remoteIndexPathUploader; if (isRemoteStoreClusterStateEnabled(settings)) { - indexCreationListener = new RemoteUploadPathIndexCreationListener(settings, repositoriesServiceReference::get); + remoteIndexPathUploader = new RemoteIndexPathUploader( + settings, + repositoriesServiceReference::get, + clusterService.getClusterSettings() + ); remoteClusterStateService = new RemoteClusterStateService( nodeEnvironment.nodeId(), repositoriesServiceReference::get, @@ -737,11 +741,11 @@ protected Node( clusterService.getClusterSettings(), threadPool::preciseRelativeTimeInNanos, threadPool, - indexCreationListener + List.of(remoteIndexPathUploader) ); } else { remoteClusterStateService = null; - indexCreationListener = null; + remoteIndexPathUploader = null; } // collect engine factory providers from plugins @@ -1318,7 +1322,7 @@ protected Node( b.bind(SearchRequestSlowLog.class).toInstance(searchRequestSlowLog); b.bind(MetricsRegistry.class).toInstance(metricsRegistry); b.bind(RemoteClusterStateService.class).toProvider(() -> remoteClusterStateService); - b.bind(RemoteUploadPathIndexCreationListener.class).toProvider(() -> indexCreationListener); + b.bind(RemoteIndexPathUploader.class).toProvider(() -> remoteIndexPathUploader); b.bind(PersistedStateRegistry.class).toInstance(persistedStateRegistry); b.bind(SegmentReplicationStatsTracker.class).toInstance(segmentReplicationStatsTracker); b.bind(SearchRequestOperationsCompositeListenerFactory.class).toInstance(searchRequestOperationsCompositeListenerFactory); @@ -1468,9 +1472,7 @@ public Node start() throws NodeValidationException { if (remoteClusterStateService != null) { remoteClusterStateService.start(); } - final RemoteUploadPathIndexCreationListener indexCreationListener = injector.getInstance( - RemoteUploadPathIndexCreationListener.class - ); + final RemoteIndexPathUploader indexCreationListener = injector.getInstance(RemoteIndexPathUploader.class); if (indexCreationListener != null) { indexCreationListener.start(); } diff --git a/server/src/main/java/org/opensearch/repositories/blobstore/ChecksumBlobStoreFormat.java b/server/src/main/java/org/opensearch/repositories/blobstore/ChecksumBlobStoreFormat.java index 1be096dd92577..3e6052a5ef820 100644 --- a/server/src/main/java/org/opensearch/repositories/blobstore/ChecksumBlobStoreFormat.java +++ b/server/src/main/java/org/opensearch/repositories/blobstore/ChecksumBlobStoreFormat.java @@ -57,7 +57,6 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.core.compress.Compressor; -import org.opensearch.core.xcontent.MediaType; import org.opensearch.core.xcontent.MediaTypeRegistry; import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.core.xcontent.ToXContent; @@ -191,32 +190,9 @@ public void write( final String name, final Compressor compressor, final ToXContent.Params params - ) throws IOException { - write(obj, blobContainer, name, compressor, params, false, XContentType.SMILE); - } - - /** - * Writes blob with resolving the blob name using {@link #blobName} method. - *

- * The blob will optionally by compressed. - * - * @param obj object to be serialized - * @param blobContainer blob container - * @param name blob name - * @param compressor whether to use compression - * @param params ToXContent params - */ - public void write( - final T obj, - final BlobContainer blobContainer, - final String name, - final Compressor compressor, - final ToXContent.Params params, - boolean skipHeaderFooter, - MediaType mediaType ) throws IOException { final String blobName = blobName(name); - final BytesReference bytes = serialize(obj, blobName, compressor, params, skipHeaderFooter, mediaType); + final BytesReference bytes = serialize(obj, blobName, compressor, params); blobContainer.writeBlob(blobName, bytes.streamInput(), bytes.length(), false); } @@ -232,17 +208,7 @@ public void writeAsync( final ToXContent.Params params ) throws IOException { // use NORMAL priority by default - this.writeAsyncWithPriority( - obj, - blobContainer, - name, - compressor, - WritePriority.NORMAL, - listener, - params, - false, - XContentType.SMILE - ); + this.writeAsyncWithPriority(obj, blobContainer, name, compressor, WritePriority.NORMAL, listener, params); } /** @@ -260,30 +226,7 @@ public void writeAsyncWithUrgentPriority( ActionListener listener, final ToXContent.Params params ) throws IOException { - this.writeAsyncWithPriority( - obj, - blobContainer, - name, - compressor, - WritePriority.URGENT, - listener, - params, - false, - XContentType.SMILE - ); - } - - public void writeAsyncWithUrgentPriority( - final T obj, - final BlobContainer blobContainer, - final String name, - final Compressor compressor, - ActionListener listener, - final ToXContent.Params params, - boolean skipHeaderFooter, - MediaType type - ) throws IOException { - this.writeAsyncWithPriority(obj, blobContainer, name, compressor, WritePriority.URGENT, listener, params, skipHeaderFooter, type); + this.writeAsyncWithPriority(obj, blobContainer, name, compressor, WritePriority.URGENT, listener, params); } /** @@ -305,17 +248,15 @@ private void writeAsyncWithPriority( final Compressor compressor, final WritePriority priority, ActionListener listener, - final ToXContent.Params params, - boolean skipHeaderFooter, - MediaType mediaType + final ToXContent.Params params ) throws IOException { if (blobContainer instanceof AsyncMultiStreamBlobContainer == false) { - write(obj, blobContainer, name, compressor, params, skipHeaderFooter, mediaType); + write(obj, blobContainer, name, compressor, params); listener.onResponse(null); return; } final String blobName = blobName(name); - final BytesReference bytes = serialize(obj, blobName, compressor, params, skipHeaderFooter, mediaType); + final BytesReference bytes = serialize(obj, blobName, compressor, params); final String resourceDescription = "ChecksumBlobStoreFormat.writeAsyncWithPriority(blob=\"" + blobName + "\")"; try (IndexInput input = new ByteArrayIndexInput(resourceDescription, BytesReference.toBytes(bytes))) { long expectedChecksum; @@ -349,17 +290,6 @@ private void writeAsyncWithPriority( public BytesReference serialize(final T obj, final String blobName, final Compressor compressor, final ToXContent.Params params) throws IOException { - return serialize(obj, blobName, compressor, params, false, XContentType.SMILE); - } - - public BytesReference serialize( - final T obj, - final String blobName, - final Compressor compressor, - final ToXContent.Params params, - boolean skipHeaderFooter, - MediaType type - ) throws IOException { try (BytesStreamOutput outputStream = new BytesStreamOutput()) { try ( OutputStreamIndexOutput indexOutput = new OutputStreamIndexOutput( @@ -369,9 +299,7 @@ public BytesReference serialize( BUFFER_SIZE ) ) { - if (skipHeaderFooter == false) { - CodecUtil.writeHeader(indexOutput, codec, VERSION); - } + CodecUtil.writeHeader(indexOutput, codec, VERSION); try (OutputStream indexOutputOutputStream = new IndexOutputOutputStream(indexOutput) { @Override public void close() throws IOException { @@ -380,7 +308,7 @@ public void close() throws IOException { } }; XContentBuilder builder = MediaTypeRegistry.contentBuilder( - type, + XContentType.SMILE, compressor.threadLocalOutputStream(indexOutputOutputStream) ) ) { @@ -388,9 +316,7 @@ public void close() throws IOException { obj.toXContent(builder, params); builder.endObject(); } - if (skipHeaderFooter == false) { - CodecUtil.writeFooter(indexOutput); - } + CodecUtil.writeFooter(indexOutput); } return outputStream.bytes(); } diff --git a/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java b/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java index 6a831f636df6f..012d2623b45df 100644 --- a/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java +++ b/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java @@ -71,7 +71,7 @@ import org.opensearch.gateway.remote.RemotePersistenceStats; import org.opensearch.index.recovery.RemoteStoreRestoreService; import org.opensearch.index.recovery.RemoteStoreRestoreService.RemoteRestoreResult; -import org.opensearch.index.remote.RemoteUploadPathIndexCreationListener; +import org.opensearch.index.remote.RemoteIndexPathUploader; import org.opensearch.node.Node; import org.opensearch.repositories.RepositoriesService; import org.opensearch.repositories.fs.FsRepository; @@ -482,14 +482,15 @@ public void testDataOnlyNodePersistence() throws Exception { Collections.emptyMap(), transportService.getThreadPool() ); + ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); return new RemoteClusterStateService( nodeEnvironment.nodeId(), repositoriesServiceSupplier, settings, - new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), + clusterSettings, () -> 0L, threadPool, - new RemoteUploadPathIndexCreationListener(settings, repositoriesServiceSupplier) + List.of(new RemoteIndexPathUploader(settings, repositoriesServiceSupplier, clusterSettings)) ); } else { return null; diff --git a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java index e760b915ed260..9ddbce3f794e6 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -38,8 +38,8 @@ import org.opensearch.core.index.Index; import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.gateway.remote.ClusterMetadataManifest.UploadedIndexMetadata; +import org.opensearch.index.remote.RemoteIndexPathUploader; import org.opensearch.index.remote.RemoteStoreUtils; -import org.opensearch.index.remote.RemoteUploadPathIndexCreationListener; import org.opensearch.indices.IndicesModule; import org.opensearch.repositories.FilterRepository; import org.opensearch.repositories.RepositoriesService; @@ -156,7 +156,7 @@ public void setup() { clusterSettings, () -> 0L, threadPool, - new RemoteUploadPathIndexCreationListener(settings, repositoriesServiceSupplier) + List.of(new RemoteIndexPathUploader(settings, repositoriesServiceSupplier, clusterSettings)) ); } @@ -175,16 +175,17 @@ public void testFailWriteFullMetadataNonClusterManagerNode() throws IOException public void testFailInitializationWhenRemoteStateDisabled() { final Settings settings = Settings.builder().build(); + ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); assertThrows( AssertionError.class, () -> new RemoteClusterStateService( "test-node-id", repositoriesServiceSupplier, settings, - new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), + clusterSettings, () -> 0L, threadPool, - new RemoteUploadPathIndexCreationListener(settings, repositoriesServiceSupplier) + List.of(new RemoteIndexPathUploader(settings, repositoriesServiceSupplier, clusterSettings)) ) ); } From 44fcf9c392cf4a6a8e9690eb429e6a8aaf0ce81b Mon Sep 17 00:00:00 2001 From: Ashish Singh Date: Wed, 17 Apr 2024 00:25:39 +0530 Subject: [PATCH 06/15] Add more UTs Signed-off-by: Ashish Singh --- .../index/remote/RemoteIndexPath.java | 2 +- .../index/remote/RemoteIndexPathUploader.java | 27 +- .../index/remote/RemoteIndexPathTests.java | 45 ++- .../remote/RemoteIndexPathUploaderTests.java | 271 ++++++++++++++++++ 4 files changed, 322 insertions(+), 23 deletions(-) create mode 100644 server/src/test/java/org/opensearch/index/remote/RemoteIndexPathUploaderTests.java diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteIndexPath.java b/server/src/main/java/org/opensearch/index/remote/RemoteIndexPath.java index 83448df427c84..b44cb1e8ec16f 100644 --- a/server/src/main/java/org/opensearch/index/remote/RemoteIndexPath.java +++ b/server/src/main/java/org/opensearch/index/remote/RemoteIndexPath.java @@ -132,7 +132,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws return builder; } - public static RemoteIndexPath fromXContent(XContentParser ignored) throws IOException { + public static RemoteIndexPath fromXContent(XContentParser ignored) { throw new UnsupportedOperationException("RemoteIndexPath.fromXContent() is not supported"); } } diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteIndexPathUploader.java b/server/src/main/java/org/opensearch/index/remote/RemoteIndexPathUploader.java index 2d0fc8406f783..4687690fcd685 100644 --- a/server/src/main/java/org/opensearch/index/remote/RemoteIndexPathUploader.java +++ b/server/src/main/java/org/opensearch/index/remote/RemoteIndexPathUploader.java @@ -63,6 +63,10 @@ public class RemoteIndexPathUploader implements IndexMetadataUploadInterceptor { private static final String TIMEOUT_EXCEPTION_MSG = "Timed out waiting while uploading remote index path file for indexes=%s"; private static final String UPLOAD_EXCEPTION_MSG = "Exception occurred while uploading remote index paths for indexes=%s"; + static final String TRANSLOG_REPO_NAME_KEY = Node.NODE_ATTRIBUTES.getKey() + + RemoteStoreNodeAttribute.REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY; + static final String SEGMENT_REPO_NAME_KEY = Node.NODE_ATTRIBUTES.getKey() + + RemoteStoreNodeAttribute.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY; private static final Logger logger = LogManager.getLogger(RemoteIndexPathUploader.class); @@ -76,11 +80,12 @@ public class RemoteIndexPathUploader implements IndexMetadataUploadInterceptor { private BlobStoreRepository segmentRepository; public RemoteIndexPathUploader(Settings settings, Supplier repositoriesService, ClusterSettings clusterSettings) { - this.settings = settings; - this.repositoriesService = repositoriesService; + this.settings = Objects.requireNonNull(settings); + this.repositoriesService = Objects.requireNonNull(repositoriesService); isRemoteDataAttributePresent = isRemoteDataAttributePresent(settings); // If the remote data attributes are not present, then there is no effect of translog and segment being same or different or null. isTranslogSegmentRepoSame = isTranslogSegmentRepoSame(); + Objects.requireNonNull(clusterSettings); indexMetadataUploadTimeout = clusterSettings.get(INDEX_METADATA_UPLOAD_TIMEOUT_SETTING); clusterSettings.addSettingsUpdateConsumer(INDEX_METADATA_UPLOAD_TIMEOUT_SETTING, this::setIndexMetadataUploadTimeout); } @@ -117,6 +122,7 @@ public void interceptIndexCreation(List indexMetadataList, Action exception ); actionListener.onFailure(ex); + return; } if (exceptionList.size() > 0) { RemoteStateTransferException ex = new RemoteStateTransferException( @@ -124,6 +130,7 @@ public void interceptIndexCreation(List indexMetadataList, Action ); exceptionList.forEach(ex::addSuppressed); actionListener.onFailure(ex); + return; } actionListener.onResponse(null); } @@ -191,21 +198,13 @@ public void start() { // If remote store data attributes are not present than we skip this. return; } - translogRepository = (BlobStoreRepository) validateAndGetRepository( - Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY - ); - segmentRepository = (BlobStoreRepository) validateAndGetRepository( - Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY - ); + translogRepository = (BlobStoreRepository) validateAndGetRepository(TRANSLOG_REPO_NAME_KEY); + segmentRepository = (BlobStoreRepository) validateAndGetRepository(SEGMENT_REPO_NAME_KEY); } private boolean isTranslogSegmentRepoSame() { - String translogRepoName = settings.get( - Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY - ); - String segmentRepoName = settings.get( - Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY - ); + String translogRepoName = settings.get(TRANSLOG_REPO_NAME_KEY); + String segmentRepoName = settings.get(SEGMENT_REPO_NAME_KEY); return Objects.equals(translogRepoName, segmentRepoName); } diff --git a/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathTests.java b/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathTests.java index 1ce05f204e5d0..c1072ebc00e54 100644 --- a/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathTests.java +++ b/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathTests.java @@ -12,6 +12,7 @@ import org.opensearch.core.xcontent.MediaTypeRegistry; 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; @@ -22,6 +23,11 @@ import java.util.Map; import java.util.TreeMap; +import org.mockito.Mockito; + +import static org.opensearch.index.remote.RemoteStoreEnums.DataCategory.TRANSLOG; +import static org.opensearch.index.remote.RemoteStoreEnums.DataType.LOCK_FILES; + public class RemoteIndexPathTests extends OpenSearchTestCase { /** @@ -33,7 +39,7 @@ public void testToXContentWithSegmentRepo() throws IOException { 2, new BlobPath().add("djsd878ndjh").add("hcs87cj8"), PathType.HASHED_PREFIX, - PathHashAlgorithm.FNV_1A, + PathHashAlgorithm.FNV_1A_BASE64, RemoteIndexPath.SEGMENT_PATH ); XContentBuilder xContentBuilder = MediaTypeRegistry.contentBuilder(MediaTypeRegistry.JSON); @@ -41,7 +47,7 @@ public void testToXContentWithSegmentRepo() throws IOException { xContentBuilder = indexPath.toXContent(xContentBuilder, ToXContent.EMPTY_PARAMS); xContentBuilder.endObject(); String expected = "{\"version\":\"1\",\"index_uuid\":\"djjsid73he8yd7usduh\",\"shard_count\":2," - + "\"path_type\":\"HASHED_PREFIX\",\"path_hash_algorithm\":\"FNV_1A\",\"paths\":[" + + "\"path_type\":\"HASHED_PREFIX\",\"path_hash_algorithm\":\"FNV_1A_BASE64\",\"paths\":[" + "\"9BmBinD5HYs/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/0/segments/data/\"," + "\"ExCNOD8_5ew/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/1/segments/data/\"," + "\"z8wtf0yr2l4/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/0/segments/metadata/\"," @@ -60,7 +66,7 @@ public void testToXContentForTranslogRepoOnly() throws IOException { 2, new BlobPath().add("djsd878ndjh").add("hcs87cj8"), PathType.HASHED_PREFIX, - PathHashAlgorithm.FNV_1A, + PathHashAlgorithm.FNV_1A_BASE64, RemoteIndexPath.TRANSLOG_PATH ); XContentBuilder xContentBuilder = MediaTypeRegistry.contentBuilder(MediaTypeRegistry.JSON); @@ -68,7 +74,7 @@ public void testToXContentForTranslogRepoOnly() throws IOException { xContentBuilder = indexPath.toXContent(xContentBuilder, ToXContent.EMPTY_PARAMS); xContentBuilder.endObject(); String expected = "{\"version\":\"1\",\"index_uuid\":\"djjsid73he8yd7usduh\",\"shard_count\":2," - + "\"path_type\":\"HASHED_PREFIX\",\"path_hash_algorithm\":\"FNV_1A\",\"paths\":[" + + "\"path_type\":\"HASHED_PREFIX\",\"path_hash_algorithm\":\"FNV_1A_BASE64\",\"paths\":[" + "\"2EaVODaKBck/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/0/translog/data/\"," + "\"dTS2VqEOUNo/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/1/translog/data/\"," + "\"PVNKNGonmZw/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/0/translog/metadata/\"," @@ -88,7 +94,7 @@ public void testToXContentForBothRepos() throws IOException { 3, new BlobPath().add("nxf9yv0").add("c3ejoi"), PathType.HASHED_PREFIX, - PathHashAlgorithm.FNV_1A, + PathHashAlgorithm.FNV_1A_BASE64, pathCreationMap ); XContentBuilder xContentBuilder = MediaTypeRegistry.contentBuilder(MediaTypeRegistry.JSON); @@ -96,7 +102,7 @@ public void testToXContentForBothRepos() throws IOException { xContentBuilder = indexPath.toXContent(xContentBuilder, ToXContent.EMPTY_PARAMS); xContentBuilder.endObject(); String expected = "{\"version\":\"1\",\"index_uuid\":\"csbdqiu8a7sdnjdks\",\"shard_count\":3,\"path_type\":" - + "\"HASHED_PREFIX\",\"path_hash_algorithm\":\"FNV_1A\",\"paths\":[" + + "\"HASHED_PREFIX\",\"path_hash_algorithm\":\"FNV_1A_BASE64\",\"paths\":[" + "\"Cjo0F6kNjYk/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/0/segments/data/\"," + "\"kpayyhxct1I/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/1/segments/data/\"," + "\"p2RlgnHeIgc/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/2/segments/data/\"," @@ -123,14 +129,37 @@ public void testRemoteIndexPathWithInvalidPathCreationMap() throws IOException { 2, new BlobPath().add("djsd878ndjh").add("hcs87cj8"), PathType.HASHED_PREFIX, - PathHashAlgorithm.FNV_1A, + PathHashAlgorithm.FNV_1A_BASE64, new HashMap<>() ) ); assertEquals( "Invalid input in RemoteIndexPath constructor indexUUID=djjsid73he8yd7usduh shardCount=2 " - + "basePath=[djsd878ndjh][hcs87cj8] pathType=HASHED_PREFIX pathHashAlgorithm=FNV_1A pathCreationMap={}", + + "basePath=[djsd878ndjh][hcs87cj8] pathType=HASHED_PREFIX pathHashAlgorithm=FNV_1A_BASE64 pathCreationMap={}", ex.getMessage() ); } + + public void testFromXContent() { + UnsupportedOperationException ex = assertThrows( + UnsupportedOperationException.class, + () -> RemoteIndexPath.fromXContent(Mockito.mock(XContentParser.class)) + ); + assertEquals("RemoteIndexPath.fromXContent() is not supported", ex.getMessage()); + } + + public void testInvalidPathCreationMap() { + IllegalArgumentException ex = assertThrows( + IllegalArgumentException.class, + () -> new RemoteIndexPath( + "djjsid73he8yd7usduh", + 2, + new BlobPath().add("djsd878ndjh").add("hcs87cj8"), + PathType.HASHED_PREFIX, + PathHashAlgorithm.FNV_1A_BASE64, + Map.of(TRANSLOG, List.of(LOCK_FILES)) + ) + ); + assertEquals("pathCreationMap={TRANSLOG=[LOCK_FILES]} is having illegal combination of category and type", ex.getMessage()); + } } diff --git a/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathUploaderTests.java b/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathUploaderTests.java new file mode 100644 index 0000000000000..d574e4df967dc --- /dev/null +++ b/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathUploaderTests.java @@ -0,0 +1,271 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.index.remote; + +import org.opensearch.Version; +import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.common.SetOnce; +import org.opensearch.common.UUIDs; +import org.opensearch.common.blobstore.AsyncMultiStreamBlobContainer; +import org.opensearch.common.blobstore.BlobContainer; +import org.opensearch.common.blobstore.BlobPath; +import org.opensearch.common.blobstore.BlobStore; +import org.opensearch.common.compress.DeflateCompressor; +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.unit.TimeValue; +import org.opensearch.core.action.ActionListener; +import org.opensearch.gateway.remote.RemoteClusterStateService; +import org.opensearch.gateway.remote.RemoteClusterStateService.RemoteStateTransferException; +import org.opensearch.index.remote.RemoteStoreEnums.PathType; +import org.opensearch.node.Node; +import org.opensearch.node.remotestore.RemoteStoreNodeAttribute; +import org.opensearch.repositories.RepositoriesService; +import org.opensearch.repositories.blobstore.BlobStoreRepository; +import org.opensearch.test.OpenSearchTestCase; +import org.junit.Before; + +import java.io.IOException; +import java.io.InputStream; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicLong; + +import org.mockito.Mockito; + +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; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class RemoteIndexPathUploaderTests extends OpenSearchTestCase { + + private static final String CLUSTER_STATE_REPO_KEY = Node.NODE_ATTRIBUTES.getKey() + + RemoteStoreNodeAttribute.REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY; + + private static final String TRANSLOG_REPO_NAME = "translog-repo"; + private static final String SEGMENT_REPO_NAME = "segment-repo"; + + private Settings settings; + private ClusterSettings clusterSettings; + private RepositoriesService repositoriesService; + private BlobStoreRepository repository; + private BlobStore blobStore; + private BlobContainer blobContainer; + private BlobPath basePath; + private List indexMetadataList; + private final AtomicLong successCount = new AtomicLong(); + private final AtomicLong failureCount = new AtomicLong(); + + @Before + public void setup() { + settings = Settings.builder() + .put(RemoteIndexPathUploader.TRANSLOG_REPO_NAME_KEY, TRANSLOG_REPO_NAME) + .put(RemoteIndexPathUploader.SEGMENT_REPO_NAME_KEY, TRANSLOG_REPO_NAME) + .put(CLUSTER_STATE_REPO_KEY, TRANSLOG_REPO_NAME) + .put(RemoteClusterStateService.REMOTE_CLUSTER_STATE_ENABLED_SETTING.getKey(), true) + .build(); + clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + basePath = BlobPath.cleanPath().add("test"); + repositoriesService = mock(RepositoriesService.class); + repository = mock(BlobStoreRepository.class); + when(repositoriesService.repository(anyString())).thenReturn(repository); + blobStore = mock(BlobStore.class); + when(repository.blobStore()).thenReturn(blobStore); + when(repositoriesService.repository(TRANSLOG_REPO_NAME)).thenReturn(repository); + when(repository.basePath()).thenReturn(basePath); + when(repository.getCompressor()).thenReturn(new DeflateCompressor()); + blobContainer = mock(BlobContainer.class); + when(blobStore.blobContainer(any(BlobPath.class))).thenReturn(blobContainer); + + Map remoteCustomData = Map.of( + PathType.NAME, + HASHED_PREFIX.name(), + RemoteStoreEnums.PathHashAlgorithm.NAME, + RemoteStoreEnums.PathHashAlgorithm.FNV_1A_BASE64.name() + ); + Settings idxSettings = Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) + .put(IndexMetadata.SETTING_INDEX_UUID, UUIDs.randomBase64UUID()) + .build(); + IndexMetadata indexMetadata = new IndexMetadata.Builder("test").settings(idxSettings) + .numberOfShards(1) + .numberOfReplicas(0) + .putCustom(IndexMetadata.REMOTE_STORE_CUSTOM_KEY, remoteCustomData) + .build(); + indexMetadataList = List.of(indexMetadata); + } + + public void testInterceptWithNoRemoteDataAttributes() throws IOException { + Settings settings = Settings.Builder.EMPTY_SETTINGS; + clusterSettings.applySettings(settings); + RemoteIndexPathUploader remoteIndexPathUploader = new RemoteIndexPathUploader(settings, () -> repositoriesService, clusterSettings); + List indexMetadataList = Mockito.mock(List.class); + ActionListener actionListener = ActionListener.wrap( + res -> successCount.incrementAndGet(), + ex -> failureCount.incrementAndGet() + ); + remoteIndexPathUploader.interceptIndexCreation(indexMetadataList, actionListener); + assertEquals(1, successCount.get()); + assertEquals(0, failureCount.get()); + verify(indexMetadataList, times(0)).stream(); + } + + public void testInterceptWithEmptyIndexMetadataList() throws IOException { + RemoteIndexPathUploader remoteIndexPathUploader = new RemoteIndexPathUploader(settings, () -> repositoriesService, clusterSettings); + remoteIndexPathUploader.start(); + ActionListener actionListener = ActionListener.wrap( + res -> successCount.incrementAndGet(), + ex -> failureCount.incrementAndGet() + ); + remoteIndexPathUploader.interceptIndexCreation(Collections.emptyList(), actionListener); + assertEquals(1, successCount.get()); + assertEquals(0, failureCount.get()); + } + + public void testInterceptWithEmptyEligibleIndexMetadataList() throws IOException { + RemoteIndexPathUploader remoteIndexPathUploader = new RemoteIndexPathUploader(settings, () -> repositoriesService, clusterSettings); + remoteIndexPathUploader.start(); + ActionListener actionListener = ActionListener.wrap( + res -> successCount.incrementAndGet(), + ex -> failureCount.incrementAndGet() + ); + + // Case 1 - Null remoteCustomData + List indexMetadataList = new ArrayList<>(); + IndexMetadata indexMetadata = mock(IndexMetadata.class); + indexMetadataList.add(indexMetadata); + remoteIndexPathUploader.interceptIndexCreation(indexMetadataList, actionListener); + assertEquals(1, successCount.get()); + assertEquals(0, failureCount.get()); + + // Case 2 - Empty remoteCustomData + when(indexMetadata.getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY)).thenReturn(new HashMap<>()); + remoteIndexPathUploader.interceptIndexCreation(indexMetadataList, actionListener); + assertEquals(2, successCount.get()); + assertEquals(0, failureCount.get()); + + // Case 3 - RemoteStoreEnums.PathType.NAME not in remoteCustomData map + Map remoteCustomData = Map.of("test", "test"); + when(indexMetadata.getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY)).thenReturn(remoteCustomData); + remoteIndexPathUploader.interceptIndexCreation(indexMetadataList, actionListener); + assertEquals(3, successCount.get()); + assertEquals(0, failureCount.get()); + + // Case 4 - RemoteStoreEnums.PathType.NAME is not HASHED_PREFIX + remoteCustomData = Map.of(PathType.NAME, randomFrom(FIXED, HASHED_INFIX).name()); + when(indexMetadata.getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY)).thenReturn(remoteCustomData); + remoteIndexPathUploader.interceptIndexCreation(indexMetadataList, actionListener); + assertEquals(4, successCount.get()); + assertEquals(0, failureCount.get()); + } + + public void testInterceptWithSameRepo() throws IOException { + RemoteIndexPathUploader remoteIndexPathUploader = new RemoteIndexPathUploader(settings, () -> repositoriesService, clusterSettings); + remoteIndexPathUploader.start(); + ActionListener actionListener = ActionListener.wrap( + res -> successCount.incrementAndGet(), + ex -> failureCount.incrementAndGet() + ); + remoteIndexPathUploader.interceptIndexCreation(indexMetadataList, actionListener); + assertEquals(1, successCount.get()); + assertEquals(0, failureCount.get()); + verify(blobContainer, times(1)).writeBlob(anyString(), any(InputStream.class), anyLong(), anyBoolean()); + } + + public void testInterceptWithDifferentRepo() throws IOException { + Settings settings = Settings.builder() + .put(this.settings) + .put(RemoteIndexPathUploader.SEGMENT_REPO_NAME_KEY, SEGMENT_REPO_NAME) + .build(); + when(repositoriesService.repository(SEGMENT_REPO_NAME)).thenReturn(repository); + RemoteIndexPathUploader remoteIndexPathUploader = new RemoteIndexPathUploader(settings, () -> repositoriesService, clusterSettings); + remoteIndexPathUploader.start(); + ActionListener actionListener = ActionListener.wrap( + res -> successCount.incrementAndGet(), + ex -> failureCount.incrementAndGet() + ); + remoteIndexPathUploader.interceptIndexCreation(indexMetadataList, actionListener); + assertEquals(1, successCount.get()); + assertEquals(0, failureCount.get()); + verify(blobContainer, times(2)).writeBlob(anyString(), any(InputStream.class), anyLong(), anyBoolean()); + } + + public void testInterceptWithLatchAwaitTimeout() throws IOException { + blobContainer = mock(AsyncMultiStreamBlobContainer.class); + when(blobStore.blobContainer(any(BlobPath.class))).thenReturn(blobContainer); + RemoteIndexPathUploader remoteIndexPathUploader = new RemoteIndexPathUploader(settings, () -> repositoriesService, clusterSettings); + remoteIndexPathUploader.start(); + + Settings settings = Settings.builder() + .put(this.settings) + .put(RemoteClusterStateService.INDEX_METADATA_UPLOAD_TIMEOUT_SETTING.getKey(), TimeValue.ZERO) + .build(); + clusterSettings.applySettings(settings); + SetOnce exceptionSetOnce = new SetOnce<>(); + ActionListener actionListener = ActionListener.wrap(res -> successCount.incrementAndGet(), ex -> { + failureCount.incrementAndGet(); + exceptionSetOnce.set(ex); + }); + remoteIndexPathUploader.interceptIndexCreation(indexMetadataList, actionListener); + assertEquals(0, successCount.get()); + assertEquals(1, failureCount.get()); + assertTrue(exceptionSetOnce.get() instanceof RemoteStateTransferException); + assertTrue( + exceptionSetOnce.get().getMessage().contains("Timed out waiting while uploading remote index path file for indexes=[test/") + ); + verify(blobContainer, times(0)).writeBlob(anyString(), any(InputStream.class), anyLong(), anyBoolean()); + } + + public void testInterceptWithInterruptedExceptionDuringLatchAwait() throws Exception { + AsyncMultiStreamBlobContainer asyncMultiStreamBlobContainer = mock(AsyncMultiStreamBlobContainer.class); + when(blobStore.blobContainer(any(BlobPath.class))).thenReturn(asyncMultiStreamBlobContainer); + RemoteIndexPathUploader remoteIndexPathUploader = new RemoteIndexPathUploader(settings, () -> repositoriesService, clusterSettings); + remoteIndexPathUploader.start(); + Settings settings = Settings.builder() + .put(this.settings) + .put(RemoteClusterStateService.INDEX_METADATA_UPLOAD_TIMEOUT_SETTING.getKey(), TimeValue.timeValueSeconds(1)) + .build(); + clusterSettings.applySettings(settings); + SetOnce exceptionSetOnce = new SetOnce<>(); + ActionListener actionListener = ActionListener.wrap(res -> successCount.incrementAndGet(), ex -> { + failureCount.incrementAndGet(); + exceptionSetOnce.set(ex); + }); + Thread thread = new Thread(() -> { + try { + remoteIndexPathUploader.interceptIndexCreation(indexMetadataList, actionListener); + } catch (IOException e) { + throw new AssertionError(e); + } catch (Exception e) { + assertTrue(e instanceof InterruptedException); + assertEquals("sleep interrupted", e.getMessage()); + } + }); + thread.start(); + Thread.sleep(10); + thread.interrupt(); + + assertBusy(() -> { + assertEquals(0, successCount.get()); + assertEquals(1, failureCount.get()); + }); + } + +} From 8f4aa0a982661cd7f64d00a46742086b47a2d266 Mon Sep 17 00:00:00 2001 From: Ashish Singh Date: Wed, 17 Apr 2024 21:24:40 +0530 Subject: [PATCH 07/15] Add path creation map in toXContent Signed-off-by: Ashish Singh --- .../index/remote/RemoteIndexPath.java | 10 +++++ .../blobstore/ChecksumBlobStoreFormat.java | 3 +- .../index/remote/RemoteIndexPathTests.java | 37 +++---------------- 3 files changed, 18 insertions(+), 32 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteIndexPath.java b/server/src/main/java/org/opensearch/index/remote/RemoteIndexPath.java index b44cb1e8ec16f..b70362d58bed1 100644 --- a/server/src/main/java/org/opensearch/index/remote/RemoteIndexPath.java +++ b/server/src/main/java/org/opensearch/index/remote/RemoteIndexPath.java @@ -20,9 +20,11 @@ import org.opensearch.index.remote.RemoteStorePathStrategy.PathInput; import java.io.IOException; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.stream.Collectors; import static org.opensearch.index.remote.RemoteStoreEnums.DataCategory.SEGMENTS; import static org.opensearch.index.remote.RemoteStoreEnums.DataCategory.TRANSLOG; @@ -45,6 +47,7 @@ public class RemoteIndexPath implements ToXContentFragment { static final String KEY_VERSION = "version"; static final String KEY_INDEX_UUID = "index_uuid"; static final String KEY_SHARD_COUNT = "shard_count"; + static final String KEY_PATH_CREATION_MAP = "path_creation_map"; static final String KEY_PATHS = "paths"; private final String indexUUID; private final int shardCount; @@ -112,6 +115,13 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws if (Objects.nonNull(pathHashAlgorithm)) { builder.field(PathHashAlgorithm.NAME, pathHashAlgorithm.name()); } + + Map> pathMap = new HashMap<>(); + for (Map.Entry> entry : pathCreationMap.entrySet()) { + pathMap.put(entry.getKey().getName(), entry.getValue().stream().map(DataType::getName).collect(Collectors.toList())); + } + builder.field(KEY_PATH_CREATION_MAP); + builder.map(pathMap); builder.startArray(KEY_PATHS); for (Map.Entry> entry : pathCreationMap.entrySet()) { DataCategory dataCategory = entry.getKey(); diff --git a/server/src/main/java/org/opensearch/repositories/blobstore/ChecksumBlobStoreFormat.java b/server/src/main/java/org/opensearch/repositories/blobstore/ChecksumBlobStoreFormat.java index 3e6052a5ef820..294022f00a2c1 100644 --- a/server/src/main/java/org/opensearch/repositories/blobstore/ChecksumBlobStoreFormat.java +++ b/server/src/main/java/org/opensearch/repositories/blobstore/ChecksumBlobStoreFormat.java @@ -300,6 +300,7 @@ public BytesReference serialize(final T obj, final String blobName, final Compre ) ) { CodecUtil.writeHeader(indexOutput, codec, VERSION); + XContentType xContentType = XContentType.SMILE; try (OutputStream indexOutputOutputStream = new IndexOutputOutputStream(indexOutput) { @Override public void close() throws IOException { @@ -308,7 +309,7 @@ public void close() throws IOException { } }; XContentBuilder builder = MediaTypeRegistry.contentBuilder( - XContentType.SMILE, + xContentType, compressor.threadLocalOutputStream(indexOutputOutputStream) ) ) { diff --git a/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathTests.java b/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathTests.java index c1072ebc00e54..8ddbd383756e7 100644 --- a/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathTests.java +++ b/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathTests.java @@ -46,14 +46,8 @@ public void testToXContentWithSegmentRepo() throws IOException { xContentBuilder.startObject(); xContentBuilder = indexPath.toXContent(xContentBuilder, ToXContent.EMPTY_PARAMS); xContentBuilder.endObject(); - String expected = "{\"version\":\"1\",\"index_uuid\":\"djjsid73he8yd7usduh\",\"shard_count\":2," - + "\"path_type\":\"HASHED_PREFIX\",\"path_hash_algorithm\":\"FNV_1A_BASE64\",\"paths\":[" - + "\"9BmBinD5HYs/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/0/segments/data/\"," - + "\"ExCNOD8_5ew/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/1/segments/data/\"," - + "\"z8wtf0yr2l4/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/0/segments/metadata/\"," - + "\"VheHVwFlExE/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/1/segments/metadata/\"," - + "\"IgFKbsDeUpQ/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/0/segments/lock_files/\"," - + "\"pA3gy_GZtns/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/1/segments/lock_files/\"]}"; + String expected = + "{\"version\":\"1\",\"index_uuid\":\"djjsid73he8yd7usduh\",\"shard_count\":2,\"path_type\":\"HASHED_PREFIX\",\"path_hash_algorithm\":\"FNV_1A_BASE64\",\"path_creation_map\":{\"segments\":[\"data\",\"metadata\",\"lock_files\"]},\"paths\":[\"9BmBinD5HYs/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/0/segments/data/\",\"ExCNOD8_5ew/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/1/segments/data/\",\"z8wtf0yr2l4/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/0/segments/metadata/\",\"VheHVwFlExE/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/1/segments/metadata/\",\"IgFKbsDeUpQ/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/0/segments/lock_files/\",\"pA3gy_GZtns/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/1/segments/lock_files/\"]}"; assertEquals(expected, xContentBuilder.toString()); } @@ -73,12 +67,8 @@ public void testToXContentForTranslogRepoOnly() throws IOException { xContentBuilder.startObject(); xContentBuilder = indexPath.toXContent(xContentBuilder, ToXContent.EMPTY_PARAMS); xContentBuilder.endObject(); - String expected = "{\"version\":\"1\",\"index_uuid\":\"djjsid73he8yd7usduh\",\"shard_count\":2," - + "\"path_type\":\"HASHED_PREFIX\",\"path_hash_algorithm\":\"FNV_1A_BASE64\",\"paths\":[" - + "\"2EaVODaKBck/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/0/translog/data/\"," - + "\"dTS2VqEOUNo/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/1/translog/data/\"," - + "\"PVNKNGonmZw/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/0/translog/metadata/\"," - + "\"NXmt0Y6NjA8/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/1/translog/metadata/\"]}"; + String expected = + "{\"version\":\"1\",\"index_uuid\":\"djjsid73he8yd7usduh\",\"shard_count\":2,\"path_type\":\"HASHED_PREFIX\",\"path_hash_algorithm\":\"FNV_1A_BASE64\",\"path_creation_map\":{\"translog\":[\"data\",\"metadata\"]},\"paths\":[\"2EaVODaKBck/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/0/translog/data/\",\"dTS2VqEOUNo/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/1/translog/data/\",\"PVNKNGonmZw/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/0/translog/metadata/\",\"NXmt0Y6NjA8/djsd878ndjh/hcs87cj8/djjsid73he8yd7usduh/1/translog/metadata/\"]}"; assertEquals(expected, xContentBuilder.toString()); } @@ -101,23 +91,8 @@ public void testToXContentForBothRepos() throws IOException { xContentBuilder.startObject(); xContentBuilder = indexPath.toXContent(xContentBuilder, ToXContent.EMPTY_PARAMS); xContentBuilder.endObject(); - String expected = "{\"version\":\"1\",\"index_uuid\":\"csbdqiu8a7sdnjdks\",\"shard_count\":3,\"path_type\":" - + "\"HASHED_PREFIX\",\"path_hash_algorithm\":\"FNV_1A_BASE64\",\"paths\":[" - + "\"Cjo0F6kNjYk/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/0/segments/data/\"," - + "\"kpayyhxct1I/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/1/segments/data/\"," - + "\"p2RlgnHeIgc/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/2/segments/data/\"," - + "\"gkPIurBtB1w/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/0/segments/metadata/\"," - + "\"Y4YhlbxAB1c/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/1/segments/metadata/\"," - + "\"HYc8fyVPouI/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/2/segments/metadata/\"," - + "\"igzyZCz1ysI/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/0/segments/lock_files/\"," - + "\"uEluEiYmptk/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/1/segments/lock_files/\"," - + "\"TfAD8f06_7A/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/2/segments/lock_files/\"," - + "\"QqKEpasbEGs/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/0/translog/data/\"," - + "\"sNyoimoe1Bw/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/1/translog/data/\"," - + "\"d4YQtONfq50/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/2/translog/data/\"," - + "\"zLr4UXjK8T4/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/0/translog/metadata/\"," - + "\"_s8i7ZmlXGE/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/1/translog/metadata/\"," - + "\"tvtD3-k5ISg/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/2/translog/metadata/\"]}"; + String expected = + "{\"version\":\"1\",\"index_uuid\":\"csbdqiu8a7sdnjdks\",\"shard_count\":3,\"path_type\":\"HASHED_PREFIX\",\"path_hash_algorithm\":\"FNV_1A_BASE64\",\"path_creation_map\":{\"translog\":[\"data\",\"metadata\"],\"segments\":[\"data\",\"metadata\",\"lock_files\"]},\"paths\":[\"Cjo0F6kNjYk/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/0/segments/data/\",\"kpayyhxct1I/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/1/segments/data/\",\"p2RlgnHeIgc/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/2/segments/data/\",\"gkPIurBtB1w/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/0/segments/metadata/\",\"Y4YhlbxAB1c/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/1/segments/metadata/\",\"HYc8fyVPouI/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/2/segments/metadata/\",\"igzyZCz1ysI/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/0/segments/lock_files/\",\"uEluEiYmptk/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/1/segments/lock_files/\",\"TfAD8f06_7A/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/2/segments/lock_files/\",\"QqKEpasbEGs/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/0/translog/data/\",\"sNyoimoe1Bw/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/1/translog/data/\",\"d4YQtONfq50/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/2/translog/data/\",\"zLr4UXjK8T4/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/0/translog/metadata/\",\"_s8i7ZmlXGE/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/1/translog/metadata/\",\"tvtD3-k5ISg/nxf9yv0/c3ejoi/csbdqiu8a7sdnjdks/2/translog/metadata/\"]}"; assertEquals(expected, xContentBuilder.toString()); } From dd1efedd5e813dd649e2baf2327807e6a6db2d69 Mon Sep 17 00:00:00 2001 From: Ashish Singh Date: Thu, 18 Apr 2024 00:37:45 +0530 Subject: [PATCH 08/15] Add IT Signed-off-by: Ashish Singh --- .../RemoteStoreUploadIndexPathIT.java | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreUploadIndexPathIT.java diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreUploadIndexPathIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreUploadIndexPathIT.java new file mode 100644 index 0000000000000..c39cec96aa476 --- /dev/null +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreUploadIndexPathIT.java @@ -0,0 +1,108 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.remotestore; + +import org.opensearch.action.admin.indices.delete.DeleteIndexRequest; +import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.common.settings.Settings; +import org.opensearch.core.util.FileSystemUtils; +import org.opensearch.index.remote.RemoteIndexPath; +import org.opensearch.index.remote.RemoteStoreEnums; +import org.opensearch.test.OpenSearchIntegTestCase; + +import java.io.IOException; +import java.util.Locale; +import java.util.concurrent.ExecutionException; + +import static org.opensearch.gateway.remote.RemoteClusterStateService.REMOTE_CLUSTER_STATE_ENABLED_SETTING; +import static org.opensearch.indices.RemoteStoreSettings.CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING; +import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked; + +@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0) +public class RemoteStoreUploadIndexPathIT extends RemoteStoreBaseIntegTestCase { + + private final String INDEX_NAME = "remote-store-test-idx-1"; + + @Override + protected Settings nodeSettings(int nodeOrdinal) { + return Settings.builder().put(super.nodeSettings(nodeOrdinal)).put(REMOTE_CLUSTER_STATE_ENABLED_SETTING.getKey(), true).build(); + } + + /** + * Checks that the remote index path file gets created for the intended remote store path type and does not get created + * wherever not required. + */ + public void testRemoteIndexPathFileCreation() throws ExecutionException, InterruptedException, IOException { + String clusterManagerNode = internalCluster().startClusterManagerOnlyNode(); + internalCluster().startDataOnlyNodes(2); + + // Case 1 - Hashed_prefix, we would need the remote index path file to be created. + client(clusterManagerNode).admin() + .cluster() + .prepareUpdateSettings() + .setTransientSettings( + Settings.builder().put(CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING.getKey(), RemoteStoreEnums.PathType.HASHED_PREFIX) + ) + .get(); + + createIndex(INDEX_NAME, remoteStoreIndexSettings(0, 1)); + validateRemoteIndexPathFile(true); + assertAcked(client().admin().indices().delete(new DeleteIndexRequest(INDEX_NAME)).get()); + FileSystemUtils.deleteSubDirectories(translogRepoPath); + FileSystemUtils.deleteSubDirectories(segmentRepoPath); + + // Case 2 - Hashed_infix, we would not have the remote index path file created here. + client(clusterManagerNode).admin() + .cluster() + .prepareUpdateSettings() + .setTransientSettings( + Settings.builder().put(CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING.getKey(), RemoteStoreEnums.PathType.HASHED_INFIX) + ) + .get(); + createIndex(INDEX_NAME, remoteStoreIndexSettings(0, 1)); + validateRemoteIndexPathFile(false); + assertAcked(client().admin().indices().delete(new DeleteIndexRequest(INDEX_NAME)).get()); + + // Case 3 - fixed, we would not have the remote index path file created here either. + client(clusterManagerNode).admin() + .cluster() + .prepareUpdateSettings() + .setTransientSettings(Settings.builder().put(CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING.getKey(), RemoteStoreEnums.PathType.FIXED)) + .get(); + createIndex(INDEX_NAME, remoteStoreIndexSettings(0, 1)); + validateRemoteIndexPathFile(false); + assertAcked(client().admin().indices().delete(new DeleteIndexRequest(INDEX_NAME)).get()); + + } + + private void validateRemoteIndexPathFile(boolean exists) { + String indexUUID = client().admin() + .indices() + .prepareGetSettings(INDEX_NAME) + .get() + .getSetting(INDEX_NAME, IndexMetadata.SETTING_INDEX_UUID); + + assertEquals(exists, FileSystemUtils.exists(translogRepoPath.resolve(RemoteIndexPath.DIR))); + assertEquals( + exists, + FileSystemUtils.exists( + translogRepoPath.resolve(RemoteIndexPath.DIR) + .resolve(String.format(Locale.ROOT, RemoteIndexPath.FILE_NAME_FORMAT, indexUUID)) + ) + ); + assertEquals(exists, FileSystemUtils.exists(segmentRepoPath.resolve(RemoteIndexPath.DIR))); + assertEquals( + exists, + FileSystemUtils.exists( + segmentRepoPath.resolve(RemoteIndexPath.DIR) + .resolve(String.format(Locale.ROOT, RemoteIndexPath.FILE_NAME_FORMAT, indexUUID)) + ) + ); + } +} From 33337c9f8d7071b54cdc7061fa2b96126919404f Mon Sep 17 00:00:00 2001 From: Ashish Singh Date: Thu, 18 Apr 2024 12:54:18 +0530 Subject: [PATCH 09/15] Incoporate PR review feedback Signed-off-by: Ashish Singh --- ....java => IndexMetadataUploadListener.java} | 12 +-- .../remote/RemoteClusterStateService.java | 49 ++++++----- .../index/remote/RemoteIndexPath.java | 2 + .../index/remote/RemoteIndexPathUploader.java | 81 ++++++++++++------- .../index/remote/RemoteStorePathStrategy.java | 4 + .../RemoteStorePathStrategyResolver.java | 2 + .../remote/RemoteIndexPathUploaderTests.java | 20 ++--- 7 files changed, 106 insertions(+), 64 deletions(-) rename server/src/main/java/org/opensearch/gateway/remote/{IndexMetadataUploadInterceptor.java => IndexMetadataUploadListener.java} (66%) diff --git a/server/src/main/java/org/opensearch/gateway/remote/IndexMetadataUploadInterceptor.java b/server/src/main/java/org/opensearch/gateway/remote/IndexMetadataUploadListener.java similarity index 66% rename from server/src/main/java/org/opensearch/gateway/remote/IndexMetadataUploadInterceptor.java rename to server/src/main/java/org/opensearch/gateway/remote/IndexMetadataUploadListener.java index 709b61ef6efb2..a19b7aea9491c 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/IndexMetadataUploadInterceptor.java +++ b/server/src/main/java/org/opensearch/gateway/remote/IndexMetadataUploadListener.java @@ -16,19 +16,21 @@ /** * Hook for running code that needs to be executed before the upload of index metadata. Here we have introduced a hook - * for index creation (also triggerred after enabling the remote cluster statement for the first time). The Interceptor + * for index creation (also triggerred after enabling the remote cluster statement for the first time). The listener * is intended to be run in parallel and async with the index metadata upload. * * @opensearch.internal */ -public interface IndexMetadataUploadInterceptor { +public interface IndexMetadataUploadListener { /** - * Intercepts the index metadata upload flow with input containing index metadata of new indexes (or first time upload). - * The caller is expected to trigger onSuccess or onFailure of the {@code ActionListener}. + * Runs before the new index upload of index metadata (or first time upload). The caller is expected to trigger + * onSuccess or onFailure of the {@code ActionListener}. * * @param indexMetadataList list of index metadata of new indexes (or first time index metadata upload). * @param actionListener listener to be invoked on success or failure. */ - void interceptIndexCreation(List indexMetadataList, ActionListener actionListener) throws IOException; + void beforeNewIndexUpload(List indexMetadataList, ActionListener actionListener) throws IOException; + + String getThreadpoolName(); } diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java index dbd2754cd3a1d..ce42d0ecf4915 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -161,7 +161,7 @@ public class RemoteClusterStateService implements Closeable { private final Settings settings; private final LongSupplier relativeTimeNanosSupplier; private final ThreadPool threadpool; - private final List indexMetadataUploadInterceptors; + private final List indexMetadataUploadListeners; private BlobStoreRepository blobStoreRepository; private BlobStoreTransferService blobStoreTransferService; private volatile TimeValue slowWriteLoggingThreshold; @@ -192,7 +192,7 @@ public RemoteClusterStateService( ClusterSettings clusterSettings, LongSupplier relativeTimeNanosSupplier, ThreadPool threadPool, - List indexMetadataUploadInterceptors + List indexMetadataUploadListeners ) { assert isRemoteStoreClusterStateEnabled(settings) : "Remote cluster state is not enabled"; this.nodeId = nodeId; @@ -209,7 +209,7 @@ public RemoteClusterStateService( clusterSettings.addSettingsUpdateConsumer(GLOBAL_METADATA_UPLOAD_TIMEOUT_SETTING, this::setGlobalMetadataUploadTimeout); clusterSettings.addSettingsUpdateConsumer(METADATA_MANIFEST_UPLOAD_TIMEOUT_SETTING, this::setMetadataManifestUploadTimeout); this.remoteStateStats = new RemotePersistenceStats(); - this.indexMetadataUploadInterceptors = indexMetadataUploadInterceptors; + this.indexMetadataUploadListeners = indexMetadataUploadListeners; } private BlobStoreTransferService getBlobStoreTransferService() { @@ -319,7 +319,7 @@ public ClusterMetadataManifest writeIncrementalMetadata( .collect(Collectors.toMap(UploadedIndexMetadata::getIndexName, Function.identity())); List toUpload = new ArrayList<>(); - final Map indexNamePreviousVersionMap = new HashMap<>(previousStateIndexMetadataVersionByName); + List newIndexMetadataList = new ArrayList<>(); for (final IndexMetadata indexMetadata : clusterState.metadata().indices().values()) { final Long previousVersion = previousStateIndexMetadataVersionByName.get(indexMetadata.getIndex().getName()); if (previousVersion == null || indexMetadata.getVersion() != previousVersion) { @@ -335,12 +335,12 @@ public ClusterMetadataManifest writeIncrementalMetadata( numIndicesUnchanged++; } previousStateIndexMetadataVersionByName.remove(indexMetadata.getIndex().getName()); + // Adding the indexMetadata to newIndexMetadataList if there is no previous version present for the index. + if (previousVersion == null) { + newIndexMetadataList.add(indexMetadata); + } } - List newIndexMetadataList = toUpload.stream() - /* If the previous state's index metadata version is null, then this is index creation */ - .filter(indexMetadata -> Objects.isNull(indexNamePreviousVersionMap.get(indexMetadata.getIndex().getName()))) - .collect(Collectors.toList()); List uploadedIndexMetadataList = writeIndexMetadataParallel(clusterState, toUpload, newIndexMetadataList); uploadedIndexMetadataList.forEach( uploadedIndexMetadata -> allUploadedIndexMetadata.put(uploadedIndexMetadata.getIndexName(), uploadedIndexMetadata) @@ -454,8 +454,8 @@ private List writeIndexMetadataParallel( List toUpload, List newIndexMetadataList ) throws IOException { - assert CollectionUtils.isEmpty(indexMetadataUploadInterceptors) == false : "indexMetadataUploadInterceptors can not be empty"; - int latchCount = toUpload.size() + indexMetadataUploadInterceptors.size(); + assert CollectionUtils.isEmpty(indexMetadataUploadListeners) == false : "indexMetadataUploadInterceptors can not be empty"; + int latchCount = toUpload.size() + indexMetadataUploadListeners.size(); List exceptionList = Collections.synchronizedList(new ArrayList<>(latchCount)); final CountDownLatch latch = new CountDownLatch(latchCount); List result = new ArrayList<>(toUpload.size()); @@ -482,14 +482,16 @@ private List writeIndexMetadataParallel( writeIndexMetadataAsync(clusterState, indexMetadata, latchedActionListener); } - for (IndexMetadataUploadInterceptor interceptor : indexMetadataUploadInterceptors) { + for (IndexMetadataUploadListener listener : indexMetadataUploadListeners) { // We are submitting the task for async execution to ensure that we are not blocking the cluster state upload - String interceptorName = interceptor.getClass().getSimpleName(); - threadpool.executor(ThreadPool.Names.GENERIC).execute(() -> { + String interceptorName = listener.getClass().getSimpleName(); + String threadPoolName = listener.getThreadpoolName(); + assert ThreadPool.THREAD_POOL_TYPES.containsKey(threadPoolName) && ThreadPool.Names.SAME.equals(threadPoolName) == false; + threadpool.executor(threadPoolName).execute(() -> { try { - interceptor.interceptIndexCreation( + listener.beforeNewIndexUpload( newIndexMetadataList, - getIndexMetadataUploadInterceptorListener(newIndexMetadataList, latch, exceptionList, interceptorName) + getIndexMetadataUploadActionListener(newIndexMetadataList, latch, exceptionList, interceptorName) ); } catch (IOException e) { exceptionList.add( @@ -538,20 +540,31 @@ private List writeIndexMetadataParallel( return result; } - private ActionListener getIndexMetadataUploadInterceptorListener( + private ActionListener getIndexMetadataUploadActionListener( List newIndexMetadataList, CountDownLatch latch, List exceptionList, String interceptorName ) { + long startTime = System.nanoTime(); return new LatchedActionListener<>( ActionListener.wrap( ignored -> logger.trace( - new ParameterizedMessage("{} : Intercepted {} successfully", interceptorName, newIndexMetadataList) + new ParameterizedMessage( + "{} : Intercepted {} successfully tookTimeNs={}", + interceptorName, + newIndexMetadataList, + (System.nanoTime() - startTime) + ) ), ex -> { logger.error( - new ParameterizedMessage("{} : Exception during interception of {}", interceptorName, newIndexMetadataList), + new ParameterizedMessage( + "{} : Exception during interception of {} tookTimeNs={}", + interceptorName, + newIndexMetadataList, + (System.nanoTime() - startTime) + ), ex ); exceptionList.add(ex); diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteIndexPath.java b/server/src/main/java/org/opensearch/index/remote/RemoteIndexPath.java index b70362d58bed1..27042eaab6a6d 100644 --- a/server/src/main/java/org/opensearch/index/remote/RemoteIndexPath.java +++ b/server/src/main/java/org/opensearch/index/remote/RemoteIndexPath.java @@ -9,6 +9,7 @@ package org.opensearch.index.remote; import org.apache.logging.log4j.message.ParameterizedMessage; +import org.opensearch.common.annotation.ExperimentalApi; import org.opensearch.common.blobstore.BlobPath; import org.opensearch.core.xcontent.ToXContentFragment; import org.opensearch.core.xcontent.XContentBuilder; @@ -37,6 +38,7 @@ * * @opensearch.internal */ +@ExperimentalApi public class RemoteIndexPath implements ToXContentFragment { public static final Map> TRANSLOG_PATH = Map.of(TRANSLOG, List.of(DATA, METADATA)); diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteIndexPathUploader.java b/server/src/main/java/org/opensearch/index/remote/RemoteIndexPathUploader.java index 4687690fcd685..be9b2d17c45e4 100644 --- a/server/src/main/java/org/opensearch/index/remote/RemoteIndexPathUploader.java +++ b/server/src/main/java/org/opensearch/index/remote/RemoteIndexPathUploader.java @@ -13,6 +13,7 @@ import org.apache.logging.log4j.message.ParameterizedMessage; import org.opensearch.action.LatchedActionListener; import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.common.annotation.ExperimentalApi; import org.opensearch.common.blobstore.BlobContainer; import org.opensearch.common.blobstore.BlobPath; import org.opensearch.common.settings.ClusterSettings; @@ -20,7 +21,7 @@ import org.opensearch.common.unit.TimeValue; import org.opensearch.core.action.ActionListener; import org.opensearch.core.index.Index; -import org.opensearch.gateway.remote.IndexMetadataUploadInterceptor; +import org.opensearch.gateway.remote.IndexMetadataUploadListener; import org.opensearch.gateway.remote.RemoteClusterStateService; import org.opensearch.gateway.remote.RemoteClusterStateService.RemoteStateTransferException; import org.opensearch.node.Node; @@ -29,6 +30,7 @@ import org.opensearch.repositories.Repository; import org.opensearch.repositories.blobstore.BlobStoreRepository; import org.opensearch.repositories.blobstore.ChecksumBlobStoreFormat; +import org.opensearch.threadpool.ThreadPool; import java.io.IOException; import java.util.ArrayList; @@ -53,7 +55,8 @@ * Uploads the remote store path for all possible combinations of {@link org.opensearch.index.remote.RemoteStoreEnums.DataCategory} * and {@link org.opensearch.index.remote.RemoteStoreEnums.DataType} for each shard of an index. */ -public class RemoteIndexPathUploader implements IndexMetadataUploadInterceptor { +@ExperimentalApi +public class RemoteIndexPathUploader implements IndexMetadataUploadListener { public static final ChecksumBlobStoreFormat REMOTE_INDEX_PATH_FORMAT = new ChecksumBlobStoreFormat<>( "remote-index-path", @@ -91,48 +94,64 @@ public RemoteIndexPathUploader(Settings settings, Supplier } @Override - public void interceptIndexCreation(List indexMetadataList, ActionListener actionListener) throws IOException { + public String getThreadpoolName() { + return ThreadPool.Names.GENERIC; + } + + @Override + public void beforeNewIndexUpload(List indexMetadataList, ActionListener actionListener) throws IOException { if (isRemoteDataAttributePresent == false) { + logger.trace("Skipping beforeNewIndexUpload as isRemoteDataAttributePresent=false"); actionListener.onResponse(null); return; } - List eligibleList = indexMetadataList.stream().filter(this::requiresPathUpload).collect(Collectors.toList()); - int latchCount = eligibleList.size() * (isTranslogSegmentRepoSame ? 1 : 2); - CountDownLatch latch = new CountDownLatch(latchCount); - List exceptionList = Collections.synchronizedList(new ArrayList<>(latchCount)); - for (IndexMetadata indexMetadata : eligibleList) { - writeIndexPathAsync(indexMetadata, latch, exceptionList); - } - String indexNames = eligibleList.stream().map(IndexMetadata::getIndex).map(Index::toString).collect(Collectors.joining(",")); - + long startTime = System.nanoTime(); + boolean success = false; try { - if (latch.await(indexMetadataUploadTimeout.millis(), TimeUnit.MILLISECONDS) == false) { + List eligibleList = indexMetadataList.stream().filter(this::requiresPathUpload).collect(Collectors.toList()); + int latchCount = eligibleList.size() * (isTranslogSegmentRepoSame ? 1 : 2); + CountDownLatch latch = new CountDownLatch(latchCount); + List exceptionList = Collections.synchronizedList(new ArrayList<>(latchCount)); + for (IndexMetadata indexMetadata : eligibleList) { + writeIndexPathAsync(indexMetadata, latch, exceptionList); + } + String indexNames = eligibleList.stream().map(IndexMetadata::getIndex).map(Index::toString).collect(Collectors.joining(",")); + logger.trace(new ParameterizedMessage("Remote index path upload started for {}", indexNames)); + + try { + if (latch.await(indexMetadataUploadTimeout.millis(), TimeUnit.MILLISECONDS) == false) { + RemoteStateTransferException ex = new RemoteStateTransferException( + String.format(Locale.ROOT, TIMEOUT_EXCEPTION_MSG, indexNames) + ); + exceptionList.forEach(ex::addSuppressed); + actionListener.onFailure(ex); + return; + } + } catch (InterruptedException exception) { + exceptionList.forEach(exception::addSuppressed); RemoteStateTransferException ex = new RemoteStateTransferException( - String.format(Locale.ROOT, TIMEOUT_EXCEPTION_MSG, indexNames) + String.format(Locale.ROOT, TIMEOUT_EXCEPTION_MSG, indexNames), + exception + ); + actionListener.onFailure(ex); + return; + } + if (exceptionList.size() > 0) { + RemoteStateTransferException ex = new RemoteStateTransferException( + String.format(Locale.ROOT, UPLOAD_EXCEPTION_MSG, indexNames) ); exceptionList.forEach(ex::addSuppressed); actionListener.onFailure(ex); return; } - } catch (InterruptedException exception) { - exceptionList.forEach(exception::addSuppressed); - RemoteStateTransferException ex = new RemoteStateTransferException( - String.format(Locale.ROOT, TIMEOUT_EXCEPTION_MSG, indexNames), - exception - ); - actionListener.onFailure(ex); - return; - } - if (exceptionList.size() > 0) { - RemoteStateTransferException ex = new RemoteStateTransferException( - String.format(Locale.ROOT, UPLOAD_EXCEPTION_MSG, indexNames) - ); - exceptionList.forEach(ex::addSuppressed); - actionListener.onFailure(ex); - return; + success = true; + actionListener.onResponse(null); + } finally { + long tookTimeNs = System.nanoTime() - startTime; + logger.trace(new ParameterizedMessage("executed beforeNewIndexUpload status={} tookTimeNs={}", success, tookTimeNs)); } - actionListener.onResponse(null); + } private void writeIndexPathAsync(IndexMetadata idxMD, CountDownLatch latch, List exceptionList) throws IOException { diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteStorePathStrategy.java b/server/src/main/java/org/opensearch/index/remote/RemoteStorePathStrategy.java index 775f8fe19e4ef..c58f6c3faac84 100644 --- a/server/src/main/java/org/opensearch/index/remote/RemoteStorePathStrategy.java +++ b/server/src/main/java/org/opensearch/index/remote/RemoteStorePathStrategy.java @@ -10,6 +10,7 @@ import org.apache.logging.log4j.message.ParameterizedMessage; import org.opensearch.common.Nullable; +import org.opensearch.common.annotation.ExperimentalApi; import org.opensearch.common.annotation.PublicApi; import org.opensearch.common.blobstore.BlobPath; import org.opensearch.index.remote.RemoteStoreEnums.DataCategory; @@ -25,6 +26,7 @@ * @opensearch.internal */ @PublicApi(since = "2.14.0") +@ExperimentalApi public class RemoteStorePathStrategy { private final PathType type; @@ -74,6 +76,7 @@ public BlobPath generatePath(PathInput pathInput) { * @opensearch.internal */ @PublicApi(since = "2.14.0") + @ExperimentalApi public static class PathInput { private final BlobPath basePath; private final String indexUUID; @@ -122,6 +125,7 @@ public static Builder builder() { * @opensearch.internal */ @PublicApi(since = "2.14.0") + @ExperimentalApi public static class Builder { private BlobPath basePath; private String indexUUID; 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 a33f7522daaae..5cd69dfa679a5 100644 --- a/server/src/main/java/org/opensearch/index/remote/RemoteStorePathStrategyResolver.java +++ b/server/src/main/java/org/opensearch/index/remote/RemoteStorePathStrategyResolver.java @@ -9,6 +9,7 @@ package org.opensearch.index.remote; import org.opensearch.Version; +import org.opensearch.common.annotation.ExperimentalApi; import org.opensearch.index.remote.RemoteStoreEnums.PathHashAlgorithm; import org.opensearch.index.remote.RemoteStoreEnums.PathType; import org.opensearch.indices.RemoteStoreSettings; @@ -20,6 +21,7 @@ * * @opensearch.internal */ +@ExperimentalApi public class RemoteStorePathStrategyResolver { private final RemoteStoreSettings remoteStoreSettings; diff --git a/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathUploaderTests.java b/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathUploaderTests.java index d574e4df967dc..07bf041ce8312 100644 --- a/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathUploaderTests.java +++ b/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathUploaderTests.java @@ -121,7 +121,7 @@ public void testInterceptWithNoRemoteDataAttributes() throws IOException { res -> successCount.incrementAndGet(), ex -> failureCount.incrementAndGet() ); - remoteIndexPathUploader.interceptIndexCreation(indexMetadataList, actionListener); + remoteIndexPathUploader.beforeNewIndexUpload(indexMetadataList, actionListener); assertEquals(1, successCount.get()); assertEquals(0, failureCount.get()); verify(indexMetadataList, times(0)).stream(); @@ -134,7 +134,7 @@ public void testInterceptWithEmptyIndexMetadataList() throws IOException { res -> successCount.incrementAndGet(), ex -> failureCount.incrementAndGet() ); - remoteIndexPathUploader.interceptIndexCreation(Collections.emptyList(), actionListener); + remoteIndexPathUploader.beforeNewIndexUpload(Collections.emptyList(), actionListener); assertEquals(1, successCount.get()); assertEquals(0, failureCount.get()); } @@ -151,27 +151,27 @@ public void testInterceptWithEmptyEligibleIndexMetadataList() throws IOException List indexMetadataList = new ArrayList<>(); IndexMetadata indexMetadata = mock(IndexMetadata.class); indexMetadataList.add(indexMetadata); - remoteIndexPathUploader.interceptIndexCreation(indexMetadataList, actionListener); + remoteIndexPathUploader.beforeNewIndexUpload(indexMetadataList, actionListener); assertEquals(1, successCount.get()); assertEquals(0, failureCount.get()); // Case 2 - Empty remoteCustomData when(indexMetadata.getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY)).thenReturn(new HashMap<>()); - remoteIndexPathUploader.interceptIndexCreation(indexMetadataList, actionListener); + remoteIndexPathUploader.beforeNewIndexUpload(indexMetadataList, actionListener); assertEquals(2, successCount.get()); assertEquals(0, failureCount.get()); // Case 3 - RemoteStoreEnums.PathType.NAME not in remoteCustomData map Map remoteCustomData = Map.of("test", "test"); when(indexMetadata.getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY)).thenReturn(remoteCustomData); - remoteIndexPathUploader.interceptIndexCreation(indexMetadataList, actionListener); + remoteIndexPathUploader.beforeNewIndexUpload(indexMetadataList, actionListener); assertEquals(3, successCount.get()); assertEquals(0, failureCount.get()); // Case 4 - RemoteStoreEnums.PathType.NAME is not HASHED_PREFIX remoteCustomData = Map.of(PathType.NAME, randomFrom(FIXED, HASHED_INFIX).name()); when(indexMetadata.getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY)).thenReturn(remoteCustomData); - remoteIndexPathUploader.interceptIndexCreation(indexMetadataList, actionListener); + remoteIndexPathUploader.beforeNewIndexUpload(indexMetadataList, actionListener); assertEquals(4, successCount.get()); assertEquals(0, failureCount.get()); } @@ -183,7 +183,7 @@ public void testInterceptWithSameRepo() throws IOException { res -> successCount.incrementAndGet(), ex -> failureCount.incrementAndGet() ); - remoteIndexPathUploader.interceptIndexCreation(indexMetadataList, actionListener); + remoteIndexPathUploader.beforeNewIndexUpload(indexMetadataList, actionListener); assertEquals(1, successCount.get()); assertEquals(0, failureCount.get()); verify(blobContainer, times(1)).writeBlob(anyString(), any(InputStream.class), anyLong(), anyBoolean()); @@ -201,7 +201,7 @@ public void testInterceptWithDifferentRepo() throws IOException { res -> successCount.incrementAndGet(), ex -> failureCount.incrementAndGet() ); - remoteIndexPathUploader.interceptIndexCreation(indexMetadataList, actionListener); + remoteIndexPathUploader.beforeNewIndexUpload(indexMetadataList, actionListener); assertEquals(1, successCount.get()); assertEquals(0, failureCount.get()); verify(blobContainer, times(2)).writeBlob(anyString(), any(InputStream.class), anyLong(), anyBoolean()); @@ -223,7 +223,7 @@ public void testInterceptWithLatchAwaitTimeout() throws IOException { failureCount.incrementAndGet(); exceptionSetOnce.set(ex); }); - remoteIndexPathUploader.interceptIndexCreation(indexMetadataList, actionListener); + remoteIndexPathUploader.beforeNewIndexUpload(indexMetadataList, actionListener); assertEquals(0, successCount.get()); assertEquals(1, failureCount.get()); assertTrue(exceptionSetOnce.get() instanceof RemoteStateTransferException); @@ -250,7 +250,7 @@ public void testInterceptWithInterruptedExceptionDuringLatchAwait() throws Excep }); Thread thread = new Thread(() -> { try { - remoteIndexPathUploader.interceptIndexCreation(indexMetadataList, actionListener); + remoteIndexPathUploader.beforeNewIndexUpload(indexMetadataList, actionListener); } catch (IOException e) { throw new AssertionError(e); } catch (Exception e) { From 9d8073f21e467f121342518b495e341651f45f6f Mon Sep 17 00:00:00 2001 From: Ashish Singh Date: Thu, 18 Apr 2024 15:38:02 +0530 Subject: [PATCH 10/15] Add BlobStoreFormat for plaintext json uploads to remote store Signed-off-by: Ashish Singh --- .../transfer/RemoteTransferContainer.java | 10 +-- .../index/remote/RemoteIndexPathUploader.java | 21 ++--- .../blobstore/BlobStoreFormat.java | 90 +++++++++++++++++++ .../RemoteTransferContainerTests.java | 12 +-- 4 files changed, 106 insertions(+), 27 deletions(-) create mode 100644 server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreFormat.java diff --git a/server/src/main/java/org/opensearch/common/blobstore/transfer/RemoteTransferContainer.java b/server/src/main/java/org/opensearch/common/blobstore/transfer/RemoteTransferContainer.java index cd2ef22327ebb..cbd1852202d1c 100644 --- a/server/src/main/java/org/opensearch/common/blobstore/transfer/RemoteTransferContainer.java +++ b/server/src/main/java/org/opensearch/common/blobstore/transfer/RemoteTransferContainer.java @@ -52,7 +52,7 @@ public class RemoteTransferContainer implements Closeable { private final String remoteFileName; private final boolean failTransferIfFileExists; private final WritePriority writePriority; - private final long expectedChecksum; + private final Long expectedChecksum; private final OffsetRangeInputStreamSupplier offsetRangeInputStreamSupplier; private final boolean isRemoteDataIntegritySupported; private final AtomicBoolean readBlock = new AtomicBoolean(); @@ -79,7 +79,7 @@ public RemoteTransferContainer( boolean failTransferIfFileExists, WritePriority writePriority, OffsetRangeInputStreamSupplier offsetRangeInputStreamSupplier, - long expectedChecksum, + Long expectedChecksum, boolean isRemoteDataIntegritySupported ) { this( @@ -115,7 +115,7 @@ public RemoteTransferContainer( boolean failTransferIfFileExists, WritePriority writePriority, OffsetRangeInputStreamSupplier offsetRangeInputStreamSupplier, - long expectedChecksum, + Long expectedChecksum, boolean isRemoteDataIntegritySupported, Map metadata ) { @@ -230,7 +230,7 @@ private LocalStreamSupplier getMultipartStreamSupplier( } private boolean isRemoteDataIntegrityCheckPossible() { - return isRemoteDataIntegritySupported; + return isRemoteDataIntegritySupported && Objects.nonNull(expectedChecksum); } private void finalizeUpload(boolean uploadSuccessful) throws IOException { @@ -238,7 +238,7 @@ private void finalizeUpload(boolean uploadSuccessful) throws IOException { return; } - if (uploadSuccessful) { + if (uploadSuccessful && Objects.nonNull(expectedChecksum)) { long actualChecksum = getActualChecksum(); if (actualChecksum != expectedChecksum) { throw new CorruptIndexException( diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteIndexPathUploader.java b/server/src/main/java/org/opensearch/index/remote/RemoteIndexPathUploader.java index be9b2d17c45e4..5e2b52dbedf2c 100644 --- a/server/src/main/java/org/opensearch/index/remote/RemoteIndexPathUploader.java +++ b/server/src/main/java/org/opensearch/index/remote/RemoteIndexPathUploader.java @@ -22,14 +22,13 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.core.index.Index; import org.opensearch.gateway.remote.IndexMetadataUploadListener; -import org.opensearch.gateway.remote.RemoteClusterStateService; import org.opensearch.gateway.remote.RemoteClusterStateService.RemoteStateTransferException; import org.opensearch.node.Node; import org.opensearch.node.remotestore.RemoteStoreNodeAttribute; import org.opensearch.repositories.RepositoriesService; import org.opensearch.repositories.Repository; +import org.opensearch.repositories.blobstore.BlobStoreFormat; import org.opensearch.repositories.blobstore.BlobStoreRepository; -import org.opensearch.repositories.blobstore.ChecksumBlobStoreFormat; import org.opensearch.threadpool.ThreadPool; import java.io.IOException; @@ -58,11 +57,7 @@ @ExperimentalApi public class RemoteIndexPathUploader implements IndexMetadataUploadListener { - public static final ChecksumBlobStoreFormat REMOTE_INDEX_PATH_FORMAT = new ChecksumBlobStoreFormat<>( - "remote-index-path", - RemoteIndexPath.FILE_NAME_FORMAT, - RemoteIndexPath::fromXContent - ); + public static final BlobStoreFormat REMOTE_INDEX_PATH_FORMAT = new BlobStoreFormat<>(RemoteIndexPath.FILE_NAME_FORMAT); private static final String TIMEOUT_EXCEPTION_MSG = "Timed out waiting while uploading remote index path file for indexes=%s"; private static final String UPLOAD_EXCEPTION_MSG = "Exception occurred while uploading remote index paths for indexes=%s"; @@ -174,9 +169,7 @@ private void writeIndexPathAsync(IndexMetadata idxMD, CountDownLatch latch, List new RemoteIndexPath(indexUUID, shardCount, translogBasePath, pathType, hashAlgorithm, pathCreationMap), translogBlobContainer, indexUUID, - translogRepository.getCompressor(), - getUploadPathLatchedActionListener(idxMD, latch, exceptionList, pathCreationMap), - RemoteClusterStateService.FORMAT_PARAMS + getUploadPathLatchedActionListener(idxMD, latch, exceptionList, pathCreationMap) ); } else { // If the repositories are different, then we need to upload one file per segment and translog containing their individual @@ -185,9 +178,7 @@ private void writeIndexPathAsync(IndexMetadata idxMD, CountDownLatch latch, List new RemoteIndexPath(indexUUID, shardCount, translogBasePath, pathType, hashAlgorithm, TRANSLOG_PATH), translogBlobContainer, indexUUID, - translogRepository.getCompressor(), - getUploadPathLatchedActionListener(idxMD, latch, exceptionList, TRANSLOG_PATH), - RemoteClusterStateService.FORMAT_PARAMS + getUploadPathLatchedActionListener(idxMD, latch, exceptionList, TRANSLOG_PATH) ); BlobPath segmentBasePath = segmentRepository.basePath(); @@ -196,9 +187,7 @@ private void writeIndexPathAsync(IndexMetadata idxMD, CountDownLatch latch, List new RemoteIndexPath(indexUUID, shardCount, segmentBasePath, pathType, hashAlgorithm, SEGMENT_PATH), segmentBlobContainer, indexUUID, - segmentRepository.getCompressor(), - getUploadPathLatchedActionListener(idxMD, latch, exceptionList, SEGMENT_PATH), - RemoteClusterStateService.FORMAT_PARAMS + getUploadPathLatchedActionListener(idxMD, latch, exceptionList, SEGMENT_PATH) ); } } diff --git a/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreFormat.java b/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreFormat.java new file mode 100644 index 0000000000000..810dcadc56e08 --- /dev/null +++ b/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreFormat.java @@ -0,0 +1,90 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.repositories.blobstore; + +import org.apache.lucene.store.IndexInput; +import org.opensearch.common.blobstore.AsyncMultiStreamBlobContainer; +import org.opensearch.common.blobstore.BlobContainer; +import org.opensearch.common.blobstore.stream.write.WritePriority; +import org.opensearch.common.blobstore.transfer.RemoteTransferContainer; +import org.opensearch.common.blobstore.transfer.stream.OffsetRangeIndexInputStream; +import org.opensearch.common.lucene.store.ByteArrayIndexInput; +import org.opensearch.common.xcontent.XContentFactory; +import org.opensearch.core.action.ActionListener; +import org.opensearch.core.common.bytes.BytesReference; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.core.xcontent.XContentBuilder; + +import java.io.IOException; +import java.util.Locale; + +/** + * Standard format that has only content for writes. Read interface does not exist as it not yet required. + * + * @opensearch.internal + */ +public class BlobStoreFormat { + + private final String blobNameFormat; + + /** + * @param blobNameFormat format of the blobname in {@link String#format} format + */ + public BlobStoreFormat(String blobNameFormat) { + this.blobNameFormat = blobNameFormat; + } + + private String blobName(String name) { + return String.format(Locale.ROOT, blobNameFormat, name); + } + + private BytesReference serialize(final T obj) throws IOException { + try (XContentBuilder xContentBuilder = XContentFactory.jsonBuilder()) { + xContentBuilder.startObject(); + obj.toXContent(xContentBuilder, ToXContent.EMPTY_PARAMS); + xContentBuilder.endObject(); + return BytesReference.bytes(xContentBuilder); + } + } + + public void writeAsyncWithUrgentPriority(T obj, BlobContainer blobContainer, String name, ActionListener listener) + throws IOException { + if (blobContainer instanceof AsyncMultiStreamBlobContainer == false) { + write(obj, blobContainer, name); + listener.onResponse(null); + return; + } + String blobName = blobName(name); + BytesReference bytes = serialize(obj); + String resourceDescription = "BlobStoreFormat.writeAsyncWithPriority(blob=\"" + blobName + "\")"; + try (IndexInput input = new ByteArrayIndexInput(resourceDescription, BytesReference.toBytes(bytes))) { + try ( + RemoteTransferContainer remoteTransferContainer = new RemoteTransferContainer( + blobName, + blobName, + bytes.length(), + true, + WritePriority.URGENT, + (size, position) -> new OffsetRangeIndexInputStream(input, size, position), + null, + ((AsyncMultiStreamBlobContainer) blobContainer).remoteIntegrityCheckSupported() + ) + ) { + ((AsyncMultiStreamBlobContainer) blobContainer).asyncBlobUpload(remoteTransferContainer.createWriteContext(), listener); + } + } + } + + private void write(final T obj, final BlobContainer blobContainer, final String name) throws IOException { + String blobName = blobName(name); + BytesReference bytes = serialize(obj); + blobContainer.writeBlob(blobName, bytes.streamInput(), bytes.length(), false); + } + +} diff --git a/server/src/test/java/org/opensearch/common/blobstore/transfer/RemoteTransferContainerTests.java b/server/src/test/java/org/opensearch/common/blobstore/transfer/RemoteTransferContainerTests.java index 074f659850c7b..44f6a0e11251c 100644 --- a/server/src/test/java/org/opensearch/common/blobstore/transfer/RemoteTransferContainerTests.java +++ b/server/src/test/java/org/opensearch/common/blobstore/transfer/RemoteTransferContainerTests.java @@ -67,7 +67,7 @@ public OffsetRangeInputStream get(long size, long position) throws IOException { return new OffsetRangeFileInputStream(testFile, size, position); } }, - 0, + 0L, false ) ) { @@ -89,7 +89,7 @@ public OffsetRangeInputStream get(long size, long position) throws IOException { return new OffsetRangeFileInputStream(testFile, size, position); } }, - 0, + 0L, false ) ) { @@ -155,7 +155,7 @@ public OffsetRangeInputStream get(long size, long position) throws IOException { return new OffsetRangeFileInputStream(testFile, size, position); } }, - 0, + 0L, false ) ) { @@ -223,7 +223,7 @@ public OffsetRangeInputStream get(long size, long position) throws IOException { return new OffsetRangeFileInputStream(testFile, size, position); } }, - 0, + 0L, isRemoteDataIntegritySupported ) ) { @@ -286,7 +286,7 @@ public OffsetRangeInputStream get(long size, long position) throws IOException { return new RateLimitingOffsetRangeInputStream(offsetRangeIndexInputStream, rateLimiterSupplier, null); } }, - 0, + 0L, true ) ) { @@ -347,7 +347,7 @@ public OffsetRangeInputStream get(long size, long position) throws IOException { return new RateLimitingOffsetRangeInputStream(offsetRangeIndexInputStream, rateLimiterSupplier, null); } }, - 0, + 0L, true ) ) { From 7ef0d1a7a94b191a1487a7153405bf1be1fb891b Mon Sep 17 00:00:00 2001 From: Ashish Singh Date: Fri, 19 Apr 2024 10:34:54 +0530 Subject: [PATCH 11/15] Incoporate PR review feedback Signed-off-by: Ashish Singh --- .../remote/RemoteClusterStateService.java | 96 +++++++++++-------- .../index/remote/RemoteIndexPath.java | 9 ++ .../index/remote/RemoteIndexPathUploader.java | 62 ++++++------ 3 files changed, 93 insertions(+), 74 deletions(-) diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java index ce42d0ecf4915..a44aa1ce21153 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -27,7 +27,6 @@ import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.io.IOUtils; import org.opensearch.core.action.ActionListener; -import org.opensearch.core.common.util.CollectionUtils; import org.opensearch.core.index.Index; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.gateway.remote.ClusterMetadataManifest.UploadedIndexMetadata; @@ -179,6 +178,7 @@ public class RemoteClusterStateService implements Closeable { // ToXContent Params with gateway mode. // We are using gateway context mode to persist all custom metadata. public static final ToXContent.Params FORMAT_PARAMS; + static { Map params = new HashMap<>(1); params.put(Metadata.CONTEXT_MODE_PARAM, Metadata.CONTEXT_MODE_GATEWAY); @@ -446,7 +446,7 @@ private String writeGlobalMetadata(ClusterState clusterState) throws IOException * Uploads provided IndexMetadata's to remote store in parallel. The call is blocking so the method waits for upload to finish and then return. * * @param clusterState current ClusterState - * @param toUpload list of IndexMetadata to upload + * @param toUpload list of IndexMetadata to upload * @return {@code List} list of IndexMetadata uploaded to remote */ private List writeIndexMetadataParallel( @@ -454,7 +454,7 @@ private List writeIndexMetadataParallel( List toUpload, List newIndexMetadataList ) throws IOException { - assert CollectionUtils.isEmpty(indexMetadataUploadListeners) == false : "indexMetadataUploadInterceptors can not be empty"; + assert Objects.nonNull(indexMetadataUploadListeners) : "indexMetadataUploadListeners can not be null"; int latchCount = toUpload.size() + indexMetadataUploadListeners.size(); List exceptionList = Collections.synchronizedList(new ArrayList<>(latchCount)); final CountDownLatch latch = new CountDownLatch(latchCount); @@ -482,24 +482,7 @@ private List writeIndexMetadataParallel( writeIndexMetadataAsync(clusterState, indexMetadata, latchedActionListener); } - for (IndexMetadataUploadListener listener : indexMetadataUploadListeners) { - // We are submitting the task for async execution to ensure that we are not blocking the cluster state upload - String interceptorName = listener.getClass().getSimpleName(); - String threadPoolName = listener.getThreadpoolName(); - assert ThreadPool.THREAD_POOL_TYPES.containsKey(threadPoolName) && ThreadPool.Names.SAME.equals(threadPoolName) == false; - threadpool.executor(threadPoolName).execute(() -> { - try { - listener.beforeNewIndexUpload( - newIndexMetadataList, - getIndexMetadataUploadActionListener(newIndexMetadataList, latch, exceptionList, interceptorName) - ); - } catch (IOException e) { - exceptionList.add( - new RemoteStateTransferException("Exception occurred while running interceptIndexCreation in " + interceptorName, e) - ); - } - }); - } + invokeIndexMetadataUploadListeners(newIndexMetadataList, latch, exceptionList); try { if (latch.await(getIndexMetadataUploadTimeout().millis(), TimeUnit.MILLISECONDS) == false) { @@ -540,19 +523,50 @@ private List writeIndexMetadataParallel( return result; } + /** + * Invokes the index metadata upload listener. + */ + private void invokeIndexMetadataUploadListeners( + List newIndexMetadataList, + CountDownLatch latch, + List exceptionList + ) { + for (IndexMetadataUploadListener listener : indexMetadataUploadListeners) { + // We are submitting the task for async execution to ensure that we are not blocking the cluster state upload + String listenerName = listener.getClass().getSimpleName(); + String threadPoolName = listener.getThreadpoolName(); + assert ThreadPool.THREAD_POOL_TYPES.containsKey(threadPoolName) && ThreadPool.Names.SAME.equals(threadPoolName) == false; + threadpool.executor(threadPoolName).execute(() -> { + try { + listener.beforeNewIndexUpload( + newIndexMetadataList, + getIndexMetadataUploadActionListener(newIndexMetadataList, latch, exceptionList, listenerName) + ); + } catch (IOException e) { + exceptionList.add( + new RemoteStateTransferException( + "Exception occurred while running invokeIndexMetadataUploadListeners in " + listenerName, + e + ) + ); + } + }); + } + } + private ActionListener getIndexMetadataUploadActionListener( List newIndexMetadataList, CountDownLatch latch, List exceptionList, - String interceptorName + String listenerName ) { long startTime = System.nanoTime(); return new LatchedActionListener<>( ActionListener.wrap( ignored -> logger.trace( new ParameterizedMessage( - "{} : Intercepted {} successfully tookTimeNs={}", - interceptorName, + "{} : Invoked listener={} successfully tookTimeNs={}", + listenerName, newIndexMetadataList, (System.nanoTime() - startTime) ) @@ -560,8 +574,8 @@ private ActionListener getIndexMetadataUploadActionListener( ex -> { logger.error( new ParameterizedMessage( - "{} : Exception during interception of {} tookTimeNs={}", - interceptorName, + "{} : Exception during invocation of listener={} tookTimeNs={}", + listenerName, newIndexMetadataList, (System.nanoTime() - startTime) ), @@ -577,8 +591,8 @@ private ActionListener getIndexMetadataUploadActionListener( /** * Allows async Upload of IndexMetadata to remote * - * @param clusterState current ClusterState - * @param indexMetadata {@link IndexMetadata} to upload + * @param clusterState current ClusterState + * @param indexMetadata {@link IndexMetadata} to upload * @param latchedActionListener listener to respond back on after upload finishes */ private void writeIndexMetadataAsync( @@ -795,7 +809,7 @@ static String getManifestFileName(long term, long version, boolean committed) { (committed ? "C" : "P"), // C for committed and P for published RemoteStoreUtils.invertLong(System.currentTimeMillis()), String.valueOf(MANIFEST_CURRENT_CODEC_VERSION) // Keep the codec version at last place only, during read we reads last place to - // determine codec version. + // determine codec version. ); } @@ -808,7 +822,7 @@ static String indexMetadataFileName(IndexMetadata indexMetadata) { RemoteStoreUtils.invertLong(indexMetadata.getVersion()), RemoteStoreUtils.invertLong(System.currentTimeMillis()), String.valueOf(INDEX_METADATA_CURRENT_CODEC_VERSION) // Keep the codec version at last place only, during read we reads last - // place to determine codec version. + // place to determine codec version. ); } @@ -830,8 +844,8 @@ private BlobPath getManifestFolderPath(String clusterName, String clusterUUID) { /** * Fetch latest index metadata from remote cluster state * - * @param clusterUUID uuid of cluster state to refer to in remote - * @param clusterName name of the cluster + * @param clusterUUID uuid of cluster state to refer to in remote + * @param clusterName name of the cluster * @param clusterMetadataManifest manifest file of cluster * @return {@code Map} latest IndexUUID to IndexMetadata map */ @@ -853,8 +867,8 @@ private Map getIndexMetadataMap( /** * Fetch index metadata from remote cluster state * - * @param clusterUUID uuid of cluster state to refer to in remote - * @param clusterName name of the cluster + * @param clusterUUID uuid of cluster state to refer to in remote + * @param clusterName name of the cluster * @param uploadedIndexMetadata {@link UploadedIndexMetadata} contains details about remote location of index metadata * @return {@link IndexMetadata} */ @@ -1046,6 +1060,7 @@ private List createClusterChain(final Map trimClusterUUIDs( @@ -1107,7 +1122,7 @@ private boolean isValidClusterUUID(ClusterMetadataManifest manifest) { * * @param clusterUUID uuid of cluster state to refer to in remote * @param clusterName name of the cluster - * @param limit max no of files to fetch + * @param limit max no of files to fetch * @return all manifest file names */ private List getManifestFileNames(String clusterName, String clusterUUID, int limit) throws IllegalStateException { @@ -1180,7 +1195,7 @@ private int getManifestCodecVersion(String fileName) { if (splitName.length == SPLITED_MANIFEST_FILE_LENGTH) { return Integer.parseInt(splitName[splitName.length - 1]); // Last value would be codec version. } else if (splitName.length < SPLITED_MANIFEST_FILE_LENGTH) { // Where codec is not part of file name, i.e. default codec version 0 - // is used. + // is used. return ClusterMetadataManifest.CODEC_V0; } else { throw new IllegalArgumentException("Manifest file name is corrupted"); @@ -1212,7 +1227,7 @@ public RemoteStateTransferException(String errorDesc, Throwable cause) { /** * Purges all remote cluster state against provided cluster UUIDs * - * @param clusterName name of the cluster + * @param clusterName name of the cluster * @param clusterUUIDs clusteUUIDs for which the remote state needs to be purged */ void deleteStaleUUIDsClusterMetadata(String clusterName, List clusterUUIDs) { @@ -1245,8 +1260,8 @@ public void onFailure(Exception e) { /** * Deletes older than last {@code versionsToRetain} manifests. Also cleans up unreferenced IndexMetadata associated with older manifests * - * @param clusterName name of the cluster - * @param clusterUUID uuid of cluster state to refer to in remote + * @param clusterName name of the cluster + * @param clusterUUID uuid of cluster state to refer to in remote * @param manifestsToRetain no of latest manifest files to keep in remote */ // package private for testing @@ -1365,7 +1380,8 @@ private void deleteStalePaths(String clusterName, String clusterUUID, List> TRANSLOG_PATH = Map.of(TRANSLOG, List.of(DATA, METADATA)); public static final Map> SEGMENT_PATH = Map.of(SEGMENTS, List.of(DataType.values())); + public static final Map> COMBINED_PATH; + + static { + Map> combinedPath = new HashMap<>(); + combinedPath.putAll(TRANSLOG_PATH); + combinedPath.putAll(SEGMENT_PATH); + COMBINED_PATH = Collections.unmodifiableMap(combinedPath); + } private static final String DEFAULT_VERSION = "1"; public static final String DIR = "remote-index-path"; public static final String FILE_NAME_FORMAT = "remote_path_%s"; diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteIndexPathUploader.java b/server/src/main/java/org/opensearch/index/remote/RemoteIndexPathUploader.java index 5e2b52dbedf2c..1984c3f462300 100644 --- a/server/src/main/java/org/opensearch/index/remote/RemoteIndexPathUploader.java +++ b/server/src/main/java/org/opensearch/index/remote/RemoteIndexPathUploader.java @@ -34,7 +34,6 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -45,6 +44,7 @@ import java.util.stream.Collectors; import static org.opensearch.gateway.remote.RemoteClusterStateService.INDEX_METADATA_UPLOAD_TIMEOUT_SETTING; +import static org.opensearch.index.remote.RemoteIndexPath.COMBINED_PATH; import static org.opensearch.index.remote.RemoteIndexPath.SEGMENT_PATH; import static org.opensearch.index.remote.RemoteIndexPath.TRANSLOG_PATH; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteDataAttributePresent; @@ -96,7 +96,7 @@ public String getThreadpoolName() { @Override public void beforeNewIndexUpload(List indexMetadataList, ActionListener actionListener) throws IOException { if (isRemoteDataAttributePresent == false) { - logger.trace("Skipping beforeNewIndexUpload as isRemoteDataAttributePresent=false"); + logger.trace("Skipping beforeNewIndexUpload as there are no remote indexes"); actionListener.onResponse(null); return; } @@ -150,6 +150,24 @@ public void beforeNewIndexUpload(List indexMetadataList, ActionLi } private void writeIndexPathAsync(IndexMetadata idxMD, CountDownLatch latch, List exceptionList) throws IOException { + if (isTranslogSegmentRepoSame) { + // If the repositories are same, then we need to upload a single file containing paths for both translog and segments. + writePathToRemoteStore(idxMD, translogRepository, latch, exceptionList, COMBINED_PATH); + } else { + // If the repositories are different, then we need to upload one file per segment and translog containing their individual + // paths. + writePathToRemoteStore(idxMD, translogRepository, latch, exceptionList, TRANSLOG_PATH); + writePathToRemoteStore(idxMD, segmentRepository, latch, exceptionList, SEGMENT_PATH); + } + } + + private void writePathToRemoteStore( + IndexMetadata idxMD, + BlobStoreRepository repository, + CountDownLatch latch, + List exceptionList, + Map> pathCreationMap + ) throws IOException { Map remoteCustomData = idxMD.getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY); RemoteStoreEnums.PathType pathType = RemoteStoreEnums.PathType.valueOf(remoteCustomData.get(RemoteStoreEnums.PathType.NAME)); RemoteStoreEnums.PathHashAlgorithm hashAlgorithm = RemoteStoreEnums.PathHashAlgorithm.valueOf( @@ -157,39 +175,15 @@ private void writeIndexPathAsync(IndexMetadata idxMD, CountDownLatch latch, List ); String indexUUID = idxMD.getIndexUUID(); int shardCount = idxMD.getNumberOfShards(); - BlobPath translogBasePath = translogRepository.basePath(); - BlobContainer translogBlobContainer = translogRepository.blobStore().blobContainer(translogBasePath.add(RemoteIndexPath.DIR)); - - if (isTranslogSegmentRepoSame) { - // If the repositories are same, then we need to upload a single file containing paths for both translog and segments. - Map> pathCreationMap = new HashMap<>(); - pathCreationMap.putAll(TRANSLOG_PATH); - pathCreationMap.putAll(SEGMENT_PATH); - REMOTE_INDEX_PATH_FORMAT.writeAsyncWithUrgentPriority( - new RemoteIndexPath(indexUUID, shardCount, translogBasePath, pathType, hashAlgorithm, pathCreationMap), - translogBlobContainer, - indexUUID, - getUploadPathLatchedActionListener(idxMD, latch, exceptionList, pathCreationMap) - ); - } else { - // If the repositories are different, then we need to upload one file per segment and translog containing their individual - // paths. - REMOTE_INDEX_PATH_FORMAT.writeAsyncWithUrgentPriority( - new RemoteIndexPath(indexUUID, shardCount, translogBasePath, pathType, hashAlgorithm, TRANSLOG_PATH), - translogBlobContainer, - indexUUID, - getUploadPathLatchedActionListener(idxMD, latch, exceptionList, TRANSLOG_PATH) - ); + BlobPath basePath = repository.basePath(); + BlobContainer blobContainer = repository.blobStore().blobContainer(basePath.add(RemoteIndexPath.DIR)); + REMOTE_INDEX_PATH_FORMAT.writeAsyncWithUrgentPriority( + new RemoteIndexPath(indexUUID, shardCount, basePath, pathType, hashAlgorithm, pathCreationMap), + blobContainer, + indexUUID, + getUploadPathLatchedActionListener(idxMD, latch, exceptionList, pathCreationMap) + ); - BlobPath segmentBasePath = segmentRepository.basePath(); - BlobContainer segmentBlobContainer = segmentRepository.blobStore().blobContainer(segmentBasePath.add(RemoteIndexPath.DIR)); - REMOTE_INDEX_PATH_FORMAT.writeAsyncWithUrgentPriority( - new RemoteIndexPath(indexUUID, shardCount, segmentBasePath, pathType, hashAlgorithm, SEGMENT_PATH), - segmentBlobContainer, - indexUUID, - getUploadPathLatchedActionListener(idxMD, latch, exceptionList, SEGMENT_PATH) - ); - } } private Repository validateAndGetRepository(String repoSetting) { From b1c5d7324c0016c55ee078c1066f0cc6e81c7bca Mon Sep 17 00:00:00 2001 From: Ashish Singh Date: Fri, 19 Apr 2024 14:15:17 +0530 Subject: [PATCH 12/15] Add javadoc to BlobStoreFormat class Signed-off-by: Ashish Singh --- .../repositories/blobstore/BlobStoreFormat.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreFormat.java b/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreFormat.java index 810dcadc56e08..0e8a98e1c62f6 100644 --- a/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreFormat.java +++ b/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreFormat.java @@ -25,7 +25,13 @@ import java.util.Locale; /** - * Standard format that has only content for writes. Read interface does not exist as it not yet required. + * Standard format that has only content for writes. Read interface does not exist as it not yet required. This format + * should be used for writing data from in-memory to remote store where there is no need for checksum and the client + * library for the remote store has inbuilt checksum capabilities while upload and download both. This format would + * serialise the data in Json format and store it on remote store as is. This does not support compression yet (this + * can be changed as required). In comparison to {@link ChecksumBlobStoreFormat}, this format does not add any additional + * metadata (like header and footer) to the content. Hence, this format does not depend on {@code CodecUtil} from + * Lucene library. * * @opensearch.internal */ From a57054b371f788a60a5c451b14f402544c1448af Mon Sep 17 00:00:00 2001 From: Ashish Singh Date: Mon, 22 Apr 2024 11:09:08 +0530 Subject: [PATCH 13/15] Incoporate PR review feedback Signed-off-by: Ashish Singh --- .../remote/IndexMetadataUploadListener.java | 3 +-- .../remote/RemoteClusterStateService.java | 19 +++++-------------- .../index/remote/RemoteIndexPathUploader.java | 18 ++++++++++++++++-- .../main/java/org/opensearch/node/Node.java | 6 +++--- .../blobstore/ChecksumBlobStoreFormat.java | 3 +-- .../remote/RemoteIndexPathUploaderTests.java | 2 -- 6 files changed, 26 insertions(+), 25 deletions(-) diff --git a/server/src/main/java/org/opensearch/gateway/remote/IndexMetadataUploadListener.java b/server/src/main/java/org/opensearch/gateway/remote/IndexMetadataUploadListener.java index a19b7aea9491c..64cd27858c6a8 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/IndexMetadataUploadListener.java +++ b/server/src/main/java/org/opensearch/gateway/remote/IndexMetadataUploadListener.java @@ -11,7 +11,6 @@ import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.core.action.ActionListener; -import java.io.IOException; import java.util.List; /** @@ -30,7 +29,7 @@ public interface IndexMetadataUploadListener { * @param indexMetadataList list of index metadata of new indexes (or first time index metadata upload). * @param actionListener listener to be invoked on success or failure. */ - void beforeNewIndexUpload(List indexMetadataList, ActionListener actionListener) throws IOException; + void beforeNewIndexUpload(List indexMetadataList, ActionListener actionListener); String getThreadpoolName(); } diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java index a44aa1ce21153..a7fbf752108e2 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -524,7 +524,7 @@ private List writeIndexMetadataParallel( } /** - * Invokes the index metadata upload listener. + * Invokes the index metadata upload listener but does not wait for the execution to complete. */ private void invokeIndexMetadataUploadListeners( List newIndexMetadataList, @@ -537,19 +537,10 @@ private void invokeIndexMetadataUploadListeners( String threadPoolName = listener.getThreadpoolName(); assert ThreadPool.THREAD_POOL_TYPES.containsKey(threadPoolName) && ThreadPool.Names.SAME.equals(threadPoolName) == false; threadpool.executor(threadPoolName).execute(() -> { - try { - listener.beforeNewIndexUpload( - newIndexMetadataList, - getIndexMetadataUploadActionListener(newIndexMetadataList, latch, exceptionList, listenerName) - ); - } catch (IOException e) { - exceptionList.add( - new RemoteStateTransferException( - "Exception occurred while running invokeIndexMetadataUploadListeners in " + listenerName, - e - ) - ); - } + listener.beforeNewIndexUpload( + newIndexMetadataList, + getIndexMetadataUploadActionListener(newIndexMetadataList, latch, exceptionList, listenerName) + ); }); } } diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteIndexPathUploader.java b/server/src/main/java/org/opensearch/index/remote/RemoteIndexPathUploader.java index 1984c3f462300..8165ae594d41e 100644 --- a/server/src/main/java/org/opensearch/index/remote/RemoteIndexPathUploader.java +++ b/server/src/main/java/org/opensearch/index/remote/RemoteIndexPathUploader.java @@ -53,6 +53,8 @@ /** * Uploads the remote store path for all possible combinations of {@link org.opensearch.index.remote.RemoteStoreEnums.DataCategory} * and {@link org.opensearch.index.remote.RemoteStoreEnums.DataType} for each shard of an index. + * + * @opensearch.internal */ @ExperimentalApi public class RemoteIndexPathUploader implements IndexMetadataUploadListener { @@ -94,7 +96,7 @@ public String getThreadpoolName() { } @Override - public void beforeNewIndexUpload(List indexMetadataList, ActionListener actionListener) throws IOException { + public void beforeNewIndexUpload(List indexMetadataList, ActionListener actionListener) { if (isRemoteDataAttributePresent == false) { logger.trace("Skipping beforeNewIndexUpload as there are no remote indexes"); actionListener.onResponse(null); @@ -109,7 +111,16 @@ public void beforeNewIndexUpload(List indexMetadataList, ActionLi CountDownLatch latch = new CountDownLatch(latchCount); List exceptionList = Collections.synchronizedList(new ArrayList<>(latchCount)); for (IndexMetadata indexMetadata : eligibleList) { - writeIndexPathAsync(indexMetadata, latch, exceptionList); + try { + writeIndexPathAsync(indexMetadata, latch, exceptionList); + } catch (IOException exception) { + RemoteStateTransferException ex = new RemoteStateTransferException( + String.format(Locale.ROOT, UPLOAD_EXCEPTION_MSG, List.of(indexMetadata.getIndex().getName())) + ); + exceptionList.forEach(ex::addSuppressed); + actionListener.onFailure(ex); + return; + } } String indexNames = eligibleList.stream().map(IndexMetadata::getIndex).map(Index::toString).collect(Collectors.joining(",")); logger.trace(new ParameterizedMessage("Remote index path upload started for {}", indexNames)); @@ -142,6 +153,9 @@ public void beforeNewIndexUpload(List indexMetadataList, ActionLi } success = true; actionListener.onResponse(null); + } catch (Exception ex) { + actionListener.onFailure(ex); + throw ex; } finally { long tookTimeNs = System.nanoTime() - startTime; logger.trace(new ParameterizedMessage("executed beforeNewIndexUpload status={} tookTimeNs={}", success, tookTimeNs)); diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index 70357a707d6c0..3e3348ae43a0b 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -1472,9 +1472,9 @@ public Node start() throws NodeValidationException { if (remoteClusterStateService != null) { remoteClusterStateService.start(); } - final RemoteIndexPathUploader indexCreationListener = injector.getInstance(RemoteIndexPathUploader.class); - if (indexCreationListener != null) { - indexCreationListener.start(); + final RemoteIndexPathUploader remoteIndexPathUploader = injector.getInstance(RemoteIndexPathUploader.class); + if (remoteIndexPathUploader != null) { + remoteIndexPathUploader.start(); } // Load (and maybe upgrade) the metadata stored on disk final GatewayMetaState gatewayMetaState = injector.getInstance(GatewayMetaState.class); diff --git a/server/src/main/java/org/opensearch/repositories/blobstore/ChecksumBlobStoreFormat.java b/server/src/main/java/org/opensearch/repositories/blobstore/ChecksumBlobStoreFormat.java index 294022f00a2c1..3e6052a5ef820 100644 --- a/server/src/main/java/org/opensearch/repositories/blobstore/ChecksumBlobStoreFormat.java +++ b/server/src/main/java/org/opensearch/repositories/blobstore/ChecksumBlobStoreFormat.java @@ -300,7 +300,6 @@ public BytesReference serialize(final T obj, final String blobName, final Compre ) ) { CodecUtil.writeHeader(indexOutput, codec, VERSION); - XContentType xContentType = XContentType.SMILE; try (OutputStream indexOutputOutputStream = new IndexOutputOutputStream(indexOutput) { @Override public void close() throws IOException { @@ -309,7 +308,7 @@ public void close() throws IOException { } }; XContentBuilder builder = MediaTypeRegistry.contentBuilder( - xContentType, + XContentType.SMILE, compressor.threadLocalOutputStream(indexOutputOutputStream) ) ) { diff --git a/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathUploaderTests.java b/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathUploaderTests.java index 07bf041ce8312..c70523cf542a3 100644 --- a/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathUploaderTests.java +++ b/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathUploaderTests.java @@ -251,8 +251,6 @@ public void testInterceptWithInterruptedExceptionDuringLatchAwait() throws Excep Thread thread = new Thread(() -> { try { remoteIndexPathUploader.beforeNewIndexUpload(indexMetadataList, actionListener); - } catch (IOException e) { - throw new AssertionError(e); } catch (Exception e) { assertTrue(e instanceof InterruptedException); assertEquals("sleep interrupted", e.getMessage()); From a58d712f0fb8b8b067e7315cf7efaf97bfcb2cab Mon Sep 17 00:00:00 2001 From: Ashish Singh Date: Mon, 22 Apr 2024 16:55:22 +0530 Subject: [PATCH 14/15] Incoporate PR review feedback Signed-off-by: Ashish Singh --- .../remote/IndexMetadataUploadListener.java | 20 ++- .../remote/RemoteClusterStateService.java | 14 +- .../index/remote/RemoteIndexPathUploader.java | 78 ++++++----- .../main/java/org/opensearch/node/Node.java | 1 + .../blobstore/AbstractBlobStoreFormat.java | 130 ++++++++++++++++++ .../blobstore/ChecksumBlobStoreFormat.java | 74 +--------- ...Format.java => ConfigBlobStoreFormat.java} | 41 ++---- .../GatewayMetaStatePersistedStateTests.java | 2 +- .../RemoteClusterStateServiceTests.java | 4 +- .../remote/RemoteIndexPathUploaderTests.java | 85 +++++++++--- 10 files changed, 278 insertions(+), 171 deletions(-) create mode 100644 server/src/main/java/org/opensearch/repositories/blobstore/AbstractBlobStoreFormat.java rename server/src/main/java/org/opensearch/repositories/blobstore/{BlobStoreFormat.java => ConfigBlobStoreFormat.java} (66%) diff --git a/server/src/main/java/org/opensearch/gateway/remote/IndexMetadataUploadListener.java b/server/src/main/java/org/opensearch/gateway/remote/IndexMetadataUploadListener.java index 64cd27858c6a8..f9158c9260747 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/IndexMetadataUploadListener.java +++ b/server/src/main/java/org/opensearch/gateway/remote/IndexMetadataUploadListener.java @@ -10,8 +10,11 @@ import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.core.action.ActionListener; +import org.opensearch.threadpool.ThreadPool; import java.util.List; +import java.util.Objects; +import java.util.concurrent.ExecutorService; /** * Hook for running code that needs to be executed before the upload of index metadata. Here we have introduced a hook @@ -20,7 +23,16 @@ * * @opensearch.internal */ -public interface IndexMetadataUploadListener { +public abstract class IndexMetadataUploadListener { + + private final ExecutorService executorService; + + public IndexMetadataUploadListener(ThreadPool threadPool, String threadPoolName) { + Objects.requireNonNull(threadPool); + Objects.requireNonNull(threadPoolName); + assert ThreadPool.THREAD_POOL_TYPES.containsKey(threadPoolName) && ThreadPool.Names.SAME.equals(threadPoolName) == false; + this.executorService = threadPool.executor(threadPoolName); + } /** * Runs before the new index upload of index metadata (or first time upload). The caller is expected to trigger @@ -29,7 +41,9 @@ public interface IndexMetadataUploadListener { * @param indexMetadataList list of index metadata of new indexes (or first time index metadata upload). * @param actionListener listener to be invoked on success or failure. */ - void beforeNewIndexUpload(List indexMetadataList, ActionListener actionListener); + public final void onNewIndexUpload(List indexMetadataList, ActionListener actionListener) { + executorService.execute(() -> doOnNewIndexUpload(indexMetadataList, actionListener)); + } - String getThreadpoolName(); + protected abstract void doOnNewIndexUpload(List indexMetadataList, ActionListener actionListener); } diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java index a7fbf752108e2..d2f927c827e5b 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -532,17 +532,13 @@ private void invokeIndexMetadataUploadListeners( List exceptionList ) { for (IndexMetadataUploadListener listener : indexMetadataUploadListeners) { - // We are submitting the task for async execution to ensure that we are not blocking the cluster state upload String listenerName = listener.getClass().getSimpleName(); - String threadPoolName = listener.getThreadpoolName(); - assert ThreadPool.THREAD_POOL_TYPES.containsKey(threadPoolName) && ThreadPool.Names.SAME.equals(threadPoolName) == false; - threadpool.executor(threadPoolName).execute(() -> { - listener.beforeNewIndexUpload( - newIndexMetadataList, - getIndexMetadataUploadActionListener(newIndexMetadataList, latch, exceptionList, listenerName) - ); - }); + listener.onNewIndexUpload( + newIndexMetadataList, + getIndexMetadataUploadActionListener(newIndexMetadataList, latch, exceptionList, listenerName) + ); } + } private ActionListener getIndexMetadataUploadActionListener( diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteIndexPathUploader.java b/server/src/main/java/org/opensearch/index/remote/RemoteIndexPathUploader.java index 8165ae594d41e..1ac7e41014d23 100644 --- a/server/src/main/java/org/opensearch/index/remote/RemoteIndexPathUploader.java +++ b/server/src/main/java/org/opensearch/index/remote/RemoteIndexPathUploader.java @@ -27,8 +27,8 @@ import org.opensearch.node.remotestore.RemoteStoreNodeAttribute; import org.opensearch.repositories.RepositoriesService; import org.opensearch.repositories.Repository; -import org.opensearch.repositories.blobstore.BlobStoreFormat; import org.opensearch.repositories.blobstore.BlobStoreRepository; +import org.opensearch.repositories.blobstore.ConfigBlobStoreFormat; import org.opensearch.threadpool.ThreadPool; import java.io.IOException; @@ -57,9 +57,11 @@ * @opensearch.internal */ @ExperimentalApi -public class RemoteIndexPathUploader implements IndexMetadataUploadListener { +public class RemoteIndexPathUploader extends IndexMetadataUploadListener { - public static final BlobStoreFormat REMOTE_INDEX_PATH_FORMAT = new BlobStoreFormat<>(RemoteIndexPath.FILE_NAME_FORMAT); + public static final ConfigBlobStoreFormat REMOTE_INDEX_PATH_FORMAT = new ConfigBlobStoreFormat<>( + RemoteIndexPath.FILE_NAME_FORMAT + ); private static final String TIMEOUT_EXCEPTION_MSG = "Timed out waiting while uploading remote index path file for indexes=%s"; private static final String UPLOAD_EXCEPTION_MSG = "Exception occurred while uploading remote index paths for indexes=%s"; @@ -79,7 +81,13 @@ public class RemoteIndexPathUploader implements IndexMetadataUploadListener { private BlobStoreRepository translogRepository; private BlobStoreRepository segmentRepository; - public RemoteIndexPathUploader(Settings settings, Supplier repositoriesService, ClusterSettings clusterSettings) { + public RemoteIndexPathUploader( + ThreadPool threadPool, + Settings settings, + Supplier repositoriesService, + ClusterSettings clusterSettings + ) { + super(threadPool, ThreadPool.Names.GENERIC); this.settings = Objects.requireNonNull(settings); this.repositoriesService = Objects.requireNonNull(repositoriesService); isRemoteDataAttributePresent = isRemoteDataAttributePresent(settings); @@ -91,12 +99,7 @@ public RemoteIndexPathUploader(Settings settings, Supplier } @Override - public String getThreadpoolName() { - return ThreadPool.Names.GENERIC; - } - - @Override - public void beforeNewIndexUpload(List indexMetadataList, ActionListener actionListener) { + protected void doOnNewIndexUpload(List indexMetadataList, ActionListener actionListener) { if (isRemoteDataAttributePresent == false) { logger.trace("Skipping beforeNewIndexUpload as there are no remote indexes"); actionListener.onResponse(null); @@ -105,24 +108,16 @@ public void beforeNewIndexUpload(List indexMetadataList, ActionLi long startTime = System.nanoTime(); boolean success = false; + List eligibleList = indexMetadataList.stream().filter(this::requiresPathUpload).collect(Collectors.toList()); + String indexNames = eligibleList.stream().map(IndexMetadata::getIndex).map(Index::toString).collect(Collectors.joining(",")); + int latchCount = eligibleList.size() * (isTranslogSegmentRepoSame ? 1 : 2); + CountDownLatch latch = new CountDownLatch(latchCount); + List exceptionList = Collections.synchronizedList(new ArrayList<>(latchCount)); try { - List eligibleList = indexMetadataList.stream().filter(this::requiresPathUpload).collect(Collectors.toList()); - int latchCount = eligibleList.size() * (isTranslogSegmentRepoSame ? 1 : 2); - CountDownLatch latch = new CountDownLatch(latchCount); - List exceptionList = Collections.synchronizedList(new ArrayList<>(latchCount)); for (IndexMetadata indexMetadata : eligibleList) { - try { - writeIndexPathAsync(indexMetadata, latch, exceptionList); - } catch (IOException exception) { - RemoteStateTransferException ex = new RemoteStateTransferException( - String.format(Locale.ROOT, UPLOAD_EXCEPTION_MSG, List.of(indexMetadata.getIndex().getName())) - ); - exceptionList.forEach(ex::addSuppressed); - actionListener.onFailure(ex); - return; - } + writeIndexPathAsync(indexMetadata, latch, exceptionList); } - String indexNames = eligibleList.stream().map(IndexMetadata::getIndex).map(Index::toString).collect(Collectors.joining(",")); + logger.trace(new ParameterizedMessage("Remote index path upload started for {}", indexNames)); try { @@ -153,9 +148,13 @@ public void beforeNewIndexUpload(List indexMetadataList, ActionLi } success = true; actionListener.onResponse(null); - } catch (Exception ex) { + } catch (Exception exception) { + RemoteStateTransferException ex = new RemoteStateTransferException( + String.format(Locale.ROOT, UPLOAD_EXCEPTION_MSG, indexNames), + exception + ); + exceptionList.forEach(ex::addSuppressed); actionListener.onFailure(ex); - throw ex; } finally { long tookTimeNs = System.nanoTime() - startTime; logger.trace(new ParameterizedMessage("executed beforeNewIndexUpload status={} tookTimeNs={}", success, tookTimeNs)); @@ -163,7 +162,7 @@ public void beforeNewIndexUpload(List indexMetadataList, ActionLi } - private void writeIndexPathAsync(IndexMetadata idxMD, CountDownLatch latch, List exceptionList) throws IOException { + private void writeIndexPathAsync(IndexMetadata idxMD, CountDownLatch latch, List exceptionList) { if (isTranslogSegmentRepoSame) { // If the repositories are same, then we need to upload a single file containing paths for both translog and segments. writePathToRemoteStore(idxMD, translogRepository, latch, exceptionList, COMBINED_PATH); @@ -181,7 +180,7 @@ private void writePathToRemoteStore( CountDownLatch latch, List exceptionList, Map> pathCreationMap - ) throws IOException { + ) { Map remoteCustomData = idxMD.getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY); RemoteStoreEnums.PathType pathType = RemoteStoreEnums.PathType.valueOf(remoteCustomData.get(RemoteStoreEnums.PathType.NAME)); RemoteStoreEnums.PathHashAlgorithm hashAlgorithm = RemoteStoreEnums.PathHashAlgorithm.valueOf( @@ -191,13 +190,20 @@ private void writePathToRemoteStore( int shardCount = idxMD.getNumberOfShards(); BlobPath basePath = repository.basePath(); BlobContainer blobContainer = repository.blobStore().blobContainer(basePath.add(RemoteIndexPath.DIR)); - REMOTE_INDEX_PATH_FORMAT.writeAsyncWithUrgentPriority( - new RemoteIndexPath(indexUUID, shardCount, basePath, pathType, hashAlgorithm, pathCreationMap), - blobContainer, - indexUUID, - getUploadPathLatchedActionListener(idxMD, latch, exceptionList, pathCreationMap) - ); - + ActionListener actionListener = getUploadPathLatchedActionListener(idxMD, latch, exceptionList, pathCreationMap); + try { + REMOTE_INDEX_PATH_FORMAT.writeAsyncWithUrgentPriority( + new RemoteIndexPath(indexUUID, shardCount, basePath, pathType, hashAlgorithm, pathCreationMap), + blobContainer, + indexUUID, + actionListener + ); + } catch (IOException ioException) { + RemoteStateTransferException ex = new RemoteStateTransferException( + String.format(Locale.ROOT, UPLOAD_EXCEPTION_MSG, List.of(idxMD.getIndex().getName())) + ); + actionListener.onFailure(ioException); + } } private Repository validateAndGetRepository(String repoSetting) { diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index 3e3348ae43a0b..47f128af438a6 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -730,6 +730,7 @@ protected Node( final RemoteIndexPathUploader remoteIndexPathUploader; if (isRemoteStoreClusterStateEnabled(settings)) { remoteIndexPathUploader = new RemoteIndexPathUploader( + threadPool, settings, repositoriesServiceReference::get, clusterService.getClusterSettings() diff --git a/server/src/main/java/org/opensearch/repositories/blobstore/AbstractBlobStoreFormat.java b/server/src/main/java/org/opensearch/repositories/blobstore/AbstractBlobStoreFormat.java new file mode 100644 index 0000000000000..f49e738040305 --- /dev/null +++ b/server/src/main/java/org/opensearch/repositories/blobstore/AbstractBlobStoreFormat.java @@ -0,0 +1,130 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.repositories.blobstore; + +import org.apache.lucene.codecs.CodecUtil; +import org.apache.lucene.store.OutputStreamIndexOutput; +import org.opensearch.common.blobstore.BlobContainer; +import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.common.lucene.store.IndexOutputOutputStream; +import org.opensearch.common.xcontent.XContentType; +import org.opensearch.core.common.bytes.BytesReference; +import org.opensearch.core.compress.Compressor; +import org.opensearch.core.xcontent.MediaTypeRegistry; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.core.xcontent.XContentBuilder; + +import java.io.IOException; +import java.io.OutputStream; +import java.util.Locale; +import java.util.Objects; + +/** + * Provides common methods, variables that can be used by the implementors. + * + * @opensearch.internal + */ +public class AbstractBlobStoreFormat { + + private static final int BUFFER_SIZE = 4096; + + private final String blobNameFormat; + + private final boolean skipHeaderFooter; + + /** + * @param blobNameFormat format of the blobname in {@link String#format} format + */ + public AbstractBlobStoreFormat(String blobNameFormat, boolean skipHeaderFooter) { + this.blobNameFormat = blobNameFormat; + this.skipHeaderFooter = skipHeaderFooter; + } + + protected String blobName(String name) { + return String.format(Locale.ROOT, blobNameFormat, name); + } + + /** + * Writes blob with resolving the blob name using {@link #blobName} method. + *

+ * The blob will optionally by compressed. + * + * @param obj object to be serialized + * @param blobContainer blob container + * @param name blob name + * @param compressor whether to use compression + * @param params ToXContent params + * @param codec codec used + * @param version version used + */ + protected void write( + final T obj, + final BlobContainer blobContainer, + final String name, + final Compressor compressor, + final ToXContent.Params params, + XContentType xContentType, + String codec, + Integer version + ) throws IOException { + final String blobName = blobName(name); + final BytesReference bytes = serialize(obj, blobName, compressor, params, xContentType, codec, version); + blobContainer.writeBlob(blobName, bytes.streamInput(), bytes.length(), false); + } + + public BytesReference serialize( + final T obj, + final String blobName, + final Compressor compressor, + final ToXContent.Params params, + XContentType xContentType, + String codec, + Integer version + ) throws IOException { + assert skipHeaderFooter || (Objects.nonNull(codec) && Objects.nonNull(version)); + try (BytesStreamOutput outputStream = new BytesStreamOutput()) { + try ( + OutputStreamIndexOutput indexOutput = new OutputStreamIndexOutput( + "ChecksumBlobStoreFormat.writeBlob(blob=\"" + blobName + "\")", + blobName, + outputStream, + BUFFER_SIZE + ) + ) { + if (skipHeaderFooter == false) { + CodecUtil.writeHeader(indexOutput, codec, version); + } + try (OutputStream indexOutputOutputStream = new IndexOutputOutputStream(indexOutput) { + @Override + public void close() { + // this is important since some of the XContentBuilders write bytes on close. + // in order to write the footer we need to prevent closing the actual index input. + } + }; + XContentBuilder builder = MediaTypeRegistry.contentBuilder( + xContentType, + compressor.threadLocalOutputStream(indexOutputOutputStream) + ) + ) { + builder.startObject(); + obj.toXContent(builder, params); + builder.endObject(); + } + if (skipHeaderFooter == false) { + CodecUtil.writeFooter(indexOutput); + } + } + return outputStream.bytes(); + } + } + + protected String getBlobNameFormat() { + return blobNameFormat; + } +} diff --git a/server/src/main/java/org/opensearch/repositories/blobstore/ChecksumBlobStoreFormat.java b/server/src/main/java/org/opensearch/repositories/blobstore/ChecksumBlobStoreFormat.java index 3e6052a5ef820..de09fe2622ba2 100644 --- a/server/src/main/java/org/opensearch/repositories/blobstore/ChecksumBlobStoreFormat.java +++ b/server/src/main/java/org/opensearch/repositories/blobstore/ChecksumBlobStoreFormat.java @@ -38,7 +38,6 @@ import org.apache.lucene.store.ByteBuffersDataInput; import org.apache.lucene.store.ByteBuffersIndexInput; import org.apache.lucene.store.IndexInput; -import org.apache.lucene.store.OutputStreamIndexOutput; import org.apache.lucene.util.BytesRef; import org.opensearch.cluster.metadata.Metadata; import org.opensearch.common.CheckedFunction; @@ -48,26 +47,21 @@ import org.opensearch.common.blobstore.transfer.RemoteTransferContainer; import org.opensearch.common.blobstore.transfer.stream.OffsetRangeIndexInputStream; import org.opensearch.common.io.Streams; -import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.common.lucene.store.ByteArrayIndexInput; -import org.opensearch.common.lucene.store.IndexOutputOutputStream; import org.opensearch.common.xcontent.LoggingDeprecationHandler; import org.opensearch.common.xcontent.XContentHelper; import org.opensearch.common.xcontent.XContentType; import org.opensearch.core.action.ActionListener; import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.core.compress.Compressor; -import org.opensearch.core.xcontent.MediaTypeRegistry; import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.core.xcontent.ToXContent; -import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; import org.opensearch.gateway.CorruptStateException; import org.opensearch.index.store.exception.ChecksumCombinationException; import org.opensearch.snapshots.SnapshotInfo; import java.io.IOException; -import java.io.OutputStream; import java.util.Arrays; import java.util.HashMap; import java.util.Locale; @@ -80,7 +74,7 @@ * * @opensearch.internal */ -public final class ChecksumBlobStoreFormat { +public final class ChecksumBlobStoreFormat extends AbstractBlobStoreFormat { // Serialization parameters to specify correct context for metadata serialization public static final ToXContent.Params SNAPSHOT_ONLY_FORMAT_PARAMS; @@ -98,12 +92,8 @@ public final class ChecksumBlobStoreFormat { // The format version public static final int VERSION = 1; - private static final int BUFFER_SIZE = 4096; - private final String codec; - private final String blobNameFormat; - private final CheckedFunction reader; /** @@ -112,8 +102,8 @@ public final class ChecksumBlobStoreFormat { * @param reader prototype object that can deserialize T from XContent */ public ChecksumBlobStoreFormat(String codec, String blobNameFormat, CheckedFunction reader) { + super(blobNameFormat, false); this.reader = reader; - this.blobNameFormat = blobNameFormat; this.codec = codec; } @@ -130,7 +120,7 @@ public T read(BlobContainer blobContainer, String name, NamedXContentRegistry na } public String blobName(String name) { - return String.format(Locale.ROOT, blobNameFormat, name); + return String.format(Locale.ROOT, getBlobNameFormat(), name); } public T deserialize(String blobName, NamedXContentRegistry namedXContentRegistry, BytesReference bytes) throws IOException { @@ -170,30 +160,7 @@ public T deserialize(String blobName, NamedXContentRegistry namedXContentRegistr * @param compressor whether to use compression */ public void write(final T obj, final BlobContainer blobContainer, final String name, final Compressor compressor) throws IOException { - write(obj, blobContainer, name, compressor, SNAPSHOT_ONLY_FORMAT_PARAMS); - } - - /** - * Writes blob with resolving the blob name using {@link #blobName} method. - *

- * The blob will optionally by compressed. - * - * @param obj object to be serialized - * @param blobContainer blob container - * @param name blob name - * @param compressor whether to use compression - * @param params ToXContent params - */ - public void write( - final T obj, - final BlobContainer blobContainer, - final String name, - final Compressor compressor, - final ToXContent.Params params - ) throws IOException { - final String blobName = blobName(name); - final BytesReference bytes = serialize(obj, blobName, compressor, params); - blobContainer.writeBlob(blobName, bytes.streamInput(), bytes.length(), false); + write(obj, blobContainer, name, compressor, SNAPSHOT_ONLY_FORMAT_PARAMS, XContentType.SMILE, codec, VERSION); } /** @@ -251,7 +218,7 @@ private void writeAsyncWithPriority( final ToXContent.Params params ) throws IOException { if (blobContainer instanceof AsyncMultiStreamBlobContainer == false) { - write(obj, blobContainer, name, compressor, params); + write(obj, blobContainer, name, compressor, params, XContentType.SMILE, codec, VERSION); listener.onResponse(null); return; } @@ -290,35 +257,6 @@ private void writeAsyncWithPriority( public BytesReference serialize(final T obj, final String blobName, final Compressor compressor, final ToXContent.Params params) throws IOException { - try (BytesStreamOutput outputStream = new BytesStreamOutput()) { - try ( - OutputStreamIndexOutput indexOutput = new OutputStreamIndexOutput( - "ChecksumBlobStoreFormat.writeBlob(blob=\"" + blobName + "\")", - blobName, - outputStream, - BUFFER_SIZE - ) - ) { - CodecUtil.writeHeader(indexOutput, codec, VERSION); - try (OutputStream indexOutputOutputStream = new IndexOutputOutputStream(indexOutput) { - @Override - public void close() throws IOException { - // this is important since some of the XContentBuilders write bytes on close. - // in order to write the footer we need to prevent closing the actual index input. - } - }; - XContentBuilder builder = MediaTypeRegistry.contentBuilder( - XContentType.SMILE, - compressor.threadLocalOutputStream(indexOutputOutputStream) - ) - ) { - builder.startObject(); - obj.toXContent(builder, params); - builder.endObject(); - } - CodecUtil.writeFooter(indexOutput); - } - return outputStream.bytes(); - } + return serialize(obj, blobName, compressor, params, XContentType.SMILE, codec, VERSION); } } diff --git a/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreFormat.java b/server/src/main/java/org/opensearch/repositories/blobstore/ConfigBlobStoreFormat.java similarity index 66% rename from server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreFormat.java rename to server/src/main/java/org/opensearch/repositories/blobstore/ConfigBlobStoreFormat.java index 0e8a98e1c62f6..98aca64498d44 100644 --- a/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreFormat.java +++ b/server/src/main/java/org/opensearch/repositories/blobstore/ConfigBlobStoreFormat.java @@ -15,17 +15,16 @@ import org.opensearch.common.blobstore.transfer.RemoteTransferContainer; import org.opensearch.common.blobstore.transfer.stream.OffsetRangeIndexInputStream; import org.opensearch.common.lucene.store.ByteArrayIndexInput; -import org.opensearch.common.xcontent.XContentFactory; +import org.opensearch.common.xcontent.XContentType; import org.opensearch.core.action.ActionListener; import org.opensearch.core.common.bytes.BytesReference; +import org.opensearch.core.compress.NoneCompressor; import org.opensearch.core.xcontent.ToXContent; -import org.opensearch.core.xcontent.XContentBuilder; import java.io.IOException; -import java.util.Locale; /** - * Standard format that has only content for writes. Read interface does not exist as it not yet required. This format + * Format for writing short configurations to remote. Read interface does not exist as it not yet required. This format * should be used for writing data from in-memory to remote store where there is no need for checksum and the client * library for the remote store has inbuilt checksum capabilities while upload and download both. This format would * serialise the data in Json format and store it on remote store as is. This does not support compression yet (this @@ -35,39 +34,24 @@ * * @opensearch.internal */ -public class BlobStoreFormat { - - private final String blobNameFormat; +public class ConfigBlobStoreFormat extends AbstractBlobStoreFormat { /** * @param blobNameFormat format of the blobname in {@link String#format} format */ - public BlobStoreFormat(String blobNameFormat) { - this.blobNameFormat = blobNameFormat; - } - - private String blobName(String name) { - return String.format(Locale.ROOT, blobNameFormat, name); - } - - private BytesReference serialize(final T obj) throws IOException { - try (XContentBuilder xContentBuilder = XContentFactory.jsonBuilder()) { - xContentBuilder.startObject(); - obj.toXContent(xContentBuilder, ToXContent.EMPTY_PARAMS); - xContentBuilder.endObject(); - return BytesReference.bytes(xContentBuilder); - } + public ConfigBlobStoreFormat(String blobNameFormat) { + super(blobNameFormat, true); } public void writeAsyncWithUrgentPriority(T obj, BlobContainer blobContainer, String name, ActionListener listener) throws IOException { if (blobContainer instanceof AsyncMultiStreamBlobContainer == false) { - write(obj, blobContainer, name); + write(obj, blobContainer, name, new NoneCompressor(), ToXContent.EMPTY_PARAMS, XContentType.JSON, null, null); listener.onResponse(null); return; } String blobName = blobName(name); - BytesReference bytes = serialize(obj); + BytesReference bytes = serialize(obj, blobName, new NoneCompressor(), ToXContent.EMPTY_PARAMS, XContentType.JSON, null, null); String resourceDescription = "BlobStoreFormat.writeAsyncWithPriority(blob=\"" + blobName + "\")"; try (IndexInput input = new ByteArrayIndexInput(resourceDescription, BytesReference.toBytes(bytes))) { try ( @@ -79,18 +63,11 @@ public void writeAsyncWithUrgentPriority(T obj, BlobContainer blobContainer, Str WritePriority.URGENT, (size, position) -> new OffsetRangeIndexInputStream(input, size, position), null, - ((AsyncMultiStreamBlobContainer) blobContainer).remoteIntegrityCheckSupported() + false ) ) { ((AsyncMultiStreamBlobContainer) blobContainer).asyncBlobUpload(remoteTransferContainer.createWriteContext(), listener); } } } - - private void write(final T obj, final BlobContainer blobContainer, final String name) throws IOException { - String blobName = blobName(name); - BytesReference bytes = serialize(obj); - blobContainer.writeBlob(blobName, bytes.streamInput(), bytes.length(), false); - } - } diff --git a/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java b/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java index 012d2623b45df..3ba98c44f8d3e 100644 --- a/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java +++ b/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java @@ -490,7 +490,7 @@ public void testDataOnlyNodePersistence() throws Exception { clusterSettings, () -> 0L, threadPool, - List.of(new RemoteIndexPathUploader(settings, repositoriesServiceSupplier, clusterSettings)) + List.of(new RemoteIndexPathUploader(threadPool, settings, repositoriesServiceSupplier, clusterSettings)) ); } else { return null; diff --git a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java index 9ddbce3f794e6..9f321cd62847c 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -156,7 +156,7 @@ public void setup() { clusterSettings, () -> 0L, threadPool, - List.of(new RemoteIndexPathUploader(settings, repositoriesServiceSupplier, clusterSettings)) + List.of(new RemoteIndexPathUploader(threadPool, settings, repositoriesServiceSupplier, clusterSettings)) ); } @@ -185,7 +185,7 @@ public void testFailInitializationWhenRemoteStateDisabled() { clusterSettings, () -> 0L, threadPool, - List.of(new RemoteIndexPathUploader(settings, repositoriesServiceSupplier, clusterSettings)) + List.of(new RemoteIndexPathUploader(threadPool, settings, repositoriesServiceSupplier, clusterSettings)) ) ); } diff --git a/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathUploaderTests.java b/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathUploaderTests.java index c70523cf542a3..2e4dd15ccb581 100644 --- a/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathUploaderTests.java +++ b/server/src/test/java/org/opensearch/index/remote/RemoteIndexPathUploaderTests.java @@ -29,6 +29,9 @@ import org.opensearch.repositories.RepositoriesService; import org.opensearch.repositories.blobstore.BlobStoreRepository; import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.threadpool.TestThreadPool; +import org.opensearch.threadpool.ThreadPool; +import org.junit.After; import org.junit.Before; import java.io.IOException; @@ -62,6 +65,7 @@ public class RemoteIndexPathUploaderTests extends OpenSearchTestCase { private static final String TRANSLOG_REPO_NAME = "translog-repo"; private static final String SEGMENT_REPO_NAME = "segment-repo"; + private final ThreadPool threadPool = new TestThreadPool(getTestName()); private Settings settings; private ClusterSettings clusterSettings; private RepositoriesService repositoriesService; @@ -112,35 +116,56 @@ public void setup() { indexMetadataList = List.of(indexMetadata); } - public void testInterceptWithNoRemoteDataAttributes() throws IOException { + @After + public void tearDown() throws Exception { + super.tearDown(); + terminate(threadPool); + } + + public void testInterceptWithNoRemoteDataAttributes() { Settings settings = Settings.Builder.EMPTY_SETTINGS; clusterSettings.applySettings(settings); - RemoteIndexPathUploader remoteIndexPathUploader = new RemoteIndexPathUploader(settings, () -> repositoriesService, clusterSettings); + RemoteIndexPathUploader remoteIndexPathUploader = new RemoteIndexPathUploader( + threadPool, + settings, + () -> repositoriesService, + clusterSettings + ); List indexMetadataList = Mockito.mock(List.class); ActionListener actionListener = ActionListener.wrap( res -> successCount.incrementAndGet(), ex -> failureCount.incrementAndGet() ); - remoteIndexPathUploader.beforeNewIndexUpload(indexMetadataList, actionListener); + remoteIndexPathUploader.doOnNewIndexUpload(indexMetadataList, actionListener); assertEquals(1, successCount.get()); assertEquals(0, failureCount.get()); verify(indexMetadataList, times(0)).stream(); } - public void testInterceptWithEmptyIndexMetadataList() throws IOException { - RemoteIndexPathUploader remoteIndexPathUploader = new RemoteIndexPathUploader(settings, () -> repositoriesService, clusterSettings); + public void testInterceptWithEmptyIndexMetadataList() { + RemoteIndexPathUploader remoteIndexPathUploader = new RemoteIndexPathUploader( + threadPool, + settings, + () -> repositoriesService, + clusterSettings + ); remoteIndexPathUploader.start(); ActionListener actionListener = ActionListener.wrap( res -> successCount.incrementAndGet(), ex -> failureCount.incrementAndGet() ); - remoteIndexPathUploader.beforeNewIndexUpload(Collections.emptyList(), actionListener); + remoteIndexPathUploader.doOnNewIndexUpload(Collections.emptyList(), actionListener); assertEquals(1, successCount.get()); assertEquals(0, failureCount.get()); } - public void testInterceptWithEmptyEligibleIndexMetadataList() throws IOException { - RemoteIndexPathUploader remoteIndexPathUploader = new RemoteIndexPathUploader(settings, () -> repositoriesService, clusterSettings); + public void testInterceptWithEmptyEligibleIndexMetadataList() { + RemoteIndexPathUploader remoteIndexPathUploader = new RemoteIndexPathUploader( + threadPool, + settings, + () -> repositoriesService, + clusterSettings + ); remoteIndexPathUploader.start(); ActionListener actionListener = ActionListener.wrap( res -> successCount.incrementAndGet(), @@ -151,39 +176,44 @@ public void testInterceptWithEmptyEligibleIndexMetadataList() throws IOException List indexMetadataList = new ArrayList<>(); IndexMetadata indexMetadata = mock(IndexMetadata.class); indexMetadataList.add(indexMetadata); - remoteIndexPathUploader.beforeNewIndexUpload(indexMetadataList, actionListener); + remoteIndexPathUploader.doOnNewIndexUpload(indexMetadataList, actionListener); assertEquals(1, successCount.get()); assertEquals(0, failureCount.get()); // Case 2 - Empty remoteCustomData when(indexMetadata.getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY)).thenReturn(new HashMap<>()); - remoteIndexPathUploader.beforeNewIndexUpload(indexMetadataList, actionListener); + remoteIndexPathUploader.doOnNewIndexUpload(indexMetadataList, actionListener); assertEquals(2, successCount.get()); assertEquals(0, failureCount.get()); // Case 3 - RemoteStoreEnums.PathType.NAME not in remoteCustomData map Map remoteCustomData = Map.of("test", "test"); when(indexMetadata.getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY)).thenReturn(remoteCustomData); - remoteIndexPathUploader.beforeNewIndexUpload(indexMetadataList, actionListener); + remoteIndexPathUploader.doOnNewIndexUpload(indexMetadataList, actionListener); assertEquals(3, successCount.get()); assertEquals(0, failureCount.get()); // Case 4 - RemoteStoreEnums.PathType.NAME is not HASHED_PREFIX remoteCustomData = Map.of(PathType.NAME, randomFrom(FIXED, HASHED_INFIX).name()); when(indexMetadata.getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY)).thenReturn(remoteCustomData); - remoteIndexPathUploader.beforeNewIndexUpload(indexMetadataList, actionListener); + remoteIndexPathUploader.doOnNewIndexUpload(indexMetadataList, actionListener); assertEquals(4, successCount.get()); assertEquals(0, failureCount.get()); } public void testInterceptWithSameRepo() throws IOException { - RemoteIndexPathUploader remoteIndexPathUploader = new RemoteIndexPathUploader(settings, () -> repositoriesService, clusterSettings); + RemoteIndexPathUploader remoteIndexPathUploader = new RemoteIndexPathUploader( + threadPool, + settings, + () -> repositoriesService, + clusterSettings + ); remoteIndexPathUploader.start(); ActionListener actionListener = ActionListener.wrap( res -> successCount.incrementAndGet(), ex -> failureCount.incrementAndGet() ); - remoteIndexPathUploader.beforeNewIndexUpload(indexMetadataList, actionListener); + remoteIndexPathUploader.doOnNewIndexUpload(indexMetadataList, actionListener); assertEquals(1, successCount.get()); assertEquals(0, failureCount.get()); verify(blobContainer, times(1)).writeBlob(anyString(), any(InputStream.class), anyLong(), anyBoolean()); @@ -195,13 +225,18 @@ public void testInterceptWithDifferentRepo() throws IOException { .put(RemoteIndexPathUploader.SEGMENT_REPO_NAME_KEY, SEGMENT_REPO_NAME) .build(); when(repositoriesService.repository(SEGMENT_REPO_NAME)).thenReturn(repository); - RemoteIndexPathUploader remoteIndexPathUploader = new RemoteIndexPathUploader(settings, () -> repositoriesService, clusterSettings); + RemoteIndexPathUploader remoteIndexPathUploader = new RemoteIndexPathUploader( + threadPool, + settings, + () -> repositoriesService, + clusterSettings + ); remoteIndexPathUploader.start(); ActionListener actionListener = ActionListener.wrap( res -> successCount.incrementAndGet(), ex -> failureCount.incrementAndGet() ); - remoteIndexPathUploader.beforeNewIndexUpload(indexMetadataList, actionListener); + remoteIndexPathUploader.doOnNewIndexUpload(indexMetadataList, actionListener); assertEquals(1, successCount.get()); assertEquals(0, failureCount.get()); verify(blobContainer, times(2)).writeBlob(anyString(), any(InputStream.class), anyLong(), anyBoolean()); @@ -210,7 +245,12 @@ public void testInterceptWithDifferentRepo() throws IOException { public void testInterceptWithLatchAwaitTimeout() throws IOException { blobContainer = mock(AsyncMultiStreamBlobContainer.class); when(blobStore.blobContainer(any(BlobPath.class))).thenReturn(blobContainer); - RemoteIndexPathUploader remoteIndexPathUploader = new RemoteIndexPathUploader(settings, () -> repositoriesService, clusterSettings); + RemoteIndexPathUploader remoteIndexPathUploader = new RemoteIndexPathUploader( + threadPool, + settings, + () -> repositoriesService, + clusterSettings + ); remoteIndexPathUploader.start(); Settings settings = Settings.builder() @@ -223,7 +263,7 @@ public void testInterceptWithLatchAwaitTimeout() throws IOException { failureCount.incrementAndGet(); exceptionSetOnce.set(ex); }); - remoteIndexPathUploader.beforeNewIndexUpload(indexMetadataList, actionListener); + remoteIndexPathUploader.doOnNewIndexUpload(indexMetadataList, actionListener); assertEquals(0, successCount.get()); assertEquals(1, failureCount.get()); assertTrue(exceptionSetOnce.get() instanceof RemoteStateTransferException); @@ -236,7 +276,12 @@ public void testInterceptWithLatchAwaitTimeout() throws IOException { public void testInterceptWithInterruptedExceptionDuringLatchAwait() throws Exception { AsyncMultiStreamBlobContainer asyncMultiStreamBlobContainer = mock(AsyncMultiStreamBlobContainer.class); when(blobStore.blobContainer(any(BlobPath.class))).thenReturn(asyncMultiStreamBlobContainer); - RemoteIndexPathUploader remoteIndexPathUploader = new RemoteIndexPathUploader(settings, () -> repositoriesService, clusterSettings); + RemoteIndexPathUploader remoteIndexPathUploader = new RemoteIndexPathUploader( + threadPool, + settings, + () -> repositoriesService, + clusterSettings + ); remoteIndexPathUploader.start(); Settings settings = Settings.builder() .put(this.settings) @@ -250,7 +295,7 @@ public void testInterceptWithInterruptedExceptionDuringLatchAwait() throws Excep }); Thread thread = new Thread(() -> { try { - remoteIndexPathUploader.beforeNewIndexUpload(indexMetadataList, actionListener); + remoteIndexPathUploader.onNewIndexUpload(indexMetadataList, actionListener); } catch (Exception e) { assertTrue(e instanceof InterruptedException); assertEquals("sleep interrupted", e.getMessage()); From a8bf7af6711b409e85261b5faa2c98c2265edbbf Mon Sep 17 00:00:00 2001 From: Ashish Singh Date: Tue, 23 Apr 2024 18:58:00 +0530 Subject: [PATCH 15/15] Incoporate PR review feedback Signed-off-by: Ashish Singh --- ...{AbstractBlobStoreFormat.java => BaseBlobStoreFormat.java} | 4 ++-- .../repositories/blobstore/ChecksumBlobStoreFormat.java | 2 +- .../repositories/blobstore/ConfigBlobStoreFormat.java | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) rename server/src/main/java/org/opensearch/repositories/blobstore/{AbstractBlobStoreFormat.java => BaseBlobStoreFormat.java} (96%) diff --git a/server/src/main/java/org/opensearch/repositories/blobstore/AbstractBlobStoreFormat.java b/server/src/main/java/org/opensearch/repositories/blobstore/BaseBlobStoreFormat.java similarity index 96% rename from server/src/main/java/org/opensearch/repositories/blobstore/AbstractBlobStoreFormat.java rename to server/src/main/java/org/opensearch/repositories/blobstore/BaseBlobStoreFormat.java index f49e738040305..262a32fa1e74d 100644 --- a/server/src/main/java/org/opensearch/repositories/blobstore/AbstractBlobStoreFormat.java +++ b/server/src/main/java/org/opensearch/repositories/blobstore/BaseBlobStoreFormat.java @@ -30,7 +30,7 @@ * * @opensearch.internal */ -public class AbstractBlobStoreFormat { +public class BaseBlobStoreFormat { private static final int BUFFER_SIZE = 4096; @@ -41,7 +41,7 @@ public class AbstractBlobStoreFormat { /** * @param blobNameFormat format of the blobname in {@link String#format} format */ - public AbstractBlobStoreFormat(String blobNameFormat, boolean skipHeaderFooter) { + public BaseBlobStoreFormat(String blobNameFormat, boolean skipHeaderFooter) { this.blobNameFormat = blobNameFormat; this.skipHeaderFooter = skipHeaderFooter; } diff --git a/server/src/main/java/org/opensearch/repositories/blobstore/ChecksumBlobStoreFormat.java b/server/src/main/java/org/opensearch/repositories/blobstore/ChecksumBlobStoreFormat.java index de09fe2622ba2..e567e1b626c5a 100644 --- a/server/src/main/java/org/opensearch/repositories/blobstore/ChecksumBlobStoreFormat.java +++ b/server/src/main/java/org/opensearch/repositories/blobstore/ChecksumBlobStoreFormat.java @@ -74,7 +74,7 @@ * * @opensearch.internal */ -public final class ChecksumBlobStoreFormat extends AbstractBlobStoreFormat { +public final class ChecksumBlobStoreFormat extends BaseBlobStoreFormat { // Serialization parameters to specify correct context for metadata serialization public static final ToXContent.Params SNAPSHOT_ONLY_FORMAT_PARAMS; diff --git a/server/src/main/java/org/opensearch/repositories/blobstore/ConfigBlobStoreFormat.java b/server/src/main/java/org/opensearch/repositories/blobstore/ConfigBlobStoreFormat.java index 98aca64498d44..18c718ca2110e 100644 --- a/server/src/main/java/org/opensearch/repositories/blobstore/ConfigBlobStoreFormat.java +++ b/server/src/main/java/org/opensearch/repositories/blobstore/ConfigBlobStoreFormat.java @@ -34,7 +34,7 @@ * * @opensearch.internal */ -public class ConfigBlobStoreFormat extends AbstractBlobStoreFormat { +public class ConfigBlobStoreFormat extends BaseBlobStoreFormat { /** * @param blobNameFormat format of the blobname in {@link String#format} format