From bc2574c8d430508a9099297f51d539a99eb9afcf Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Thu, 30 Jan 2020 13:24:55 -0500 Subject: [PATCH 1/3] Reject translog retention settings --- .../reference/index-modules/translog.asciidoc | 37 ------------------- .../migration/migrate_8_0/indices.asciidoc | 8 ++++ .../test/indices.stats/20_translog.yml | 25 +++---------- .../metadata/MetaDataCreateIndexService.java | 6 +++ .../elasticsearch/index/IndexSettings.java | 12 +----- .../MetaDataCreateIndexServiceTests.java | 24 +++++++++++- .../test/InternalSettingsPlugin.java | 3 +- 7 files changed, 46 insertions(+), 69 deletions(-) diff --git a/docs/reference/index-modules/translog.asciidoc b/docs/reference/index-modules/translog.asciidoc index e349875d14bee..73a309647185b 100644 --- a/docs/reference/index-modules/translog.asciidoc +++ b/docs/reference/index-modules/translog.asciidoc @@ -75,40 +75,3 @@ update, or bulk request. This setting accepts the following parameters: operations, to prevent recoveries from taking too long. Once the maximum size has been reached a flush will happen, generating a new Lucene commit point. Defaults to `512mb`. - -[float] -[[index-modules-translog-retention]] -==== Translog retention - -deprecated::[7.4.0, translog retention settings are deprecated in favor of -<>. These settings are -effectively ignored since 7.4 and will be removed in a future version]. - -If an index is not using <> to -retain historical operations then {es} recovers each replica shard by replaying -operations from the primary's translog. This means it is important for the -primary to preserve extra operations in its translog in case it needs to -rebuild a replica. Moreover it is important for each replica to preserve extra -operations in its translog in case it is promoted to primary and then needs to -rebuild its own replicas in turn. The following settings control how much -translog is retained for peer recoveries. - -`index.translog.retention.size`:: - - This controls the total size of translog files to keep for each shard. - Keeping more translog files increases the chance of performing an operation - based sync when recovering a replica. If the translog files are not - sufficient, replica recovery will fall back to a file based sync. Defaults to - `512mb`. This setting is ignored, and should not be set, if soft deletes are - enabled. Soft deletes are enabled by default in indices created in {es} - versions 7.0.0 and later. - -`index.translog.retention.age`:: - - This controls the maximum duration for which translog files are kept by each - shard. Keeping more translog files increases the chance of performing an - operation based sync when recovering replicas. If the translog files are not - sufficient, replica recovery will fall back to a file based sync. Defaults to - `12h`. This setting is ignored, and should not be set, if soft deletes are - enabled. Soft deletes are enabled by default in indices created in {es} - versions 7.0.0 and later. diff --git a/docs/reference/migration/migrate_8_0/indices.asciidoc b/docs/reference/migration/migrate_8_0/indices.asciidoc index 03acac7753539..62a5042fbde8f 100644 --- a/docs/reference/migration/migrate_8_0/indices.asciidoc +++ b/docs/reference/migration/migrate_8_0/indices.asciidoc @@ -43,3 +43,11 @@ Creating indices with soft deletes disabled was deprecated in 7.6 and is no longer supported in 8.0. The setting index.soft_deletes.enabled can no longer be set to false. As the setting defaults to true, simply leave the setting unset. + +[float] +==== Translog retention settings are no longer supported + +Translog retention settings `index.translog.retention.size` and +`index.translog.retention.age` were effectively ignored in 7.4, +deprecated in 7.7, and removed in 8.0 in favor of +<>. diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.stats/20_translog.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.stats/20_translog.yml index 8678a7b3d2c03..9912db128b7de 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.stats/20_translog.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.stats/20_translog.yml @@ -44,39 +44,26 @@ - match: { indices.test.primaries.translog.uncommitted_operations: 0 } --- -"Translog retention settings are deprecated": +"Translog retention settings are no long supported": - skip: - version: " - 7.6.99" - reason: "translog retention settings are deprecated in 7.6" - features: "warnings" + version: " - 7.9.99" + reason: "translog retention settings are no longer supported in 8.0" - do: - warnings: - - Translog retention settings [index.translog.retention.age] and [index.translog.retention.size] - are deprecated and effectively ignored. They will be removed in a future version. + catch: /illegal_argument_exception/ indices.create: index: test body: settings: index.translog.retention.size: 128mb - do: - indices.put_settings: + indices.create: index: test - body: - index.number_of_replicas: 0 - do: - warnings: - - Translog retention settings [index.translog.retention.age] and [index.translog.retention.size] - are deprecated and effectively ignored. They will be removed in a future version. + catch: /illegal_argument_exception/ indices.put_settings: index: test body: index.translog.retention.age: 1h - - do: - indices.put_settings: - index: test - body: - index.translog.retention.age: null - index.translog.retention.size: null --- "Translog last modified age stats": diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java index 6534036a7e275..1a80af0572529 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java @@ -952,6 +952,12 @@ public static int calculateNumRoutingShards(int numShards, Version indexVersionC } public static void validateTranslogRetentionSettings(Settings indexSettings) { + if (IndexMetaData.SETTING_INDEX_VERSION_CREATED.get(indexSettings).onOrAfter(Version.V_8_0_0) && + (IndexSettings.INDEX_TRANSLOG_RETENTION_AGE_SETTING.exists(indexSettings) + || IndexSettings.INDEX_TRANSLOG_RETENTION_SIZE_SETTING.exists(indexSettings))) { + throw new IllegalArgumentException("Translog retention settings [index.translog.retention.age] " + + "and [index.translog.retention.size] are no longer supported. Please do not specify values for these settings"); + } if (IndexSettings.INDEX_SOFT_DELETES_SETTING.get(indexSettings) && (IndexSettings.INDEX_TRANSLOG_RETENTION_AGE_SETTING.exists(indexSettings) || IndexSettings.INDEX_TRANSLOG_RETENTION_SIZE_SETTING.exists(indexSettings))) { diff --git a/server/src/main/java/org/elasticsearch/index/IndexSettings.java b/server/src/main/java/org/elasticsearch/index/IndexSettings.java index 328d1ede2b416..78eeec56e75df 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexSettings.java +++ b/server/src/main/java/org/elasticsearch/index/IndexSettings.java @@ -245,7 +245,7 @@ public final class IndexSettings { * Controls how long translog files that are no longer needed for persistence reasons * will be kept around before being deleted. Keeping more files is useful to increase * the chance of ops based recoveries for indices with soft-deletes disabled. - * This setting will be ignored if soft-deletes is used in peer recoveries (default in 7.4). + * TODO: Remove this setting in 9.0. **/ public static final Setting INDEX_TRANSLOG_RETENTION_AGE_SETTING = Setting.timeSetting("index.translog.retention.age", settings -> TimeValue.MINUS_ONE, @@ -255,19 +255,11 @@ public final class IndexSettings { * Controls how many translog files that are no longer needed for persistence reasons * will be kept around before being deleted. Keeping more files is useful to increase * the chance of ops based recoveries for indices with soft-deletes disabled. - * This setting will be ignored if soft-deletes is used in peer recoveries (default in 7.4). + * TODO: Remove this setting in 9.0. **/ public static final Setting INDEX_TRANSLOG_RETENTION_SIZE_SETTING = Setting.byteSizeSetting("index.translog.retention.size", settings -> "-1", Property.Dynamic, Property.IndexScope); - /** - * Controls the number of translog files that are no longer needed for persistence reasons will be kept around before being deleted. - * This is a safeguard making sure that the translog deletion policy won't keep too many translog files especially when they're small. - * This setting is intentionally not registered, it is only used in tests - **/ - public static final Setting INDEX_TRANSLOG_RETENTION_TOTAL_FILES_SETTING = - Setting.intSetting("index.translog.retention.total_files", 100, 0, Setting.Property.IndexScope); - /** * Controls the maximum length of time since a retention lease is created or renewed before it is considered expired. */ diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexServiceTests.java index bbfc9d7cafa47..b7f437e80af70 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexServiceTests.java @@ -88,6 +88,7 @@ import static org.elasticsearch.cluster.metadata.IndexMetaData.INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING; import static org.elasticsearch.cluster.metadata.IndexMetaData.INDEX_NUMBER_OF_SHARDS_SETTING; import static org.elasticsearch.cluster.metadata.IndexMetaData.INDEX_READ_ONLY_BLOCK; +import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_INDEX_VERSION_CREATED; import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_REPLICAS; import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS; import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_READ_ONLY; @@ -951,7 +952,7 @@ public void testRejectWithSoftDeletesDisabled() { + "Please do not specify a value for setting [index.soft_deletes.enabled].")); } - public void testValidateTranslogRetentionSettings() { + public void testRejectTranslogRetentionSettings() { request = new CreateIndexClusterStateUpdateRequest("create index", "test", "test"); final Settings.Builder settings = Settings.builder(); if (randomBoolean()) { @@ -959,6 +960,27 @@ public void testValidateTranslogRetentionSettings() { } else { settings.put(IndexSettings.INDEX_TRANSLOG_RETENTION_SIZE_SETTING.getKey(), between(1, 128) + "mb"); } + if (randomBoolean()) { + settings.put(SETTING_INDEX_VERSION_CREATED.getKey(), + VersionUtils.randomVersionBetween(random(), Version.V_8_0_0, Version.CURRENT)); + } + request.settings(settings.build()); + IllegalArgumentException error = expectThrows(IllegalArgumentException.class, + () -> aggregateIndexSettings(ClusterState.EMPTY_STATE, request, List.of(), Map.of(), + null, Settings.EMPTY, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS)); + assertThat(error.getMessage(), equalTo("Translog retention settings [index.translog.retention.age] " + + "and [index.translog.retention.size] are no longer supported. Please do not specify values for these settings")); + } + + public void testDeprecateTranslogRetentionSettings() { + request = new CreateIndexClusterStateUpdateRequest("create index", "test", "test"); + final Settings.Builder settings = Settings.builder(); + if (randomBoolean()) { + settings.put(IndexSettings.INDEX_TRANSLOG_RETENTION_AGE_SETTING.getKey(), TimeValue.timeValueMillis(between(1, 120))); + } else { + settings.put(IndexSettings.INDEX_TRANSLOG_RETENTION_SIZE_SETTING.getKey(), between(1, 128) + "mb"); + } + settings.put(SETTING_INDEX_VERSION_CREATED.getKey(), VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0)); request.settings(settings.build()); aggregateIndexSettings(ClusterState.EMPTY_STATE, request, List.of(), Map.of(), null, Settings.EMPTY, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS); diff --git a/test/framework/src/main/java/org/elasticsearch/test/InternalSettingsPlugin.java b/test/framework/src/main/java/org/elasticsearch/test/InternalSettingsPlugin.java index b921e392f4f35..246dac18ef8a2 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/InternalSettingsPlugin.java +++ b/test/framework/src/main/java/org/elasticsearch/test/InternalSettingsPlugin.java @@ -53,8 +53,7 @@ public List> getSettings() { IndexService.GLOBAL_CHECKPOINT_SYNC_INTERVAL_SETTING, IndexService.RETENTION_LEASE_SYNC_INTERVAL_SETTING, IndexSettings.FILE_BASED_RECOVERY_THRESHOLD_SETTING, - IndexModule.INDEX_QUERY_CACHE_EVERYTHING_SETTING, - IndexSettings.INDEX_TRANSLOG_RETENTION_TOTAL_FILES_SETTING + IndexModule.INDEX_QUERY_CACHE_EVERYTHING_SETTING ); } } From 3cb97f38e2c44f0ec099109fbe89844009b3d2ee Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Thu, 30 Jan 2020 14:47:56 -0500 Subject: [PATCH 2/3] fix docs --- docs/reference/index-modules/history-retention.asciidoc | 7 ------- docs/reference/indices/flush.asciidoc | 3 +-- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/docs/reference/index-modules/history-retention.asciidoc b/docs/reference/index-modules/history-retention.asciidoc index 8df79625c88d0..6ace77c3533ff 100644 --- a/docs/reference/index-modules/history-retention.asciidoc +++ b/docs/reference/index-modules/history-retention.asciidoc @@ -49,13 +49,6 @@ index since it can no longer simply replay the missing history. The expiry time of a retention lease defaults to `12h` which should be long enough for most reasonable recovery scenarios. -Soft deletes are enabled by default on indices created in recent versions, but -they can be explicitly enabled or disabled at index creation time. If soft -deletes are disabled then peer recoveries can still sometimes take place by -copying just the missing operations from the translog -<>. {ccr-cap} will not function if soft deletes are disabled. - [float] === History retention settings diff --git a/docs/reference/indices/flush.asciidoc b/docs/reference/indices/flush.asciidoc index 5729909e94f71..72bec9da50c8f 100644 --- a/docs/reference/indices/flush.asciidoc +++ b/docs/reference/indices/flush.asciidoc @@ -38,8 +38,7 @@ unflushed transaction log against the cost of performing each flush. Once each operation has been flushed it is permanently stored in the Lucene index. This may mean that there is no need to maintain an additional copy of it -in the transaction log, unless <>. The transaction log is made up of multiple files, +in the transaction log. The transaction log is made up of multiple files, called _generations_, and {es} will delete any generation files once they are no longer needed, freeing up disk space. From 6802872b136d9afff1cc41916b92b7608630c3fc Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Thu, 30 Jan 2020 16:47:02 -0500 Subject: [PATCH 3/3] fix outdated test --- .../test/java/org/elasticsearch/index/IndexServiceTests.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/IndexServiceTests.java b/server/src/test/java/org/elasticsearch/index/IndexServiceTests.java index 55b65dbe11665..36a04a8c1020e 100644 --- a/server/src/test/java/org/elasticsearch/index/IndexServiceTests.java +++ b/server/src/test/java/org/elasticsearch/index/IndexServiceTests.java @@ -376,11 +376,6 @@ public void testAsyncTranslogTrimActuallyWorks() throws Exception { assertTrue(indexService.getTrimTranslogTask().mustReschedule()); client().prepareIndex("test").setId("1").setSource("{\"foo\": \"bar\"}", XContentType.JSON).get(); client().admin().indices().prepareFlush("test").get(); - client().admin().indices().prepareUpdateSettings("test") - .setSettings(Settings.builder() - .put(IndexSettings.INDEX_TRANSLOG_RETENTION_SIZE_SETTING.getKey(), -1) - .put(IndexSettings.INDEX_TRANSLOG_RETENTION_AGE_SETTING.getKey(), -1)) - .get(); IndexShard shard = indexService.getShard(0); assertBusy(() -> assertThat(IndexShardTestCase.getTranslog(shard).totalOperations(), equalTo(0))); }