Skip to content

Delete backing snapshot when searchable snapshot index is deleted #75565

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.

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions docs/changelog/75565.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 75565
summary: Delete backing snapshot when searchable snapshot index is deleted
area: Snapshot/Restore
type: enhancement
issues: []
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
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,6 @@ protected void masterOperation(
ClusterState state,
final ActionListener<AcknowledgedResponse> listener
) {
snapshotsService.deleteSnapshots(request, listener.map(v -> AcknowledgedResponse.TRUE));
snapshotsService.deleteSnapshotsByName(request, listener.map(v -> AcknowledgedResponse.TRUE));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,37 @@
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.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.Map;
import java.util.Objects;
import java.util.Set;

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 All @@ -60,13 +71,15 @@ public void deleteIndices(final DeleteIndexClusterStateUpdateRequest request, fi
throw new IllegalArgumentException("Index name is required");
}

clusterService.submitStateUpdateTask("delete-index " + Arrays.toString(request.indices()),
clusterService.submitStateUpdateTask(
"delete-index " + Arrays.toString(request.indices()),
new AckedClusterStateUpdateTask(Priority.URGENT, request, listener) {
@Override
public ClusterState execute(final ClusterState currentState) {
return deleteIndices(currentState, Sets.newHashSet(request.indices()));
}
});
}
);
}

/**
Expand All @@ -81,8 +94,13 @@ public ClusterState deleteIndices(ClusterState currentState, Set<Index> indices)
IndexAbstraction.DataStream parent = meta.getIndicesLookup().get(im.getIndex().getName()).getParentDataStream();
if (parent != null) {
if (parent.getWriteIndex().equals(im)) {
throw new IllegalArgumentException("index [" + index.getName() + "] is the write index for data stream [" +
parent.getName() + "] and cannot be deleted");
throw new IllegalArgumentException(
"index ["
+ index.getName()
+ "] is the write index for data stream ["
+ parent.getName()
+ "] and cannot be deleted"
);
} else {
backingIndices.put(index, parent.getDataStream());
}
Expand All @@ -93,8 +111,11 @@ public ClusterState deleteIndices(ClusterState currentState, Set<Index> indices)
// Check if index deletion conflicts with any running snapshots
Set<Index> snapshottingIndices = SnapshotsService.snapshottingIndices(currentState, indicesToDelete);
if (snapshottingIndices.isEmpty() == false) {
throw new SnapshotInProgressException("Cannot delete indices that are being snapshotted: " + snapshottingIndices +
". Try again after snapshot finishes or cancel the currently running snapshot.");
throw new SnapshotInProgressException(
"Cannot delete indices that are being snapshotted: "
+ snapshottingIndices
+ ". Try again after snapshot finishes or cancel the currently running snapshot."
);
}

RoutingTable.Builder routingTableBuilder = RoutingTable.builder(currentState.routingTable());
Expand All @@ -117,8 +138,22 @@ public ClusterState deleteIndices(ClusterState currentState, Set<Index> indices)
// add tombstones to the cluster state for each deleted index
final IndexGraveyard currentGraveyard = graveyardBuilder.addTombstones(indices).build(settings);
metadataBuilder.indexGraveyard(currentGraveyard); // the new graveyard set on the metadata
logger.trace("{} tombstones purged from the cluster state. Previous tombstone size: {}. Current tombstone size: {}.",
graveyardBuilder.getNumPurged(), previousGraveyardSize, currentGraveyard.getTombstones().size());
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);
for (Map.Entry<String, Set<SnapshotId>> snapshotToDelete : snapshotsToDelete.entrySet()) {
repositories = repositories.addSnapshotsToDelete(snapshotToDelete.getKey(), snapshotToDelete.getValue());
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 we risk getting an IllegalArgumentException from RepositoriesMetadata.withUpdate when the repo has been deleted while deleting the index.

}
metadataBuilder.putCustom(RepositoriesMetadata.TYPE, repositories);
}

Metadata newMetadata = metadataBuilder.build();
ClusterBlocks blocks = clusterBlocksBuilder.build();
Expand All @@ -134,12 +169,73 @@ public ClusterState deleteIndices(ClusterState currentState, Set<Index> indices)
}

return allocationService.reroute(
ClusterState.builder(currentState)
.routingTable(routingTableBuilder.build())
.metadata(newMetadata)
.blocks(blocks)
.customs(customs)
.build(),
"deleted indices [" + indices + "]");
ClusterState.builder(currentState)
.routingTable(routingTableBuilder.build())
.metadata(newMetadata)
.blocks(blocks)
.customs(customs)
.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
}
assert otherSettings.getAsBoolean(SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION, false) : other;
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)) {
final RepositoriesMetadata repositories = currentState.metadata().custom(RepositoriesMetadata.TYPE, RepositoriesMetadata.EMPTY);
for (RepositoryMetadata repository : repositories.repositories()) {
if (repositoryUuid.equals(repository.uuid())) {
return repository.name();
}
}
}
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

return indexSettings.get(SEARCHABLE_SNAPSHOTS_REPOSITORY_NAME_SETTING_KEY);
}
}
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