diff --git a/plugins/repository-azure/src/main/java/org/elasticsearch/cloud/azure/storage/AzureStorageService.java b/plugins/repository-azure/src/main/java/org/elasticsearch/cloud/azure/storage/AzureStorageService.java index 3e854ab9c709e..6343541aed323 100644 --- a/plugins/repository-azure/src/main/java/org/elasticsearch/cloud/azure/storage/AzureStorageService.java +++ b/plugins/repository-azure/src/main/java/org/elasticsearch/cloud/azure/storage/AzureStorageService.java @@ -26,6 +26,7 @@ import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.unit.ByteSizeUnit; import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.unit.TimeValue; @@ -42,6 +43,9 @@ */ public interface AzureStorageService { + ByteSizeValue MIN_CHUNK_SIZE = new ByteSizeValue(1, ByteSizeUnit.BYTES); + ByteSizeValue MAX_CHUNK_SIZE = new ByteSizeValue(64, ByteSizeUnit.MB); + final class Storage { public static final String PREFIX = "cloud.azure.storage."; @@ -58,7 +62,7 @@ final class Storage { public static final Setting LOCATION_MODE_SETTING = Setting.simpleString("repositories.azure.location_mode", Property.NodeScope); public static final Setting CHUNK_SIZE_SETTING = - Setting.byteSizeSetting("repositories.azure.chunk_size", new ByteSizeValue(-1), Property.NodeScope); + Setting.byteSizeSetting("repositories.azure.chunk_size", MAX_CHUNK_SIZE, MIN_CHUNK_SIZE, MAX_CHUNK_SIZE, Property.NodeScope); public static final Setting COMPRESS_SETTING = Setting.boolSetting("repositories.azure.compress", false, Property.NodeScope); } diff --git a/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureRepository.java b/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureRepository.java index 1033578d52d18..17227971e495b 100644 --- a/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureRepository.java +++ b/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureRepository.java @@ -40,15 +40,14 @@ import org.elasticsearch.common.blobstore.BlobStore; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; -import org.elasticsearch.common.settings.SettingsException; -import org.elasticsearch.common.unit.ByteSizeUnit; import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.env.Environment; import org.elasticsearch.repositories.RepositoryVerificationException; import org.elasticsearch.repositories.blobstore.BlobStoreRepository; import org.elasticsearch.snapshots.SnapshotCreationException; -import static org.elasticsearch.cloud.azure.storage.AzureStorageSettings.getEffectiveSetting; +import static org.elasticsearch.cloud.azure.storage.AzureStorageService.MAX_CHUNK_SIZE; +import static org.elasticsearch.cloud.azure.storage.AzureStorageService.MIN_CHUNK_SIZE; import static org.elasticsearch.cloud.azure.storage.AzureStorageSettings.getValue; /** @@ -64,8 +63,6 @@ */ public class AzureRepository extends BlobStoreRepository { - private static final ByteSizeValue MAX_CHUNK_SIZE = new ByteSizeValue(64, ByteSizeUnit.MB); - public static final String TYPE = "azure"; public static final class Repository { @@ -75,7 +72,7 @@ public static final class Repository { public static final Setting BASE_PATH_SETTING = Setting.simpleString("base_path", Property.NodeScope); public static final Setting LOCATION_MODE_SETTING = Setting.simpleString("location_mode", Property.NodeScope); public static final Setting CHUNK_SIZE_SETTING = - Setting.byteSizeSetting("chunk_size", MAX_CHUNK_SIZE, Property.NodeScope); + Setting.byteSizeSetting("chunk_size", MAX_CHUNK_SIZE, MIN_CHUNK_SIZE, MAX_CHUNK_SIZE, Property.NodeScope); public static final Setting COMPRESS_SETTING = Setting.boolSetting("compress", false, Property.NodeScope); } @@ -92,14 +89,7 @@ public AzureRepository(RepositoryMetaData metadata, Environment environment, blobStore = new AzureBlobStore(metadata, environment.settings(), storageService); String container = getValue(metadata.settings(), settings, Repository.CONTAINER_SETTING, Storage.CONTAINER_SETTING); - ByteSizeValue configuredChunkSize = getValue(metadata.settings(), settings, Repository.CHUNK_SIZE_SETTING, Storage.CHUNK_SIZE_SETTING); - if (configuredChunkSize.getMb() > MAX_CHUNK_SIZE.getMb()) { - Setting setting = getEffectiveSetting(metadata.settings(), Repository.CHUNK_SIZE_SETTING, Storage.CHUNK_SIZE_SETTING); - throw new SettingsException("[" + setting.getKey() + "] must not exceed [" + MAX_CHUNK_SIZE + "] but is set to [" + configuredChunkSize + "]."); - } else { - this.chunkSize = configuredChunkSize; - } - + this.chunkSize = getValue(metadata.settings(), settings, Repository.CHUNK_SIZE_SETTING, Storage.CHUNK_SIZE_SETTING); this.compress = getValue(metadata.settings(), settings, Repository.COMPRESS_SETTING, Storage.COMPRESS_SETTING); String modeStr = getValue(metadata.settings(), settings, Repository.LOCATION_MODE_SETTING, Storage.LOCATION_MODE_SETTING); Boolean forcedReadonly = metadata.settings().getAsBoolean("readonly", null); diff --git a/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureRepositorySettingsTests.java b/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureRepositorySettingsTests.java index 99b79fc3b3212..014fa88cc4d8d 100644 --- a/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureRepositorySettingsTests.java +++ b/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureRepositorySettingsTests.java @@ -21,24 +21,19 @@ import com.microsoft.azure.storage.LocationMode; import com.microsoft.azure.storage.StorageException; -import com.microsoft.azure.storage.blob.CloudBlobClient; import org.elasticsearch.cloud.azure.storage.AzureStorageService; -import org.elasticsearch.cloud.azure.storage.AzureStorageServiceImpl; -import org.elasticsearch.cloud.azure.storage.AzureStorageSettings; import org.elasticsearch.cluster.metadata.RepositoryMetaData; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.unit.ByteSizeUnit; +import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.env.Environment; -import org.elasticsearch.env.NodeEnvironment; import org.elasticsearch.test.ESTestCase; import java.io.IOException; -import java.net.URI; import java.net.URISyntaxException; -import static org.elasticsearch.cloud.azure.storage.AzureStorageServiceImpl.blobNameFromUri; import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.nullValue; public class AzureRepositorySettingsTests extends ESTestCase { @@ -103,4 +98,30 @@ public void testReadonlyWithPrimaryAndSecondaryOnlyAndReadonlyOff() throws Stora .put("readonly", false) .build()).isReadOnly(), is(false)); } + + public void testChunkSize() throws StorageException, IOException, URISyntaxException { + // default chunk size + AzureRepository azureRepository = azureRepository(Settings.EMPTY); + assertEquals(AzureStorageService.MAX_CHUNK_SIZE, azureRepository.chunkSize()); + + // chunk size in settings + int size = randomIntBetween(1, 64); + azureRepository = azureRepository(Settings.builder().put("chunk_size", size + "mb").build()); + assertEquals(new ByteSizeValue(size, ByteSizeUnit.MB), azureRepository.chunkSize()); + + // zero bytes is not allowed + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> + azureRepository(Settings.builder().put("chunk_size", "0").build())); + assertEquals("Failed to parse value [0] for setting [chunk_size] must be >= 1b", e.getMessage()); + + // negative bytes not allowed + e = expectThrows(IllegalArgumentException.class, () -> + azureRepository(Settings.builder().put("chunk_size", "-1").build())); + assertEquals("Failed to parse value [-1] for setting [chunk_size] must be >= 1b", e.getMessage()); + + // greater than max chunk size not allowed + e = expectThrows(IllegalArgumentException.class, () -> + azureRepository(Settings.builder().put("chunk_size", "65mb").build())); + assertEquals("Failed to parse value [65mb] for setting [chunk_size] must be <= 64mb", e.getMessage()); + } } diff --git a/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageRepository.java b/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageRepository.java index c0a82f8266ad2..b7473a2d5fa6e 100644 --- a/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageRepository.java +++ b/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageRepository.java @@ -46,6 +46,10 @@ public class GoogleCloudStorageRepository extends BlobStoreRepository { + // package private for testing + static final ByteSizeValue MIN_CHUNK_SIZE = new ByteSizeValue(1, ByteSizeUnit.BYTES); + static final ByteSizeValue MAX_CHUNK_SIZE = new ByteSizeValue(100, ByteSizeUnit.MB); + public static final String TYPE = "gcs"; public static final TimeValue NO_TIMEOUT = timeValueMillis(-1); @@ -57,7 +61,7 @@ public class GoogleCloudStorageRepository extends BlobStoreRepository { public static final Setting COMPRESS = boolSetting("compress", false, Property.NodeScope, Property.Dynamic); public static final Setting CHUNK_SIZE = - byteSizeSetting("chunk_size", new ByteSizeValue(100, ByteSizeUnit.MB), Property.NodeScope, Property.Dynamic); + byteSizeSetting("chunk_size", MAX_CHUNK_SIZE, MIN_CHUNK_SIZE, MAX_CHUNK_SIZE, Property.NodeScope, Property.Dynamic); public static final Setting APPLICATION_NAME = new Setting<>("application_name", GoogleCloudStoragePlugin.NAME, Function.identity(), Property.NodeScope, Property.Dynamic); public static final Setting SERVICE_ACCOUNT = @@ -77,9 +81,9 @@ public GoogleCloudStorageRepository(RepositoryMetaData metadata, Environment env GoogleCloudStorageService storageService) throws Exception { super(metadata, environment.settings(), namedXContentRegistry); - String bucket = get(BUCKET, metadata); - String application = get(APPLICATION_NAME, metadata); - String serviceAccount = get(SERVICE_ACCOUNT, metadata); + String bucket = getSetting(BUCKET, metadata); + String application = getSetting(APPLICATION_NAME, metadata); + String serviceAccount = getSetting(SERVICE_ACCOUNT, metadata); String basePath = BASE_PATH.get(metadata.settings()); if (Strings.hasLength(basePath)) { @@ -105,8 +109,8 @@ public GoogleCloudStorageRepository(RepositoryMetaData metadata, Environment env readTimeout = timeout; } - this.compress = get(COMPRESS, metadata); - this.chunkSize = get(CHUNK_SIZE, metadata); + this.compress = getSetting(COMPRESS, metadata); + this.chunkSize = getSetting(CHUNK_SIZE, metadata); logger.debug("using bucket [{}], base_path [{}], chunk_size [{}], compress [{}], application [{}]", bucket, basePath, chunkSize, compress, application); @@ -139,7 +143,7 @@ protected ByteSizeValue chunkSize() { /** * Get a given setting from the repository settings, throwing a {@link RepositoryException} if the setting does not exist or is empty. */ - static T get(Setting setting, RepositoryMetaData metadata) { + static T getSetting(Setting setting, RepositoryMetaData metadata) { T value = setting.get(metadata.settings()); if (value == null) { throw new RepositoryException(metadata.name(), "Setting [" + setting.getKey() + "] is not defined for repository"); diff --git a/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobStoreRepositoryTests.java b/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobStoreRepositoryTests.java index 15cd647dc5cc3..eeb877dabf51c 100644 --- a/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobStoreRepositoryTests.java +++ b/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobStoreRepositoryTests.java @@ -20,9 +20,11 @@ package org.elasticsearch.repositories.gcs; import com.google.api.services.storage.Storage; +import org.elasticsearch.cluster.metadata.RepositoryMetaData; import org.elasticsearch.common.blobstore.gcs.MockHttpTransport; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeUnit; +import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.env.Environment; import org.elasticsearch.plugin.repository.gcs.GoogleCloudStoragePlugin; @@ -80,4 +82,42 @@ public Storage createClient(String serviceAccount, String application, TimeValue return storage.get(); } } + + public void testChunkSize() { + // default chunk size + RepositoryMetaData repositoryMetaData = new RepositoryMetaData("repo", GoogleCloudStorageRepository.TYPE, Settings.EMPTY); + ByteSizeValue chunkSize = GoogleCloudStorageRepository.getSetting(GoogleCloudStorageRepository.CHUNK_SIZE, repositoryMetaData); + assertEquals(GoogleCloudStorageRepository.MAX_CHUNK_SIZE, chunkSize); + + // chunk size in settings + int size = randomIntBetween(1, 100); + repositoryMetaData = new RepositoryMetaData("repo", GoogleCloudStorageRepository.TYPE, + Settings.builder().put("chunk_size", size + "mb").build()); + chunkSize = GoogleCloudStorageRepository.getSetting(GoogleCloudStorageRepository.CHUNK_SIZE, repositoryMetaData); + assertEquals(new ByteSizeValue(size, ByteSizeUnit.MB), chunkSize); + + // zero bytes is not allowed + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> { + RepositoryMetaData repoMetaData = new RepositoryMetaData("repo", GoogleCloudStorageRepository.TYPE, + Settings.builder().put("chunk_size", "0").build()); + GoogleCloudStorageRepository.getSetting(GoogleCloudStorageRepository.CHUNK_SIZE, repoMetaData); + }); + assertEquals("Failed to parse value [0] for setting [chunk_size] must be >= 1b", e.getMessage()); + + // negative bytes not allowed + e = expectThrows(IllegalArgumentException.class, () -> { + RepositoryMetaData repoMetaData = new RepositoryMetaData("repo", GoogleCloudStorageRepository.TYPE, + Settings.builder().put("chunk_size", "-1").build()); + GoogleCloudStorageRepository.getSetting(GoogleCloudStorageRepository.CHUNK_SIZE, repoMetaData); + }); + assertEquals("Failed to parse value [-1] for setting [chunk_size] must be >= 1b", e.getMessage()); + + // greater than max chunk size not allowed + e = expectThrows(IllegalArgumentException.class, () -> { + RepositoryMetaData repoMetaData = new RepositoryMetaData("repo", GoogleCloudStorageRepository.TYPE, + Settings.builder().put("chunk_size", "101mb").build()); + GoogleCloudStorageRepository.getSetting(GoogleCloudStorageRepository.CHUNK_SIZE, repoMetaData); + }); + assertEquals("Failed to parse value [101mb] for setting [chunk_size] must be <= 100mb", e.getMessage()); + } }