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 elastic#39073

Author: henningandersen

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");

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<ByteSizeValue> CHUNK_SIZE_SETTING =
             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);
         public static final Setting<Boolean> 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()), '/');

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<String> BASE_PATH =
             simpleString("base_path", Property.NodeScope, Property.Dynamic);
-    static final Setting<Boolean> COMPRESS =
-            boolSetting("compress", false, Property.NodeScope, Property.Dynamic);
     static final Setting<ByteSizeValue> CHUNK_SIZE =
             byteSizeSetting("chunk_size", MAX_CHUNK_SIZE, MIN_CHUNK_SIZE, MAX_CHUNK_SIZE, Property.NodeScope, Property.Dynamic);
     static final Setting<String> 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());

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);

plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Repository.java

@@ -122,12 +122,6 @@ class S3Repository extends BlobStoreRepository {
     static final Setting<ByteSizeValue> 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<Boolean> 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;

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,

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<Boolean> 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);

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<ByteSizeValue> 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<Boolean> COMPRESS_SETTING = Setting.boolSetting("compress", false, Property.NodeScope);
-    public static final Setting<Boolean> 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());

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,40 +247,28 @@ 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));
 
         final RepositoriesService repositoriesService = getInstanceFromNode(RepositoriesService.class);
         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;
     }