From 06ad2af978047477a1bcc47ca653c92e2973ab7f Mon Sep 17 00:00:00 2001 From: Henning Andersen Date: Mon, 25 Feb 2019 09:43:01 +0100 Subject: [PATCH] Unify blob store compress setting Blob store compression was all implemented generally, except reading the setting for it. Moved the setting to BlobStoreRepository to unify this. Also removed deprecated env setting 'repositories.fs.compress'. This is a follow up on #39073 --- .../repositories/url/URLRepository.java | 2 +- .../repositories/azure/AzureRepository.java | 5 +--- .../gcs/GoogleCloudStorageRepository.java | 8 +----- .../repositories/hdfs/HdfsRepository.java | 2 +- .../repositories/s3/S3Repository.java | 8 +----- .../common/settings/ClusterSettings.java | 1 - .../blobstore/BlobStoreRepository.java | 16 +++++++---- .../repositories/fs/FsRepository.java | 10 +------ .../blobstore/BlobStoreRepositoryTests.java | 27 +++++-------------- 9 files changed, 24 insertions(+), 55 deletions(-) diff --git a/modules/repository-url/src/main/java/org/elasticsearch/repositories/url/URLRepository.java b/modules/repository-url/src/main/java/org/elasticsearch/repositories/url/URLRepository.java index d314ce912ef66..4728e1b0d9eb6 100644 --- a/modules/repository-url/src/main/java/org/elasticsearch/repositories/url/URLRepository.java +++ b/modules/repository-url/src/main/java/org/elasticsearch/repositories/url/URLRepository.java @@ -83,7 +83,7 @@ public class URLRepository extends BlobStoreRepository { */ public URLRepository(RepositoryMetaData metadata, Environment environment, NamedXContentRegistry namedXContentRegistry) { - super(metadata, environment.settings(), false, namedXContentRegistry); + super(metadata, environment.settings(), namedXContentRegistry); if (URL_SETTING.exists(metadata.settings()) == false && REPOSITORIES_URL_SETTING.exists(environment.settings()) == false) { throw new RepositoryException(metadata.name(), "missing url"); 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 078e0e698aa51..f1790347c736a 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 @@ -75,21 +75,18 @@ public static final class Repository { s -> LocationMode.PRIMARY_ONLY.toString(), s -> LocationMode.valueOf(s.toUpperCase(Locale.ROOT)), Property.NodeScope); public static final Setting CHUNK_SIZE_SETTING = 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); public static final Setting READONLY_SETTING = Setting.boolSetting("readonly", false, Property.NodeScope); } private final BlobPath basePath; private final ByteSizeValue chunkSize; - private final Environment environment; private final AzureStorageService storageService; private final boolean readonly; public AzureRepository(RepositoryMetaData metadata, Environment environment, NamedXContentRegistry namedXContentRegistry, AzureStorageService storageService) { - super(metadata, environment.settings(), Repository.COMPRESS_SETTING.get(metadata.settings()), namedXContentRegistry); + super(metadata, environment.settings(), namedXContentRegistry); this.chunkSize = Repository.CHUNK_SIZE_SETTING.get(metadata.settings()); - this.environment = environment; this.storageService = storageService; final String basePath = Strings.trimLeadingCharacter(Repository.BASE_PATH_SETTING.get(metadata.settings()), '/'); 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 3192691d84389..9bcd6a8f6c527 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 @@ -25,7 +25,6 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.blobstore.BlobPath; import org.elasticsearch.common.settings.Setting; -import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeUnit; import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.xcontent.NamedXContentRegistry; @@ -36,7 +35,6 @@ import java.util.function.Function; import static org.elasticsearch.common.settings.Setting.Property; -import static org.elasticsearch.common.settings.Setting.boolSetting; import static org.elasticsearch.common.settings.Setting.byteSizeSetting; import static org.elasticsearch.common.settings.Setting.simpleString; @@ -53,13 +51,10 @@ class GoogleCloudStorageRepository extends BlobStoreRepository { simpleString("bucket", Property.NodeScope, Property.Dynamic); static final Setting BASE_PATH = simpleString("base_path", Property.NodeScope, Property.Dynamic); - static final Setting COMPRESS = - boolSetting("compress", false, Property.NodeScope, Property.Dynamic); static final Setting CHUNK_SIZE = byteSizeSetting("chunk_size", MAX_CHUNK_SIZE, MIN_CHUNK_SIZE, MAX_CHUNK_SIZE, Property.NodeScope, Property.Dynamic); static final Setting CLIENT_NAME = new Setting<>("client", "default", Function.identity()); - private final Settings settings; private final GoogleCloudStorageService storageService; private final BlobPath basePath; private final ByteSizeValue chunkSize; @@ -69,8 +64,7 @@ class GoogleCloudStorageRepository extends BlobStoreRepository { GoogleCloudStorageRepository(RepositoryMetaData metadata, Environment environment, NamedXContentRegistry namedXContentRegistry, GoogleCloudStorageService storageService) { - super(metadata, environment.settings(), getSetting(COMPRESS, metadata), namedXContentRegistry); - this.settings = environment.settings(); + super(metadata, environment.settings(), namedXContentRegistry); this.storageService = storageService; String basePath = BASE_PATH.get(metadata.settings()); diff --git a/plugins/repository-hdfs/src/main/java/org/elasticsearch/repositories/hdfs/HdfsRepository.java b/plugins/repository-hdfs/src/main/java/org/elasticsearch/repositories/hdfs/HdfsRepository.java index bba1b0031c85a..ac0ed7d24cf5e 100644 --- a/plugins/repository-hdfs/src/main/java/org/elasticsearch/repositories/hdfs/HdfsRepository.java +++ b/plugins/repository-hdfs/src/main/java/org/elasticsearch/repositories/hdfs/HdfsRepository.java @@ -68,7 +68,7 @@ public final class HdfsRepository extends BlobStoreRepository { public HdfsRepository(RepositoryMetaData metadata, Environment environment, NamedXContentRegistry namedXContentRegistry) { - super(metadata, environment.settings(), metadata.settings().getAsBoolean("compress", false), namedXContentRegistry); + super(metadata, environment.settings(), namedXContentRegistry); this.environment = environment; this.chunkSize = metadata.settings().getAsBytesSize("chunk_size", null); diff --git a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Repository.java b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Repository.java index 72ce6f8bf1f3e..5dfce233c9bb0 100644 --- a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Repository.java +++ b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Repository.java @@ -122,12 +122,6 @@ class S3Repository extends BlobStoreRepository { static final Setting CHUNK_SIZE_SETTING = Setting.byteSizeSetting("chunk_size", new ByteSizeValue(1, ByteSizeUnit.GB), new ByteSizeValue(5, ByteSizeUnit.MB), new ByteSizeValue(5, ByteSizeUnit.TB)); - /** - * When set to true metadata files are stored in compressed format. This setting doesn’t affect index - * files that are already compressed by default. Defaults to false. - */ - static final Setting COMPRESS_SETTING = Setting.boolSetting("compress", false); - /** * Sets the S3 storage class type for the backup files. Values may be standard, reduced_redundancy, * standard_ia. Defaults to standard. @@ -172,7 +166,7 @@ class S3Repository extends BlobStoreRepository { final Settings settings, final NamedXContentRegistry namedXContentRegistry, final S3Service service) { - super(metadata, settings, COMPRESS_SETTING.get(metadata.settings()), namedXContentRegistry); + super(metadata, settings, namedXContentRegistry); this.service = service; this.repositoryMetaData = metadata; diff --git a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java index 61c3dd9adadad..ebbc830a905c4 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java @@ -199,7 +199,6 @@ public void apply(Settings value, Settings current, Settings previous) { FilterAllocationDecider.CLUSTER_ROUTING_EXCLUDE_GROUP_SETTING, FilterAllocationDecider.CLUSTER_ROUTING_REQUIRE_GROUP_SETTING, FsRepository.REPOSITORIES_CHUNK_SIZE_SETTING, - FsRepository.REPOSITORIES_COMPRESS_SETTING, FsRepository.REPOSITORIES_LOCATION_SETTING, IndicesQueryCache.INDICES_CACHE_QUERY_SIZE_SETTING, IndicesQueryCache.INDICES_CACHE_QUERY_COUNT_SETTING, diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index 8858f46a39e82..4fee2fad41600 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -56,6 +56,7 @@ import org.elasticsearch.common.lucene.Lucene; import org.elasticsearch.common.lucene.store.InputStreamIndexInput; import org.elasticsearch.common.metrics.CounterMetric; +import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeUnit; import org.elasticsearch.common.unit.ByteSizeValue; @@ -193,6 +194,13 @@ public abstract class BlobStoreRepository extends AbstractLifecycleComponent imp private static final String DATA_BLOB_PREFIX = "__"; + /** + * When set to true metadata files are stored in compressed format. This setting doesn’t affect index + * files that are already compressed by default. Changing the setting does not invalidate existing files since reads + * do not observe the setting, instead they examine the file to see if it is compressed or not. + */ + public static final Setting COMPRESS_SETTING = Setting.boolSetting("compress", false, Setting.Property.NodeScope); + private final Settings settings; private final boolean compress; @@ -225,17 +233,15 @@ public abstract class BlobStoreRepository extends AbstractLifecycleComponent imp /** * Constructs new BlobStoreRepository - * - * @param metadata The metadata for this repository including name and settings + * @param metadata The metadata for this repository including name and settings * @param settings Settings for the node this repository object is created on - * @param compress true if metadata and snapshot files should be compressed */ - protected BlobStoreRepository(RepositoryMetaData metadata, Settings settings, boolean compress, + protected BlobStoreRepository(RepositoryMetaData metadata, Settings settings, NamedXContentRegistry namedXContentRegistry) { this.settings = settings; - this.compress = compress; this.metadata = metadata; this.namedXContentRegistry = namedXContentRegistry; + this.compress = COMPRESS_SETTING.get(metadata.settings()); snapshotRateLimiter = getRateLimiter(metadata.settings(), "max_snapshot_bytes_per_sec", new ByteSizeValue(40, ByteSizeUnit.MB)); restoreRateLimiter = getRateLimiter(metadata.settings(), "max_restore_bytes_per_sec", new ByteSizeValue(40, ByteSizeUnit.MB)); readOnly = metadata.settings().getAsBoolean("readonly", false); diff --git a/server/src/main/java/org/elasticsearch/repositories/fs/FsRepository.java b/server/src/main/java/org/elasticsearch/repositories/fs/FsRepository.java index ea438f03bf11e..e3e986c1eca9a 100644 --- a/server/src/main/java/org/elasticsearch/repositories/fs/FsRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/fs/FsRepository.java @@ -61,9 +61,6 @@ public class FsRepository extends BlobStoreRepository { new ByteSizeValue(Long.MAX_VALUE), new ByteSizeValue(5), new ByteSizeValue(Long.MAX_VALUE), Property.NodeScope); public static final Setting REPOSITORIES_CHUNK_SIZE_SETTING = Setting.byteSizeSetting("repositories.fs.chunk_size", new ByteSizeValue(Long.MAX_VALUE), new ByteSizeValue(5), new ByteSizeValue(Long.MAX_VALUE), Property.NodeScope); - public static final Setting COMPRESS_SETTING = Setting.boolSetting("compress", false, Property.NodeScope); - public static final Setting REPOSITORIES_COMPRESS_SETTING = - Setting.boolSetting("repositories.fs.compress", false, Property.NodeScope, Property.Deprecated); private final Environment environment; private ByteSizeValue chunkSize; @@ -75,7 +72,7 @@ public class FsRepository extends BlobStoreRepository { */ public FsRepository(RepositoryMetaData metadata, Environment environment, NamedXContentRegistry namedXContentRegistry) { - super(metadata, environment.settings(), calculateCompress(metadata, environment), namedXContentRegistry); + super(metadata, environment.settings(), namedXContentRegistry); this.environment = environment; String location = REPOSITORIES_LOCATION_SETTING.get(metadata.settings()); if (location.isEmpty()) { @@ -106,11 +103,6 @@ public FsRepository(RepositoryMetaData metadata, Environment environment, this.basePath = BlobPath.cleanPath(); } - private static boolean calculateCompress(RepositoryMetaData metadata, Environment environment) { - return COMPRESS_SETTING.exists(metadata.settings()) - ? COMPRESS_SETTING.get(metadata.settings()) : REPOSITORIES_COMPRESS_SETTING.get(environment.settings()); - } - @Override protected BlobStore createBlobStore() throws Exception { final String location = REPOSITORIES_LOCATION_SETTING.get(metadata.settings()); diff --git a/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryTests.java b/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryTests.java index a09560c54ce43..7246b708dd168 100644 --- a/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryTests.java +++ b/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryTests.java @@ -22,7 +22,6 @@ import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse; import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.client.Client; -import org.elasticsearch.cluster.metadata.RepositoryMetaData; import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeUnit; @@ -248,33 +247,20 @@ public void testBadChunksize() throws Exception { .get()); } - public void testFsRepositoryCompressDeprecated() { - final Path location = ESIntegTestCase.randomRepoPath(node().settings()); - final Settings settings = Settings.builder().put(node().settings()).put("location", location).build(); - final RepositoryMetaData metaData = new RepositoryMetaData("test-repo", REPO_TYPE, settings); - - Settings useCompressSettings = Settings.builder() - .put(node().getEnvironment().settings()) - .put(FsRepository.REPOSITORIES_COMPRESS_SETTING.getKey(), true) - .build(); - Environment useCompressEnvironment = - new Environment(useCompressSettings, node().getEnvironment().configFile()); - - new FsRepository(metaData, useCompressEnvironment, null); - - assertWarnings("[repositories.fs.compress] setting was deprecated in Elasticsearch and will be removed in a future release!" + - " See the breaking changes documentation for the next major version."); - } - private BlobStoreRepository setupRepo() { final Client client = client(); final Path location = ESIntegTestCase.randomRepoPath(node().settings()); final String repositoryName = "test-repo"; + Settings.Builder repoSettings = Settings.builder().put(node().settings()).put("location", location); + boolean compress = randomBoolean(); + if (compress) { + repoSettings.put(BlobStoreRepository.COMPRESS_SETTING.getKey(), true); + } AcknowledgedResponse putRepositoryResponse = client.admin().cluster().preparePutRepository(repositoryName) .setType(REPO_TYPE) - .setSettings(Settings.builder().put(node().settings()).put("location", location)) + .setSettings(repoSettings) .get(); assertThat(putRepositoryResponse.isAcknowledged(), equalTo(true)); @@ -282,6 +268,7 @@ private BlobStoreRepository setupRepo() { final BlobStoreRepository repository = (BlobStoreRepository) repositoriesService.repository(repositoryName); assertThat("getBlobContainer has to be lazy initialized", repository.getBlobContainer(), nullValue()); + assertEquals("Compress must be set to", compress, repository.isCompress()); return repository; }