Fixes default chunk size for Azure repositories (#22577)

Before, the default chunk size for Azure repositories was
-1 bytes, which meant that if the chunk_size was not set on
the Azure repository, nor as a node setting, then no data
files would get written as part of the snapshot (because
the BlobStoreRepository's PartSliceStream does not know
how to process negative chunk sizes).

This commit fixes the default chunk size for Azure repositories
to be the same as the maximum chunk size.  This commit also
adds tests for both the Azure and Google Cloud repositories to
ensure only valid chunk sizes can be set.

Closes #22513
This commit is contained in:
Ali Beyad 2017-01-12 07:59:22 -06:00 committed by GitHub
parent df703dce0a
commit bdf836a286
5 changed files with 88 additions and 29 deletions

View File

@ -26,6 +26,7 @@ import org.elasticsearch.common.blobstore.BlobMetaData;
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 @@ import java.util.function.Function;
*/
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 @@ public interface AzureStorageService {
public static final Setting<String> LOCATION_MODE_SETTING =
Setting.simpleString("repositories.azure.location_mode", Property.NodeScope);
public static final Setting<ByteSizeValue> 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<Boolean> COMPRESS_SETTING =
Setting.boolSetting("repositories.azure.compress", false, Property.NodeScope);
}

View File

@ -40,15 +40,14 @@ import org.elasticsearch.common.blobstore.BlobPath;
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 @@ import static org.elasticsearch.cloud.azure.storage.AzureStorageSettings.getValu
*/
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 class AzureRepository extends BlobStoreRepository {
public static final Setting<String> BASE_PATH_SETTING = Setting.simpleString("base_path", Property.NodeScope);
public static final Setting<String> LOCATION_MODE_SETTING = Setting.simpleString("location_mode", Property.NodeScope);
public static final Setting<ByteSizeValue> 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<Boolean> COMPRESS_SETTING = Setting.boolSetting("compress", false, Property.NodeScope);
}
@ -92,14 +89,7 @@ public class AzureRepository extends BlobStoreRepository {
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<ByteSizeValue> 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);

View File

@ -21,24 +21,19 @@ package org.elasticsearch.repositories.azure;
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 class AzureRepositorySettingsTests extends ESTestCase {
.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());
}
}

View File

@ -46,6 +46,10 @@ import static org.elasticsearch.common.unit.TimeValue.timeValueMillis;
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<Boolean> COMPRESS =
boolSetting("compress", false, Property.NodeScope, Property.Dynamic);
public static final Setting<ByteSizeValue> 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<String> APPLICATION_NAME =
new Setting<>("application_name", GoogleCloudStoragePlugin.NAME, Function.identity(), Property.NodeScope, Property.Dynamic);
public static final Setting<String> SERVICE_ACCOUNT =
@ -77,9 +81,9 @@ public class GoogleCloudStorageRepository extends BlobStoreRepository {
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 class GoogleCloudStorageRepository extends BlobStoreRepository {
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 @@ public class GoogleCloudStorageRepository extends BlobStoreRepository {
/**
* Get a given setting from the repository settings, throwing a {@link RepositoryException} if the setting does not exist or is empty.
*/
static <T> T get(Setting<T> setting, RepositoryMetaData metadata) {
static <T> T getSetting(Setting<T> 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");

View File

@ -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 class GoogleCloudStorageBlobStoreRepositoryTests extends ESBlobStoreRepos
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());
}
}