Skip to content

Commit

Permalink
Unify blob store compress setting (#39346)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
henningandersen committed Feb 28, 2019
1 parent 54cbf1a commit ac7ec99
Show file tree
Hide file tree
Showing 11 changed files with 39 additions and 55 deletions.
2 changes: 2 additions & 0 deletions docs/reference/migration/migrate_8_0.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ coming[8.0.0]

* <<breaking_80_analysis_changes>>
* <<breaking_80_mappings_changes>>
* <<breaking_80_snapshots_changes>>

[float]
=== Indices created before 7.0
Expand All @@ -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[]
13 changes: 13 additions & 0 deletions docs/reference/migration/migrate_8_0/snapshots.asciidoc
Original file line number Diff line number Diff line change
@@ -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 <<modules-snapshots>> for information
on the `compress` setting.
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()), '/');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand All @@ -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;
Expand All @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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()) {
Expand Down Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down

0 comments on commit ac7ec99

Please sign in to comment.