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

Delete backing snapshot when searchable snapshot index is deleted #75565

Closed
wants to merge 18 commits into from
Closed

Delete backing snapshot when searchable snapshot index is deleted #75565

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
fdf484c
add snapshots to delete to repository metadata
tlrx Jul 15, 2021
ba6b73b
add snapshots to delete metadata
tlrx Jul 16, 2021
a446683
prevent snapshots to delete to be mounted/restored/cloned
tlrx Jul 16, 2021
704c8ea
trigger snapshot deletions
tlrx Jul 19, 2021
955a5fc
add settings
tlrx Jul 21, 2021
982d6ea
Merge branch 'master' into delete-snap-on-searchable-index-deletion
tlrx Aug 2, 2021
4e641ab
Merge branch 'master' into delete-snap-on-searchable-index-deletion
tlrx Aug 3, 2021
c799368
Merge branch 'master' into delete-snap-on-searchable-index-deletion
tlrx Sep 3, 2021
e1995d7
Remove other repository name check
tlrx Sep 3, 2021
e13a11a
Do not re read repository name
tlrx Sep 3, 2021
4df53d7
Delete snapshots using UUIDs
tlrx Sep 3, 2021
3e8dc65
rework snapshot deletion
tlrx Sep 6, 2021
e1f17d5
Merge branch 'master' into delete-snap-on-searchable-index-deletion
tlrx Sep 6, 2021
b7bcf80
Merge branch 'master' into delete-snap-on-searchable-index-deletion
tlrx Sep 7, 2021
0bcaf26
Merge branch 'master' into delete-snap-on-searchable-index-deletion
tlrx Sep 14, 2021
83dfa73
fix random int in test
tlrx Sep 14, 2021
eb0e89e
Merge branch 'master' into delete-snap-on-searchable-index-deletion
tlrx Sep 23, 2021
b1f36f8
Update docs/changelog/75565.yaml
tlrx Feb 2, 2022
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
8 changes: 7 additions & 1 deletion ...java/org/elasticsearch/action/admin/cluster/repositories/get/GetRepositoriesResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,13 @@ public void writeTo(StreamOutput out) throws IOException {
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
repositories.toXContent(builder, new DelegatingMapParams(Map.of(RepositoriesMetadata.HIDE_GENERATIONS_PARAM, "true"), params));
repositories.toXContent(
builder,
new DelegatingMapParams(
Map.of(RepositoriesMetadata.HIDE_GENERATIONS_PARAM, "true", RepositoriesMetadata.HIDE_SNAPSHOTS_TO_DELETE, "true"),
params
)
);
builder.endObject();
return builder;
}
Expand Down
94 changes: 94 additions & 0 deletions server/src/main/java/org/elasticsearch/cluster/metadata/MetadataDeleteIndexService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,40 @@
import org.elasticsearch.cluster.AckedClusterStateUpdateTask;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.RestoreInProgress;
import org.elasticsearch.cluster.SnapshotDeletionsInProgress;
import org.elasticsearch.cluster.block.ClusterBlocks;
import org.elasticsearch.cluster.routing.RoutingTable;
import org.elasticsearch.cluster.routing.allocation.AllocationService;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.Priority;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.ImmutableOpenMap;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.set.Sets;
import org.elasticsearch.index.Index;
import org.elasticsearch.repositories.RepositoryMissingException;
import org.elasticsearch.snapshots.RestoreService;
import org.elasticsearch.snapshots.SearchableSnapshotsSettings;
import org.elasticsearch.snapshots.SnapshotId;
import org.elasticsearch.snapshots.SnapshotInProgressException;
import org.elasticsearch.snapshots.SnapshotsService;

import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;

import static java.util.Collections.emptyList;
import static org.elasticsearch.snapshots.SearchableSnapshotsSettings.SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION;
import static org.elasticsearch.snapshots.SearchableSnapshotsSettings.SEARCHABLE_SNAPSHOTS_REPOSITORY_NAME_SETTING_KEY;
import static org.elasticsearch.snapshots.SearchableSnapshotsSettings.SEARCHABLE_SNAPSHOTS_REPOSITORY_UUID_SETTING_KEY;
import static org.elasticsearch.snapshots.SearchableSnapshotsSettings.SEARCHABLE_SNAPSHOTS_SNAPSHOT_NAME_SETTING_KEY;
import static org.elasticsearch.snapshots.SearchableSnapshotsSettings.SEARCHABLE_SNAPSHOTS_SNAPSHOT_UUID_SETTING_KEY;

/**
* Deletes indices.
*/
Expand Down Expand Up @@ -120,6 +134,23 @@ public ClusterState deleteIndices(ClusterState currentState, Set<Index> indices)
logger.trace("{} tombstones purged from the cluster state. Previous tombstone size: {}. Current tombstone size: {}.",
graveyardBuilder.getNumPurged(), previousGraveyardSize, currentGraveyard.getTombstones().size());

