diff --git a/plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3Repository.java b/plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3Repository.java index 5fc2114f0af66..afdc3c1665958 100644 --- a/plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3Repository.java +++ b/plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3Repository.java @@ -204,19 +204,19 @@ class S3Repository extends MeteredBlobStoreRepository { private final S3Service service; - private final String bucket; + private String bucket; - private final ByteSizeValue bufferSize; + private ByteSizeValue bufferSize; - private final ByteSizeValue chunkSize; + private ByteSizeValue chunkSize; - private final BlobPath basePath; + private BlobPath basePath; - private final boolean serverSideEncryption; + private boolean serverSideEncryption; - private final String storageClass; + private String storageClass; - private final String cannedACL; + private String cannedACL; private RepositoryMetadata repositoryMetadata; @@ -282,66 +282,11 @@ class S3Repository extends MeteredBlobStoreRepository { this.s3AsyncService = s3AsyncService; this.multipartUploadEnabled = multipartUploadEnabled; this.pluginConfigPath = pluginConfigPath; - - this.repositoryMetadata = metadata; this.asyncUploadUtils = asyncUploadUtils; this.priorityExecutorBuilder = priorityExecutorBuilder; this.normalExecutorBuilder = normalExecutorBuilder; - // Parse and validate the user's S3 Storage Class setting - this.bucket = BUCKET_SETTING.get(metadata.settings()); - if (bucket == null) { - throw new RepositoryException(metadata.name(), "No bucket defined for s3 repository"); - } - - this.bufferSize = BUFFER_SIZE_SETTING.get(metadata.settings()); - this.chunkSize = CHUNK_SIZE_SETTING.get(metadata.settings()); - - // We make sure that chunkSize is bigger or equal than/to bufferSize - if (this.chunkSize.getBytes() < bufferSize.getBytes()) { - throw new RepositoryException( - metadata.name(), - CHUNK_SIZE_SETTING.getKey() - + " (" - + this.chunkSize - + ") can't be lower than " - + BUFFER_SIZE_SETTING.getKey() - + " (" - + bufferSize - + ")." - ); - } - - final String basePath = BASE_PATH_SETTING.get(metadata.settings()); - if (Strings.hasLength(basePath)) { - this.basePath = new BlobPath().add(basePath); - } else { - this.basePath = BlobPath.cleanPath(); - } - - this.serverSideEncryption = SERVER_SIDE_ENCRYPTION_SETTING.get(metadata.settings()); - - this.storageClass = STORAGE_CLASS_SETTING.get(metadata.settings()); - this.cannedACL = CANNED_ACL_SETTING.get(metadata.settings()); - - if (S3ClientSettings.checkDeprecatedCredentials(metadata.settings())) { - // provided repository settings - deprecationLogger.deprecate( - "s3_repository_secret_settings", - "Using s3 access/secret key from repository settings. Instead " - + "store these in named clients and the opensearch keystore for secure settings." - ); - } - - logger.debug( - "using bucket [{}], chunk_size [{}], server_side_encryption [{}], buffer_size [{}], cannedACL [{}], storageClass [{}]", - bucket, - chunkSize, - serverSideEncryption, - bufferSize, - cannedACL, - storageClass - ); + readRepositoryMetadata(metadata); } private static Map buildLocation(RepositoryMetadata metadata) { @@ -420,10 +365,11 @@ public boolean isReloadable() { } @Override - public void reload(RepositoryMetadata newRepositoryMetadata, boolean compress) { + public void reload(RepositoryMetadata newRepositoryMetadata) { // Reload configs for S3Repository - super.reload(newRepositoryMetadata, compress); + super.reload(newRepositoryMetadata); repositoryMetadata = newRepositoryMetadata; + readRepositoryMetadata(repositoryMetadata); // Reload configs for S3RepositoryPlugin final Map clientsSettings = S3ClientSettings.load(repositoryMetadata.settings(), pluginConfigPath); @@ -432,7 +378,71 @@ public void reload(RepositoryMetadata newRepositoryMetadata, boolean compress) { // Reload configs for S3BlobStore BlobStore blobStore = getBlobStore(); - blobStore.reload(newRepositoryMetadata); + blobStore.reload(repositoryMetadata); + } + + /** + * Reloads the values derived from the Repository Metadata + * + * @param repositoryMetadata RepositoryMetadata instance to derive the values from + */ + private void readRepositoryMetadata(RepositoryMetadata repositoryMetadata) { + this.repositoryMetadata = metadata; + + // Parse and validate the user's S3 Storage Class setting + this.bucket = BUCKET_SETTING.get(metadata.settings()); + if (bucket == null) { + throw new RepositoryException(metadata.name(), "No bucket defined for s3 repository"); + } + + this.bufferSize = BUFFER_SIZE_SETTING.get(metadata.settings()); + this.chunkSize = CHUNK_SIZE_SETTING.get(metadata.settings()); + + // We make sure that chunkSize is bigger or equal than/to bufferSize + if (this.chunkSize.getBytes() < bufferSize.getBytes()) { + throw new RepositoryException( + metadata.name(), + CHUNK_SIZE_SETTING.getKey() + + " (" + + this.chunkSize + + ") can't be lower than " + + BUFFER_SIZE_SETTING.getKey() + + " (" + + bufferSize + + ")." + ); + } + + final String basePath = BASE_PATH_SETTING.get(metadata.settings()); + if (Strings.hasLength(basePath)) { + this.basePath = new BlobPath().add(basePath); + } else { + this.basePath = BlobPath.cleanPath(); + } + + this.serverSideEncryption = SERVER_SIDE_ENCRYPTION_SETTING.get(metadata.settings()); + + this.storageClass = STORAGE_CLASS_SETTING.get(metadata.settings()); + this.cannedACL = CANNED_ACL_SETTING.get(metadata.settings()); + + if (S3ClientSettings.checkDeprecatedCredentials(metadata.settings())) { + // provided repository settings + deprecationLogger.deprecate( + "s3_repository_secret_settings", + "Using s3 access/secret key from repository settings. Instead " + + "store these in named clients and the opensearch keystore for secure settings." + ); + } + + logger.debug( + "using bucket [{}], chunk_size [{}], server_side_encryption [{}], buffer_size [{}], cannedACL [{}], storageClass [{}]", + bucket, + chunkSize, + serverSideEncryption, + bufferSize, + cannedACL, + storageClass + ); } @Override diff --git a/server/src/main/java/org/opensearch/repositories/RepositoriesService.java b/server/src/main/java/org/opensearch/repositories/RepositoriesService.java index fa4cf99328ac2..b867f16e1f495 100644 --- a/server/src/main/java/org/opensearch/repositories/RepositoriesService.java +++ b/server/src/main/java/org/opensearch/repositories/RepositoriesService.java @@ -83,7 +83,6 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -import static org.opensearch.repositories.blobstore.BlobStoreRepository.COMPRESS_SETTING; import static org.opensearch.repositories.blobstore.BlobStoreRepository.REMOTE_STORE_INDEX_SHALLOW_COPY; /** @@ -459,7 +458,7 @@ public void applyClusterState(ClusterChangedEvent event) { // Previous version is different from the version in settings logger.debug("updating repository [{}]", repositoryMetadata.name()); if (repository.isSystemRepository() && repository.isReloadable()) { - repository.reload(repositoryMetadata, COMPRESS_SETTING.get(repositoryMetadata.settings())); + repository.reload(repositoryMetadata); } else { closeRepository(repository); archiveRepositoryStats(repository, state.version()); diff --git a/server/src/main/java/org/opensearch/repositories/Repository.java b/server/src/main/java/org/opensearch/repositories/Repository.java index c46640071a284..e05581d22e0fa 100644 --- a/server/src/main/java/org/opensearch/repositories/Repository.java +++ b/server/src/main/java/org/opensearch/repositories/Repository.java @@ -452,5 +452,5 @@ default boolean isReloadable() { /** * Reload the repository inplace */ - default void reload(RepositoryMetadata repositoryMetadata, boolean compress) {} + default void reload(RepositoryMetadata repositoryMetadata) {} } diff --git a/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java index b2ed0289864ec..b980441346482 100644 --- a/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java @@ -413,7 +413,7 @@ protected BlobStoreRepository( final RecoverySettings recoverySettings ) { // Read RepositoryMetadata as the first step - readRepositoryMetadata(repositoryMetadata, compress); + readRepositoryMetadata(repositoryMetadata); isSystemRepository = SYSTEM_REPOSITORY_SETTING.get(metadata.settings()); this.namedXContentRegistry = namedXContentRegistry; @@ -423,17 +423,16 @@ protected BlobStoreRepository( } @Override - public void reload(RepositoryMetadata repositoryMetadata, boolean compress) { - readRepositoryMetadata(repositoryMetadata, compress); + public void reload(RepositoryMetadata repositoryMetadata) { + readRepositoryMetadata(repositoryMetadata); } /** * Reloads the values derived from the Repository Metadata * * @param repositoryMetadata RepositoryMetadata instance to derive the values from - * @param compress boolean representing whether compression is to be used */ - private void readRepositoryMetadata(RepositoryMetadata repositoryMetadata, boolean compress) { + private void readRepositoryMetadata(RepositoryMetadata repositoryMetadata) { this.metadata = repositoryMetadata; supportURLRepo = SUPPORT_URL_REPO.get(metadata.settings()); @@ -445,7 +444,9 @@ private void readRepositoryMetadata(RepositoryMetadata repositoryMetadata, boole cacheRepositoryData = CACHE_REPOSITORY_DATA.get(metadata.settings()); bufferSize = Math.toIntExact(BUFFER_SIZE_SETTING.get(metadata.settings()).getBytes()); maxShardBlobDeleteBatch = MAX_SNAPSHOT_SHARD_BLOB_DELETE_BATCH_SIZE.get(metadata.settings()); - compressor = compress ? COMPRESSION_TYPE_SETTING.get(metadata.settings()) : CompressorRegistry.none(); + compressor = COMPRESS_SETTING.get(metadata.settings()) + ? COMPRESSION_TYPE_SETTING.get(metadata.settings()) + : CompressorRegistry.none(); } @Override diff --git a/server/src/main/java/org/opensearch/repositories/blobstore/MeteredBlobStoreRepository.java b/server/src/main/java/org/opensearch/repositories/blobstore/MeteredBlobStoreRepository.java index 5f076b2c5fcfd..d47bf147a740a 100644 --- a/server/src/main/java/org/opensearch/repositories/blobstore/MeteredBlobStoreRepository.java +++ b/server/src/main/java/org/opensearch/repositories/blobstore/MeteredBlobStoreRepository.java @@ -71,8 +71,8 @@ public MeteredBlobStoreRepository( } @Override - public void reload(RepositoryMetadata repositoryMetadata, boolean compress) { - super.reload(repositoryMetadata, compress); + public void reload(RepositoryMetadata repositoryMetadata) { + super.reload(repositoryMetadata); // Not adding any additional reload logic here is intentional as the constructor only // initializes the repositoryInfo from the repo metadata, which cannot be changed. diff --git a/server/src/main/java/org/opensearch/repositories/fs/ReloadableFsRepository.java b/server/src/main/java/org/opensearch/repositories/fs/ReloadableFsRepository.java index fe816f9d6f511..a40864413c226 100644 --- a/server/src/main/java/org/opensearch/repositories/fs/ReloadableFsRepository.java +++ b/server/src/main/java/org/opensearch/repositories/fs/ReloadableFsRepository.java @@ -54,8 +54,8 @@ public boolean isReloadable() { } @Override - public void reload(RepositoryMetadata repositoryMetadata, boolean compress) { - super.reload(repositoryMetadata, compress); + public void reload(RepositoryMetadata repositoryMetadata) { + super.reload(repositoryMetadata); metadata = repositoryMetadata; // TODO - deduplicate the below block diff --git a/server/src/test/java/org/opensearch/repositories/fs/ReloadableFsRepositoryTests.java b/server/src/test/java/org/opensearch/repositories/fs/ReloadableFsRepositoryTests.java index 92986bfa45326..a1c9f582438a9 100644 --- a/server/src/test/java/org/opensearch/repositories/fs/ReloadableFsRepositoryTests.java +++ b/server/src/test/java/org/opensearch/repositories/fs/ReloadableFsRepositoryTests.java @@ -69,7 +69,7 @@ public void testIsReloadable() { public void testCompressReload() { assertEquals(CompressorRegistry.none(), repository.getCompressor()); updateCompressionTypeToDefault(); - repository.reload(metadata, true); + repository.reload(metadata); assertEquals(CompressorRegistry.defaultCompressor(), repository.getCompressor()); } @@ -99,7 +99,7 @@ public void testCompressionTypeReload() { .put(FsRepository.BASE_PATH_SETTING.getKey(), "my_base_path") .build(); metadata = new RepositoryMetadata("test", "fs", settings); - repository.reload(metadata, true); + repository.reload(metadata); assertEquals(CompressorRegistry.getCompressor(ZstdCompressor.NAME.toUpperCase(Locale.ROOT)), repository.getCompressor()); }