Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fail snapshot operations early on repository corruption #30140

Merged
merged 4 commits into from
Apr 27, 2018
Merged

Fail snapshot operations early on repository corruption #30140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b6bdc2e
Fix NullPointerException when creating or deleting a snapshot
tlrx Apr 25, 2018
ff72347
Update PR number
tlrx Apr 25, 2018
46d4748
Apply feedback
tlrx Apr 26, 2018
2177da2
Update Changelog
tlrx Apr 27, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/CHANGELOG.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@

=== Bug Fixes

Fail snapshot operations early when creating or deleting a snapshot on a repository that has been
written to by an older Elasticsearch after writing to it with a newer Elasticsearch version. ({pull}30140[#30140])

=== Regressions

=== Known Issues
Expand Down
12 changes: 6 additions & 6 deletions docs/reference/modules/snapshots.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,12 @@ If you register same snapshot repository with multiple clusters, only
one cluster should have write access to the repository. All other clusters
connected to that repository should set the repository to `readonly` mode.

NOTE: The snapshot format can change across major versions, so if you have
clusters on different major versions trying to write the same repository,
new snapshots written by one version will not be visible to the other. While
setting the repository to `readonly` on all but one of the clusters should work
with multiple clusters differing by one major version, it is not a supported
configuration.
IMPORTANT: The snapshot format can change across major versions, so if you have
clusters on different versions trying to write the same repository, snapshots
written by one version may not be visible to the other and the repository could
be corrupted. While setting the repository to `readonly` on all but one of the
clusters should work with multiple clusters differing by one major version, it
is not a supported configuration.

[source,js]
-----------------------------------
Expand Down
6 changes: 3 additions & 3 deletions ...g/elasticsearch/action/admin/cluster/snapshots/status/TransportSnapshotsStatusAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,9 +230,9 @@ private SnapshotsStatusResponse buildResponse(SnapshotsStatusRequest request, Li
SnapshotInfo snapshotInfo = snapshotsService.snapshot(repositoryName, snapshotId);
List<SnapshotIndexShardStatus> shardStatusBuilder = new ArrayList<>();
if (snapshotInfo.state().completed()) {
Map<ShardId, IndexShardSnapshotStatus> shardStatues =
snapshotsService.snapshotShards(request.repository(), snapshotInfo);
for (Map.Entry<ShardId, IndexShardSnapshotStatus> shardStatus : shardStatues.entrySet()) {
Map<ShardId, IndexShardSnapshotStatus> shardStatuses =
snapshotsService.snapshotShards(repositoryName, repositoryData, snapshotInfo);
for (Map.Entry<ShardId, IndexShardSnapshotStatus> shardStatus : shardStatuses.entrySet()) {
IndexShardSnapshotStatus.Copy lastSnapshotStatus = shardStatus.getValue().asCopy();
shardStatusBuilder.add(new SnapshotIndexShardStatus(shardStatus.getKey(), lastSnapshotStatus));
}
Expand Down
39 changes: 22 additions & 17 deletions server/src/main/java/org/elasticsearch/repositories/RepositoryData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,13 +230,6 @@ public Set<SnapshotId> getSnapshots(final IndexId indexId) {
return snapshotIds;
}

/**
* Initializes the indices in the repository metadata; returns a new instance.
*/
public RepositoryData initIndices(final Map<IndexId, Set<SnapshotId>> indexSnapshots) {
return new RepositoryData(genId, snapshotIds, snapshotStates, indexSnapshots, incompatibleSnapshotIds);
}

@Override
public boolean equals(Object obj) {
if (this == obj) {
Expand Down Expand Up @@ -352,9 +345,10 @@ public XContentBuilder snapshotsToXContent(final XContentBuilder builder, final
* Reads an instance of {@link RepositoryData} from x-content, loading the snapshots and indices metadata.
*/
public static RepositoryData snapshotsFromXContent(final XContentParser parser, long genId) throws IOException {
Map<String, SnapshotId> snapshots = new HashMap<>();
Map<String, SnapshotState> snapshotStates = new HashMap<>();
Map<IndexId, Set<SnapshotId>> indexSnapshots = new HashMap<>();
final Map<String, SnapshotId> snapshots = new HashMap<>();
final Map<String, SnapshotState> snapshotStates = new HashMap<>();
final Map<IndexId, Set<SnapshotId>> indexSnapshots = new HashMap<>();

if (parser.nextToken() == XContentParser.Token.START_OBJECT) {
while (parser.nextToken() == XContentParser.Token.FIELD_NAME) {
String field = parser.currentName();
Expand Down Expand Up @@ -397,17 +391,18 @@ public static RepositoryData snapshotsFromXContent(final XContentParser parser,
throw new ElasticsearchParseException("start object expected [indices]");
}
while (parser.nextToken() != XContentParser.Token.END_OBJECT) {
String indexName = parser.currentName();
String indexId = null;
Set<SnapshotId> snapshotIds = new LinkedHashSet<>();
final String indexName = parser.currentName();
final Set<SnapshotId> snapshotIds = new LinkedHashSet<>();

IndexId indexId = null;
if (parser.nextToken() != XContentParser.Token.START_OBJECT) {
throw new ElasticsearchParseException("start object expected index[" + indexName + "]");
}
while (parser.nextToken() != XContentParser.Token.END_OBJECT) {
String indexMetaFieldName = parser.currentName();
final String indexMetaFieldName = parser.currentName();
parser.nextToken();
if (INDEX_ID.equals(indexMetaFieldName)) {
indexId = parser.text();
indexId = new IndexId(indexName, parser.text());
} else if (SNAPSHOTS.equals(indexMetaFieldName)) {
if (parser.currentToken() != XContentParser.Token.START_ARRAY) {
throw new ElasticsearchParseException("start array expected [snapshots]");
Expand All @@ -428,12 +423,22 @@ public static RepositoryData snapshotsFromXContent(final XContentParser parser,
// since we already have the name/uuid combo in the snapshots array
uuid = parser.text();
}
snapshotIds.add(snapshots.get(uuid));

SnapshotId snapshotId = snapshots.get(uuid);
if (snapshotId != null) {
snapshotIds.add(snapshotId);
} else {
// A snapshotted index references a snapshot which does not exist in
// the list of snapshots. This can happen when multiple clusters in
// different versions create or delete snapshot in the same repository.
throw new ElasticsearchParseException("Detected a corrupted repository, index " + indexId
+ " references an unknown snapshot uuid [" + uuid + "]");
}
}
}
}
assert indexId != null;
indexSnapshots.put(new IndexId(indexName, indexId), snapshotIds);
indexSnapshots.put(indexId, snapshotIds);
}
} else {
throw new ElasticsearchParseException("unknown field name [" + field + "]");
Expand Down
3 changes: 1 addition & 2 deletions server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -592,10 +592,9 @@ public List<SnapshotsInProgress.Entry> currentSnapshots(final String repository,
* @return map of shard id to snapshot status
*/
public Map<ShardId, IndexShardSnapshotStatus> snapshotShards(final String repositoryName,
final RepositoryData repositoryData,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this is a small optimization in the case of the Snapshot Status API where the repository index was loaded two times.

final SnapshotInfo snapshotInfo) throws IOException {
final Repository repository = repositoriesService.repository(repositoryName);
final RepositoryData repositoryData = repository.getRepositoryData();

final Map<ShardId, IndexShardSnapshotStatus> shardStatus = new HashMap<>();
for (String index : snapshotInfo.indices()) {
IndexId indexId = repositoryData.resolveIndexId(index);
Expand Down
87 changes: 86 additions & 1 deletion server/src/test/java/org/elasticsearch/repositories/RepositoryDataTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@

package org.elasticsearch.repositories;

import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.common.UUIDs;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.elasticsearch.snapshots.SnapshotId;
import org.elasticsearch.snapshots.SnapshotState;
Expand All @@ -39,7 +42,11 @@
import java.util.Map;
import java.util.Set;

import static java.util.Collections.emptySet;
import static java.util.Collections.singleton;
import static org.elasticsearch.repositories.RepositoryData.EMPTY_REPO_GEN;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan;

/**
Expand Down Expand Up @@ -101,15 +108,18 @@ public void testAddSnapshots() {
public void testInitIndices() {
final int numSnapshots = randomIntBetween(1, 30);
final Map<String, SnapshotId> snapshotIds = new HashMap<>(numSnapshots);
final Map<String, SnapshotState> snapshotStates = new HashMap<>(numSnapshots);
for (int i = 0; i < numSnapshots; i++) {
final SnapshotId snapshotId = new SnapshotId(randomAlphaOfLength(8), UUIDs.randomBase64UUID());
snapshotIds.put(snapshotId.getUUID(), snapshotId);
snapshotStates.put(snapshotId.getUUID(), randomFrom(SnapshotState.values()));
}
RepositoryData repositoryData = new RepositoryData(EMPTY_REPO_GEN, snapshotIds,
Collections.emptyMap(), Collections.emptyMap(), Collections.emptyList());
// test that initializing indices works
Map<IndexId, Set<SnapshotId>> indices = randomIndices(snapshotIds);
RepositoryData newRepoData = repositoryData.initIndices(indices);
RepositoryData newRepoData = new RepositoryData(repositoryData.getGenId(), snapshotIds, snapshotStates, indices,
new ArrayList<>(repositoryData.getIncompatibleSnapshotIds()));
List<SnapshotId> expected = new ArrayList<>(repositoryData.getSnapshotIds());
Collections.sort(expected);
List<SnapshotId> actual = new ArrayList<>(newRepoData.getSnapshotIds());
Expand Down Expand Up @@ -153,6 +163,81 @@ public void testGetSnapshotState() {
assertNull(repositoryData.getSnapshotState(new SnapshotId(randomAlphaOfLength(8), UUIDs.randomBase64UUID())));
}

public void testIndexThatReferencesAnUnknownSnapshot() throws IOException {
final XContent xContent = randomFrom(XContentType.values()).xContent();
final RepositoryData repositoryData = generateRandomRepoData();

XContentBuilder builder = XContentBuilder.builder(xContent);
repositoryData.snapshotsToXContent(builder, ToXContent.EMPTY_PARAMS);
RepositoryData parsedRepositoryData = RepositoryData.snapshotsFromXContent(createParser(builder), repositoryData.getGenId());
assertEquals(repositoryData, parsedRepositoryData);

Map<String, SnapshotId> snapshotIds = new HashMap<>();
Map<String, SnapshotState> snapshotStates = new HashMap<>();
for (SnapshotId snapshotId : parsedRepositoryData.getSnapshotIds()) {
snapshotIds.put(snapshotId.getUUID(), snapshotId);
snapshotStates.put(snapshotId.getUUID(), parsedRepositoryData.getSnapshotState(snapshotId));
}

final IndexId corruptedIndexId = randomFrom(parsedRepositoryData.getIndices().values());

Map<IndexId, Set<SnapshotId>> indexSnapshots = new HashMap<>();
for (Map.Entry<String, IndexId> snapshottedIndex : parsedRepositoryData.getIndices().entrySet()) {
IndexId indexId = snapshottedIndex.getValue();
Set<SnapshotId> snapshotsIds = new LinkedHashSet<>(parsedRepositoryData.getSnapshots(indexId));
if (corruptedIndexId.equals(indexId)) {
snapshotsIds.add(new SnapshotId("_uuid", "_does_not_exist"));
}
indexSnapshots.put(indexId, snapshotsIds);
}
assertNotNull(corruptedIndexId);

RepositoryData corruptedRepositoryData = new RepositoryData(parsedRepositoryData.getGenId(), snapshotIds, snapshotStates,
indexSnapshots, new ArrayList<>(parsedRepositoryData.getIncompatibleSnapshotIds()));

final XContentBuilder corruptedBuilder = XContentBuilder.builder(xContent);
corruptedRepositoryData.snapshotsToXContent(corruptedBuilder, ToXContent.EMPTY_PARAMS);

ElasticsearchParseException e = expectThrows(ElasticsearchParseException.class, () ->
RepositoryData.snapshotsFromXContent(createParser(corruptedBuilder), corruptedRepositoryData.getGenId()));
assertThat(e.getMessage(), equalTo("Detected a corrupted repository, index " + corruptedIndexId + " references an unknown " +
"snapshot uuid [_does_not_exist]"));
}

public void testIndexThatReferenceANullSnapshot() throws IOException {
final XContentBuilder builder = XContentBuilder.builder(randomFrom(XContentType.JSON).xContent());
builder.startObject();
{
builder.startArray("snapshots");
builder.value(new SnapshotId("_name", "_uuid"));
builder.endArray();

builder.startObject("indices");
{
builder.startObject("docs");
{
builder.field("id", "_id");
builder.startArray("snapshots");
{
builder.startObject();
if (randomBoolean()) {
builder.field("name", "_name");
}
builder.endObject();
}
builder.endArray();
}
builder.endObject();
}
builder.endObject();
}
builder.endObject();

ElasticsearchParseException e = expectThrows(ElasticsearchParseException.class, () ->
RepositoryData.snapshotsFromXContent(createParser(builder), randomNonNegativeLong()));
assertThat(e.getMessage(), equalTo("Detected a corrupted repository, index [docs/_id] references an unknown snapshot uuid [null]"));
}

public static RepositoryData generateRandomRepoData() {
final int numIndices = randomIntBetween(1, 30);
final List<IndexId> indices = new ArrayList<>(numIndices);
Expand Down