// add snapshot(s) marked as to delete to the cluster state
final Map<String, Set<SnapshotId>> snapshotsToDelete = listOfSnapshotsToDelete(currentState, indicesToDelete);
if (snapshotsToDelete.isEmpty() == false) {
RepositoriesMetadata repositories = currentState.metadata().custom(RepositoriesMetadata.TYPE, RepositoriesMetadata.EMPTY);
boolean changed = false;
for (Map.Entry<String, Set<SnapshotId>> snapshotToDelete : snapshotsToDelete.entrySet()) {
RepositoryMetadata repository = repositories.repository(snapshotToDelete.getKey());
if (repository != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be null?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think it can be null and actually we don't need to re read the repository metadata here so I pushed e1995d7

repositories = repositories.addSnapshotsToDelete(repository.name(), snapshotToDelete.getValue());
changed = true;
}
}
if (changed) {
metadataBuilder.putCustom(RepositoriesMetadata.TYPE, repositories);
}
}

Metadata newMetadata = metadataBuilder.build();
ClusterBlocks blocks = clusterBlocksBuilder.build();

Expand All @@ -142,4 +173,67 @@ public ClusterState deleteIndices(ClusterState currentState, Set<Index> indices)
.build(),
"deleted indices [" + indices + "]");
}

private static Map<String, Set<SnapshotId>> listOfSnapshotsToDelete(final ClusterState currentState, final Set<Index> indicesToDelete) {
final Map<String, Set<SnapshotId>> snapshotsToDelete = new HashMap<>();

for (Index indexToDelete : indicesToDelete) {
final Settings indexSettings = currentState.metadata().getIndexSafe(indexToDelete).getSettings();
if (SearchableSnapshotsSettings.isSearchableSnapshotIndexWithSnapshotDeletion(indexSettings) == false) {
continue;
}

final String repositoryName = repositoryNameFromIndexSettings(currentState, indexSettings);
final String snapshotName = indexSettings.get(SEARCHABLE_SNAPSHOTS_SNAPSHOT_NAME_SETTING_KEY);
final String snapshotUuid = indexSettings.get(SEARCHABLE_SNAPSHOTS_SNAPSHOT_UUID_SETTING_KEY);

boolean canDeleteSnapshot = true;

// TODO change this to an assertion once it becomes impossible to delete a snapshot that is mounted as an index
if (currentState.custom(SnapshotDeletionsInProgress.TYPE, SnapshotDeletionsInProgress.EMPTY)
.getEntries().stream().anyMatch(entry -> entry.getSnapshots().contains(new SnapshotId(snapshotName, snapshotUuid)))) {
continue; // this snapshot is part of an existing snapshot deletion in progress, nothing to do
}

for (IndexMetadata other : currentState.metadata()) {
if (indicesToDelete.contains(other.getIndex())) {
continue; // do not check indices that are going to be deleted
}
final Settings otherSettings = other.getSettings();
if (SearchableSnapshotsSettings.isSearchableSnapshotStore(otherSettings) == false) {
continue; // other index is not a searchable snapshot index, skip
}
final String otherSnapshotUuid = otherSettings.get(SEARCHABLE_SNAPSHOTS_SNAPSHOT_UUID_SETTING_KEY);
if (Objects.equals(snapshotUuid, otherSnapshotUuid) == false) {
continue; // other index is backed by a different snapshot, skip
}
final String otherRepositoryName = repositoryNameFromIndexSettings(currentState, otherSettings);
if (Objects.equals(repositoryName, otherRepositoryName) == false) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this might break in some odd corner cases involving registering the same repository under multiple names (maybe without repository UUIDs). But do we need to check this? By this point we know the snapshot UUID matches, that should be enough to tell us not to delete it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree and I removed this check in e1995d7

continue; // other index is backed by a snapshot from a different repository, skip
}
assert otherSettings.getAsBoolean(SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, false) : other;
DaveCTurner marked this conversation as resolved.
Show resolved Hide resolved
canDeleteSnapshot = false; // another index is using the same snapshot, do not delete
break;
}
if (canDeleteSnapshot) {
snapshotsToDelete.computeIfAbsent(repositoryName, r -> new HashSet<>())
.add(new SnapshotId(indexSettings.get(SEARCHABLE_SNAPSHOTS_SNAPSHOT_NAME_SETTING_KEY), snapshotUuid));
}
}
return snapshotsToDelete;
}

