From edbff0fd726bd18bb14f92c7b00a83a5ef6fb5f5 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 6 Jan 2020 13:55:26 +0100 Subject: [PATCH 01/26] start --- qa/snapshot-repository-downgrade/build.gradle | 102 +++++++++ .../upgrades/ClusterDowngradeIT.java | 28 +++ .../test/mixed_cluster/10_basic.yml | 69 ++++++ .../test/old_cluster/10_basic.yml | 207 ++++++++++++++++++ .../test/upgraded_cluster/10_basic.yml | 129 +++++++++++ 5 files changed, 535 insertions(+) create mode 100644 qa/snapshot-repository-downgrade/build.gradle create mode 100644 qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java create mode 100644 qa/snapshot-repository-downgrade/src/test/resources/rest-api-spec/test/mixed_cluster/10_basic.yml create mode 100644 qa/snapshot-repository-downgrade/src/test/resources/rest-api-spec/test/old_cluster/10_basic.yml create mode 100644 qa/snapshot-repository-downgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/10_basic.yml diff --git a/qa/snapshot-repository-downgrade/build.gradle b/qa/snapshot-repository-downgrade/build.gradle new file mode 100644 index 0000000000000..bc62d0361695f --- /dev/null +++ b/qa/snapshot-repository-downgrade/build.gradle @@ -0,0 +1,102 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import org.elasticsearch.gradle.Version +import org.elasticsearch.gradle.info.BuildParams +import org.elasticsearch.gradle.testclusters.RestTestRunnerTask + +apply plugin: 'elasticsearch.testclusters' +apply plugin: 'elasticsearch.standalone-test' + +tasks.register("bwcTest") { + description = 'Runs backwards compatibility tests.' + group = 'verification' +} + +for (Version bwcVersion : bwcVersions.indexCompatible) { + String baseName = "v${bwcVersion}" + + testClusters { + "${baseName}" { + versions = [bwcVersion.toString(), project.version] + numberOfNodes = 2 + setting 'path.repo', "${buildDir}/cluster/shared/repo/${baseName}" + javaHome = BuildParams.runtimeJavaHome + } + } + + tasks.register("${baseName}#oldClusterTest", RestTestRunnerTask) { + useCluster testClusters."${baseName}" + mustRunAfter(precommit) + doFirst { + project.delete("${buildDir}/cluster/shared/repo/${baseName}") + } + + systemProperty 'tests.is_old_cluster', 'true' + } + + tasks.register("${baseName}#upgradedClusterTest", RestTestRunnerTask) { + useCluster testClusters."${baseName}" + dependsOn "${baseName}#oldClusterTest" + doFirst { + testClusters."${baseName}".goToNextVersion() + } + systemProperty 'tests.is_old_cluster', 'false' + } + + tasks.matching { it.name.startsWith(baseName) && it.name.endsWith("ClusterTest") }.configureEach { + it.systemProperty 'tests.old_cluster_version', bwcVersion.toString().minus("-SNAPSHOT") + it.systemProperty 'tests.path.repo', "${buildDir}/cluster/shared/repo/${baseName}" + it.nonInputProperties.systemProperty('tests.rest.cluster', "${-> testClusters."${baseName}".allHttpSocketURI.join(",")}") + it.nonInputProperties.systemProperty('tests.clustername', "${-> testClusters."${baseName}".getName()}") + } + + if (project.bwc_tests_enabled) { + bwcTest.dependsOn( + tasks.register("${baseName}#bwcTest") { + dependsOn tasks.named("${baseName}#upgradedClusterTest") + } + ) + } +} + +task bwcTestSnapshots { + if (project.bwc_tests_enabled) { + for (final def version : bwcVersions.unreleasedIndexCompatible) { + dependsOn "v${version}#bwcTest" + } + } +} + +check.dependsOn(bwcTestSnapshots) + +configurations { + testArtifacts.extendsFrom testRuntime +} + +task testJar(type: Jar) { + appendix 'test' + from sourceSets.test.output +} + +artifacts { + testArtifacts testJar +} + +test.enabled = false diff --git a/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java b/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java new file mode 100644 index 0000000000000..d88d86914c6a0 --- /dev/null +++ b/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java @@ -0,0 +1,28 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.upgrades; + +/** + * Downgrade tests that verify that a snapshot repository is not getting corrupted and continues to function properly for downgrades. + */ +public class ClusterDowngradeIT extends AbstractFullClusterRestartTestCase { + + +} diff --git a/qa/snapshot-repository-downgrade/src/test/resources/rest-api-spec/test/mixed_cluster/10_basic.yml b/qa/snapshot-repository-downgrade/src/test/resources/rest-api-spec/test/mixed_cluster/10_basic.yml new file mode 100644 index 0000000000000..fa86389f0db2a --- /dev/null +++ b/qa/snapshot-repository-downgrade/src/test/resources/rest-api-spec/test/mixed_cluster/10_basic.yml @@ -0,0 +1,69 @@ +--- +"Verify that we can still find things with the template": + - do: + search_template: + rest_total_hits_as_int: true + index: test_search_template + body: + id: test_search_template + params: + f1: v5_old + - match: { hits.total: 1 } + +--- +"Verify custom cluster metadata still exists during upgrade": + - do: + snapshot.get_repository: + repository: my_repo + - is_true: my_repo + + - do: + ingest.get_pipeline: + id: "my_pipeline" + - match: { my_pipeline.description: "_description" } + +--- +"Use the percolate query in mixed cluster": + - do: + search: + rest_total_hits_as_int: true + index: queries + body: + query: + percolate: + field: query + document: + field1: value + - match: { hits.total: 1 } + - match: { hits.hits.0._id: q1 } + + - do: + search: + rest_total_hits_as_int: true + index: queries + body: + sort: id + query: + percolate: + field: query + document: + field1: value + field2: value + - match: { hits.total: 2 } + - match: { hits.hits.0._id: q1 } + - match: { hits.hits.1._id: q2 } + + - do: + search: + rest_total_hits_as_int: true + index: queries + body: + query: + percolate: + field: query + document: + field2: value + field3: value + - match: { hits.total: 1 } + - match: { hits.hits.0._id: q3 } + diff --git a/qa/snapshot-repository-downgrade/src/test/resources/rest-api-spec/test/old_cluster/10_basic.yml b/qa/snapshot-repository-downgrade/src/test/resources/rest-api-spec/test/old_cluster/10_basic.yml new file mode 100644 index 0000000000000..e1ffcea930a42 --- /dev/null +++ b/qa/snapshot-repository-downgrade/src/test/resources/rest-api-spec/test/old_cluster/10_basic.yml @@ -0,0 +1,207 @@ +--- +"Create things in the cluster state that we'll validate are there after the upgrade": + - do: + snapshot.create_repository: + repository: my_repo + verify: false + body: + type: url + settings: + url: "http://snapshot.test" + - match: { "acknowledged": true } + + - do: + ingest.put_pipeline: + id: "my_pipeline" + body: > + { + "description": "_description", + "processors": [ + ] + } + - match: { "acknowledged": true } + + - do: + bulk: + refresh: true + body: + - '{"index": {"_index": "test_search_template"}}' + - '{"f1": "v1_old"}' + - '{"index": {"_index": "test_search_template"}}' + - '{"f1": "v2_old"}' + - '{"index": {"_index": "test_search_template"}}' + - '{"f1": "v3_old"}' + - '{"index": {"_index": "test_search_template"}}' + - '{"f1": "v4_old"}' + - '{"index": {"_index": "test_search_template"}}' + - '{"f1": "v5_old"}' + + - do: + put_script: + id: test_search_template + body: + script: + lang: mustache + source: + query: + match: + f1: "{{f1}}" + - match: { acknowledged: true } + + - do: + search_template: + rest_total_hits_as_int: true + index: test_search_template + body: + id: test_search_template + params: + f1: v5_old + - match: { hits.total: 1 } + +--- +"Index percolator queries and use the percolate query in old cluster": + - do: + indices.create: + index: queries + body: + mappings: + properties: + id: + type: keyword + query: + type: percolator + field1: + type: keyword + field2: + type: keyword + field3: + type: keyword + + - do: + index: + index: queries + id: q1 + body: + id: q1 + query: + term: + field1: value + + - do: + index: + index: queries + id: q2 + body: + id: q2 + query: + bool: + must: + - term: + field1: value + - term: + field2: value + + - do: + index: + index: queries + id: q3 + body: + id: q3 + query: + bool: + minimum_should_match: 2 + should: + - term: + field2: value + - term: + field3: value + + - do: + indices.refresh: + index: queries + + - do: + search: + rest_total_hits_as_int: true + index: queries + body: + query: + percolate: + field: query + document: + field1: value + - match: { hits.total: 1 } + - match: { hits.hits.0._id: q1 } + + - do: + search: + rest_total_hits_as_int: true + index: queries + body: + sort: id + query: + percolate: + field: query + document: + field1: value + field2: value + - match: { hits.total: 2 } + - match: { hits.hits.0._id: q1 } + - match: { hits.hits.1._id: q2 } + + - do: + search: + rest_total_hits_as_int: true + index: queries + body: + sort: id + query: + percolate: + field: query + document: + field2: value + field3: value + - match: { hits.total: 1 } + - match: { hits.hits.0._id: q3 } + +--- +"Create a task result record in the old cluster": + - do: + indices.create: + index: reindexed_index + body: + settings: + index: + number_of_replicas: 0 + - do: + bulk: + refresh: true + body: + - '{"index": {"_index": "reindexed_index"}}' + - '{"f1": "1"}' + - '{"index": {"_index": "reindexed_index"}}' + - '{"f1": "2"}' + - '{"index": {"_index": "reindexed_index"}}' + - '{"f1": "3"}' + - '{"index": {"_index": "reindexed_index"}}' + - '{"f1": "4"}' + - '{"index": {"_index": "reindexed_index"}}' + - '{"f1": "5"}' + + - do: + reindex: + wait_for_completion: false + body: + source: + index: reindexed_index + size: 1 + dest: + index: reindexed_index_copy + - match: {task: '/.+:\d+/'} + - set: {task: task} + + - do: + tasks.get: + wait_for_completion: true + task_id: $task + diff --git a/qa/snapshot-repository-downgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/10_basic.yml b/qa/snapshot-repository-downgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/10_basic.yml new file mode 100644 index 0000000000000..4368ffd602f58 --- /dev/null +++ b/qa/snapshot-repository-downgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/10_basic.yml @@ -0,0 +1,129 @@ +--- +"Verify that we can still find things with the template": + - do: + search_template: + rest_total_hits_as_int: true + index: test_search_template + body: + id: test_search_template + params: + f1: v5_old + - match: { hits.total: 1 } + +--- +"Verify custom cluster metadata still exists after rolling upgrade": + - do: + snapshot.get_repository: + repository: my_repo + - is_true: my_repo + + - do: + ingest.get_pipeline: + id: "my_pipeline" + - match: { my_pipeline.description: "_description" } + +--- +"Index percolator query and use the percolate query in upgraded cluster": + - do: + index: + index: queries + id: q4 + refresh: true + body: + id: q4 + query: + bool: + minimum_should_match: 2 + should: + - term: + field1: value + - term: + field2: value + + - do: + search: + rest_total_hits_as_int: true + index: queries + body: + query: + percolate: + field: query + document: + field1: value + - match: { hits.total: 1 } + - match: { hits.hits.0._id: q1 } + + - do: + search: + rest_total_hits_as_int: true + index: queries + body: + sort: id + query: + percolate: + field: query + document: + field1: value + field2: value + - match: { hits.total: 3 } + - match: { hits.hits.0._id: q1 } + - match: { hits.hits.1._id: q2 } + - match: { hits.hits.2._id: q4 } + + - do: + search: + rest_total_hits_as_int: true + index: queries + body: + query: + percolate: + field: query + document: + field2: value + field3: value + - match: { hits.total: 1 } + - match: { hits.hits.0._id: q3 } + +--- +"Find a task result record from the old cluster": + - skip: + features: headers + + - do: + search: + rest_total_hits_as_int: true + index: .tasks + body: + query: + match_all: {} + - match: { hits.total: 1 } + - match: { hits.hits.0._id: '/.+:\d+/' } + - set: {hits.hits.0._id: task_id} + + - do: + tasks.get: + task_id: $task_id + + - is_false: node_failures + - is_true: task + + - do: + headers: { "X-Opaque-Id": "Reindexing Again" } + reindex: + wait_for_completion: false + body: + source: + index: reindexed_index_copy + size: 1 + dest: + index: reindexed_index_another_copy + - match: { task: '/.+:\d+/' } + - set: { task: task_id } + + - do: + tasks.get: + wait_for_completion: true + task_id: $task_id + - match: { task.headers.X-Opaque-Id: "Reindexing Again" } + + From a7306db7049a2ecadce23c59cd51f945d587b13a Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 6 Jan 2020 19:05:32 +0100 Subject: [PATCH 02/26] works --- qa/snapshot-repository-downgrade/build.gradle | 20 +- .../test/mixed_cluster/10_basic.yml | 69 ------ .../test/old_cluster/10_basic.yml | 207 ------------------ .../test/upgraded_cluster/10_basic.yml | 129 ----------- 4 files changed, 17 insertions(+), 408 deletions(-) delete mode 100644 qa/snapshot-repository-downgrade/src/test/resources/rest-api-spec/test/mixed_cluster/10_basic.yml delete mode 100644 qa/snapshot-repository-downgrade/src/test/resources/rest-api-spec/test/old_cluster/10_basic.yml delete mode 100644 qa/snapshot-repository-downgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/10_basic.yml diff --git a/qa/snapshot-repository-downgrade/build.gradle b/qa/snapshot-repository-downgrade/build.gradle index bc62d0361695f..5081818f816bd 100644 --- a/qa/snapshot-repository-downgrade/build.gradle +++ b/qa/snapshot-repository-downgrade/build.gradle @@ -39,6 +39,13 @@ for (Version bwcVersion : bwcVersions.indexCompatible) { setting 'path.repo', "${buildDir}/cluster/shared/repo/${baseName}" javaHome = BuildParams.runtimeJavaHome } + + "${baseName}-downgraded" { + versions = [bwcVersion.toString()] + numberOfNodes = 2 + setting 'path.repo', "${buildDir}/cluster/shared/repo/${baseName}" + javaHome = BuildParams.runtimeJavaHome + } } tasks.register("${baseName}#oldClusterTest", RestTestRunnerTask) { @@ -60,17 +67,24 @@ for (Version bwcVersion : bwcVersions.indexCompatible) { systemProperty 'tests.is_old_cluster', 'false' } + tasks.register("${baseName}#downgradedClusterTest", RestTestRunnerTask) { + useCluster testClusters."${baseName}-downgraded" + dependsOn "${baseName}#upgradedClusterTest" + systemProperty 'tests.is_old_cluster', 'true' + } + tasks.matching { it.name.startsWith(baseName) && it.name.endsWith("ClusterTest") }.configureEach { it.systemProperty 'tests.old_cluster_version', bwcVersion.toString().minus("-SNAPSHOT") it.systemProperty 'tests.path.repo', "${buildDir}/cluster/shared/repo/${baseName}" - it.nonInputProperties.systemProperty('tests.rest.cluster', "${-> testClusters."${baseName}".allHttpSocketURI.join(",")}") - it.nonInputProperties.systemProperty('tests.clustername', "${-> testClusters."${baseName}".getName()}") + def clusterName = it.name.contains("downgraded") ? "${baseName}-downgraded" : "${baseName}" + it.nonInputProperties.systemProperty('tests.rest.cluster', "${-> testClusters."${clusterName}".allHttpSocketURI.join(",")}") + it.nonInputProperties.systemProperty('tests.clustername', "${-> testClusters."${clusterName}".getName()}") } if (project.bwc_tests_enabled) { bwcTest.dependsOn( tasks.register("${baseName}#bwcTest") { - dependsOn tasks.named("${baseName}#upgradedClusterTest") + dependsOn tasks.named("${baseName}#downgradedClusterTest") } ) } diff --git a/qa/snapshot-repository-downgrade/src/test/resources/rest-api-spec/test/mixed_cluster/10_basic.yml b/qa/snapshot-repository-downgrade/src/test/resources/rest-api-spec/test/mixed_cluster/10_basic.yml deleted file mode 100644 index fa86389f0db2a..0000000000000 --- a/qa/snapshot-repository-downgrade/src/test/resources/rest-api-spec/test/mixed_cluster/10_basic.yml +++ /dev/null @@ -1,69 +0,0 @@ ---- -"Verify that we can still find things with the template": - - do: - search_template: - rest_total_hits_as_int: true - index: test_search_template - body: - id: test_search_template - params: - f1: v5_old - - match: { hits.total: 1 } - ---- -"Verify custom cluster metadata still exists during upgrade": - - do: - snapshot.get_repository: - repository: my_repo - - is_true: my_repo - - - do: - ingest.get_pipeline: - id: "my_pipeline" - - match: { my_pipeline.description: "_description" } - ---- -"Use the percolate query in mixed cluster": - - do: - search: - rest_total_hits_as_int: true - index: queries - body: - query: - percolate: - field: query - document: - field1: value - - match: { hits.total: 1 } - - match: { hits.hits.0._id: q1 } - - - do: - search: - rest_total_hits_as_int: true - index: queries - body: - sort: id - query: - percolate: - field: query - document: - field1: value - field2: value - - match: { hits.total: 2 } - - match: { hits.hits.0._id: q1 } - - match: { hits.hits.1._id: q2 } - - - do: - search: - rest_total_hits_as_int: true - index: queries - body: - query: - percolate: - field: query - document: - field2: value - field3: value - - match: { hits.total: 1 } - - match: { hits.hits.0._id: q3 } - diff --git a/qa/snapshot-repository-downgrade/src/test/resources/rest-api-spec/test/old_cluster/10_basic.yml b/qa/snapshot-repository-downgrade/src/test/resources/rest-api-spec/test/old_cluster/10_basic.yml deleted file mode 100644 index e1ffcea930a42..0000000000000 --- a/qa/snapshot-repository-downgrade/src/test/resources/rest-api-spec/test/old_cluster/10_basic.yml +++ /dev/null @@ -1,207 +0,0 @@ ---- -"Create things in the cluster state that we'll validate are there after the upgrade": - - do: - snapshot.create_repository: - repository: my_repo - verify: false - body: - type: url - settings: - url: "http://snapshot.test" - - match: { "acknowledged": true } - - - do: - ingest.put_pipeline: - id: "my_pipeline" - body: > - { - "description": "_description", - "processors": [ - ] - } - - match: { "acknowledged": true } - - - do: - bulk: - refresh: true - body: - - '{"index": {"_index": "test_search_template"}}' - - '{"f1": "v1_old"}' - - '{"index": {"_index": "test_search_template"}}' - - '{"f1": "v2_old"}' - - '{"index": {"_index": "test_search_template"}}' - - '{"f1": "v3_old"}' - - '{"index": {"_index": "test_search_template"}}' - - '{"f1": "v4_old"}' - - '{"index": {"_index": "test_search_template"}}' - - '{"f1": "v5_old"}' - - - do: - put_script: - id: test_search_template - body: - script: - lang: mustache - source: - query: - match: - f1: "{{f1}}" - - match: { acknowledged: true } - - - do: - search_template: - rest_total_hits_as_int: true - index: test_search_template - body: - id: test_search_template - params: - f1: v5_old - - match: { hits.total: 1 } - ---- -"Index percolator queries and use the percolate query in old cluster": - - do: - indices.create: - index: queries - body: - mappings: - properties: - id: - type: keyword - query: - type: percolator - field1: - type: keyword - field2: - type: keyword - field3: - type: keyword - - - do: - index: - index: queries - id: q1 - body: - id: q1 - query: - term: - field1: value - - - do: - index: - index: queries - id: q2 - body: - id: q2 - query: - bool: - must: - - term: - field1: value - - term: - field2: value - - - do: - index: - index: queries - id: q3 - body: - id: q3 - query: - bool: - minimum_should_match: 2 - should: - - term: - field2: value - - term: - field3: value - - - do: - indices.refresh: - index: queries - - - do: - search: - rest_total_hits_as_int: true - index: queries - body: - query: - percolate: - field: query - document: - field1: value - - match: { hits.total: 1 } - - match: { hits.hits.0._id: q1 } - - - do: - search: - rest_total_hits_as_int: true - index: queries - body: - sort: id - query: - percolate: - field: query - document: - field1: value - field2: value - - match: { hits.total: 2 } - - match: { hits.hits.0._id: q1 } - - match: { hits.hits.1._id: q2 } - - - do: - search: - rest_total_hits_as_int: true - index: queries - body: - sort: id - query: - percolate: - field: query - document: - field2: value - field3: value - - match: { hits.total: 1 } - - match: { hits.hits.0._id: q3 } - ---- -"Create a task result record in the old cluster": - - do: - indices.create: - index: reindexed_index - body: - settings: - index: - number_of_replicas: 0 - - do: - bulk: - refresh: true - body: - - '{"index": {"_index": "reindexed_index"}}' - - '{"f1": "1"}' - - '{"index": {"_index": "reindexed_index"}}' - - '{"f1": "2"}' - - '{"index": {"_index": "reindexed_index"}}' - - '{"f1": "3"}' - - '{"index": {"_index": "reindexed_index"}}' - - '{"f1": "4"}' - - '{"index": {"_index": "reindexed_index"}}' - - '{"f1": "5"}' - - - do: - reindex: - wait_for_completion: false - body: - source: - index: reindexed_index - size: 1 - dest: - index: reindexed_index_copy - - match: {task: '/.+:\d+/'} - - set: {task: task} - - - do: - tasks.get: - wait_for_completion: true - task_id: $task - diff --git a/qa/snapshot-repository-downgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/10_basic.yml b/qa/snapshot-repository-downgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/10_basic.yml deleted file mode 100644 index 4368ffd602f58..0000000000000 --- a/qa/snapshot-repository-downgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/10_basic.yml +++ /dev/null @@ -1,129 +0,0 @@ ---- -"Verify that we can still find things with the template": - - do: - search_template: - rest_total_hits_as_int: true - index: test_search_template - body: - id: test_search_template - params: - f1: v5_old - - match: { hits.total: 1 } - ---- -"Verify custom cluster metadata still exists after rolling upgrade": - - do: - snapshot.get_repository: - repository: my_repo - - is_true: my_repo - - - do: - ingest.get_pipeline: - id: "my_pipeline" - - match: { my_pipeline.description: "_description" } - ---- -"Index percolator query and use the percolate query in upgraded cluster": - - do: - index: - index: queries - id: q4 - refresh: true - body: - id: q4 - query: - bool: - minimum_should_match: 2 - should: - - term: - field1: value - - term: - field2: value - - - do: - search: - rest_total_hits_as_int: true - index: queries - body: - query: - percolate: - field: query - document: - field1: value - - match: { hits.total: 1 } - - match: { hits.hits.0._id: q1 } - - - do: - search: - rest_total_hits_as_int: true - index: queries - body: - sort: id - query: - percolate: - field: query - document: - field1: value - field2: value - - match: { hits.total: 3 } - - match: { hits.hits.0._id: q1 } - - match: { hits.hits.1._id: q2 } - - match: { hits.hits.2._id: q4 } - - - do: - search: - rest_total_hits_as_int: true - index: queries - body: - query: - percolate: - field: query - document: - field2: value - field3: value - - match: { hits.total: 1 } - - match: { hits.hits.0._id: q3 } - ---- -"Find a task result record from the old cluster": - - skip: - features: headers - - - do: - search: - rest_total_hits_as_int: true - index: .tasks - body: - query: - match_all: {} - - match: { hits.total: 1 } - - match: { hits.hits.0._id: '/.+:\d+/' } - - set: {hits.hits.0._id: task_id} - - - do: - tasks.get: - task_id: $task_id - - - is_false: node_failures - - is_true: task - - - do: - headers: { "X-Opaque-Id": "Reindexing Again" } - reindex: - wait_for_completion: false - body: - source: - index: reindexed_index_copy - size: 1 - dest: - index: reindexed_index_another_copy - - match: { task: '/.+:\d+/' } - - set: { task: task_id } - - - do: - tasks.get: - wait_for_completion: true - task_id: $task_id - - match: { task.headers.X-Opaque-Id: "Reindexing Again" } - - From 78907a1688cebb9cb1cad40cd7b99cafb8befa3b Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 7 Jan 2020 00:46:07 +0100 Subject: [PATCH 03/26] works --- qa/snapshot-repository-downgrade/build.gradle | 13 +- .../upgrades/ClusterDowngradeIT.java | 163 +++++++++++++++++- 2 files changed, 170 insertions(+), 6 deletions(-) diff --git a/qa/snapshot-repository-downgrade/build.gradle b/qa/snapshot-repository-downgrade/build.gradle index 5081818f816bd..62fc438dd108f 100644 --- a/qa/snapshot-repository-downgrade/build.gradle +++ b/qa/snapshot-repository-downgrade/build.gradle @@ -29,6 +29,10 @@ tasks.register("bwcTest") { group = 'verification' } +dependencies { + testCompile project(':client:rest-high-level') +} + for (Version bwcVersion : bwcVersions.indexCompatible) { String baseName = "v${bwcVersion}" @@ -41,7 +45,7 @@ for (Version bwcVersion : bwcVersions.indexCompatible) { } "${baseName}-downgraded" { - versions = [bwcVersion.toString()] + version = bwcVersion.toString() numberOfNodes = 2 setting 'path.repo', "${buildDir}/cluster/shared/repo/${baseName}" javaHome = BuildParams.runtimeJavaHome @@ -54,8 +58,7 @@ for (Version bwcVersion : bwcVersions.indexCompatible) { doFirst { project.delete("${buildDir}/cluster/shared/repo/${baseName}") } - - systemProperty 'tests.is_old_cluster', 'true' + systemProperty 'tests.rest.suite', 'old_cluster' } tasks.register("${baseName}#upgradedClusterTest", RestTestRunnerTask) { @@ -64,13 +67,13 @@ for (Version bwcVersion : bwcVersions.indexCompatible) { doFirst { testClusters."${baseName}".goToNextVersion() } - systemProperty 'tests.is_old_cluster', 'false' + systemProperty 'tests.rest.suite', 'upgraded_cluster' } tasks.register("${baseName}#downgradedClusterTest", RestTestRunnerTask) { useCluster testClusters."${baseName}-downgraded" dependsOn "${baseName}#upgradedClusterTest" - systemProperty 'tests.is_old_cluster', 'true' + systemProperty 'tests.rest.suite', 'downgraded_cluster' } tasks.matching { it.name.startsWith(baseName) && it.name.endsWith("ClusterTest") }.configureEach { diff --git a/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java b/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java index d88d86914c6a0..ab3087938a30c 100644 --- a/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java +++ b/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java @@ -19,10 +19,171 @@ package org.elasticsearch.upgrades; +import org.elasticsearch.action.admin.cluster.repositories.put.PutRepositoryRequest; +import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotRequest; +import org.elasticsearch.action.admin.cluster.snapshots.delete.DeleteSnapshotRequest; +import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotRequest; +import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse; +import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotStatus; +import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotsStatusRequest; +import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotsStatusResponse; +import org.elasticsearch.client.Node; +import org.elasticsearch.client.Request; +import org.elasticsearch.client.RequestOptions; +import org.elasticsearch.client.Response; +import org.elasticsearch.client.RestClient; +import org.elasticsearch.client.RestHighLevelClient; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.DeprecationHandler; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.test.rest.ESRestTestCase; + +import java.io.IOException; +import java.io.InputStream; +import java.net.HttpURLConnection; +import java.util.List; +import java.util.Locale; +import java.util.Map; + +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.is; + /** * Downgrade tests that verify that a snapshot repository is not getting corrupted and continues to function properly for downgrades. */ -public class ClusterDowngradeIT extends AbstractFullClusterRestartTestCase { +public class ClusterDowngradeIT extends ESRestTestCase { + + protected enum ClusterType { + OLD, + UPGRADED, + DOWNGRADED; + + public static ClusterType parse(String value) { + switch (value) { + case "old_cluster": + return OLD; + case "upgraded_cluster": + return UPGRADED; + case "downgraded_cluster": + return DOWNGRADED; + default: + throw new AssertionError("unknown cluster type: " + value); + } + } + } + + protected static final ClusterType CLUSTER_TYPE = ClusterType.parse(System.getProperty("tests.rest.suite")); + + @Override + protected boolean preserveIndicesUponCompletion() { + return true; + } + + @Override + protected boolean preserveSnapshotsUponCompletion() { + return true; + } + + @Override + protected boolean preserveReposUponCompletion() { + return true; + } + + @Override + protected boolean preserveTemplatesUponCompletion() { + return true; + } + + @Override + protected boolean preserveClusterSettings() { + return true; + } + + @Override + protected boolean preserveRollupJobsUponCompletion() { + return true; + } + + @Override + protected boolean preserveILMPoliciesUponCompletion() { + return true; + } + + @Override + protected boolean preserveSLMPoliciesUponCompletion() { + return true; + } + @SuppressWarnings("unchecked") + public void testCreateSnapshot() throws IOException { + final String repoName = "repo"; + try (RestHighLevelClient client = new RestHighLevelClient(RestClient.builder(adminClient().getNodes().toArray(new Node[0])))) { + if (CLUSTER_TYPE == ClusterType.OLD) { + createIndex(client, "idx-1", 3); + } + if (CLUSTER_TYPE == ClusterType.OLD || CLUSTER_TYPE == ClusterType.DOWNGRADED) { + assertThat(client.snapshot().createRepository(new PutRepositoryRequest(repoName).type("fs").settings( + Settings.builder().put("location", ".")), RequestOptions.DEFAULT).isAcknowledged(), is(true)); + } + client.snapshot().create( + new CreateSnapshotRequest(repoName, "snapshot-" + CLUSTER_TYPE.toString().toLowerCase(Locale.ROOT)) + .waitForCompletion(true), RequestOptions.DEFAULT); + client.snapshot().create( + new CreateSnapshotRequest(repoName, "snapshot-to-delete").waitForCompletion(true), RequestOptions.DEFAULT); + client.snapshot().delete(new DeleteSnapshotRequest(repoName, "snapshot-to-delete"), RequestOptions.DEFAULT); + final List> snapshots; + Response response = client.getLowLevelClient().performRequest(new Request("GET", "/_snapshot/" + repoName + "/_all")); + try (InputStream entity = response.getEntity().getContent(); + XContentParser parser = JsonXContent.jsonXContent.createParser( + xContentRegistry(), DeprecationHandler.THROW_UNSUPPORTED_OPERATION, entity)) { + final Map raw = parser.map(); + logger.error(raw); + // Bwc lookup since the format of the snapshots response changed between versions + if (raw.containsKey("snapshots")) { + snapshots = (List>) raw.get("snapshots"); + } else { + snapshots = (List>) ((List>) raw.get("responses")).get(0).get("snapshots"); + } + } + switch (CLUSTER_TYPE) { + case OLD: + assertThat(snapshots, hasSize(1)); + break; + case UPGRADED: + assertThat(snapshots, hasSize(2)); + break; + case DOWNGRADED: + assertThat(snapshots, hasSize(3)); + } + final SnapshotsStatusResponse statusResponse = client.snapshot().status( + new SnapshotsStatusRequest(repoName, new String[]{"snapshot-old"}), RequestOptions.DEFAULT); + for (SnapshotStatus status : statusResponse.getSnapshots()) { + assertThat(status.getShardsStats().getFailedShards(), is(0)); + } + if (CLUSTER_TYPE == ClusterType.DOWNGRADED) { + wipeAllIndices(); + final RestoreSnapshotResponse restoreSnapshotResponse = client.snapshot().restore( + new RestoreSnapshotRequest().repository(repoName).snapshot("snapshot-old").waitForCompletion(true), + RequestOptions.DEFAULT); + assertThat(restoreSnapshotResponse.getRestoreInfo().failedShards(), is(0)); + assertThat(restoreSnapshotResponse.getRestoreInfo().successfulShards(), greaterThanOrEqualTo(3)); + } + } + } + private void createIndex(RestHighLevelClient client, String name, int shards) throws IOException { + final Request putIndexRequest = new Request("PUT", "/" + name); + putIndexRequest.setJsonEntity("{\n" + + " \"settings\" : {\n" + + " \"index\" : {\n" + + " \"number_of_shards\" : " + shards + ", \n" + + " \"number_of_replicas\" : 0 \n" + + " }\n" + + " }\n" + + "}"); + final Response response = client.getLowLevelClient().performRequest(putIndexRequest); + assertThat(response.getStatusLine().getStatusCode(), is(HttpURLConnection.HTTP_OK)); + } } From ea223c98659431942650dd7314cf1d96b42c8a70 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 7 Jan 2020 00:52:21 +0100 Subject: [PATCH 04/26] works --- .../test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java | 1 - 1 file changed, 1 deletion(-) diff --git a/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java b/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java index ab3087938a30c..9eb723e6821e7 100644 --- a/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java +++ b/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java @@ -139,7 +139,6 @@ public void testCreateSnapshot() throws IOException { XContentParser parser = JsonXContent.jsonXContent.createParser( xContentRegistry(), DeprecationHandler.THROW_UNSUPPORTED_OPERATION, entity)) { final Map raw = parser.map(); - logger.error(raw); // Bwc lookup since the format of the snapshots response changed between versions if (raw.containsKey("snapshots")) { snapshots = (List>) raw.get("snapshots"); From e9832b04d3907326be5c4e6faa051bfe2763e55a Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 7 Jan 2020 04:25:39 +0100 Subject: [PATCH 05/26] reproduces at last --- qa/snapshot-repository-downgrade/build.gradle | 16 ++++++++++++--- .../upgrades/ClusterDowngradeIT.java | 20 +++++++++++++++++-- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/qa/snapshot-repository-downgrade/build.gradle b/qa/snapshot-repository-downgrade/build.gradle index 62fc438dd108f..d4f81e1c8cadb 100644 --- a/qa/snapshot-repository-downgrade/build.gradle +++ b/qa/snapshot-repository-downgrade/build.gradle @@ -45,7 +45,7 @@ for (Version bwcVersion : bwcVersions.indexCompatible) { } "${baseName}-downgraded" { - version = bwcVersion.toString() + versions = [bwcVersion.toString(), project.version] numberOfNodes = 2 setting 'path.repo', "${buildDir}/cluster/shared/repo/${baseName}" javaHome = BuildParams.runtimeJavaHome @@ -76,10 +76,20 @@ for (Version bwcVersion : bwcVersions.indexCompatible) { systemProperty 'tests.rest.suite', 'downgraded_cluster' } + tasks.register("${baseName}#reUpgradedClusterTest", RestTestRunnerTask) { + useCluster testClusters."${baseName}-downgraded" + dependsOn "${baseName}#downgradedClusterTest" + doFirst { + testClusters."${baseName}-downgraded".goToNextVersion() + } + systemProperty 'tests.rest.suite', 're_upgraded_cluster' + } + + tasks.matching { it.name.startsWith(baseName) && it.name.endsWith("ClusterTest") }.configureEach { it.systemProperty 'tests.old_cluster_version', bwcVersion.toString().minus("-SNAPSHOT") it.systemProperty 'tests.path.repo', "${buildDir}/cluster/shared/repo/${baseName}" - def clusterName = it.name.contains("downgraded") ? "${baseName}-downgraded" : "${baseName}" + def clusterName = it.name.contains("downgraded") || it.name.contains("reUpgraded") ? "${baseName}-downgraded" : "${baseName}" it.nonInputProperties.systemProperty('tests.rest.cluster', "${-> testClusters."${clusterName}".allHttpSocketURI.join(",")}") it.nonInputProperties.systemProperty('tests.clustername', "${-> testClusters."${clusterName}".getName()}") } @@ -87,7 +97,7 @@ for (Version bwcVersion : bwcVersions.indexCompatible) { if (project.bwc_tests_enabled) { bwcTest.dependsOn( tasks.register("${baseName}#bwcTest") { - dependsOn tasks.named("${baseName}#downgradedClusterTest") + dependsOn tasks.named("${baseName}#reUpgradedClusterTest") } ) } diff --git a/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java b/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java index 9eb723e6821e7..1093d794bf53c 100644 --- a/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java +++ b/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java @@ -58,7 +58,8 @@ public class ClusterDowngradeIT extends ESRestTestCase { protected enum ClusterType { OLD, UPGRADED, - DOWNGRADED; + DOWNGRADED, + RE_UPGRADED; public static ClusterType parse(String value) { switch (value) { @@ -68,6 +69,8 @@ public static ClusterType parse(String value) { return UPGRADED; case "downgraded_cluster": return DOWNGRADED; + case "re_upgraded_cluster": + return RE_UPGRADED; default: throw new AssertionError("unknown cluster type: " + value); } @@ -120,7 +123,7 @@ protected boolean preserveSLMPoliciesUponCompletion() { public void testCreateSnapshot() throws IOException { final String repoName = "repo"; try (RestHighLevelClient client = new RestHighLevelClient(RestClient.builder(adminClient().getNodes().toArray(new Node[0])))) { - if (CLUSTER_TYPE == ClusterType.OLD) { + if (CLUSTER_TYPE == ClusterType.OLD || CLUSTER_TYPE == ClusterType.DOWNGRADED) { createIndex(client, "idx-1", 3); } if (CLUSTER_TYPE == ClusterType.OLD || CLUSTER_TYPE == ClusterType.DOWNGRADED) { @@ -155,6 +158,9 @@ public void testCreateSnapshot() throws IOException { break; case DOWNGRADED: assertThat(snapshots, hasSize(3)); + break; + case RE_UPGRADED: + assertThat(snapshots, hasSize(4)); } final SnapshotsStatusResponse statusResponse = client.snapshot().status( new SnapshotsStatusRequest(repoName, new String[]{"snapshot-old"}), RequestOptions.DEFAULT); @@ -168,6 +174,16 @@ public void testCreateSnapshot() throws IOException { RequestOptions.DEFAULT); assertThat(restoreSnapshotResponse.getRestoreInfo().failedShards(), is(0)); assertThat(restoreSnapshotResponse.getRestoreInfo().successfulShards(), greaterThanOrEqualTo(3)); + } else if (CLUSTER_TYPE == ClusterType.RE_UPGRADED) { + for (ClusterType value : ClusterType.values()) { + wipeAllIndices(); + final RestoreSnapshotResponse restoreSnapshotResponse = client.snapshot().restore( + new RestoreSnapshotRequest().repository(repoName) + .snapshot("snapshot-" + value.toString().toLowerCase(Locale.ROOT)).waitForCompletion(true), + RequestOptions.DEFAULT); + assertThat(restoreSnapshotResponse.getRestoreInfo().failedShards(), is(0)); + assertThat(restoreSnapshotResponse.getRestoreInfo().successfulShards(), greaterThanOrEqualTo(3)); + } } } } From b3497874d745e6586fc3e6074c64e775bd3feeba Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 7 Jan 2020 04:47:28 +0100 Subject: [PATCH 06/26] fixed --- .../snapshots/SnapshotsService.java | 34 +++++++++++++++---- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index cfb2112b4044d..a4dbd72016352 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -80,6 +80,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -270,6 +271,7 @@ public void createSnapshot(final CreateSnapshotRequest request, final ActionList final StepListener repositoryDataListener = new StepListener<>(); repositoriesService.repository(repositoryName).getRepositoryData(repositoryDataListener); repositoryDataListener.whenComplete(repositoryData -> { + final boolean hasOldFormatSnapshots = hasOldVersionSnapshots(repositoryName, repositoryData); clusterService.submitStateUpdateTask("create_snapshot [" + snapshotName + ']', new ClusterStateUpdateTask() { private SnapshotsInProgress.Entry newSnapshot = null; @@ -303,7 +305,8 @@ public ClusterState execute(ClusterState currentState) { repositoryData.getGenId(), null, request.userMetadata(), - clusterService.state().nodes().getMinNodeVersion().onOrAfter(SHARD_GEN_IN_REPO_DATA_VERSION)); + hasOldFormatSnapshots == false && + clusterService.state().nodes().getMinNodeVersion().onOrAfter(SHARD_GEN_IN_REPO_DATA_VERSION)); initializingSnapshots.add(newSnapshot.snapshot()); snapshots = new SnapshotsInProgress(newSnapshot); } else { @@ -351,6 +354,18 @@ public TimeValue timeout() { }, listener::onFailure); } + private boolean hasOldVersionSnapshots(String repositoryName, RepositoryData repositoryData) { + final List snapshotIds = List.copyOf(repositoryData.getSnapshotIds()); + final boolean hasOldFormatSnapshots; + if (snapshotIds.isEmpty()) { + hasOldFormatSnapshots = false; + } else { + hasOldFormatSnapshots = snapshots(repositoryName, snapshotIds, false).stream() + .anyMatch(snapshotInfo -> snapshotInfo.version().before(SHARD_GEN_IN_REPO_DATA_VERSION)); + } + return hasOldFormatSnapshots; + } + /** * Validates snapshot request * @@ -1402,12 +1417,17 @@ private void deleteSnapshotFromRepository(Snapshot snapshot, @Nullable ActionLis Version version) { threadPool.executor(ThreadPool.Names.SNAPSHOT).execute(ActionRunnable.wrap(listener, l -> { Repository repository = repositoriesService.repository(snapshot.getRepository()); - repository.deleteSnapshot(snapshot.getSnapshotId(), repositoryStateId, version.onOrAfter(SHARD_GEN_IN_REPO_DATA_VERSION), - ActionListener.wrap(v -> { - logger.info("snapshot [{}] deleted", snapshot); - removeSnapshotDeletionFromClusterState(snapshot, null, l); - }, ex -> removeSnapshotDeletionFromClusterState(snapshot, ex, l) - )); + repository.getRepositoryData(ActionListener.wrap(repositoryData -> { + //TODO: Check snapshot versions without the deleted snapshot and add this logic to repo cleanup + final boolean hasOldFormatSnapshots = hasOldVersionSnapshots(snapshot.getRepository(), repositoryData); + repository.deleteSnapshot(snapshot.getSnapshotId(), repositoryStateId, + hasOldFormatSnapshots == false && version.onOrAfter(SHARD_GEN_IN_REPO_DATA_VERSION), + ActionListener.wrap(v -> { + logger.info("snapshot [{}] deleted", snapshot); + removeSnapshotDeletionFromClusterState(snapshot, null, l); + }, ex -> removeSnapshotDeletionFromClusterState(snapshot, ex, l) + )); + }, ex -> removeSnapshotDeletionFromClusterState(snapshot, ex, l))); })); } From b61b8e9a67947a59af36cdc45b342119cea9ba33 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 7 Jan 2020 05:01:21 +0100 Subject: [PATCH 07/26] nicer --- .../TransportCleanupRepositoryAction.java | 20 ++++++++++++++----- .../snapshots/SnapshotsService.java | 11 +++++----- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java index 1273485823f76..4cfaf6abd6d88 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java @@ -77,6 +77,8 @@ public final class TransportCleanupRepositoryAction extends TransportMasterNodeA private final RepositoriesService repositoriesService; + private final SnapshotsService snapshotsService; + @Override protected String executor() { return ThreadPool.Names.GENERIC; @@ -84,11 +86,13 @@ protected String executor() { @Inject public TransportCleanupRepositoryAction(TransportService transportService, ClusterService clusterService, - RepositoriesService repositoriesService, ThreadPool threadPool, ActionFilters actionFilters, + RepositoriesService repositoriesService, SnapshotsService snapshotsService, + ThreadPool threadPool, ActionFilters actionFilters, IndexNameExpressionResolver indexNameExpressionResolver) { super(CleanupRepositoryAction.NAME, transportService, clusterService, threadPool, actionFilters, CleanupRepositoryRequest::new, indexNameExpressionResolver); this.repositoriesService = repositoriesService; + this.snapshotsService = snapshotsService; // We add a state applier that will remove any dangling repository cleanup actions on master failover. // This is safe to do since cleanups will increment the repository state id before executing any operations to prevent concurrent // operations from corrupting the repository. This is the same safety mechanism used by snapshot deletes. @@ -214,10 +218,16 @@ public void clusterStateProcessed(String source, ClusterState oldState, ClusterS startedCleanup = true; logger.debug("Initialized repository cleanup in cluster state for [{}][{}]", repositoryName, repositoryStateId); threadPool.executor(ThreadPool.Names.SNAPSHOT).execute(ActionRunnable.wrap(listener, - l -> blobStoreRepository.cleanup( - repositoryStateId, - newState.nodes().getMinNodeVersion().onOrAfter(SnapshotsService.SHARD_GEN_IN_REPO_DATA_VERSION), - ActionListener.wrap(result -> after(null, result), e -> after(e, null))))); + l -> { + final boolean hasOldFormatSnapshots = snapshotsService.hasOldVersionSnapshots( + repositoryName, repositoryData, null); + blobStoreRepository.cleanup( + repositoryStateId, + hasOldFormatSnapshots == false && + newState.nodes().getMinNodeVersion().onOrAfter(SnapshotsService.SHARD_GEN_IN_REPO_DATA_VERSION), + ActionListener.wrap(result -> after(null, result), e -> after(e, null))); + } + )); } private void after(@Nullable Exception failure, @Nullable RepositoryCleanupResult result) { diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index a4dbd72016352..025e9f25fd003 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -271,7 +271,7 @@ public void createSnapshot(final CreateSnapshotRequest request, final ActionList final StepListener repositoryDataListener = new StepListener<>(); repositoriesService.repository(repositoryName).getRepositoryData(repositoryDataListener); repositoryDataListener.whenComplete(repositoryData -> { - final boolean hasOldFormatSnapshots = hasOldVersionSnapshots(repositoryName, repositoryData); + final boolean hasOldFormatSnapshots = hasOldVersionSnapshots(repositoryName, repositoryData, null); clusterService.submitStateUpdateTask("create_snapshot [" + snapshotName + ']', new ClusterStateUpdateTask() { private SnapshotsInProgress.Entry newSnapshot = null; @@ -354,14 +354,15 @@ public TimeValue timeout() { }, listener::onFailure); } - private boolean hasOldVersionSnapshots(String repositoryName, RepositoryData repositoryData) { + public boolean hasOldVersionSnapshots(String repositoryName, RepositoryData repositoryData, @Nullable SnapshotId excluded) { final List snapshotIds = List.copyOf(repositoryData.getSnapshotIds()); final boolean hasOldFormatSnapshots; if (snapshotIds.isEmpty()) { hasOldFormatSnapshots = false; } else { hasOldFormatSnapshots = snapshots(repositoryName, snapshotIds, false).stream() - .anyMatch(snapshotInfo -> snapshotInfo.version().before(SHARD_GEN_IN_REPO_DATA_VERSION)); + .anyMatch(snapshotInfo -> (excluded == null || snapshotInfo.snapshotId().equals(excluded) == false) + && snapshotInfo.version().before(SHARD_GEN_IN_REPO_DATA_VERSION)); } return hasOldFormatSnapshots; } @@ -1418,8 +1419,8 @@ private void deleteSnapshotFromRepository(Snapshot snapshot, @Nullable ActionLis threadPool.executor(ThreadPool.Names.SNAPSHOT).execute(ActionRunnable.wrap(listener, l -> { Repository repository = repositoriesService.repository(snapshot.getRepository()); repository.getRepositoryData(ActionListener.wrap(repositoryData -> { - //TODO: Check snapshot versions without the deleted snapshot and add this logic to repo cleanup - final boolean hasOldFormatSnapshots = hasOldVersionSnapshots(snapshot.getRepository(), repositoryData); + final boolean hasOldFormatSnapshots = + hasOldVersionSnapshots(snapshot.getRepository(), repositoryData, snapshot.getSnapshotId()); repository.deleteSnapshot(snapshot.getSnapshotId(), repositoryStateId, hasOldFormatSnapshots == false && version.onOrAfter(SHARD_GEN_IN_REPO_DATA_VERSION), ActionListener.wrap(v -> { From 3dbe0c4a8f5ce92bb25ee5f8dda2e156af7a996e Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 7 Jan 2020 05:14:26 +0100 Subject: [PATCH 08/26] add assertion --- .../main/java/org/elasticsearch/snapshots/SnapshotsService.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index 025e9f25fd003..6dac5a74d1bcb 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -364,6 +364,8 @@ public boolean hasOldVersionSnapshots(String repositoryName, RepositoryData repo .anyMatch(snapshotInfo -> (excluded == null || snapshotInfo.snapshotId().equals(excluded) == false) && snapshotInfo.version().before(SHARD_GEN_IN_REPO_DATA_VERSION)); } + assert hasOldFormatSnapshots == false || repositoryData.shardGenerations().totalShards() == 0 : + "Found non-empty shard generations [" + repositoryData.shardGenerations() + "] but repository contained old version snapshots"; return hasOldFormatSnapshots; } From f9249816281b2640e0af2e5443427483375fd025 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 7 Jan 2020 05:36:00 +0100 Subject: [PATCH 09/26] add min version --- .../elasticsearch/repositories/RepositoryData.java | 13 +++++++++++++ .../elasticsearch/snapshots/SnapshotsService.java | 11 +++++++---- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoryData.java b/server/src/main/java/org/elasticsearch/repositories/RepositoryData.java index 63e2957aacc78..18cc43dc36221 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoryData.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoryData.java @@ -21,6 +21,7 @@ import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.ResourceNotFoundException; +import org.elasticsearch.Version; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -28,6 +29,7 @@ import org.elasticsearch.common.xcontent.XContentParserUtils; import org.elasticsearch.snapshots.SnapshotId; import org.elasticsearch.snapshots.SnapshotState; +import org.elasticsearch.snapshots.SnapshotsService; import java.io.IOException; import java.util.ArrayList; @@ -321,6 +323,7 @@ public List resolveNewIndices(final List indicesToResolve) { private static final String NAME = "name"; private static final String UUID = "uuid"; private static final String STATE = "state"; + private static final String MIN_VERSION = "min_version"; /** * Writes the snapshots metadata and the related indices metadata to x-content. @@ -361,6 +364,10 @@ public XContentBuilder snapshotsToXContent(final XContentBuilder builder, final builder.endObject(); } builder.endObject(); + if (shardGenerations.totalShards() > 0) { + // Add min version field to make it impossible for older ES versions to deserialize this object + builder.field(MIN_VERSION, SnapshotsService.SHARD_GEN_IN_REPO_DATA_VERSION.toString()); + } builder.endObject(); return builder; } @@ -468,6 +475,12 @@ public static RepositoryData snapshotsFromXContent(final XContentParser parser, shardGenerations.put(indexId, i, gens.get(i)); } } + } else if (MIN_VERSION.equals(field)) { + if (parser.nextToken() != XContentParser.Token.VALUE_STRING) { + throw new ElasticsearchParseException("version string expected [min_version]"); + } + final Version version = Version.fromString(parser.text()); + assert version.onOrAfter(SnapshotsService.SHARD_GEN_IN_REPO_DATA_VERSION); } else { throw new ElasticsearchParseException("unknown field name [" + field + "]"); } diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index 6dac5a74d1bcb..befe4130bf04f 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -80,7 +80,6 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -360,9 +359,13 @@ public boolean hasOldVersionSnapshots(String repositoryName, RepositoryData repo if (snapshotIds.isEmpty()) { hasOldFormatSnapshots = false; } else { - hasOldFormatSnapshots = snapshots(repositoryName, snapshotIds, false).stream() - .anyMatch(snapshotInfo -> (excluded == null || snapshotInfo.snapshotId().equals(excluded) == false) - && snapshotInfo.version().before(SHARD_GEN_IN_REPO_DATA_VERSION)); + if (repositoryData.shardGenerations().totalShards() > 0) { + hasOldFormatSnapshots = false; + } else { + hasOldFormatSnapshots = snapshots(repositoryName, snapshotIds, false).stream() + .anyMatch(snapshotInfo -> (excluded == null || snapshotInfo.snapshotId().equals(excluded) == false) + && snapshotInfo.version().before(SHARD_GEN_IN_REPO_DATA_VERSION)); + } } assert hasOldFormatSnapshots == false || repositoryData.shardGenerations().totalShards() == 0 : "Found non-empty shard generations [" + repositoryData.shardGenerations() + "] but repository contained old version snapshots"; From 93c57769c24ea2dc0e18268fc6ccb5561803e7d9 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 7 Jan 2020 07:55:37 +0100 Subject: [PATCH 10/26] bck --- .../org/elasticsearch/repositories/RepositoryData.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoryData.java b/server/src/main/java/org/elasticsearch/repositories/RepositoryData.java index 18cc43dc36221..1eddb89a2c901 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoryData.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoryData.java @@ -364,10 +364,11 @@ public XContentBuilder snapshotsToXContent(final XContentBuilder builder, final builder.endObject(); } builder.endObject(); - if (shardGenerations.totalShards() > 0) { + // TODO: bwc logic to allow for writing this field + //if (shardGenerations.totalShards() > 0) { // Add min version field to make it impossible for older ES versions to deserialize this object - builder.field(MIN_VERSION, SnapshotsService.SHARD_GEN_IN_REPO_DATA_VERSION.toString()); - } + //builder.field(MIN_VERSION, SnapshotsService.SHARD_GEN_IN_REPO_DATA_VERSION.toString()); + //} builder.endObject(); return builder; } From 13ce331bcc6860ea3185c173453c8eef883ddcf8 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 7 Jan 2020 08:17:53 +0100 Subject: [PATCH 11/26] fix compile --- .../TransportCleanupRepositoryAction.java | 14 +++++-------- .../snapshots/SnapshotsService.java | 20 +++++++++---------- .../snapshots/SnapshotResiliencyTests.java | 2 +- 3 files changed, 15 insertions(+), 21 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java index 4cfaf6abd6d88..1b2381ed89ec5 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java @@ -218,15 +218,11 @@ public void clusterStateProcessed(String source, ClusterState oldState, ClusterS startedCleanup = true; logger.debug("Initialized repository cleanup in cluster state for [{}][{}]", repositoryName, repositoryStateId); threadPool.executor(ThreadPool.Names.SNAPSHOT).execute(ActionRunnable.wrap(listener, - l -> { - final boolean hasOldFormatSnapshots = snapshotsService.hasOldVersionSnapshots( - repositoryName, repositoryData, null); - blobStoreRepository.cleanup( - repositoryStateId, - hasOldFormatSnapshots == false && - newState.nodes().getMinNodeVersion().onOrAfter(SnapshotsService.SHARD_GEN_IN_REPO_DATA_VERSION), - ActionListener.wrap(result -> after(null, result), e -> after(e, null))); - } + l -> blobStoreRepository.cleanup( + repositoryStateId, + newState.nodes().getMinNodeVersion().onOrAfter(SnapshotsService.SHARD_GEN_IN_REPO_DATA_VERSION) + && snapshotsService.hasOldVersionSnapshots(repositoryName, repositoryData, null) == false, + ActionListener.wrap(result -> after(null, result), e -> after(e, null))) )); } diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index befe4130bf04f..95d6777248786 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -1423,17 +1423,15 @@ private void deleteSnapshotFromRepository(Snapshot snapshot, @Nullable ActionLis Version version) { threadPool.executor(ThreadPool.Names.SNAPSHOT).execute(ActionRunnable.wrap(listener, l -> { Repository repository = repositoriesService.repository(snapshot.getRepository()); - repository.getRepositoryData(ActionListener.wrap(repositoryData -> { - final boolean hasOldFormatSnapshots = - hasOldVersionSnapshots(snapshot.getRepository(), repositoryData, snapshot.getSnapshotId()); - repository.deleteSnapshot(snapshot.getSnapshotId(), repositoryStateId, - hasOldFormatSnapshots == false && version.onOrAfter(SHARD_GEN_IN_REPO_DATA_VERSION), - ActionListener.wrap(v -> { - logger.info("snapshot [{}] deleted", snapshot); - removeSnapshotDeletionFromClusterState(snapshot, null, l); - }, ex -> removeSnapshotDeletionFromClusterState(snapshot, ex, l) - )); - }, ex -> removeSnapshotDeletionFromClusterState(snapshot, ex, l))); + repository.getRepositoryData(ActionListener.wrap(repositoryData -> repository.deleteSnapshot(snapshot.getSnapshotId(), + repositoryStateId, + version.onOrAfter(SHARD_GEN_IN_REPO_DATA_VERSION) && + hasOldVersionSnapshots(snapshot.getRepository(), repositoryData, snapshot.getSnapshotId()) == false, + ActionListener.wrap(v -> { + logger.info("snapshot [{}] deleted", snapshot); + removeSnapshotDeletionFromClusterState(snapshot, null, l); + }, ex -> removeSnapshotDeletionFromClusterState(snapshot, ex, l) + )), ex -> removeSnapshotDeletionFromClusterState(snapshot, ex, l))); })); } diff --git a/server/src/test/java/org/elasticsearch/snapshots/SnapshotResiliencyTests.java b/server/src/test/java/org/elasticsearch/snapshots/SnapshotResiliencyTests.java index 5fd6624d1d128..9b8950c813b4f 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/SnapshotResiliencyTests.java +++ b/server/src/test/java/org/elasticsearch/snapshots/SnapshotResiliencyTests.java @@ -1276,7 +1276,7 @@ searchTransportService, new SearchPhaseController(searchService::createReduceCon actionFilters, indexNameExpressionResolver )); actions.put(CleanupRepositoryAction.INSTANCE, new TransportCleanupRepositoryAction(transportService, clusterService, - repositoriesService, threadPool, actionFilters, indexNameExpressionResolver)); + repositoriesService, snapshotsService, threadPool, actionFilters, indexNameExpressionResolver)); actions.put(CreateSnapshotAction.INSTANCE, new TransportCreateSnapshotAction( transportService, clusterService, threadPool, From 04702eb2061c0cdf0b60ca48fcbfbd397802184c Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 7 Jan 2020 08:28:45 +0100 Subject: [PATCH 12/26] better efficiency --- .../java/org/elasticsearch/snapshots/SnapshotsService.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index 95d6777248786..43d0693009052 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -362,8 +362,9 @@ public boolean hasOldVersionSnapshots(String repositoryName, RepositoryData repo if (repositoryData.shardGenerations().totalShards() > 0) { hasOldFormatSnapshots = false; } else { - hasOldFormatSnapshots = snapshots(repositoryName, snapshotIds, false).stream() - .anyMatch(snapshotInfo -> (excluded == null || snapshotInfo.snapshotId().equals(excluded) == false) + final Repository repository = repositoriesService.repository(repositoryName); + hasOldFormatSnapshots = snapshotIds.stream().map(repository::getSnapshotInfo).anyMatch( + snapshotInfo -> (excluded == null || snapshotInfo.snapshotId().equals(excluded) == false) && snapshotInfo.version().before(SHARD_GEN_IN_REPO_DATA_VERSION)); } } From 3e7fef0dc6b031ff3c809da51086c379a090902c Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 7 Jan 2020 16:55:29 +0100 Subject: [PATCH 13/26] need 2 steps ... --- .../upgrades/ClusterDowngradeIT.java | 58 ++++++++++++------- .../repositories/RepositoryData.java | 9 +-- 2 files changed, 41 insertions(+), 26 deletions(-) diff --git a/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java b/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java index 1093d794bf53c..1f3bccc54e5d0 100644 --- a/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java +++ b/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java @@ -51,17 +51,24 @@ import static org.hamcrest.Matchers.is; /** - * Downgrade tests that verify that a snapshot repository is not getting corrupted and continues to function properly for downgrades. + * Downgrade tests that verify that a snapshot repository is not getting corrupted and continues to function properly during cluster + * downgrades. Concretely this test suit is simulating the following scenario: + *
    + *
  • Start from a cluster in an old version and create a snapshot containing a given index
    • + *
  • Upgrade the cluster to the current version and create another snapshot.
    • + *
  • Downgrade the cluster back to the old version and create another snapshot
    • + *
  • Once again upgrade the cluster to the current version and create a snapshot
    • + *
*/ public class ClusterDowngradeIT extends ESRestTestCase { - protected enum ClusterType { + protected enum TestStep { OLD, UPGRADED, DOWNGRADED, RE_UPGRADED; - public static ClusterType parse(String value) { + public static TestStep parse(String value) { switch (value) { case "old_cluster": return OLD; @@ -77,7 +84,7 @@ public static ClusterType parse(String value) { } } - protected static final ClusterType CLUSTER_TYPE = ClusterType.parse(System.getProperty("tests.rest.suite")); + protected static final TestStep CLUSTER_TYPE = TestStep.parse(System.getProperty("tests.rest.suite")); @Override protected boolean preserveIndicesUponCompletion() { @@ -119,14 +126,13 @@ protected boolean preserveSLMPoliciesUponCompletion() { return true; } - @SuppressWarnings("unchecked") public void testCreateSnapshot() throws IOException { final String repoName = "repo"; try (RestHighLevelClient client = new RestHighLevelClient(RestClient.builder(adminClient().getNodes().toArray(new Node[0])))) { - if (CLUSTER_TYPE == ClusterType.OLD || CLUSTER_TYPE == ClusterType.DOWNGRADED) { + if (CLUSTER_TYPE == TestStep.OLD || CLUSTER_TYPE == TestStep.DOWNGRADED) { createIndex(client, "idx-1", 3); } - if (CLUSTER_TYPE == ClusterType.OLD || CLUSTER_TYPE == ClusterType.DOWNGRADED) { + if (CLUSTER_TYPE == TestStep.OLD || CLUSTER_TYPE == TestStep.DOWNGRADED) { assertThat(client.snapshot().createRepository(new PutRepositoryRequest(repoName).type("fs").settings( Settings.builder().put("location", ".")), RequestOptions.DEFAULT).isAcknowledged(), is(true)); } @@ -137,18 +143,9 @@ public void testCreateSnapshot() throws IOException { new CreateSnapshotRequest(repoName, "snapshot-to-delete").waitForCompletion(true), RequestOptions.DEFAULT); client.snapshot().delete(new DeleteSnapshotRequest(repoName, "snapshot-to-delete"), RequestOptions.DEFAULT); final List> snapshots; - Response response = client.getLowLevelClient().performRequest(new Request("GET", "/_snapshot/" + repoName + "/_all")); - try (InputStream entity = response.getEntity().getContent(); - XContentParser parser = JsonXContent.jsonXContent.createParser( - xContentRegistry(), DeprecationHandler.THROW_UNSUPPORTED_OPERATION, entity)) { - final Map raw = parser.map(); - // Bwc lookup since the format of the snapshots response changed between versions - if (raw.containsKey("snapshots")) { - snapshots = (List>) raw.get("snapshots"); - } else { - snapshots = (List>) ((List>) raw.get("responses")).get(0).get("snapshots"); - } - } + Response response = client.getLowLevelClient().performRequest( + new Request("GET", "/_snapshot/" + repoName + "/_all")); + snapshots = readSnapshotsList(response); switch (CLUSTER_TYPE) { case OLD: assertThat(snapshots, hasSize(1)); @@ -167,15 +164,15 @@ public void testCreateSnapshot() throws IOException { for (SnapshotStatus status : statusResponse.getSnapshots()) { assertThat(status.getShardsStats().getFailedShards(), is(0)); } - if (CLUSTER_TYPE == ClusterType.DOWNGRADED) { + if (CLUSTER_TYPE == TestStep.DOWNGRADED) { wipeAllIndices(); final RestoreSnapshotResponse restoreSnapshotResponse = client.snapshot().restore( new RestoreSnapshotRequest().repository(repoName).snapshot("snapshot-old").waitForCompletion(true), RequestOptions.DEFAULT); assertThat(restoreSnapshotResponse.getRestoreInfo().failedShards(), is(0)); assertThat(restoreSnapshotResponse.getRestoreInfo().successfulShards(), greaterThanOrEqualTo(3)); - } else if (CLUSTER_TYPE == ClusterType.RE_UPGRADED) { - for (ClusterType value : ClusterType.values()) { + } else if (CLUSTER_TYPE == TestStep.RE_UPGRADED) { + for (TestStep value : TestStep.values()) { wipeAllIndices(); final RestoreSnapshotResponse restoreSnapshotResponse = client.snapshot().restore( new RestoreSnapshotRequest().repository(repoName) @@ -188,6 +185,23 @@ public void testCreateSnapshot() throws IOException { } } + @SuppressWarnings("unchecked") + private List> readSnapshotsList(final Response response) throws IOException { + final List> snapshots; + try (InputStream entity = response.getEntity().getContent(); + XContentParser parser = JsonXContent.jsonXContent.createParser( + xContentRegistry(), DeprecationHandler.THROW_UNSUPPORTED_OPERATION, entity)) { + final Map raw = parser.map(); + // Bwc lookup since the format of the snapshots response changed between versions + if (raw.containsKey("snapshots")) { + snapshots = (List>) raw.get("snapshots"); + } else { + snapshots = (List>) ((List>) raw.get("responses")).get(0).get("snapshots"); + } + } + return snapshots; + } + private void createIndex(RestHighLevelClient client, String name, int shards) throws IOException { final Request putIndexRequest = new Request("PUT", "/" + name); putIndexRequest.setJsonEntity("{\n" + diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoryData.java b/server/src/main/java/org/elasticsearch/repositories/RepositoryData.java index 1eddb89a2c901..f41a28686e38b 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoryData.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoryData.java @@ -364,11 +364,12 @@ public XContentBuilder snapshotsToXContent(final XContentBuilder builder, final builder.endObject(); } builder.endObject(); - // TODO: bwc logic to allow for writing this field - //if (shardGenerations.totalShards() > 0) { + if (shouldWriteShardGens) { + // TODO: write this field once 7.6 is able to read it and add tests to :qa:snapshot-repository-downgrade that make sure older + // ES versions can't corrupt the repository by writing to it and all the snapshots in it are v7.6 or newer // Add min version field to make it impossible for older ES versions to deserialize this object - //builder.field(MIN_VERSION, SnapshotsService.SHARD_GEN_IN_REPO_DATA_VERSION.toString()); - //} + // builder.field(MIN_VERSION, SnapshotsService.SHARD_GEN_IN_REPO_DATA_VERSION.toString()); + } builder.endObject(); return builder; } From 4c0499970647f6ce74c3458223b6daac4c29ac26 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 7 Jan 2020 16:59:28 +0100 Subject: [PATCH 14/26] document test --- .../org/elasticsearch/upgrades/ClusterDowngradeIT.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java b/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java index 1f3bccc54e5d0..6155753f0631e 100644 --- a/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java +++ b/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java @@ -54,11 +54,13 @@ * Downgrade tests that verify that a snapshot repository is not getting corrupted and continues to function properly during cluster * downgrades. Concretely this test suit is simulating the following scenario: *
    - *
  • Start from a cluster in an old version and create a snapshot containing a given index
    • - *
  • Upgrade the cluster to the current version and create another snapshot.
    • - *
  • Downgrade the cluster back to the old version and create another snapshot
    • - *
  • Once again upgrade the cluster to the current version and create a snapshot
    • + *
  • Start from a cluster in an old version
    • + *
  • Upgrade the cluster to the current version
    • + *
  • Downgrade the cluster back to the old version
    • + *
  • Once again upgrade the cluster to the current version
    • *
+ * TODO: Add two more steps: delete all old version snapshots from the repository, then downgrade again and verify that the repository + * is not being corrupted. This requires first merging the logic for reading the min_version field in RepositoryData back to 7.6. */ public class ClusterDowngradeIT extends ESRestTestCase { From cb509701350eb36654a9dac16e7fb62b3a5eb021 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 7 Jan 2020 17:07:23 +0100 Subject: [PATCH 15/26] drier --- qa/snapshot-repository-downgrade/build.gradle | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/qa/snapshot-repository-downgrade/build.gradle b/qa/snapshot-repository-downgrade/build.gradle index d4f81e1c8cadb..87bfa607ecee7 100644 --- a/qa/snapshot-repository-downgrade/build.gradle +++ b/qa/snapshot-repository-downgrade/build.gradle @@ -36,20 +36,16 @@ dependencies { for (Version bwcVersion : bwcVersions.indexCompatible) { String baseName = "v${bwcVersion}" - testClusters { - "${baseName}" { - versions = [bwcVersion.toString(), project.version] - numberOfNodes = 2 - setting 'path.repo', "${buildDir}/cluster/shared/repo/${baseName}" - javaHome = BuildParams.runtimeJavaHome - } + def clusterSettings = { + versions = [bwcVersion.toString(), project.version] + numberOfNodes = 2 + setting 'path.repo', "${buildDir}/cluster/shared/repo/${baseName}" + javaHome = BuildParams.runtimeJavaHome + } - "${baseName}-downgraded" { - versions = [bwcVersion.toString(), project.version] - numberOfNodes = 2 - setting 'path.repo', "${buildDir}/cluster/shared/repo/${baseName}" - javaHome = BuildParams.runtimeJavaHome - } + testClusters { + "${baseName}" clusterSettings + "${baseName}-downgraded" clusterSettings } tasks.register("${baseName}#oldClusterTest", RestTestRunnerTask) { From 5004dda3a0daec9f2ce04df12b05feab04802f8d Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 7 Jan 2020 21:05:19 +0100 Subject: [PATCH 16/26] drier --- qa/snapshot-repository-downgrade/build.gradle | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/qa/snapshot-repository-downgrade/build.gradle b/qa/snapshot-repository-downgrade/build.gradle index 87bfa607ecee7..57c9c08890aff 100644 --- a/qa/snapshot-repository-downgrade/build.gradle +++ b/qa/snapshot-repository-downgrade/build.gradle @@ -35,6 +35,7 @@ dependencies { for (Version bwcVersion : bwcVersions.indexCompatible) { String baseName = "v${bwcVersion}" + String downgradedClusterName = "${baseName}-downgraded" def clusterSettings = { versions = [bwcVersion.toString(), project.version] @@ -45,7 +46,7 @@ for (Version bwcVersion : bwcVersions.indexCompatible) { testClusters { "${baseName}" clusterSettings - "${baseName}-downgraded" clusterSettings + "${downgradedClusterName}" clusterSettings } tasks.register("${baseName}#oldClusterTest", RestTestRunnerTask) { @@ -67,25 +68,24 @@ for (Version bwcVersion : bwcVersions.indexCompatible) { } tasks.register("${baseName}#downgradedClusterTest", RestTestRunnerTask) { - useCluster testClusters."${baseName}-downgraded" + useCluster testClusters."${downgradedClusterName}" dependsOn "${baseName}#upgradedClusterTest" systemProperty 'tests.rest.suite', 'downgraded_cluster' } tasks.register("${baseName}#reUpgradedClusterTest", RestTestRunnerTask) { - useCluster testClusters."${baseName}-downgraded" + useCluster testClusters."${downgradedClusterName}" dependsOn "${baseName}#downgradedClusterTest" doFirst { - testClusters."${baseName}-downgraded".goToNextVersion() + testClusters."${downgradedClusterName}".goToNextVersion() } systemProperty 'tests.rest.suite', 're_upgraded_cluster' } - tasks.matching { it.name.startsWith(baseName) && it.name.endsWith("ClusterTest") }.configureEach { it.systemProperty 'tests.old_cluster_version', bwcVersion.toString().minus("-SNAPSHOT") it.systemProperty 'tests.path.repo', "${buildDir}/cluster/shared/repo/${baseName}" - def clusterName = it.name.contains("downgraded") || it.name.contains("reUpgraded") ? "${baseName}-downgraded" : "${baseName}" + def clusterName = it.name.contains("downgraded") || it.name.contains("reUpgraded") ? "${downgradedClusterName}" : "${baseName}" it.nonInputProperties.systemProperty('tests.rest.cluster', "${-> testClusters."${clusterName}".allHttpSocketURI.join(",")}") it.nonInputProperties.systemProperty('tests.clustername', "${-> testClusters."${clusterName}".getName()}") } From 33a3acdcb81dc0b22e2a6f52daec3a8663ea2944 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 7 Jan 2020 21:38:54 +0100 Subject: [PATCH 17/26] drier --- .../upgrades/ClusterDowngradeIT.java | 80 ++++++++++--------- 1 file changed, 41 insertions(+), 39 deletions(-) diff --git a/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java b/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java index 6155753f0631e..367bc9d7cf685 100644 --- a/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java +++ b/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java @@ -23,7 +23,6 @@ import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotRequest; import org.elasticsearch.action.admin.cluster.snapshots.delete.DeleteSnapshotRequest; import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotRequest; -import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse; import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotStatus; import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotsStatusRequest; import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotsStatusResponse; @@ -37,6 +36,7 @@ import org.elasticsearch.common.xcontent.DeprecationHandler; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.snapshots.RestoreInfo; import org.elasticsearch.test.rest.ESRestTestCase; import java.io.IOException; @@ -46,7 +46,7 @@ import java.util.Locale; import java.util.Map; -import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; @@ -54,10 +54,10 @@ * Downgrade tests that verify that a snapshot repository is not getting corrupted and continues to function properly during cluster * downgrades. Concretely this test suit is simulating the following scenario: *
    - *
  • Start from a cluster in an old version
    • - *
  • Upgrade the cluster to the current version
    • - *
  • Downgrade the cluster back to the old version
    • - *
  • Once again upgrade the cluster to the current version
    • + *
  • Start from a cluster in an old version: {@link TestStep#OLD}
    • + *
  • Upgrade the cluster to the current version: {@link TestStep#UPGRADED}
    • + *
  • Downgrade the cluster back to the old version: {@link TestStep#DOWNGRADED}
    • + *
  • Once again upgrade the cluster to the current version: {@link TestStep#RE_UPGRADED}
    • *
* TODO: Add two more steps: delete all old version snapshots from the repository, then downgrade again and verify that the repository * is not being corrupted. This requires first merging the logic for reading the min_version field in RepositoryData back to 7.6. @@ -81,12 +81,12 @@ public static TestStep parse(String value) { case "re_upgraded_cluster": return RE_UPGRADED; default: - throw new AssertionError("unknown cluster type: " + value); + throw new AssertionError("unknown test step: " + value); } } } - protected static final TestStep CLUSTER_TYPE = TestStep.parse(System.getProperty("tests.rest.suite")); + protected static final TestStep TEST_STEP = TestStep.parse(System.getProperty("tests.rest.suite")); @Override protected boolean preserveIndicesUponCompletion() { @@ -131,24 +131,21 @@ protected boolean preserveSLMPoliciesUponCompletion() { public void testCreateSnapshot() throws IOException { final String repoName = "repo"; try (RestHighLevelClient client = new RestHighLevelClient(RestClient.builder(adminClient().getNodes().toArray(new Node[0])))) { - if (CLUSTER_TYPE == TestStep.OLD || CLUSTER_TYPE == TestStep.DOWNGRADED) { - createIndex(client, "idx-1", 3); - } - if (CLUSTER_TYPE == TestStep.OLD || CLUSTER_TYPE == TestStep.DOWNGRADED) { + final int shards = 3; + if (TEST_STEP == TestStep.OLD || TEST_STEP == TestStep.DOWNGRADED) { + createIndex(client, "test-index", shards); assertThat(client.snapshot().createRepository(new PutRepositoryRequest(repoName).type("fs").settings( Settings.builder().put("location", ".")), RequestOptions.DEFAULT).isAcknowledged(), is(true)); } - client.snapshot().create( - new CreateSnapshotRequest(repoName, "snapshot-" + CLUSTER_TYPE.toString().toLowerCase(Locale.ROOT)) - .waitForCompletion(true), RequestOptions.DEFAULT); - client.snapshot().create( - new CreateSnapshotRequest(repoName, "snapshot-to-delete").waitForCompletion(true), RequestOptions.DEFAULT); - client.snapshot().delete(new DeleteSnapshotRequest(repoName, "snapshot-to-delete"), RequestOptions.DEFAULT); - final List> snapshots; - Response response = client.getLowLevelClient().performRequest( - new Request("GET", "/_snapshot/" + repoName + "/_all")); - snapshots = readSnapshotsList(response); - switch (CLUSTER_TYPE) { + createSnapshot(client, repoName, "snapshot-" + TEST_STEP.toString().toLowerCase(Locale.ROOT)); + final String snapshotToDeleteName = "snapshot-to-delete"; + // Create a snapshot and delete it right away again to test the impact of each version's cleanup functionality that is run + // as part of the snapshot delete + createSnapshot(client, repoName, snapshotToDeleteName); + client.snapshot().delete(new DeleteSnapshotRequest(repoName, snapshotToDeleteName), RequestOptions.DEFAULT); + final List> snapshots = readSnapshotsList( + client.getLowLevelClient().performRequest(new Request("GET", "/_snapshot/" + repoName + "/_all"))); + switch (TEST_STEP) { case OLD: assertThat(snapshots, hasSize(1)); break; @@ -161,34 +158,39 @@ public void testCreateSnapshot() throws IOException { case RE_UPGRADED: assertThat(snapshots, hasSize(4)); } - final SnapshotsStatusResponse statusResponse = client.snapshot().status( - new SnapshotsStatusRequest(repoName, new String[]{"snapshot-old"}), RequestOptions.DEFAULT); + final SnapshotsStatusResponse statusResponse = client.snapshot().status(new SnapshotsStatusRequest(repoName, + snapshots.stream().map(sn -> (String) sn.get("snapshot")).toArray(String[]::new)), RequestOptions.DEFAULT); for (SnapshotStatus status : statusResponse.getSnapshots()) { assertThat(status.getShardsStats().getFailedShards(), is(0)); } - if (CLUSTER_TYPE == TestStep.DOWNGRADED) { + if (TEST_STEP == TestStep.DOWNGRADED) { wipeAllIndices(); - final RestoreSnapshotResponse restoreSnapshotResponse = client.snapshot().restore( - new RestoreSnapshotRequest().repository(repoName).snapshot("snapshot-old").waitForCompletion(true), - RequestOptions.DEFAULT); - assertThat(restoreSnapshotResponse.getRestoreInfo().failedShards(), is(0)); - assertThat(restoreSnapshotResponse.getRestoreInfo().successfulShards(), greaterThanOrEqualTo(3)); - } else if (CLUSTER_TYPE == TestStep.RE_UPGRADED) { + final RestoreInfo restoreInfo = restoreSnapshot(client, repoName, "snapshot-old"); + assertThat(restoreInfo.failedShards(), is(0)); + assertThat(restoreInfo.successfulShards(), equalTo(shards)); + } else if (TEST_STEP == TestStep.RE_UPGRADED) { for (TestStep value : TestStep.values()) { wipeAllIndices(); - final RestoreSnapshotResponse restoreSnapshotResponse = client.snapshot().restore( - new RestoreSnapshotRequest().repository(repoName) - .snapshot("snapshot-" + value.toString().toLowerCase(Locale.ROOT)).waitForCompletion(true), - RequestOptions.DEFAULT); - assertThat(restoreSnapshotResponse.getRestoreInfo().failedShards(), is(0)); - assertThat(restoreSnapshotResponse.getRestoreInfo().successfulShards(), greaterThanOrEqualTo(3)); + final RestoreInfo restoreInfo = + restoreSnapshot(client, repoName, "snapshot-" + value.toString().toLowerCase(Locale.ROOT)); + assertThat(restoreInfo.failedShards(), is(0)); + assertThat(restoreInfo.successfulShards(), equalTo(shards)); } } } } + private static RestoreInfo restoreSnapshot(RestHighLevelClient client, String repoName, String name) throws IOException { + return client.snapshot().restore(new RestoreSnapshotRequest().repository(repoName).snapshot(name).waitForCompletion(true), + RequestOptions.DEFAULT).getRestoreInfo(); + } + + private static void createSnapshot(RestHighLevelClient client, String repoName, String name) throws IOException { + client.snapshot().create(new CreateSnapshotRequest(repoName, name).waitForCompletion(true), RequestOptions.DEFAULT); + } + @SuppressWarnings("unchecked") - private List> readSnapshotsList(final Response response) throws IOException { + private List> readSnapshotsList(Response response) throws IOException { final List> snapshots; try (InputStream entity = response.getEntity().getContent(); XContentParser parser = JsonXContent.jsonXContent.createParser( From 72cfb1b8dbe85afe4976311e6a4fc84994298532 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Wed, 8 Jan 2020 10:21:56 +0100 Subject: [PATCH 18/26] nicer --- .../upgrades/ClusterDowngradeIT.java | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java b/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java index 367bc9d7cf685..f6568f4164594 100644 --- a/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java +++ b/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java @@ -164,25 +164,23 @@ public void testCreateSnapshot() throws IOException { assertThat(status.getShardsStats().getFailedShards(), is(0)); } if (TEST_STEP == TestStep.DOWNGRADED) { - wipeAllIndices(); - final RestoreInfo restoreInfo = restoreSnapshot(client, repoName, "snapshot-old"); - assertThat(restoreInfo.failedShards(), is(0)); - assertThat(restoreInfo.successfulShards(), equalTo(shards)); + ensureSnapshotRestoreWorks(client, repoName, "snapshot-old", shards); } else if (TEST_STEP == TestStep.RE_UPGRADED) { for (TestStep value : TestStep.values()) { - wipeAllIndices(); - final RestoreInfo restoreInfo = - restoreSnapshot(client, repoName, "snapshot-" + value.toString().toLowerCase(Locale.ROOT)); - assertThat(restoreInfo.failedShards(), is(0)); - assertThat(restoreInfo.successfulShards(), equalTo(shards)); + ensureSnapshotRestoreWorks(client, repoName, "snapshot-" + value.toString().toLowerCase(Locale.ROOT), shards); } } } } - private static RestoreInfo restoreSnapshot(RestHighLevelClient client, String repoName, String name) throws IOException { - return client.snapshot().restore(new RestoreSnapshotRequest().repository(repoName).snapshot(name).waitForCompletion(true), - RequestOptions.DEFAULT).getRestoreInfo(); + private static void ensureSnapshotRestoreWorks(RestHighLevelClient client, String repoName, String name, + int shards) throws IOException { + wipeAllIndices(); + final RestoreInfo restoreInfo = + client.snapshot().restore(new RestoreSnapshotRequest().repository(repoName).snapshot(name).waitForCompletion(true), + RequestOptions.DEFAULT).getRestoreInfo(); + assertThat(restoreInfo.failedShards(), is(0)); + assertThat(restoreInfo.successfulShards(), equalTo(shards)); } private static void createSnapshot(RestHighLevelClient client, String repoName, String name) throws IOException { From bbb2b3d8942b4768e583f0823bda7f69de863dd3 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Wed, 8 Jan 2020 12:33:31 +0100 Subject: [PATCH 19/26] CR: rename qa project --- .../build.gradle | 0 .../test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename qa/{snapshot-repository-downgrade => repository-downgrade}/build.gradle (100%) rename qa/{snapshot-repository-downgrade => repository-downgrade}/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java (100%) diff --git a/qa/snapshot-repository-downgrade/build.gradle b/qa/repository-downgrade/build.gradle similarity index 100% rename from qa/snapshot-repository-downgrade/build.gradle rename to qa/repository-downgrade/build.gradle diff --git a/qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java b/qa/repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java similarity index 100% rename from qa/snapshot-repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java rename to qa/repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java From 1321106bad5462b06b14019ba63e0c624d35e17a Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Wed, 8 Jan 2020 12:46:26 +0100 Subject: [PATCH 20/26] CR: remove dead code --- .../upgrades/ClusterDowngradeIT.java | 15 --------------- .../elasticsearch/snapshots/SnapshotsService.java | 3 ++- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/qa/repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java b/qa/repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java index f6568f4164594..16606400c9608 100644 --- a/qa/repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java +++ b/qa/repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java @@ -113,21 +113,6 @@ protected boolean preserveClusterSettings() { return true; } - @Override - protected boolean preserveRollupJobsUponCompletion() { - return true; - } - - @Override - protected boolean preserveILMPoliciesUponCompletion() { - return true; - } - - @Override - protected boolean preserveSLMPoliciesUponCompletion() { - return true; - } - public void testCreateSnapshot() throws IOException { final String repoName = "repo"; try (RestHighLevelClient client = new RestHighLevelClient(RestClient.builder(adminClient().getNodes().toArray(new Node[0])))) { diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index 43d0693009052..d676fd27215d8 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -80,6 +80,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -354,7 +355,7 @@ public TimeValue timeout() { } public boolean hasOldVersionSnapshots(String repositoryName, RepositoryData repositoryData, @Nullable SnapshotId excluded) { - final List snapshotIds = List.copyOf(repositoryData.getSnapshotIds()); + final Collection snapshotIds = repositoryData.getSnapshotIds(); final boolean hasOldFormatSnapshots; if (snapshotIds.isEmpty()) { hasOldFormatSnapshots = false; From 2637126b9f73fd5a806f2069604cffdddfa2ec6d Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Wed, 8 Jan 2020 14:14:53 +0100 Subject: [PATCH 21/26] CR: more tests and renaming --- qa/repository-downgrade/build.gradle | 36 ++-- ...va => MultiVersionRepositoryAccessIT.java} | 181 ++++++++++++------ .../test/rest/ESRestTestCase.java | 8 +- 3 files changed, 147 insertions(+), 78 deletions(-) rename qa/repository-downgrade/src/test/java/org/elasticsearch/upgrades/{ClusterDowngradeIT.java => MultiVersionRepositoryAccessIT.java} (51%) diff --git a/qa/repository-downgrade/build.gradle b/qa/repository-downgrade/build.gradle index 57c9c08890aff..95a09ea03f4a4 100644 --- a/qa/repository-downgrade/build.gradle +++ b/qa/repository-downgrade/build.gradle @@ -35,7 +35,7 @@ dependencies { for (Version bwcVersion : bwcVersions.indexCompatible) { String baseName = "v${bwcVersion}" - String downgradedClusterName = "${baseName}-downgraded" + String revertedClusterName = "${baseName}-reverted" def clusterSettings = { versions = [bwcVersion.toString(), project.version] @@ -46,46 +46,46 @@ for (Version bwcVersion : bwcVersions.indexCompatible) { testClusters { "${baseName}" clusterSettings - "${downgradedClusterName}" clusterSettings + "${revertedClusterName}" clusterSettings } - tasks.register("${baseName}#oldClusterTest", RestTestRunnerTask) { + tasks.register("${baseName}#Step1OldClusterTest", RestTestRunnerTask) { useCluster testClusters."${baseName}" mustRunAfter(precommit) doFirst { project.delete("${buildDir}/cluster/shared/repo/${baseName}") } - systemProperty 'tests.rest.suite', 'old_cluster' + systemProperty 'tests.rest.suite', 'step1' } - tasks.register("${baseName}#upgradedClusterTest", RestTestRunnerTask) { + tasks.register("${baseName}#Step2NewClusterTest", RestTestRunnerTask) { useCluster testClusters."${baseName}" - dependsOn "${baseName}#oldClusterTest" + dependsOn "${baseName}#Step1OldClusterTest" doFirst { testClusters."${baseName}".goToNextVersion() } - systemProperty 'tests.rest.suite', 'upgraded_cluster' + systemProperty 'tests.rest.suite', 'step2' } - tasks.register("${baseName}#downgradedClusterTest", RestTestRunnerTask) { - useCluster testClusters."${downgradedClusterName}" - dependsOn "${baseName}#upgradedClusterTest" - systemProperty 'tests.rest.suite', 'downgraded_cluster' + tasks.register("${baseName}#Step3OldClusterTest", RestTestRunnerTask) { + useCluster testClusters."${revertedClusterName}" + dependsOn "${baseName}#Step2NewClusterTest" + systemProperty 'tests.rest.suite', 'step3' } - tasks.register("${baseName}#reUpgradedClusterTest", RestTestRunnerTask) { - useCluster testClusters."${downgradedClusterName}" - dependsOn "${baseName}#downgradedClusterTest" + tasks.register("${baseName}#Step4NewClusterTest", RestTestRunnerTask) { + useCluster testClusters."${revertedClusterName}" + dependsOn "${baseName}#Step3OldClusterTest" doFirst { - testClusters."${downgradedClusterName}".goToNextVersion() + testClusters."${revertedClusterName}".goToNextVersion() } - systemProperty 'tests.rest.suite', 're_upgraded_cluster' + systemProperty 'tests.rest.suite', 'step4' } tasks.matching { it.name.startsWith(baseName) && it.name.endsWith("ClusterTest") }.configureEach { it.systemProperty 'tests.old_cluster_version', bwcVersion.toString().minus("-SNAPSHOT") it.systemProperty 'tests.path.repo', "${buildDir}/cluster/shared/repo/${baseName}" - def clusterName = it.name.contains("downgraded") || it.name.contains("reUpgraded") ? "${downgradedClusterName}" : "${baseName}" + def clusterName = it.name.contains("Step3") || it.name.contains("Step4") ? "${revertedClusterName}" : "${baseName}" it.nonInputProperties.systemProperty('tests.rest.cluster', "${-> testClusters."${clusterName}".allHttpSocketURI.join(",")}") it.nonInputProperties.systemProperty('tests.clustername', "${-> testClusters."${clusterName}".getName()}") } @@ -93,7 +93,7 @@ for (Version bwcVersion : bwcVersions.indexCompatible) { if (project.bwc_tests_enabled) { bwcTest.dependsOn( tasks.register("${baseName}#bwcTest") { - dependsOn tasks.named("${baseName}#reUpgradedClusterTest") + dependsOn tasks.named("${baseName}#Step4NewClusterTest") } ) } diff --git a/qa/repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java b/qa/repository-downgrade/src/test/java/org/elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java similarity index 51% rename from qa/repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java rename to qa/repository-downgrade/src/test/java/org/elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java index 16606400c9608..c0b7309bc98a1 100644 --- a/qa/repository-downgrade/src/test/java/org/elasticsearch/upgrades/ClusterDowngradeIT.java +++ b/qa/repository-downgrade/src/test/java/org/elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java @@ -43,7 +43,6 @@ import java.io.InputStream; import java.net.HttpURLConnection; import java.util.List; -import java.util.Locale; import java.util.Map; import static org.hamcrest.Matchers.equalTo; @@ -51,35 +50,46 @@ import static org.hamcrest.Matchers.is; /** - * Downgrade tests that verify that a snapshot repository is not getting corrupted and continues to function properly during cluster - * downgrades. Concretely this test suit is simulating the following scenario: + * Tests that verify that a snapshot repository is not getting corrupted and continues to function properly when accessed by multiple + * clusters of different versions. Concretely this test suit is simulating the following scenario: *
    - *
  • Start from a cluster in an old version: {@link TestStep#OLD}
    • - *
  • Upgrade the cluster to the current version: {@link TestStep#UPGRADED}
    • - *
  • Downgrade the cluster back to the old version: {@link TestStep#DOWNGRADED}
    • - *
  • Once again upgrade the cluster to the current version: {@link TestStep#RE_UPGRADED}
    • + *
  • Start a cluster in an old version: {@link TestStep#STEP1_OLD_CLUSTER}
    • + *
  • Start a cluster running the current version: {@link TestStep#STEP2_NEW_CLUSTER}
    • + *
  • Again start a cluster in an old version: {@link TestStep#STEP3_OLD_CLUSTER}
    • + *
  • Once again start a cluster running the current version: {@link TestStep#STEP4_NEW_CLUSTER}
    • *
* TODO: Add two more steps: delete all old version snapshots from the repository, then downgrade again and verify that the repository * is not being corrupted. This requires first merging the logic for reading the min_version field in RepositoryData back to 7.6. */ -public class ClusterDowngradeIT extends ESRestTestCase { +public class MultiVersionRepositoryAccessIT extends ESRestTestCase { - protected enum TestStep { - OLD, - UPGRADED, - DOWNGRADED, - RE_UPGRADED; + private enum TestStep { + STEP1_OLD_CLUSTER("step1"), + STEP2_NEW_CLUSTER("step2"), + STEP3_OLD_CLUSTER("step3"), + STEP4_NEW_CLUSTER("step4"); + + private final String name; + + TestStep(String name) { + this.name = name; + } + + @Override + public String toString() { + return name; + } public static TestStep parse(String value) { switch (value) { - case "old_cluster": - return OLD; - case "upgraded_cluster": - return UPGRADED; - case "downgraded_cluster": - return DOWNGRADED; - case "re_upgraded_cluster": - return RE_UPGRADED; + case "step1": + return STEP1_OLD_CLUSTER; + case "step2": + return STEP2_NEW_CLUSTER; + case "step3": + return STEP3_OLD_CLUSTER; + case "step4": + return STEP4_NEW_CLUSTER; default: throw new AssertionError("unknown test step: " + value); } @@ -88,11 +98,6 @@ public static TestStep parse(String value) { protected static final TestStep TEST_STEP = TestStep.parse(System.getProperty("tests.rest.suite")); - @Override - protected boolean preserveIndicesUponCompletion() { - return true; - } - @Override protected boolean preserveSnapshotsUponCompletion() { return true; @@ -103,61 +108,115 @@ protected boolean preserveReposUponCompletion() { return true; } - @Override - protected boolean preserveTemplatesUponCompletion() { - return true; - } - - @Override - protected boolean preserveClusterSettings() { - return true; - } - - public void testCreateSnapshot() throws IOException { - final String repoName = "repo"; + public void testCreateAndRestoreSnapshot() throws IOException { + final String repoName = "testCreateAndRestoreSnapshot"; try (RestHighLevelClient client = new RestHighLevelClient(RestClient.builder(adminClient().getNodes().toArray(new Node[0])))) { final int shards = 3; - if (TEST_STEP == TestStep.OLD || TEST_STEP == TestStep.DOWNGRADED) { - createIndex(client, "test-index", shards); - assertThat(client.snapshot().createRepository(new PutRepositoryRequest(repoName).type("fs").settings( - Settings.builder().put("location", ".")), RequestOptions.DEFAULT).isAcknowledged(), is(true)); - } - createSnapshot(client, repoName, "snapshot-" + TEST_STEP.toString().toLowerCase(Locale.ROOT)); + createIndex(client, "test-index", shards); + createRepository(client, repoName, false); + createSnapshot(client, repoName, "snapshot-" + TEST_STEP); final String snapshotToDeleteName = "snapshot-to-delete"; // Create a snapshot and delete it right away again to test the impact of each version's cleanup functionality that is run // as part of the snapshot delete createSnapshot(client, repoName, snapshotToDeleteName); - client.snapshot().delete(new DeleteSnapshotRequest(repoName, snapshotToDeleteName), RequestOptions.DEFAULT); + deleteSnapshot(client, repoName, snapshotToDeleteName); + final List> snapshots = readSnapshotsList( + client.getLowLevelClient().performRequest(new Request("GET", "/_snapshot/" + repoName + "/_all"))); + // Every step creates one snapshot + assertThat(snapshots, hasSize(TEST_STEP.ordinal() + 1)); + final SnapshotsStatusResponse statusResponse = client.snapshot().status(new SnapshotsStatusRequest(repoName, + snapshots.stream().map(sn -> (String) sn.get("snapshot")).toArray(String[]::new)), RequestOptions.DEFAULT); + for (SnapshotStatus status : statusResponse.getSnapshots()) { + assertThat(status.getShardsStats().getFailedShards(), is(0)); + } + if (TEST_STEP == TestStep.STEP3_OLD_CLUSTER) { + ensureSnapshotRestoreWorks(client, repoName, "snapshot-" + TestStep.STEP1_OLD_CLUSTER, shards); + } else if (TEST_STEP == TestStep.STEP4_NEW_CLUSTER) { + for (TestStep value : TestStep.values()) { + ensureSnapshotRestoreWorks(client, repoName, "snapshot-" + value, shards); + } + } + } finally { + deleteRepository(repoName); + } + } + + public void testReadOnlyRepo() throws IOException { + final String repoName = "testReadOnlyRepo"; + try (RestHighLevelClient client = new RestHighLevelClient(RestClient.builder(adminClient().getNodes().toArray(new Node[0])))) { + final int shards = 3; + final boolean readOnly = TEST_STEP.ordinal() > 1; // only restore from read-only repo in steps 3 and 4 + createRepository(client, repoName, readOnly); + if (readOnly == false) { + createIndex(client, "test-index", shards); + createSnapshot(client, repoName, "snapshot-" + TEST_STEP); + } final List> snapshots = readSnapshotsList( client.getLowLevelClient().performRequest(new Request("GET", "/_snapshot/" + repoName + "/_all"))); switch (TEST_STEP) { - case OLD: + case STEP1_OLD_CLUSTER: assertThat(snapshots, hasSize(1)); break; - case UPGRADED: + case STEP2_NEW_CLUSTER: + case STEP4_NEW_CLUSTER: + case STEP3_OLD_CLUSTER: assertThat(snapshots, hasSize(2)); break; - case DOWNGRADED: - assertThat(snapshots, hasSize(3)); - break; - case RE_UPGRADED: - assertThat(snapshots, hasSize(4)); } final SnapshotsStatusResponse statusResponse = client.snapshot().status(new SnapshotsStatusRequest(repoName, snapshots.stream().map(sn -> (String) sn.get("snapshot")).toArray(String[]::new)), RequestOptions.DEFAULT); for (SnapshotStatus status : statusResponse.getSnapshots()) { assertThat(status.getShardsStats().getFailedShards(), is(0)); } - if (TEST_STEP == TestStep.DOWNGRADED) { - ensureSnapshotRestoreWorks(client, repoName, "snapshot-old", shards); - } else if (TEST_STEP == TestStep.RE_UPGRADED) { - for (TestStep value : TestStep.values()) { - ensureSnapshotRestoreWorks(client, repoName, "snapshot-" + value.toString().toLowerCase(Locale.ROOT), shards); - } + if (TEST_STEP == TestStep.STEP3_OLD_CLUSTER) { + ensureSnapshotRestoreWorks(client, repoName, "snapshot-" + TestStep.STEP1_OLD_CLUSTER, shards); + } else if (TEST_STEP == TestStep.STEP4_NEW_CLUSTER) { + ensureSnapshotRestoreWorks(client, repoName, "snapshot-" + TestStep.STEP1_OLD_CLUSTER, shards); + ensureSnapshotRestoreWorks(client, repoName, "snapshot-" + TestStep.STEP2_NEW_CLUSTER, shards); } } } + public void testUpgradeMovesRepoToNewMetaVersion() throws IOException { + if (TEST_STEP.ordinal() > 1) { + // Only testing the first 2 steps here + return; + } + final String repoName = "testUpgradeMovesRepoToNewMetaVersion"; + try (RestHighLevelClient client = new RestHighLevelClient(RestClient.builder(adminClient().getNodes().toArray(new Node[0])))) { + final int shards = 3; + createIndex(client, "test-index", shards); + createRepository(client, repoName, false); + createSnapshot(client, repoName, "snapshot-" + TEST_STEP); + final List> snapshots = readSnapshotsList( + client.getLowLevelClient().performRequest(new Request("GET", "/_snapshot/" + repoName + "/_all"))); + // Every step creates one snapshot + assertThat(snapshots, hasSize(TEST_STEP.ordinal() + 1)); + final SnapshotsStatusResponse statusResponse = client.snapshot().status(new SnapshotsStatusRequest(repoName, + snapshots.stream().map(sn -> (String) sn.get("snapshot")).toArray(String[]::new)), RequestOptions.DEFAULT); + for (SnapshotStatus status : statusResponse.getSnapshots()) { + assertThat(status.getShardsStats().getFailedShards(), is(0)); + } + if (TEST_STEP == TestStep.STEP1_OLD_CLUSTER) { + ensureSnapshotRestoreWorks(client, repoName, "snapshot-" + TestStep.STEP1_OLD_CLUSTER, shards); + } else { + deleteSnapshot(client, repoName, "snapshot-" + TestStep.STEP1_OLD_CLUSTER); + ensureSnapshotRestoreWorks(client, repoName, "snapshot-" + TestStep.STEP2_NEW_CLUSTER, shards); + createSnapshot(client, repoName, "snapshot-1"); + ensureSnapshotRestoreWorks(client, repoName, "snapshot-1", shards); + deleteSnapshot(client, repoName, "snapshot-" + TestStep.STEP2_NEW_CLUSTER); + createSnapshot(client, repoName, "snapshot-2"); + ensureSnapshotRestoreWorks(client, repoName, "snapshot-2", shards); + } + } finally { + deleteRepository(repoName); + } + } + + private void deleteSnapshot(RestHighLevelClient client, String repoName, String name) throws IOException { + client.snapshot().delete(new DeleteSnapshotRequest(repoName, name), RequestOptions.DEFAULT); + } + private static void ensureSnapshotRestoreWorks(RestHighLevelClient client, String repoName, String name, int shards) throws IOException { wipeAllIndices(); @@ -168,6 +227,12 @@ private static void ensureSnapshotRestoreWorks(RestHighLevelClient client, Strin assertThat(restoreInfo.successfulShards(), equalTo(shards)); } + private static void createRepository(RestHighLevelClient client, String repoName, boolean readOnly) throws IOException { + assertThat(client.snapshot().createRepository(new PutRepositoryRequest(repoName).type("fs").settings( + Settings.builder().put("location", "./" + repoName).put("readonly", readOnly)), RequestOptions.DEFAULT).isAcknowledged(), + is(true)); + } + private static void createSnapshot(RestHighLevelClient client, String repoName, String name) throws IOException { client.snapshot().create(new CreateSnapshotRequest(repoName, name).waitForCompletion(true), RequestOptions.DEFAULT); } diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java index b843ea7a3d2b5..a1968332c734b 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java @@ -620,13 +620,17 @@ protected static void wipeAllIndices() throws IOException { } } if (preserveReposUponCompletion() == false) { - logger.debug("wiping snapshot repository [{}]", repoName); - adminClient().performRequest(new Request("DELETE", "_snapshot/" + repoName)); + deleteRepository(repoName); } } return inProgressSnapshots; } + protected void deleteRepository(String repoName) throws IOException { + logger.debug("wiping snapshot repository [{}]", repoName); + adminClient().performRequest(new Request("DELETE", "_snapshot/" + repoName)); + } + /** * Remove any cluster settings. */ From 5df61002ba5a7a031f4f2ccd535d55be0ad6a094 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Wed, 8 Jan 2020 14:25:35 +0100 Subject: [PATCH 22/26] rename qa projct --- .../build.gradle | 0 .../elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename qa/{repository-downgrade => repository-multi-version}/build.gradle (100%) rename qa/{repository-downgrade => repository-multi-version}/src/test/java/org/elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java (100%) diff --git a/qa/repository-downgrade/build.gradle b/qa/repository-multi-version/build.gradle similarity index 100% rename from qa/repository-downgrade/build.gradle rename to qa/repository-multi-version/build.gradle diff --git a/qa/repository-downgrade/src/test/java/org/elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java b/qa/repository-multi-version/src/test/java/org/elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java similarity index 100% rename from qa/repository-downgrade/src/test/java/org/elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java rename to qa/repository-multi-version/src/test/java/org/elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java From cecf2d088e5c3ac398054cc6c0bd25ad18cb829e Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Wed, 8 Jan 2020 17:19:29 +0100 Subject: [PATCH 23/26] only use 2 clusters --- qa/repository-multi-version/build.gradle | 28 ++++++++++-------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/qa/repository-multi-version/build.gradle b/qa/repository-multi-version/build.gradle index 95a09ea03f4a4..1ac3dab650e0a 100644 --- a/qa/repository-multi-version/build.gradle +++ b/qa/repository-multi-version/build.gradle @@ -37,16 +37,18 @@ for (Version bwcVersion : bwcVersions.indexCompatible) { String baseName = "v${bwcVersion}" String revertedClusterName = "${baseName}-reverted" - def clusterSettings = { - versions = [bwcVersion.toString(), project.version] - numberOfNodes = 2 - setting 'path.repo', "${buildDir}/cluster/shared/repo/${baseName}" - javaHome = BuildParams.runtimeJavaHome + def clusterSettings = { v -> + return { + version = v + numberOfNodes = 2 + setting 'path.repo', "${buildDir}/cluster/shared/repo/${baseName}" + javaHome = BuildParams.runtimeJavaHome + } } testClusters { - "${baseName}" clusterSettings - "${revertedClusterName}" clusterSettings + "${baseName}" clusterSettings(bwcVersion.toString()) + "${revertedClusterName}" clusterSettings(project.version) } tasks.register("${baseName}#Step1OldClusterTest", RestTestRunnerTask) { @@ -59,16 +61,13 @@ for (Version bwcVersion : bwcVersions.indexCompatible) { } tasks.register("${baseName}#Step2NewClusterTest", RestTestRunnerTask) { - useCluster testClusters."${baseName}" + useCluster testClusters."${revertedClusterName}" dependsOn "${baseName}#Step1OldClusterTest" - doFirst { - testClusters."${baseName}".goToNextVersion() - } systemProperty 'tests.rest.suite', 'step2' } tasks.register("${baseName}#Step3OldClusterTest", RestTestRunnerTask) { - useCluster testClusters."${revertedClusterName}" + useCluster testClusters."${baseName}" dependsOn "${baseName}#Step2NewClusterTest" systemProperty 'tests.rest.suite', 'step3' } @@ -76,16 +75,13 @@ for (Version bwcVersion : bwcVersions.indexCompatible) { tasks.register("${baseName}#Step4NewClusterTest", RestTestRunnerTask) { useCluster testClusters."${revertedClusterName}" dependsOn "${baseName}#Step3OldClusterTest" - doFirst { - testClusters."${revertedClusterName}".goToNextVersion() - } systemProperty 'tests.rest.suite', 'step4' } tasks.matching { it.name.startsWith(baseName) && it.name.endsWith("ClusterTest") }.configureEach { it.systemProperty 'tests.old_cluster_version', bwcVersion.toString().minus("-SNAPSHOT") it.systemProperty 'tests.path.repo', "${buildDir}/cluster/shared/repo/${baseName}" - def clusterName = it.name.contains("Step3") || it.name.contains("Step4") ? "${revertedClusterName}" : "${baseName}" + def clusterName = it.name.contains("Step2") || it.name.contains("Step4") ? "${revertedClusterName}" : "${baseName}" it.nonInputProperties.systemProperty('tests.rest.cluster', "${-> testClusters."${clusterName}".allHttpSocketURI.join(",")}") it.nonInputProperties.systemProperty('tests.clustername', "${-> testClusters."${clusterName}".getName()}") } From f1cda9ddbde710db0a7cb92fdc6da3485695c102 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Wed, 8 Jan 2020 21:13:33 +0100 Subject: [PATCH 24/26] CR: more checks + renaming --- qa/repository-multi-version/build.gradle | 17 +++--- .../MultiVersionRepositoryAccessIT.java | 58 ++++++++++--------- 2 files changed, 39 insertions(+), 36 deletions(-) diff --git a/qa/repository-multi-version/build.gradle b/qa/repository-multi-version/build.gradle index 1ac3dab650e0a..ed062e59eee1a 100644 --- a/qa/repository-multi-version/build.gradle +++ b/qa/repository-multi-version/build.gradle @@ -35,7 +35,8 @@ dependencies { for (Version bwcVersion : bwcVersions.indexCompatible) { String baseName = "v${bwcVersion}" - String revertedClusterName = "${baseName}-reverted" + String oldClusterName = "${baseName}-old" + String newClusterName = "${baseName}-new" def clusterSettings = { v -> return { @@ -47,12 +48,12 @@ for (Version bwcVersion : bwcVersions.indexCompatible) { } testClusters { - "${baseName}" clusterSettings(bwcVersion.toString()) - "${revertedClusterName}" clusterSettings(project.version) + "${oldClusterName}" clusterSettings(bwcVersion.toString()) + "${newClusterName}" clusterSettings(project.version) } tasks.register("${baseName}#Step1OldClusterTest", RestTestRunnerTask) { - useCluster testClusters."${baseName}" + useCluster testClusters."${oldClusterName}" mustRunAfter(precommit) doFirst { project.delete("${buildDir}/cluster/shared/repo/${baseName}") @@ -61,19 +62,19 @@ for (Version bwcVersion : bwcVersions.indexCompatible) { } tasks.register("${baseName}#Step2NewClusterTest", RestTestRunnerTask) { - useCluster testClusters."${revertedClusterName}" + useCluster testClusters."${newClusterName}" dependsOn "${baseName}#Step1OldClusterTest" systemProperty 'tests.rest.suite', 'step2' } tasks.register("${baseName}#Step3OldClusterTest", RestTestRunnerTask) { - useCluster testClusters."${baseName}" + useCluster testClusters."${oldClusterName}" dependsOn "${baseName}#Step2NewClusterTest" systemProperty 'tests.rest.suite', 'step3' } tasks.register("${baseName}#Step4NewClusterTest", RestTestRunnerTask) { - useCluster testClusters."${revertedClusterName}" + useCluster testClusters."${newClusterName}" dependsOn "${baseName}#Step3OldClusterTest" systemProperty 'tests.rest.suite', 'step4' } @@ -81,7 +82,7 @@ for (Version bwcVersion : bwcVersions.indexCompatible) { tasks.matching { it.name.startsWith(baseName) && it.name.endsWith("ClusterTest") }.configureEach { it.systemProperty 'tests.old_cluster_version', bwcVersion.toString().minus("-SNAPSHOT") it.systemProperty 'tests.path.repo', "${buildDir}/cluster/shared/repo/${baseName}" - def clusterName = it.name.contains("Step2") || it.name.contains("Step4") ? "${revertedClusterName}" : "${baseName}" + def clusterName = it.name.contains("Step2") || it.name.contains("Step4") ? "${newClusterName}" : "${oldClusterName}" it.nonInputProperties.systemProperty('tests.rest.cluster', "${-> testClusters."${clusterName}".allHttpSocketURI.join(",")}") it.nonInputProperties.systemProperty('tests.clustername', "${-> testClusters."${clusterName}".getName()}") } diff --git a/qa/repository-multi-version/src/test/java/org/elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java b/qa/repository-multi-version/src/test/java/org/elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java index c0b7309bc98a1..5648e1594bd20 100644 --- a/qa/repository-multi-version/src/test/java/org/elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java +++ b/qa/repository-multi-version/src/test/java/org/elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java @@ -44,8 +44,10 @@ import java.net.HttpURLConnection; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; @@ -53,10 +55,10 @@ * Tests that verify that a snapshot repository is not getting corrupted and continues to function properly when accessed by multiple * clusters of different versions. Concretely this test suit is simulating the following scenario: *
    - *
  • Start a cluster in an old version: {@link TestStep#STEP1_OLD_CLUSTER}
    • - *
  • Start a cluster running the current version: {@link TestStep#STEP2_NEW_CLUSTER}
    • - *
  • Again start a cluster in an old version: {@link TestStep#STEP3_OLD_CLUSTER}
    • - *
  • Once again start a cluster running the current version: {@link TestStep#STEP4_NEW_CLUSTER}
    • + *
  • Start and run against a cluster in an old version: {@link TestStep#STEP1_OLD_CLUSTER}
    • + *
  • Start and run against a cluster running the current version: {@link TestStep#STEP2_NEW_CLUSTER}
    • + *
  • Run against the old version cluster from the first step: {@link TestStep#STEP3_OLD_CLUSTER}
    • + *
  • Run against the current version cluster from the second step: {@link TestStep#STEP4_NEW_CLUSTER}
    • *
* TODO: Add two more steps: delete all old version snapshots from the repository, then downgrade again and verify that the repository * is not being corrupted. This requires first merging the logic for reading the min_version field in RepositoryData back to 7.6. @@ -119,10 +121,13 @@ public void testCreateAndRestoreSnapshot() throws IOException { // Create a snapshot and delete it right away again to test the impact of each version's cleanup functionality that is run // as part of the snapshot delete createSnapshot(client, repoName, snapshotToDeleteName); + final List> snapshotsIncludingToDelete = listSnapshots(repoName); + // Every step creates one snapshot and we have to add one more for the temporary snapshot + assertThat(snapshotsIncludingToDelete, hasSize(TEST_STEP.ordinal() + 1 + 1)); + assertThat(snapshotsIncludingToDelete.stream().map( + sn -> (String) sn.get("snapshot")).collect(Collectors.toList()), hasItem(snapshotToDeleteName)); deleteSnapshot(client, repoName, snapshotToDeleteName); - final List> snapshots = readSnapshotsList( - client.getLowLevelClient().performRequest(new Request("GET", "/_snapshot/" + repoName + "/_all"))); - // Every step creates one snapshot + final List> snapshots = listSnapshots(repoName); assertThat(snapshots, hasSize(TEST_STEP.ordinal() + 1)); final SnapshotsStatusResponse statusResponse = client.snapshot().status(new SnapshotsStatusRequest(repoName, snapshots.stream().map(sn -> (String) sn.get("snapshot")).toArray(String[]::new)), RequestOptions.DEFAULT); @@ -151,8 +156,7 @@ public void testReadOnlyRepo() throws IOException { createIndex(client, "test-index", shards); createSnapshot(client, repoName, "snapshot-" + TEST_STEP); } - final List> snapshots = readSnapshotsList( - client.getLowLevelClient().performRequest(new Request("GET", "/_snapshot/" + repoName + "/_all"))); + final List> snapshots = listSnapshots(repoName); switch (TEST_STEP) { case STEP1_OLD_CLUSTER: assertThat(snapshots, hasSize(1)); @@ -188,8 +192,7 @@ public void testUpgradeMovesRepoToNewMetaVersion() throws IOException { createIndex(client, "test-index", shards); createRepository(client, repoName, false); createSnapshot(client, repoName, "snapshot-" + TEST_STEP); - final List> snapshots = readSnapshotsList( - client.getLowLevelClient().performRequest(new Request("GET", "/_snapshot/" + repoName + "/_all"))); + final List> snapshots = listSnapshots(repoName); // Every step creates one snapshot assertThat(snapshots, hasSize(TEST_STEP.ordinal() + 1)); final SnapshotsStatusResponse statusResponse = client.snapshot().status(new SnapshotsStatusRequest(repoName, @@ -217,6 +220,22 @@ private void deleteSnapshot(RestHighLevelClient client, String repoName, String client.snapshot().delete(new DeleteSnapshotRequest(repoName, name), RequestOptions.DEFAULT); } + @SuppressWarnings("unchecked") + private List> listSnapshots(String repoName) throws IOException { + try (InputStream entity = client().performRequest( + new Request("GET", "/_snapshot/" + repoName + "/_all")).getEntity().getContent(); + XContentParser parser = JsonXContent.jsonXContent.createParser( + xContentRegistry(), DeprecationHandler.THROW_UNSUPPORTED_OPERATION, entity)) { + final Map raw = parser.map(); + // Bwc lookup since the format of the snapshots response changed between versions + if (raw.containsKey("snapshots")) { + return (List>) raw.get("snapshots"); + } else { + return (List>) ((List>) raw.get("responses")).get(0).get("snapshots"); + } + } + } + private static void ensureSnapshotRestoreWorks(RestHighLevelClient client, String repoName, String name, int shards) throws IOException { wipeAllIndices(); @@ -237,23 +256,6 @@ private static void createSnapshot(RestHighLevelClient client, String repoName, client.snapshot().create(new CreateSnapshotRequest(repoName, name).waitForCompletion(true), RequestOptions.DEFAULT); } - @SuppressWarnings("unchecked") - private List> readSnapshotsList(Response response) throws IOException { - final List> snapshots; - try (InputStream entity = response.getEntity().getContent(); - XContentParser parser = JsonXContent.jsonXContent.createParser( - xContentRegistry(), DeprecationHandler.THROW_UNSUPPORTED_OPERATION, entity)) { - final Map raw = parser.map(); - // Bwc lookup since the format of the snapshots response changed between versions - if (raw.containsKey("snapshots")) { - snapshots = (List>) raw.get("snapshots"); - } else { - snapshots = (List>) ((List>) raw.get("responses")).get(0).get("snapshots"); - } - } - return snapshots; - } - private void createIndex(RestHighLevelClient client, String name, int shards) throws IOException { final Request putIndexRequest = new Request("PUT", "/" + name); putIndexRequest.setJsonEntity("{\n" + From d4a18d85b81fd98a52fdd0b258d8ee1f085c21c6 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Thu, 9 Jan 2020 10:09:35 +0100 Subject: [PATCH 25/26] nicer --- .../MultiVersionRepositoryAccessIT.java | 37 +++++++++---------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/qa/repository-multi-version/src/test/java/org/elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java b/qa/repository-multi-version/src/test/java/org/elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java index 5648e1594bd20..f3effc8c01bec 100644 --- a/qa/repository-multi-version/src/test/java/org/elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java +++ b/qa/repository-multi-version/src/test/java/org/elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java @@ -53,7 +53,7 @@ /** * Tests that verify that a snapshot repository is not getting corrupted and continues to function properly when accessed by multiple - * clusters of different versions. Concretely this test suit is simulating the following scenario: + * clusters of different versions. Concretely this test suite is simulating the following scenario: *
    *
  • Start and run against a cluster in an old version: {@link TestStep#STEP1_OLD_CLUSTER}
    • *
  • Start and run against a cluster running the current version: {@link TestStep#STEP2_NEW_CLUSTER}
    • @@ -111,7 +111,7 @@ protected boolean preserveReposUponCompletion() { } public void testCreateAndRestoreSnapshot() throws IOException { - final String repoName = "testCreateAndRestoreSnapshot"; + final String repoName = getTestName(); try (RestHighLevelClient client = new RestHighLevelClient(RestClient.builder(adminClient().getNodes().toArray(new Node[0])))) { final int shards = 3; createIndex(client, "test-index", shards); @@ -129,11 +129,7 @@ public void testCreateAndRestoreSnapshot() throws IOException { deleteSnapshot(client, repoName, snapshotToDeleteName); final List> snapshots = listSnapshots(repoName); assertThat(snapshots, hasSize(TEST_STEP.ordinal() + 1)); - final SnapshotsStatusResponse statusResponse = client.snapshot().status(new SnapshotsStatusRequest(repoName, - snapshots.stream().map(sn -> (String) sn.get("snapshot")).toArray(String[]::new)), RequestOptions.DEFAULT); - for (SnapshotStatus status : statusResponse.getSnapshots()) { - assertThat(status.getShardsStats().getFailedShards(), is(0)); - } + assertSnapshotStatusSuccessful(client, repoName, snapshots); if (TEST_STEP == TestStep.STEP3_OLD_CLUSTER) { ensureSnapshotRestoreWorks(client, repoName, "snapshot-" + TestStep.STEP1_OLD_CLUSTER, shards); } else if (TEST_STEP == TestStep.STEP4_NEW_CLUSTER) { @@ -147,7 +143,7 @@ public void testCreateAndRestoreSnapshot() throws IOException { } public void testReadOnlyRepo() throws IOException { - final String repoName = "testReadOnlyRepo"; + final String repoName = getTestName(); try (RestHighLevelClient client = new RestHighLevelClient(RestClient.builder(adminClient().getNodes().toArray(new Node[0])))) { final int shards = 3; final boolean readOnly = TEST_STEP.ordinal() > 1; // only restore from read-only repo in steps 3 and 4 @@ -167,11 +163,7 @@ public void testReadOnlyRepo() throws IOException { assertThat(snapshots, hasSize(2)); break; } - final SnapshotsStatusResponse statusResponse = client.snapshot().status(new SnapshotsStatusRequest(repoName, - snapshots.stream().map(sn -> (String) sn.get("snapshot")).toArray(String[]::new)), RequestOptions.DEFAULT); - for (SnapshotStatus status : statusResponse.getSnapshots()) { - assertThat(status.getShardsStats().getFailedShards(), is(0)); - } + assertSnapshotStatusSuccessful(client, repoName, snapshots); if (TEST_STEP == TestStep.STEP3_OLD_CLUSTER) { ensureSnapshotRestoreWorks(client, repoName, "snapshot-" + TestStep.STEP1_OLD_CLUSTER, shards); } else if (TEST_STEP == TestStep.STEP4_NEW_CLUSTER) { @@ -186,7 +178,7 @@ public void testUpgradeMovesRepoToNewMetaVersion() throws IOException { // Only testing the first 2 steps here return; } - final String repoName = "testUpgradeMovesRepoToNewMetaVersion"; + final String repoName = getTestName(); try (RestHighLevelClient client = new RestHighLevelClient(RestClient.builder(adminClient().getNodes().toArray(new Node[0])))) { final int shards = 3; createIndex(client, "test-index", shards); @@ -195,11 +187,7 @@ public void testUpgradeMovesRepoToNewMetaVersion() throws IOException { final List> snapshots = listSnapshots(repoName); // Every step creates one snapshot assertThat(snapshots, hasSize(TEST_STEP.ordinal() + 1)); - final SnapshotsStatusResponse statusResponse = client.snapshot().status(new SnapshotsStatusRequest(repoName, - snapshots.stream().map(sn -> (String) sn.get("snapshot")).toArray(String[]::new)), RequestOptions.DEFAULT); - for (SnapshotStatus status : statusResponse.getSnapshots()) { - assertThat(status.getShardsStats().getFailedShards(), is(0)); - } + assertSnapshotStatusSuccessful(client, repoName, snapshots); if (TEST_STEP == TestStep.STEP1_OLD_CLUSTER) { ensureSnapshotRestoreWorks(client, repoName, "snapshot-" + TestStep.STEP1_OLD_CLUSTER, shards); } else { @@ -216,8 +204,17 @@ public void testUpgradeMovesRepoToNewMetaVersion() throws IOException { } } + private static void assertSnapshotStatusSuccessful(RestHighLevelClient client, String repoName, + List> snapshots) throws IOException { + final SnapshotsStatusResponse statusResponse = client.snapshot().status(new SnapshotsStatusRequest(repoName, + snapshots.stream().map(sn -> (String) sn.get("snapshot")).toArray(String[]::new)), RequestOptions.DEFAULT); + for (SnapshotStatus status : statusResponse.getSnapshots()) { + assertThat(status.getShardsStats().getFailedShards(), is(0)); + } + } + private void deleteSnapshot(RestHighLevelClient client, String repoName, String name) throws IOException { - client.snapshot().delete(new DeleteSnapshotRequest(repoName, name), RequestOptions.DEFAULT); + assertThat(client.snapshot().delete(new DeleteSnapshotRequest(repoName, name), RequestOptions.DEFAULT).isAcknowledged(), is(true)); } @SuppressWarnings("unchecked") From 7d5d2565b0e8c0df84d89e0fd5f3c37d5a87199b Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Thu, 9 Jan 2020 11:52:41 +0100 Subject: [PATCH 26/26] assume old version when missing snap- blob --- .../snapshots/SnapshotsService.java | 13 +++-- .../CorruptedBlobStoreRepositoryIT.java | 47 +++++++++++++++++++ 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index d676fd27215d8..9340e1508999c 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -363,10 +363,15 @@ public boolean hasOldVersionSnapshots(String repositoryName, RepositoryData repo if (repositoryData.shardGenerations().totalShards() > 0) { hasOldFormatSnapshots = false; } else { - final Repository repository = repositoriesService.repository(repositoryName); - hasOldFormatSnapshots = snapshotIds.stream().map(repository::getSnapshotInfo).anyMatch( - snapshotInfo -> (excluded == null || snapshotInfo.snapshotId().equals(excluded) == false) - && snapshotInfo.version().before(SHARD_GEN_IN_REPO_DATA_VERSION)); + try { + final Repository repository = repositoriesService.repository(repositoryName); + hasOldFormatSnapshots = snapshotIds.stream().map(repository::getSnapshotInfo).anyMatch( + snapshotInfo -> (excluded == null || snapshotInfo.snapshotId().equals(excluded) == false) + && snapshotInfo.version().before(SHARD_GEN_IN_REPO_DATA_VERSION)); + } catch (SnapshotMissingException e) { + logger.warn("Failed to load snapshot metadata, assuming repository is in old format", e); + return true; + } } } assert hasOldFormatSnapshots == false || repositoryData.shardGenerations().totalShards() == 0 : diff --git a/server/src/test/java/org/elasticsearch/snapshots/CorruptedBlobStoreRepositoryIT.java b/server/src/test/java/org/elasticsearch/snapshots/CorruptedBlobStoreRepositoryIT.java index 2a860eda3f972..9abb107521042 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/CorruptedBlobStoreRepositoryIT.java +++ b/server/src/test/java/org/elasticsearch/snapshots/CorruptedBlobStoreRepositoryIT.java @@ -18,6 +18,7 @@ */ package org.elasticsearch.snapshots; +import org.elasticsearch.action.ActionRunnable; import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse; import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.client.Client; @@ -33,9 +34,11 @@ import org.elasticsearch.repositories.RepositoryData; import org.elasticsearch.repositories.RepositoryException; import org.elasticsearch.repositories.blobstore.BlobStoreRepository; +import org.elasticsearch.threadpool.ThreadPool; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Locale; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.hamcrest.Matchers.containsString; @@ -231,6 +234,50 @@ public void clusterStateProcessed(String source, ClusterState oldState, ClusterS .addSnapshots(snapshot).get().getSnapshots(repoName)); } + public void testHandlingMissingRootLevelSnapshotMetadata() throws Exception { + Path repo = randomRepoPath(); + final String repoName = "test-repo"; + logger.info("--> creating repository at {}", repo.toAbsolutePath()); + assertAcked(client().admin().cluster().preparePutRepository(repoName) + .setType("fs").setSettings(Settings.builder() + .put("location", repo) + .put("compress", false) + .put("chunk_size", randomIntBetween(100, 1000), ByteSizeUnit.BYTES))); + + final String snapshotPrefix = "test-snap-"; + final int snapshots = randomIntBetween(1, 2); + logger.info("--> creating [{}] snapshots", snapshots); + for (int i = 0; i < snapshots; ++i) { + // Workaround to simulate BwC situation: taking a snapshot without indices here so that we don't create any new version shard + // generations (the existence of which would short-circuit checks for the repo containing old version snapshots) + CreateSnapshotResponse createSnapshotResponse = client().admin().cluster().prepareCreateSnapshot(repoName, snapshotPrefix + i) + .setIndices().setWaitForCompletion(true).get(); + assertThat(createSnapshotResponse.getSnapshotInfo().successfulShards(), is(0)); + assertThat(createSnapshotResponse.getSnapshotInfo().successfulShards(), + equalTo(createSnapshotResponse.getSnapshotInfo().totalShards())); + } + final Repository repository = internalCluster().getCurrentMasterNodeInstance(RepositoriesService.class).repository(repoName); + final RepositoryData repositoryData = getRepositoryData(repository); + + final SnapshotId snapshotToCorrupt = randomFrom(repositoryData.getSnapshotIds()); + logger.info("--> delete root level snapshot metadata blob for snapshot [{}]", snapshotToCorrupt); + Files.delete(repo.resolve(String.format(Locale.ROOT, BlobStoreRepository.SNAPSHOT_NAME_FORMAT, snapshotToCorrupt.getUUID()))); + + logger.info("--> verify that repo is assumed in old metadata format"); + final SnapshotsService snapshotsService = internalCluster().getCurrentMasterNodeInstance(SnapshotsService.class); + final ThreadPool threadPool = internalCluster().getCurrentMasterNodeInstance(ThreadPool.class); + assertThat(PlainActionFuture.get(f -> threadPool.generic().execute( + ActionRunnable.supply(f, () -> snapshotsService.hasOldVersionSnapshots(repoName, repositoryData, null)))), is(true)); + + logger.info("--> verify that snapshot with missing root level metadata can be deleted"); + assertAcked(client().admin().cluster().prepareDeleteSnapshot(repoName, snapshotToCorrupt.getName()).get()); + + logger.info("--> verify that repository is assumed in new metadata format after removing corrupted snapshot"); + assertThat(PlainActionFuture.get(f -> threadPool.generic().execute( + ActionRunnable.supply(f, () -> snapshotsService.hasOldVersionSnapshots(repoName, getRepositoryData(repository), null)))), + is(false)); + } + private void assertRepositoryBlocked(Client client, String repo, String existingSnapshot) { logger.info("--> try to delete snapshot"); final RepositoryException repositoryException3 = expectThrows(RepositoryException.class,