From ac7ec99bea12325dcd31c6d94b666c75d593ff41 Mon Sep 17 00:00:00 2001 From: Henning Andersen <33268011+henningandersen@users.noreply.github.com> Date: Thu, 28 Feb 2019 12:34:13 +0100 Subject: [PATCH] Unify blob store compress setting (#39346) 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 --- docs/reference/migration/migrate_8_0.asciidoc | 2 ++ .../migration/migrate_8_0/snapshots.asciidoc | 13 +++++++++ .../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 +++++-------------- 11 files changed, 39 insertions(+), 55 deletions(-) create mode 100644 docs/reference/migration/migrate_8_0/snapshots.asciidoc diff --git a/docs/reference/migration/migrate_8_0.asciidoc b/docs/reference/migration/migrate_8_0.asciidoc index 58c2b1218231a..cef1290498cc8 100644 --- a/docs/reference/migration/migrate_8_0.asciidoc +++ b/docs/reference/migration/migrate_8_0.asciidoc @@ -13,6 +13,7 @@ coming[8.0.0] * <> * <> +* <> [float] === Indices created before 7.0 @@ -32,3 +33,4 @@ Elasticsearch 7.x in order to be readable by Elasticsearch 8.x. include::migrate_8_0/analysis.asciidoc[] include::migrate_8_0/mappings.asciidoc[] +include::migrate_8_0/snapshots.asciidoc[] diff --git a/docs/reference/migration/migrate_8_0/snapshots.asciidoc b/docs/reference/migration/migrate_8_0/snapshots.asciidoc new file mode 100644 index 0000000000000..52db460fa7a16 --- /dev/null +++ b/docs/reference/migration/migrate_8_0/snapshots.asciidoc @@ -0,0 +1,13 @@ +[float] +[[breaking_80_snapshots_changes]] +=== Snapshot and Restore changes + +[float] +==== Deprecated node level compress setting removed + +For shared file system repositories (`"type": "fs"`), the node level setting `repositories.fs.compress` could +previously be used to enable compression for all shared file system repositories where `compress` was not specified. +The `repositories.fs.compress` setting has been removed. + +Instead use the repository specific `compress` setting to enable compression. See <> for information +on the `compress` setting. 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; }