private static String repositoryNameFromIndexSettings(ClusterState currentState, Settings indexSettings) {
final String repositoryUuid = indexSettings.get(SEARCHABLE_SNAPSHOTS_REPOSITORY_UUID_SETTING_KEY);
if (Strings.hasLength(repositoryUuid) == false) {
return indexSettings.get(SEARCHABLE_SNAPSHOTS_REPOSITORY_NAME_SETTING_KEY);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should return null in case there is a repo-uuid on the index but no such repo was found? This works differently from RepositoriesService.indexSettingsMatchRepositoryMetadata

final RepositoriesMetadata repoMetadata = currentState.metadata().custom(RepositoriesMetadata.TYPE);
final List<RepositoryMetadata> repositories = repoMetadata == null ? emptyList() : repoMetadata.repositories();
return repositories.stream()
.filter(r -> repositoryUuid.equals(r.uuid()))
.map(RepositoryMetadata::name)
.findFirst()
.orElseThrow(() -> new RepositoryMissingException(repositoryUuid));
}
}
37 changes: 35 additions & 2 deletions server/src/main/java/org/elasticsearch/cluster/metadata/RepositoriesMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,20 @@
import org.elasticsearch.cluster.AbstractNamedDiffable;
import org.elasticsearch.cluster.NamedDiff;
import org.elasticsearch.cluster.metadata.Metadata.Custom;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.repositories.RepositoryData;
import org.elasticsearch.snapshots.SnapshotId;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.EnumSet;
import java.util.List;
Expand All @@ -45,6 +47,12 @@ public class RepositoriesMetadata extends AbstractNamedDiffable<Custom> implemen
*/
public static final String HIDE_GENERATIONS_PARAM = "hide_generations";

/**
* Serialization parameter used to hide the {@link RepositoryMetadata#snapshotsToDelete()}
* in {@link org.elasticsearch.action.admin.cluster.repositories.get.GetRepositoriesResponse}.
*/
public static final String HIDE_SNAPSHOTS_TO_DELETE = "hide_snapshots_to_delete";

private final List<RepositoryMetadata> repositories;

/**
Expand Down Expand Up @@ -79,6 +87,14 @@ public RepositoriesMetadata withUuid(String repoName, String uuid) {
return withUpdate(repoName, repositoryMetadata -> repositoryMetadata.withUuid(uuid));
}

public RepositoriesMetadata addSnapshotsToDelete(String repoName, Collection<SnapshotId> snapshotsToDelete) {
return withUpdate(repoName, repositoryMetadata -> repositoryMetadata.addSnapshotsToDelete(snapshotsToDelete));
}

public RepositoriesMetadata removeSnapshotsToDelete(String repoName, Collection<SnapshotId> snapshotsToDelete) {
return withUpdate(repoName, repositoryMetadata -> repositoryMetadata.removeSnapshotsToDelete(snapshotsToDelete));
}

private RepositoriesMetadata withUpdate(String repoName, UnaryOperator<RepositoryMetadata> update) {
int indexOfRepo = -1;
for (int i = 0; i < repositories.size(); i++) {
Expand Down Expand Up @@ -200,6 +216,7 @@ public static RepositoriesMetadata fromXContent(XContentParser parser) throws IO
Settings settings = Settings.EMPTY;
long generation = RepositoryData.UNKNOWN_REPO_GEN;
long pendingGeneration = RepositoryData.EMPTY_REPO_GEN;
final List<SnapshotId> snapshotsToDelete = new ArrayList<>();
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
if (token == XContentParser.Token.FIELD_NAME) {
String currentFieldName = parser.currentName();
Expand Down Expand Up @@ -228,6 +245,13 @@ public static RepositoriesMetadata fromXContent(XContentParser parser) throws IO
throw new ElasticsearchParseException("failed to parse repository [{}], unknown type", name);
}
pendingGeneration = parser.longValue();
} else if ("snapshots_to_delete".equals(currentFieldName)) {
if (parser.nextToken() != XContentParser.Token.START_ARRAY) {
throw new ElasticsearchParseException("failed to parse repository [{}], unknown type", name);
}
while (parser.nextToken() != XContentParser.Token.END_ARRAY) {
snapshotsToDelete.add(SnapshotId.parse(parser));
}
} else {
throw new ElasticsearchParseException("failed to parse repository [{}], unknown field [{}]",
name, currentFieldName);
Expand All @@ -239,7 +263,9 @@ public static RepositoriesMetadata fromXContent(XContentParser parser) throws IO
if (type == null) {
throw new ElasticsearchParseException("failed to parse repository [{}], missing repository type", name);
}
repository.add(new RepositoryMetadata(name, uuid, type, settings, generation, pendingGeneration));
repository.add(
new RepositoryMetadata(name, uuid, type, settings, generation, pendingGeneration, List.copyOf(snapshotsToDelete))
);
} else {
throw new ElasticsearchParseException("failed to parse repositories");
}
Expand Down Expand Up @@ -284,6 +310,13 @@ public static void toXContent(RepositoryMetadata repository, XContentBuilder bui
builder.field("generation", repository.generation());
builder.field("pending_generation", repository.pendingGeneration());
}
if (params.paramAsBoolean(HIDE_SNAPSHOTS_TO_DELETE, false) == false) {
builder.startArray("snapshots_to_delete");
for (SnapshotId snapshotToDelete : repository.snapshotsToDelete()) {
builder.value(snapshotToDelete);
}
builder.endArray();
}
builder.endObject();
}

Expand Down
Loading