From 674afd68b69801e1cb9a97a92e49177f0c7550cb Mon Sep 17 00:00:00 2001 From: Ali Beyad Date: Wed, 3 May 2017 22:29:58 -0400 Subject: [PATCH 1/5] Enhances get snapshots API to allow retrieving repository index only Currently, the get snapshots API (e.g. /_snapshot/{repositoryName}/_all) provides information about snapshots in the repository, including the snapshot state, number of shards snapshotted, failures, etc. In order to provide information about each snapshot in the repository, the call must read the snapshot metadata blob (`snap-{snapshot_uuid}.dat`) for every snapshot. In cloud-based repositories, this can be expensive, both from a cost and performance perspective. Sometimes, all the user wants is to retrieve all the names/uuids of each snapshot, and the indices that went into each snapshot, without any of the other status information about the snapshot. This minimal information can be retrieved from the repository index blob (`index-N`) without needing to read each snapshot metadata blob. This commit enhances the get snapshots API with an optional `verbose` parameter. If `verbose` is set to false on the request, then the get snapshots API will only retrieve the minimal information about each snapshot (the name, uuid, and indices in the snapshot), and only read this information from the repository index blob, thereby giving users the option to retrieve the snapshots in a repository in a more cost-effective and efficient manner. Closes #24288 --- .../snapshots/get/GetSnapshotsRequest.java | 31 +++++ .../get/GetSnapshotsRequestBuilder.java | 14 +++ .../get/TransportGetSnapshotsAction.java | 68 +++++++++-- .../admin/cluster/RestGetSnapshotsAction.java | 3 +- .../elasticsearch/snapshots/SnapshotId.java | 7 +- .../elasticsearch/snapshots/SnapshotInfo.java | 114 ++++++++++++------ .../SharedClusterSnapshotRestoreIT.java | 89 ++++++++++++++ docs/reference/modules/snapshots.asciidoc | 9 ++ 8 files changed, 285 insertions(+), 50 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java b/core/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java index fd2c97ed5d43c..e90f9e578ce37 100644 --- a/core/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java +++ b/core/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java @@ -28,6 +28,7 @@ import java.io.IOException; import static org.elasticsearch.action.ValidateActions.addValidationError; +import static org.elasticsearch.snapshots.SnapshotInfo.VERBOSE_INTRODUCED; /** * Get snapshot request @@ -43,6 +44,8 @@ public class GetSnapshotsRequest extends MasterNodeRequest private boolean ignoreUnavailable; + private boolean verbose = true; + public GetSnapshotsRequest() { } @@ -123,6 +126,7 @@ public GetSnapshotsRequest ignoreUnavailable(boolean ignoreUnavailable) { this.ignoreUnavailable = ignoreUnavailable; return this; } + /** * @return Whether snapshots should be ignored when unavailable (corrupt or temporarily not fetchable) */ @@ -130,12 +134,36 @@ public boolean ignoreUnavailable() { return ignoreUnavailable; } + /** + * Set to {@code false} to only show the snapshot names and the indices they contain. + * This is useful when the snapshots belong to a cloud-based repository where each + * blob read is a concern (cost wise and performance wise), as the snapshot names and + * indices they contain can be retrieved from a single index blob in the repository, + * whereas the rest of the information requires reading a snapshot metadata file for + * each snapshot requested. Defaults to {@code true}, which returns all information + * about each requested snapshot. + */ + public GetSnapshotsRequest verbose(boolean verbose) { + this.verbose = verbose; + return this; + } + + /** + * Returns whether the request will return a verbose response. + */ + public boolean verbose() { + return verbose; + } + @Override public void readFrom(StreamInput in) throws IOException { super.readFrom(in); repository = in.readString(); snapshots = in.readStringArray(); ignoreUnavailable = in.readBoolean(); + if (in.getVersion().onOrAfter(VERBOSE_INTRODUCED)) { + verbose = in.readBoolean(); + } } @Override @@ -144,5 +172,8 @@ public void writeTo(StreamOutput out) throws IOException { out.writeString(repository); out.writeStringArray(snapshots); out.writeBoolean(ignoreUnavailable); + if (out.getVersion().onOrAfter(VERBOSE_INTRODUCED)) { + out.writeBoolean(verbose); + } } } diff --git a/core/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestBuilder.java b/core/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestBuilder.java index 3b0ac47c69f9f..2115bd0bc3b81 100644 --- a/core/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestBuilder.java +++ b/core/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestBuilder.java @@ -96,4 +96,18 @@ public GetSnapshotsRequestBuilder setIgnoreUnavailable(boolean ignoreUnavailable return this; } + /** + * Set to {@code false} to only show the snapshot names and the indices they contain. + * This is useful when the snapshots belong to a cloud-based repository where each + * blob read is a concern (cost wise and performance wise), as the snapshot names and + * indices they contain can be retrieved from a single index blob in the repository, + * whereas the rest of the information requires reading a snapshot metadata file for + * each snapshot requested. Defaults to {@code true}, which returns all information + * about each requested snapshot. + */ + public GetSnapshotsRequestBuilder setVerbose(boolean verbose) { + request.verbose(verbose); + return this; + } + } diff --git a/core/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java b/core/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java index 88b84f36ff6ca..f2fcbeef65797 100644 --- a/core/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java +++ b/core/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java @@ -19,6 +19,7 @@ package org.elasticsearch.action.admin.cluster.snapshots.get; +import org.apache.lucene.util.CollectionUtil; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.master.TransportMasterNodeAction; @@ -30,6 +31,7 @@ import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.regex.Regex; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.repositories.IndexId; import org.elasticsearch.repositories.RepositoryData; import org.elasticsearch.snapshots.SnapshotId; import org.elasticsearch.snapshots.SnapshotInfo; @@ -39,11 +41,13 @@ import org.elasticsearch.transport.TransportService; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; /** * Transport Action for get snapshots operation @@ -76,31 +80,35 @@ protected ClusterBlockException checkBlock(GetSnapshotsRequest request, ClusterS } @Override - protected void masterOperation(final GetSnapshotsRequest request, ClusterState state, + protected void masterOperation(final GetSnapshotsRequest request, final ClusterState state, final ActionListener listener) { try { final String repository = request.repository(); - List snapshotInfoBuilder = new ArrayList<>(); final Map allSnapshotIds = new HashMap<>(); - final List currentSnapshotIds = new ArrayList<>(); - final RepositoryData repositoryData = snapshotsService.getRepositoryData(repository); + final List currentSnapshots = new ArrayList<>(); for (SnapshotInfo snapshotInfo : snapshotsService.currentSnapshots(repository)) { SnapshotId snapshotId = snapshotInfo.snapshotId(); allSnapshotIds.put(snapshotId.getName(), snapshotId); - currentSnapshotIds.add(snapshotId); + currentSnapshots.add(snapshotInfo); } + + final RepositoryData repositoryData; if (isCurrentSnapshotsOnly(request.snapshots()) == false) { + repositoryData = snapshotsService.getRepositoryData(repository); for (SnapshotId snapshotId : repositoryData.getAllSnapshotIds()) { allSnapshotIds.put(snapshotId.getName(), snapshotId); } + } else { + repositoryData = null; } + final Set toResolve = new HashSet<>(); if (isAllSnapshots(request.snapshots())) { toResolve.addAll(allSnapshotIds.values()); } else { for (String snapshotOrPattern : request.snapshots()) { if (GetSnapshotsRequest.CURRENT_SNAPSHOT.equalsIgnoreCase(snapshotOrPattern)) { - toResolve.addAll(currentSnapshotIds); + toResolve.addAll(currentSnapshots.stream().map(SnapshotInfo::snapshotId).collect(Collectors.toList())); } else if (Regex.isSimpleMatchPattern(snapshotOrPattern) == false) { if (allSnapshotIds.containsKey(snapshotOrPattern)) { toResolve.add(allSnapshotIds.get(snapshotOrPattern)); @@ -121,9 +129,23 @@ protected void masterOperation(final GetSnapshotsRequest request, ClusterState s } } - snapshotInfoBuilder.addAll(snapshotsService.snapshots( - repository, new ArrayList<>(toResolve), repositoryData.getIncompatibleSnapshotIds(), request.ignoreUnavailable())); - listener.onResponse(new GetSnapshotsResponse(snapshotInfoBuilder)); + final List snapshotInfos; + if (request.verbose()) { + final List incompatibleSnapshots = repositoryData != null ? + repositoryData.getIncompatibleSnapshotIds() : Collections.emptyList(); + snapshotInfos = snapshotsService.snapshots(repository, new ArrayList<>(toResolve), + incompatibleSnapshots, request.ignoreUnavailable()); + } else { + if (repositoryData != null) { + // want non-current snapshots as well, which are found in the repository data + snapshotInfos = buildSimpleSnapshotInfos(toResolve, repositoryData, currentSnapshots); + } else { + // only want current snapshots + snapshotInfos = currentSnapshots.stream().map(SnapshotInfo::basic).collect(Collectors.toList()); + CollectionUtil.timSort(snapshotInfos); + } + } + listener.onResponse(new GetSnapshotsResponse(snapshotInfos)); } catch (Exception e) { listener.onFailure(e); } @@ -136,4 +158,32 @@ private boolean isAllSnapshots(String[] snapshots) { private boolean isCurrentSnapshotsOnly(String[] snapshots) { return (snapshots.length == 1 && GetSnapshotsRequest.CURRENT_SNAPSHOT.equalsIgnoreCase(snapshots[0])); } + + private List buildSimpleSnapshotInfos(final Set toResolve, + final RepositoryData repositoryData, + final List currentSnapshots) { + List snapshotInfos = new ArrayList<>(); + for (SnapshotInfo snapshotInfo : currentSnapshots) { + if (toResolve.contains(snapshotInfo.snapshotId())) { + snapshotInfos.add(snapshotInfo.basic()); + toResolve.remove(snapshotInfo.snapshotId()); + } + } + Map> snapshotsToIndices = new HashMap<>(); + for (IndexId indexId : repositoryData.getIndices().values()) { + for (SnapshotId snapshotId : repositoryData.getSnapshots(indexId)) { + if (toResolve.contains(snapshotId)) { + snapshotsToIndices.computeIfAbsent(snapshotId, (k) -> new ArrayList<>()) + .add(indexId.getName()); + } + } + } + for (Map.Entry> entry : snapshotsToIndices.entrySet()) { + final List indices = entry.getValue(); + CollectionUtil.timSort(indices); + snapshotInfos.add(new SnapshotInfo(entry.getKey(), indices)); + } + CollectionUtil.timSort(snapshotInfos); + return Collections.unmodifiableList(snapshotInfos); + } } diff --git a/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestGetSnapshotsAction.java b/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestGetSnapshotsAction.java index 508953cdde8e5..7348cb5896cb4 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestGetSnapshotsAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestGetSnapshotsAction.java @@ -42,7 +42,6 @@ public RestGetSnapshotsAction(Settings settings, RestController controller) { controller.registerHandler(GET, "/_snapshot/{repository}/{snapshot}", this); } - @Override public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { String repository = request.param("repository"); @@ -50,7 +49,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC GetSnapshotsRequest getSnapshotsRequest = getSnapshotsRequest(repository).snapshots(snapshots); getSnapshotsRequest.ignoreUnavailable(request.paramAsBoolean("ignore_unavailable", getSnapshotsRequest.ignoreUnavailable())); - + getSnapshotsRequest.verbose(request.paramAsBoolean("verbose", getSnapshotsRequest.verbose())); getSnapshotsRequest.masterNodeTimeout(request.paramAsTime("master_timeout", getSnapshotsRequest.masterNodeTimeout())); return channel -> client.admin().cluster().getSnapshots(getSnapshotsRequest, new RestToXContentListener<>(channel)); } diff --git a/core/src/main/java/org/elasticsearch/snapshots/SnapshotId.java b/core/src/main/java/org/elasticsearch/snapshots/SnapshotId.java index 4866a79afb95a..ffd7547099c66 100644 --- a/core/src/main/java/org/elasticsearch/snapshots/SnapshotId.java +++ b/core/src/main/java/org/elasticsearch/snapshots/SnapshotId.java @@ -32,7 +32,7 @@ /** * SnapshotId - snapshot name + snapshot UUID */ -public final class SnapshotId implements Writeable, ToXContent { +public final class SnapshotId implements Comparable, Writeable, ToXContent { private static final String NAME = "name"; private static final String UUID = "uuid"; @@ -106,6 +106,11 @@ public int hashCode() { return hashCode; } + @Override + public int compareTo(final SnapshotId other) { + return this.name.compareTo(other.name); + } + private int computeHashCode() { return Objects.hash(name, uuid); } diff --git a/core/src/main/java/org/elasticsearch/snapshots/SnapshotInfo.java b/core/src/main/java/org/elasticsearch/snapshots/SnapshotInfo.java index 519393d49e10c..8ea8dce66aa26 100644 --- a/core/src/main/java/org/elasticsearch/snapshots/SnapshotInfo.java +++ b/core/src/main/java/org/elasticsearch/snapshots/SnapshotInfo.java @@ -35,6 +35,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Collections; +import java.util.Comparator; import java.util.List; import java.util.Objects; @@ -69,11 +70,17 @@ public final class SnapshotInfo implements Comparable, ToXContent, private static final String SUCCESSFUL_SHARDS = "successful_shards"; private static final Version VERSION_INCOMPATIBLE_INTRODUCED = Version.V_5_2_0_UNRELEASED; + public static final Version VERBOSE_INTRODUCED = Version.V_5_5_0_UNRELEASED; + + private static final Comparator COMPARATOR = + Comparator.comparing(SnapshotInfo::startTime).thenComparing(SnapshotInfo::snapshotId); private final SnapshotId snapshotId; + @Nullable private final SnapshotState state; + @Nullable private final String reason; private final List indices; @@ -91,6 +98,10 @@ public final class SnapshotInfo implements Comparable, ToXContent, private final List shardFailures; + public SnapshotInfo(SnapshotId snapshotId, List indices) { + this(snapshotId, indices, null, null, null, 0L, 0L, 0, 0, Collections.emptyList()); + } + public SnapshotInfo(SnapshotId snapshotId, List indices, long startTime) { this(snapshotId, indices, SnapshotState.IN_PROGRESS, null, Version.CURRENT, startTime, 0L, 0, 0, Collections.emptyList()); } @@ -104,8 +115,8 @@ public SnapshotInfo(SnapshotId snapshotId, List indices, long startTime, private SnapshotInfo(SnapshotId snapshotId, List indices, SnapshotState state, String reason, Version version, long startTime, long endTime, int totalShards, int successfulShards, List shardFailures) { this.snapshotId = Objects.requireNonNull(snapshotId); - this.indices = Objects.requireNonNull(indices); - this.state = Objects.requireNonNull(state); + this.indices = Collections.unmodifiableList(Objects.requireNonNull(indices)); + this.state = state; this.reason = reason; this.version = version; this.startTime = startTime; @@ -126,7 +137,11 @@ public SnapshotInfo(final StreamInput in) throws IOException { indicesListBuilder.add(in.readString()); } indices = Collections.unmodifiableList(indicesListBuilder); - state = SnapshotState.fromValue(in.readByte()); + if (in.getVersion().onOrAfter(VERBOSE_INTRODUCED)) { + state = in.readBoolean() ? SnapshotState.fromValue(in.readByte()) : null; + } else { + state = SnapshotState.fromValue(in.readByte()); + } reason = in.readOptionalString(); startTime = in.readVLong(); endTime = in.readVLong(); @@ -159,6 +174,14 @@ public static SnapshotInfo incompatible(SnapshotId snapshotId) { null, 0L, 0L, 0, 0, Collections.emptyList()); } + /** + * Gets a new {@link SnapshotInfo} instance from the given {@link SnapshotInfo} with + * all information stripped out except the snapshot id and indices. + */ + public SnapshotInfo basic() { + return new SnapshotInfo(snapshotId, indices); + } + /** * Returns snapshot id * @@ -169,25 +192,27 @@ public SnapshotId snapshotId() { } /** - * Returns snapshot state + * Returns snapshot state; {@code null} if the state is unknown. * * @return snapshot state */ + @Nullable public SnapshotState state() { return state; } /** - * Returns snapshot failure reason + * Returns snapshot failure reason; {@code null} if the snapshot succeeded. * * @return snapshot failure reason */ + @Nullable public String reason() { return reason; } /** - * Returns indices that were included into this snapshot + * Returns indices that were included in this snapshot. * * @return list of indices */ @@ -196,7 +221,8 @@ public List indices() { } /** - * Returns time when snapshot started + * Returns time when snapshot started; a value of {@code 0L} will be returned if + * {@link #state()} returns {@code null}. * * @return snapshot start time */ @@ -205,9 +231,8 @@ public long startTime() { } /** - * Returns time when snapshot ended - *

- * Can be 0L if snapshot is still running + * Returns time when snapshot ended; a value of {@code 0L} will be returned if the + * snapshot is still running or if {@link #state()} returns {@code null}. * * @return snapshot end time */ @@ -216,7 +241,8 @@ public long endTime() { } /** - * Returns total number of shards that were snapshotted + * Returns total number of shards that were snapshotted; a value of {@code 0} will + * be returned if {@link #state()} returns {@code null}. * * @return number of shards */ @@ -225,7 +251,8 @@ public int totalShards() { } /** - * Number of failed shards + * Number of failed shards; a value of {@code 0} will be returned if there were no + * failed shards, or if {@link #state()} returns {@code null}. * * @return number of failed shards */ @@ -234,7 +261,8 @@ public int failedShards() { } /** - * Returns total number of shards that were successfully snapshotted + * Returns total number of shards that were successfully snapshotted; a value of + * {@code 0} will be returned if {@link #state()} returns {@code null}. * * @return number of successful shards */ @@ -243,7 +271,8 @@ public int successfulShards() { } /** - * Returns shard failures + * Returns shard failures; an empty list will be returned if there were no shard + * failures, or if {@link #state()} returns {@code null}. * * @return shard failures */ @@ -253,7 +282,7 @@ public List shardFailures() { /** * Returns the version of elasticsearch that the snapshot was created with. Will only - * return {@code null} if {@link #state()} returns {@link SnapshotState#INCOMPATIBLE}. + * return {@code null} if {@link #state()} returns {@code null} or {@link SnapshotState#INCOMPATIBLE}. * * @return version of elasticsearch that the snapshot was created with */ @@ -263,16 +292,12 @@ public Version version() { } /** - * Compares two snapshots by their start time - * - * @param o other snapshot - * @return the value {@code 0} if snapshots were created at the same time; - * a value less than {@code 0} if this snapshot was created before snapshot {@code o}; and - * a value greater than {@code 0} if this snapshot was created after snapshot {@code o}; + * Compares two snapshots by their start time; if the start times are the same, then + * compares the two snapshots by their snapshot ids. */ @Override public int compareTo(final SnapshotInfo o) { - return Long.compare(startTime, o.startTime); + return COMPARATOR.compare(this, o); } @Override @@ -328,15 +353,15 @@ public XContentBuilder toXContent(final XContentBuilder builder, final Params pa if (version != null) { builder.field(VERSION_ID, version.id); builder.field(VERSION, version.toString()); - } else { - builder.field(VERSION, "unknown"); } builder.startArray(INDICES); for (String index : indices) { builder.value(index); } builder.endArray(); - builder.field(STATE, state); + if (state != null) { + builder.field(STATE, state); + } if (reason != null) { builder.field(REASON, reason); } @@ -349,18 +374,22 @@ public XContentBuilder toXContent(final XContentBuilder builder, final Params pa builder.field(END_TIME_IN_MILLIS, endTime); builder.timeValueField(DURATION_IN_MILLIS, DURATION, endTime - startTime); } - builder.startArray(FAILURES); - for (SnapshotShardFailure shardFailure : shardFailures) { - builder.startObject(); - shardFailure.toXContent(builder, params); + if (!shardFailures.isEmpty()) { + builder.startArray(FAILURES); + for (SnapshotShardFailure shardFailure : shardFailures) { + builder.startObject(); + shardFailure.toXContent(builder, params); + builder.endObject(); + } + builder.endArray(); + } + if (totalShards != 0) { + builder.startObject(SHARDS); + builder.field(TOTAL, totalShards); + builder.field(FAILED, failedShards()); + builder.field(SUCCESSFUL, successfulShards); builder.endObject(); } - builder.endArray(); - builder.startObject(SHARDS); - builder.field(TOTAL, totalShards); - builder.field(FAILED, failedShards()); - builder.field(SUCCESSFUL, successfulShards); - builder.endObject(); builder.endObject(); return builder; } @@ -496,10 +525,19 @@ public void writeTo(final StreamOutput out) throws IOException { for (String index : indices) { out.writeString(index); } - if (out.getVersion().before(VERSION_INCOMPATIBLE_INTRODUCED) && state == SnapshotState.INCOMPATIBLE) { - out.writeByte(SnapshotState.FAILED.value()); + if (out.getVersion().onOrAfter(VERBOSE_INTRODUCED)) { + if (state != null) { + out.writeBoolean(true); + out.writeByte(state.value()); + } else { + out.writeBoolean(false); + } } else { - out.writeByte(state.value()); + if (out.getVersion().before(VERSION_INCOMPATIBLE_INTRODUCED) && state == SnapshotState.INCOMPATIBLE) { + out.writeByte(SnapshotState.FAILED.value()); + } else { + out.writeByte(state.value()); + } } out.writeOptionalString(reason); out.writeVLong(startTime); diff --git a/core/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java b/core/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java index 67911f3e5b03d..47c73377cf7e8 100644 --- a/core/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java +++ b/core/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java @@ -88,8 +88,10 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; @@ -2754,4 +2756,91 @@ public void testSnapshotStatusOnFailedIndex() throws Exception { } } } + + public void testGetSnapshotsFromIndexBlobOnly() throws Exception { + logger.info("--> creating repository"); + final Path repoPath = randomRepoPath(); + final Client client = client(); + assertAcked(client.admin().cluster() + .preparePutRepository("test-repo") + .setType("fs") + .setVerify(false) + .setSettings(Settings.builder().put("location", repoPath))); + + logger.info("--> creating random number of indices"); + final int numIndices = randomIntBetween(1, 10); + for (int i = 0; i < numIndices; i++) { + assertAcked(prepareCreate("test-idx-" + i).setSettings(Settings.builder() + .put(SETTING_NUMBER_OF_SHARDS, 1).put(SETTING_NUMBER_OF_REPLICAS, 0))); + } + + logger.info("--> creating random number of snapshots"); + final int numSnapshots = randomIntBetween(1, 10); + final Map> indicesPerSnapshot = new HashMap<>(); + for (int i = 0; i < numSnapshots; i++) { + // index some additional docs (maybe) for each index + for (int j = 0; j < numIndices; j++) { + if (randomBoolean()) { + final int numDocs = randomIntBetween(1, 5); + for (int k = 0; k < numDocs; k++) { + index("test-idx-" + j, "doc", Integer.toString(k), "foo", "bar" + k); + } + refresh(); + } + } + final boolean all = randomBoolean(); + boolean atLeastOne = false; + List indices = new ArrayList<>(); + for (int j = 0; j < numIndices; j++) { + if (all || randomBoolean() || !atLeastOne) { + indices.add("test-idx-" + j); + atLeastOne = true; + } + } + final String snapshotName = "test-snap-" + i; + indicesPerSnapshot.put(snapshotName, indices); + client.admin().cluster() + .prepareCreateSnapshot("test-repo", snapshotName) + .setWaitForCompletion(true) + .setIndices(indices.toArray(new String[indices.size()])) + .get(); + } + + logger.info("--> verify _all returns snapshot info"); + GetSnapshotsResponse response = client().admin().cluster() + .prepareGetSnapshots("test-repo") + .setSnapshots("_all") + .setVerbose(false) + .get(); + assertEquals(indicesPerSnapshot.size(), response.getSnapshots().size()); + verifySnapshotInfo(response, indicesPerSnapshot); + + logger.info("--> verify wildcard returns snapshot info"); + response = client().admin().cluster() + .prepareGetSnapshots("test-repo") + .setSnapshots("test-snap-*") + .setVerbose(false) + .get(); + assertEquals(indicesPerSnapshot.size(), response.getSnapshots().size()); + verifySnapshotInfo(response, indicesPerSnapshot); + + logger.info("--> verify individual requests return snapshot info"); + for (int i = 0; i < numSnapshots; i++) { + response = client().admin().cluster() + .prepareGetSnapshots("test-repo") + .setSnapshots("test-snap-" + i) + .setVerbose(false) + .get(); + assertEquals(1, response.getSnapshots().size()); + verifySnapshotInfo(response, indicesPerSnapshot); + } + } + + private void verifySnapshotInfo(final GetSnapshotsResponse response, final Map> indicesPerSnapshot) { + for (SnapshotInfo snapshotInfo : response.getSnapshots()) { + final List expected = snapshotInfo.indices(); + assertEquals(expected, indicesPerSnapshot.get(snapshotInfo.snapshotId().getName())); + assertNull(snapshotInfo.state()); + } + } } diff --git a/docs/reference/modules/snapshots.asciidoc b/docs/reference/modules/snapshots.asciidoc index 94138dbdb0f34..64e9e2e1663aa 100644 --- a/docs/reference/modules/snapshots.asciidoc +++ b/docs/reference/modules/snapshots.asciidoc @@ -350,6 +350,15 @@ GET /_snapshot/my_backup/_all The command fails if some of the snapshots are unavailable. The boolean parameter `ignore_unavailable` can be used to return all snapshots that are currently available. +Getting all snapshots in the repository can be costly on cloud-based repositories, +both from a cost and performance perspective. If the only information required is +the snapshot names/uuids in the repository and the indices in each snapshot, then +the optional boolean parameter `verbose` can be set to `false` to execute a more +performant and cost-effective retrieval of the snapshots in the repository. Note +that setting `verbose` to `false` will omit all other information about the snapshot +such as status information, the number of snapshotted shards, etc. The default +value of the `verbose` parameter is `true`. + A currently running snapshot can be retrieved using the following command: [source,sh] From f99d4c41d991a2b4645dba74914a06a43d3fb3ba Mon Sep 17 00:00:00 2001 From: Ali Beyad Date: Mon, 8 May 2017 22:57:07 -0400 Subject: [PATCH 2/5] adds state to repo data --- .../get/TransportGetSnapshotsAction.java | 7 +- .../repositories/RepositoryData.java | 171 ++++++++++++------ .../blobstore/BlobStoreRepository.java | 5 +- .../elasticsearch/snapshots/SnapshotInfo.java | 8 +- .../snapshots/SnapshotsService.java | 5 +- .../index/shard/IndexShardTests.java | 2 +- .../repositories/RepositoryDataTests.java | 49 +++-- .../blobstore/BlobStoreRepositoryTests.java | 10 +- .../SharedClusterSnapshotRestoreIT.java | 2 +- .../rest-api-spec/api/snapshot.get.json | 4 + .../test/snapshot.get/10_basic.yaml | 31 ++++ 11 files changed, 202 insertions(+), 92 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java b/core/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java index f2fcbeef65797..557627f1b10f3 100644 --- a/core/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java +++ b/core/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java @@ -131,8 +131,8 @@ protected void masterOperation(final GetSnapshotsRequest request, final ClusterS final List snapshotInfos; if (request.verbose()) { - final List incompatibleSnapshots = repositoryData != null ? - repositoryData.getIncompatibleSnapshotIds() : Collections.emptyList(); + final Set incompatibleSnapshots = repositoryData != null ? + new HashSet<>(repositoryData.getIncompatibleSnapshotIds()) : Collections.emptySet(); snapshotInfos = snapshotsService.snapshots(repository, new ArrayList<>(toResolve), incompatibleSnapshots, request.ignoreUnavailable()); } else { @@ -181,7 +181,8 @@ private List buildSimpleSnapshotInfos(final Set toReso for (Map.Entry> entry : snapshotsToIndices.entrySet()) { final List indices = entry.getValue(); CollectionUtil.timSort(indices); - snapshotInfos.add(new SnapshotInfo(entry.getKey(), indices)); + final SnapshotId snapshotId = entry.getKey(); + snapshotInfos.add(new SnapshotInfo(snapshotId, indices, repositoryData.getSnapshotState(snapshotId))); } CollectionUtil.timSort(snapshotInfos); return Collections.unmodifiableList(snapshotInfos); diff --git a/core/src/main/java/org/elasticsearch/repositories/RepositoryData.java b/core/src/main/java/org/elasticsearch/repositories/RepositoryData.java index ee4a64cac87dc..ac52ac30d69d9 100644 --- a/core/src/main/java/org/elasticsearch/repositories/RepositoryData.java +++ b/core/src/main/java/org/elasticsearch/repositories/RepositoryData.java @@ -20,14 +20,17 @@ package org.elasticsearch.repositories; import org.elasticsearch.ElasticsearchParseException; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.snapshots.SnapshotId; +import org.elasticsearch.snapshots.SnapshotState; import java.io.IOException; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashSet; @@ -51,8 +54,8 @@ public final class RepositoryData { /** * An instance initialized for an empty repository. */ - public static final RepositoryData EMPTY = - new RepositoryData(EMPTY_REPO_GEN, Collections.emptyList(), Collections.emptyMap(), Collections.emptyList()); + public static final RepositoryData EMPTY = new RepositoryData(EMPTY_REPO_GEN, + Collections.emptyMap(), Collections.emptyMap(), Collections.emptyMap(), Collections.emptyList()); /** * The generational id of the index file from which the repository data was read. @@ -61,7 +64,11 @@ public final class RepositoryData { /** * The ids of the snapshots in the repository. */ - private final List snapshotIds; + private final Map snapshotIds; + /** + * The states of each snapshot in the repository. + */ + private final Map snapshotStates; /** * The indices found in the repository across all snapshots, as a name to {@link IndexId} mapping */ @@ -75,19 +82,22 @@ public final class RepositoryData { */ private final List incompatibleSnapshotIds; - public RepositoryData(long genId, List snapshotIds, Map> indexSnapshots, + public RepositoryData(long genId, + Map snapshotIds, + Map snapshotStates, + Map> indexSnapshots, List incompatibleSnapshotIds) { this.genId = genId; - this.snapshotIds = Collections.unmodifiableList(snapshotIds); - this.indices = Collections.unmodifiableMap(indexSnapshots.keySet() - .stream() - .collect(Collectors.toMap(IndexId::getName, Function.identity()))); + this.snapshotIds = Collections.unmodifiableMap(snapshotIds); + this.snapshotStates = Collections.unmodifiableMap(snapshotStates); + this.indices = Collections.unmodifiableMap(indexSnapshots.keySet().stream() + .collect(Collectors.toMap(IndexId::getName, Function.identity()))); this.indexSnapshots = Collections.unmodifiableMap(indexSnapshots); this.incompatibleSnapshotIds = Collections.unmodifiableList(incompatibleSnapshotIds); } protected RepositoryData copy() { - return new RepositoryData(genId, snapshotIds, indexSnapshots, incompatibleSnapshotIds); + return new RepositoryData(genId, snapshotIds, snapshotStates, indexSnapshots, incompatibleSnapshotIds); } /** @@ -98,17 +108,17 @@ public long getGenId() { } /** - * Returns an unmodifiable list of the snapshot ids. + * Returns an unmodifiable collection of the snapshot ids. */ - public List getSnapshotIds() { - return snapshotIds; + public Collection getSnapshotIds() { + return Collections.unmodifiableCollection(snapshotIds.values()); } /** * Returns an immutable collection of the snapshot ids in the repository that are incompatible with the * current ES version. */ - public List getIncompatibleSnapshotIds() { + public Collection getIncompatibleSnapshotIds() { return incompatibleSnapshotIds; } @@ -116,13 +126,22 @@ public List getIncompatibleSnapshotIds() { * Returns an immutable collection of all the snapshot ids in the repository, both active and * incompatible snapshots. */ - public List getAllSnapshotIds() { + public Collection getAllSnapshotIds() { List allSnapshotIds = new ArrayList<>(snapshotIds.size() + incompatibleSnapshotIds.size()); - allSnapshotIds.addAll(snapshotIds); + allSnapshotIds.addAll(snapshotIds.values()); allSnapshotIds.addAll(incompatibleSnapshotIds); return Collections.unmodifiableList(allSnapshotIds); } + /** + * Returns the {@link SnapshotState} for the given snapshot. Returns {@code null} if + * there is no state for the snapshot. + */ + @Nullable + public SnapshotState getSnapshotState(final SnapshotId snapshotId) { + return snapshotStates.get(snapshotId.getUUID()); + } + /** * Returns an unmodifiable map of the index names to {@link IndexId} in the repository. */ @@ -134,15 +153,19 @@ public Map getIndices() { * Add a snapshot and its indices to the repository; returns a new instance. If the snapshot * already exists in the repository data, this method throws an IllegalArgumentException. */ - public RepositoryData addSnapshot(final SnapshotId snapshotId, final List snapshottedIndices) { - if (snapshotIds.contains(snapshotId)) { + public RepositoryData addSnapshot(final SnapshotId snapshotId, + final SnapshotState snapshotState, + final List snapshottedIndices) { + if (snapshotIds.containsKey(snapshotId.getUUID())) { // if the snapshot id already exists in the repository data, it means an old master // that is blocked from the cluster is trying to finalize a snapshot concurrently with // the new master, so we make the operation idempotent return this; } - List snapshots = new ArrayList<>(snapshotIds); - snapshots.add(snapshotId); + Map snapshots = new HashMap<>(snapshotIds); + snapshots.put(snapshotId.getUUID(), snapshotId); + Map newSnapshotStates = new HashMap<>(snapshotStates); + newSnapshotStates.put(snapshotId.getUUID(), snapshotState); Map> allIndexSnapshots = new HashMap<>(indexSnapshots); for (final IndexId indexId : snapshottedIndices) { if (allIndexSnapshots.containsKey(indexId)) { @@ -158,17 +181,18 @@ public RepositoryData addSnapshot(final SnapshotId snapshotId, final List newSnapshotIds = snapshotIds - .stream() - .filter(id -> snapshotId.equals(id) == false) - .collect(Collectors.toList()); + Map newSnapshotIds = snapshotIds.values().stream() + .filter(id -> snapshotId.equals(id) == false) + .collect(Collectors.toMap(SnapshotId::getUUID, Function.identity())); + Map newSnapshotStates = new HashMap<>(snapshotStates); + newSnapshotStates.remove(snapshotId.getUUID()); Map> indexSnapshots = new HashMap<>(); for (final IndexId indexId : indices.values()) { Set set; @@ -176,7 +200,8 @@ public RepositoryData removeSnapshot(final SnapshotId snapshotId) { assert snapshotIds != null; if (snapshotIds.contains(snapshotId)) { if (snapshotIds.size() == 1) { - // removing the snapshot will mean no more snapshots have this index, so just skip over it + // removing the snapshot will mean no more snapshots + // have this index, so just skip over it continue; } set = new LinkedHashSet<>(snapshotIds); @@ -187,21 +212,7 @@ public RepositoryData removeSnapshot(final SnapshotId snapshotId) { indexSnapshots.put(indexId, set); } - return new RepositoryData(genId, newSnapshotIds, indexSnapshots, incompatibleSnapshotIds); - } - - /** - * Returns a new {@link RepositoryData} instance containing the same snapshot data as the - * invoking instance, with the given incompatible snapshots added to the new instance. - */ - public RepositoryData addIncompatibleSnapshots(final List incompatibleSnapshotIds) { - List newSnapshotIds = new ArrayList<>(this.snapshotIds); - List newIncompatibleSnapshotIds = new ArrayList<>(this.incompatibleSnapshotIds); - for (SnapshotId snapshotId : incompatibleSnapshotIds) { - newSnapshotIds.remove(snapshotId); - newIncompatibleSnapshotIds.add(snapshotId); - } - return new RepositoryData(this.genId, newSnapshotIds, this.indexSnapshots, newIncompatibleSnapshotIds); + return new RepositoryData(genId, newSnapshotIds, newSnapshotStates, indexSnapshots, incompatibleSnapshotIds); } /** @@ -219,7 +230,7 @@ public Set getSnapshots(final IndexId indexId) { * Initializes the indices in the repository metadata; returns a new instance. */ public RepositoryData initIndices(final Map> indexSnapshots) { - return new RepositoryData(genId, snapshotIds, indexSnapshots, incompatibleSnapshotIds); + return new RepositoryData(genId, snapshotIds, snapshotStates, indexSnapshots, incompatibleSnapshotIds); } @Override @@ -232,6 +243,7 @@ public boolean equals(Object obj) { } @SuppressWarnings("unchecked") RepositoryData that = (RepositoryData) obj; return snapshotIds.equals(that.snapshotIds) + && snapshotStates.equals(that.snapshotStates) && indices.equals(that.indices) && indexSnapshots.equals(that.indexSnapshots) && incompatibleSnapshotIds.equals(that.incompatibleSnapshotIds); @@ -239,7 +251,7 @@ public boolean equals(Object obj) { @Override public int hashCode() { - return Objects.hash(snapshotIds, indices, indexSnapshots, incompatibleSnapshotIds); + return Objects.hash(snapshotIds, snapshotStates, indices, indexSnapshots, incompatibleSnapshotIds); } /** @@ -291,6 +303,9 @@ public List resolveNewIndices(final List indicesToResolve) { private static final String INCOMPATIBLE_SNAPSHOTS = "incompatible-snapshots"; private static final String INDICES = "indices"; private static final String INDEX_ID = "id"; + private static final String NAME = "name"; + private static final String UUID = "uuid"; + private static final String STATE = "state"; /** * Writes the snapshots metadata and the related indices metadata to x-content, omitting the @@ -301,7 +316,13 @@ public XContentBuilder snapshotsToXContent(final XContentBuilder builder, final // write the snapshots list builder.startArray(SNAPSHOTS); for (final SnapshotId snapshot : getSnapshotIds()) { - snapshot.toXContent(builder, params); + builder.startObject(); + builder.field(NAME, snapshot.getName()); + builder.field(UUID, snapshot.getUUID()); + if (snapshotStates.containsKey(snapshot.getUUID())) { + builder.field(STATE, snapshotStates.get(snapshot.getUUID()).value()); + } + builder.endObject(); } builder.endArray(); // write the indices map @@ -313,7 +334,7 @@ public XContentBuilder snapshotsToXContent(final XContentBuilder builder, final Set snapshotIds = indexSnapshots.get(indexId); assert snapshotIds != null; for (final SnapshotId snapshotId : snapshotIds) { - snapshotId.toXContent(builder, params); + builder.value(snapshotId.getUUID()); } builder.endArray(); builder.endObject(); @@ -327,20 +348,47 @@ 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 { - List snapshots = new ArrayList<>(); + Map snapshots = new HashMap<>(); + Map snapshotStates = new HashMap<>(); Map> indexSnapshots = new HashMap<>(); if (parser.nextToken() == XContentParser.Token.START_OBJECT) { while (parser.nextToken() == XContentParser.Token.FIELD_NAME) { - String currentFieldName = parser.currentName(); - if (SNAPSHOTS.equals(currentFieldName)) { + String field = parser.currentName(); + if (SNAPSHOTS.equals(field)) { if (parser.nextToken() == XContentParser.Token.START_ARRAY) { while (parser.nextToken() != XContentParser.Token.END_ARRAY) { - snapshots.add(SnapshotId.fromXContent(parser)); + final SnapshotId snapshotId; + // the new format from 5.0 which contains the snapshot name and uuid + if (parser.currentToken() == XContentParser.Token.START_OBJECT) { + String name = null; + String uuid = null; + SnapshotState state = null; + while (parser.nextToken() != XContentParser.Token.END_OBJECT) { + String currentFieldName = parser.currentName(); + parser.nextToken(); + if (NAME.equals(currentFieldName)) { + name = parser.text(); + } else if (UUID.equals(currentFieldName)) { + uuid = parser.text(); + } else if (STATE.equals(currentFieldName)) { + state = SnapshotState.fromValue(parser.numberValue().byteValue()); + } + } + snapshotId = new SnapshotId(name, uuid); + if (state != null) { + snapshotStates.put(uuid, state); + } + } else { + // the old format pre 5.0 that only contains the snapshot name, use the name as the uuid too + final String name = parser.text(); + snapshotId = new SnapshotId(name, name); + } + snapshots.put(snapshotId.getUUID(), snapshotId); } } else { - throw new ElasticsearchParseException("expected array for [" + currentFieldName + "]"); + throw new ElasticsearchParseException("expected array for [" + field + "]"); } - } else if (INDICES.equals(currentFieldName)) { + } else if (INDICES.equals(field)) { if (parser.nextToken() != XContentParser.Token.START_OBJECT) { throw new ElasticsearchParseException("start object expected [indices]"); } @@ -361,7 +409,22 @@ public static RepositoryData snapshotsFromXContent(final XContentParser parser, throw new ElasticsearchParseException("start array expected [snapshots]"); } while (parser.nextToken() != XContentParser.Token.END_ARRAY) { - snapshotIds.add(SnapshotId.fromXContent(parser)); + String uuid = null; + // the old format pre 5.4.1 which contains the snapshot name and uuid + if (parser.currentToken() == XContentParser.Token.START_OBJECT) { + while (parser.nextToken() != XContentParser.Token.END_OBJECT) { + String currentFieldName = parser.currentName(); + parser.nextToken(); + if (UUID.equals(currentFieldName)) { + uuid = parser.text(); + } + } + } else { + // the new format post 5.4.1 that only contains the snapshot uuid, + // since we already have the name/uuid combo in the snapshots array + uuid = parser.text(); + } + snapshotIds.add(snapshots.get(uuid)); } } } @@ -369,13 +432,13 @@ public static RepositoryData snapshotsFromXContent(final XContentParser parser, indexSnapshots.put(new IndexId(indexName, indexId), snapshotIds); } } else { - throw new ElasticsearchParseException("unknown field name [" + currentFieldName + "]"); + throw new ElasticsearchParseException("unknown field name [" + field + "]"); } } } else { throw new ElasticsearchParseException("start object expected"); } - return new RepositoryData(genId, snapshots, indexSnapshots, Collections.emptyList()); + return new RepositoryData(genId, snapshots, snapshotStates, indexSnapshots, Collections.emptyList()); } /** @@ -419,7 +482,7 @@ public RepositoryData incompatibleSnapshotsFromXContent(final XContentParser par } else { throw new ElasticsearchParseException("start object expected"); } - return new RepositoryData(this.genId, this.snapshotIds, this.indexSnapshots, incompatibleSnapshotIds); + return new RepositoryData(this.genId, this.snapshotIds, this.snapshotStates, this.indexSnapshots, incompatibleSnapshotIds); } } diff --git a/core/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/core/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index f9fcb3a470afb..4c9ebd94de61b 100644 --- a/core/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/core/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -473,10 +473,7 @@ public SnapshotInfo finalizeSnapshot(final SnapshotId snapshotId, shardFailures); snapshotFormat.write(blobStoreSnapshot, snapshotsBlobContainer, snapshotId.getUUID()); final RepositoryData repositoryData = getRepositoryData(); - List snapshotIds = repositoryData.getSnapshotIds(); - if (!snapshotIds.contains(snapshotId)) { - writeIndexGen(repositoryData.addSnapshot(snapshotId, indices), repositoryStateId); - } + writeIndexGen(repositoryData.addSnapshot(snapshotId, blobStoreSnapshot.state(), indices), repositoryStateId); return blobStoreSnapshot; } catch (IOException ex) { throw new RepositoryException(metadata.name(), "failed to update snapshot in repository", ex); diff --git a/core/src/main/java/org/elasticsearch/snapshots/SnapshotInfo.java b/core/src/main/java/org/elasticsearch/snapshots/SnapshotInfo.java index 8ea8dce66aa26..433a631821319 100644 --- a/core/src/main/java/org/elasticsearch/snapshots/SnapshotInfo.java +++ b/core/src/main/java/org/elasticsearch/snapshots/SnapshotInfo.java @@ -98,8 +98,8 @@ public final class SnapshotInfo implements Comparable, ToXContent, private final List shardFailures; - public SnapshotInfo(SnapshotId snapshotId, List indices) { - this(snapshotId, indices, null, null, null, 0L, 0L, 0, 0, Collections.emptyList()); + public SnapshotInfo(SnapshotId snapshotId, List indices, SnapshotState state) { + this(snapshotId, indices, state, null, null, 0L, 0L, 0, 0, Collections.emptyList()); } public SnapshotInfo(SnapshotId snapshotId, List indices, long startTime) { @@ -176,10 +176,10 @@ public static SnapshotInfo incompatible(SnapshotId snapshotId) { /** * Gets a new {@link SnapshotInfo} instance from the given {@link SnapshotInfo} with - * all information stripped out except the snapshot id and indices. + * all information stripped out except the snapshot id, state, and indices. */ public SnapshotInfo basic() { - return new SnapshotInfo(snapshotId, indices); + return new SnapshotInfo(snapshotId, indices, state); } /** diff --git a/core/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/core/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index a30324a13fc91..f8abcf318086f 100644 --- a/core/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/core/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -26,8 +26,6 @@ import org.apache.lucene.util.CollectionUtil; import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.OriginalIndices; -import org.elasticsearch.action.search.ShardSearchFailure; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.cluster.ClusterChangedEvent; import org.elasticsearch.cluster.ClusterState; @@ -67,7 +65,6 @@ import org.elasticsearch.repositories.Repository; import org.elasticsearch.repositories.RepositoryData; import org.elasticsearch.repositories.RepositoryMissingException; -import org.elasticsearch.search.SearchShardTarget; import org.elasticsearch.threadpool.ThreadPool; import java.io.IOException; @@ -171,7 +168,7 @@ public SnapshotInfo snapshot(final String repositoryName, final SnapshotId snaps */ public List snapshots(final String repositoryName, final List snapshotIds, - final List incompatibleSnapshotIds, + final Set incompatibleSnapshotIds, final boolean ignoreUnavailable) { final Set snapshotSet = new HashSet<>(); final Set snapshotIdsToIterate = new HashSet<>(snapshotIds); diff --git a/core/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java b/core/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java index ab898985e43af..c925775fa5bd0 100644 --- a/core/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java +++ b/core/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java @@ -1538,7 +1538,7 @@ public MetaData getSnapshotMetaData(SnapshotInfo snapshot, List indices public RepositoryData getRepositoryData() { Map> map = new HashMap<>(); map.put(new IndexId(indexName, "blah"), emptySet()); - return new RepositoryData(EMPTY_REPO_GEN, Collections.emptyList(), map, Collections.emptyList()); + return new RepositoryData(EMPTY_REPO_GEN, Collections.emptyMap(), Collections.emptyMap(), map, Collections.emptyList()); } @Override diff --git a/core/src/test/java/org/elasticsearch/repositories/RepositoryDataTests.java b/core/src/test/java/org/elasticsearch/repositories/RepositoryDataTests.java index 6c548a38cb356..40ff1bad9767f 100644 --- a/core/src/test/java/org/elasticsearch/repositories/RepositoryDataTests.java +++ b/core/src/test/java/org/elasticsearch/repositories/RepositoryDataTests.java @@ -25,6 +25,7 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.snapshots.SnapshotId; +import org.elasticsearch.snapshots.SnapshotState; import org.elasticsearch.test.ESTestCase; import java.io.IOException; @@ -82,7 +83,8 @@ public void testAddSnapshots() { for (int i = 0; i < numOld; i++) { indices.add(indexIdMap.get(indexNames.get(i))); } - RepositoryData newRepoData = repositoryData.addSnapshot(newSnapshot, indices); + RepositoryData newRepoData = repositoryData.addSnapshot(newSnapshot, + randomFrom(SnapshotState.SUCCESS, SnapshotState.PARTIAL, SnapshotState.FAILED), indices); // verify that the new repository data has the new snapshot and its indices assertTrue(newRepoData.getSnapshotIds().contains(newSnapshot)); for (IndexId indexId : indices) { @@ -97,15 +99,21 @@ public void testAddSnapshots() { public void testInitIndices() { final int numSnapshots = randomIntBetween(1, 30); - final List snapshotIds = new ArrayList<>(numSnapshots); + final Map snapshotIds = new HashMap<>(numSnapshots); for (int i = 0; i < numSnapshots; i++) { - snapshotIds.add(new SnapshotId(randomAlphaOfLength(8), UUIDs.randomBase64UUID())); + final SnapshotId snapshotId = new SnapshotId(randomAlphaOfLength(8), UUIDs.randomBase64UUID()); + snapshotIds.put(snapshotId.getUUID(), snapshotId); } - RepositoryData repositoryData = new RepositoryData(EMPTY_REPO_GEN, snapshotIds, Collections.emptyMap(), Collections.emptyList()); + RepositoryData repositoryData = new RepositoryData(EMPTY_REPO_GEN, snapshotIds, + Collections.emptyMap(), Collections.emptyMap(), Collections.emptyList()); // test that initializing indices works Map> indices = randomIndices(snapshotIds); RepositoryData newRepoData = repositoryData.initIndices(indices); - assertEquals(repositoryData.getSnapshotIds(), newRepoData.getSnapshotIds()); + List expected = new ArrayList<>(repositoryData.getSnapshotIds()); + Collections.sort(expected); + List actual = new ArrayList<>(newRepoData.getSnapshotIds()); + Collections.sort(actual); + assertEquals(expected, actual); for (IndexId indexId : indices.keySet()) { assertEquals(indices.get(indexId), newRepoData.getSnapshots(indexId)); } @@ -136,25 +144,32 @@ public void testResolveIndexId() { assertEquals(new IndexId(notInRepoData, notInRepoData), repositoryData.resolveIndexId(notInRepoData)); } - public static RepositoryData generateRandomRepoData() { - return generateRandomRepoData(new ArrayList<>()); - } - - public static RepositoryData generateRandomRepoData(final List origSnapshotIds) { - List snapshotIds = randomSnapshots(origSnapshotIds); - return new RepositoryData(EMPTY_REPO_GEN, snapshotIds, randomIndices(snapshotIds), Collections.emptyList()); + public void testGetSnapshotState() { + final SnapshotId snapshotId = new SnapshotId(randomAlphaOfLength(8), UUIDs.randomBase64UUID()); + final SnapshotState state = randomFrom(SnapshotState.values()); + final RepositoryData repositoryData = RepositoryData.EMPTY.addSnapshot(snapshotId, state, Collections.emptyList()); + assertEquals(state, repositoryData.getSnapshotState(snapshotId)); + assertNull(repositoryData.getSnapshotState(new SnapshotId(randomAlphaOfLength(8), UUIDs.randomBase64UUID()))); } - private static List randomSnapshots(final List origSnapshotIds) { + public static RepositoryData generateRandomRepoData() { + final int numIndices = randomIntBetween(1, 30); + final List indices = new ArrayList<>(numIndices); + for (int i = 0; i < numIndices; i++) { + indices.add(new IndexId(randomAlphaOfLength(8), UUIDs.randomBase64UUID())); + } final int numSnapshots = randomIntBetween(1, 30); - final List snapshotIds = new ArrayList<>(origSnapshotIds); + RepositoryData repositoryData = RepositoryData.EMPTY; for (int i = 0; i < numSnapshots; i++) { - snapshotIds.add(new SnapshotId(randomAlphaOfLength(8), UUIDs.randomBase64UUID())); + final SnapshotId snapshotId = new SnapshotId(randomAlphaOfLength(8), UUIDs.randomBase64UUID()); + final List someIndices = indices.subList(0, randomIntBetween(1, numIndices)); + repositoryData = repositoryData.addSnapshot(snapshotId, randomFrom(SnapshotState.values()), someIndices); } - return snapshotIds; + return repositoryData; } - private static Map> randomIndices(final List snapshotIds) { + private static Map> randomIndices(final Map snapshotIdsMap) { + final List snapshotIds = new ArrayList<>(snapshotIdsMap.values()); final int totalSnapshots = snapshotIds.size(); final int numIndices = randomIntBetween(1, 30); final Map> indices = new HashMap<>(numIndices); diff --git a/core/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryTests.java b/core/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryTests.java index 87e8d595f4b99..7e4d5cc54a900 100644 --- a/core/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryTests.java +++ b/core/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryTests.java @@ -29,6 +29,7 @@ import org.elasticsearch.repositories.RepositoryData; import org.elasticsearch.repositories.RepositoryException; import org.elasticsearch.snapshots.SnapshotId; +import org.elasticsearch.snapshots.SnapshotState; import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.ESSingleNodeTestCase; @@ -143,7 +144,7 @@ public void testIndexGenerationalFiles() throws Exception { assertThat(repository.readSnapshotIndexLatestBlob(), equalTo(1L)); // removing a snapshot and writing to a new index generational file - repositoryData = repository.getRepositoryData().removeSnapshot(repositoryData.getSnapshotIds().get(0)); + repositoryData = repository.getRepositoryData().removeSnapshot(repositoryData.getSnapshotIds().iterator().next()); repository.writeIndexGen(repositoryData, repositoryData.getGenId()); assertEquals(repository.getRepositoryData(), repositoryData); assertThat(repository.latestIndexBlobId(), equalTo(2L)); @@ -181,8 +182,8 @@ public void testReadAndWriteIncompatibleSnapshots() throws Exception { for (int i = 0; i < numSnapshots; i++) { snapshotIds.add(new SnapshotId(randomAlphaOfLength(8), UUIDs.randomBase64UUID())); } - RepositoryData repositoryData = new RepositoryData(readData.getGenId(), Collections.emptyList(), Collections.emptyMap(), - snapshotIds); + RepositoryData repositoryData = new RepositoryData(readData.getGenId(), + Collections.emptyMap(), Collections.emptyMap(), Collections.emptyMap(), snapshotIds); repository.blobContainer().deleteBlob("incompatible-snapshots"); repository.writeIncompatibleSnapshots(repositoryData); readData = repository.getRepositoryData(); @@ -228,7 +229,8 @@ private RepositoryData addRandomSnapshotsToRepoData(RepositoryData repoData, boo for (int j = 0; j < numIndices; j++) { indexIds.add(new IndexId(randomAlphaOfLength(8), UUIDs.randomBase64UUID())); } - repoData = repoData.addSnapshot(snapshotId, indexIds); + repoData = repoData.addSnapshot(snapshotId, + randomFrom(SnapshotState.SUCCESS, SnapshotState.PARTIAL, SnapshotState.FAILED), indexIds); } return repoData; } diff --git a/core/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java b/core/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java index 47c73377cf7e8..381755f87388c 100644 --- a/core/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java +++ b/core/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java @@ -2840,7 +2840,7 @@ private void verifySnapshotInfo(final GetSnapshotsResponse response, final Map expected = snapshotInfo.indices(); assertEquals(expected, indicesPerSnapshot.get(snapshotInfo.snapshotId().getName())); - assertNull(snapshotInfo.state()); + assertEquals(SnapshotState.SUCCESS, snapshotInfo.state()); } } } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/snapshot.get.json b/rest-api-spec/src/main/resources/rest-api-spec/api/snapshot.get.json index 760809cdf9baa..02f5259bc2ec8 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/snapshot.get.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/snapshot.get.json @@ -25,6 +25,10 @@ "ignore_unavailable": { "type": "boolean", "description": "Whether to ignore unavailable snapshots, defaults to false which means a SnapshotMissingException is thrown" + }, + "verbose": { + "type": "boolean", + "description": "Whether to show verbose snapshot info or only show the basic info found in the repository index blob" } } }, diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.get/10_basic.yaml b/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.get/10_basic.yaml index 24a7ac6adc616..f9da8fab1e8ac 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.get/10_basic.yaml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.get/10_basic.yaml @@ -52,3 +52,34 @@ setup: ignore_unavailable: true - is_true: snapshots + +--- +"Get snapshot info when verbose is false": + - skip: + version: " - 5.4.99" + reason: verbose mode was introduced in 5.5 + + - do: + indices.create: + index: test_index + body: + settings: + number_of_shards: 1 + number_of_replicas: 0 + + - do: + snapshot.create: + repository: test_repo_get_1 + snapshot: test_snapshot + wait_for_completion: true + + - do: + snapshot.get: + repository: test_repo_get_1 + snapshot: test_snapshot + verbose: false + + - is_true: snapshots + - match: { snapshots.0.snapshot: test_snapshot } + - match: { snapshots.0.state: SUCCESS } + - is_false: snapshots.0.version From 41bb55dec682e663640f834f37c4a1bd7a1301c1 Mon Sep 17 00:00:00 2001 From: Ali Beyad Date: Tue, 9 May 2017 14:29:18 -0400 Subject: [PATCH 3/5] fix test --- .../org/elasticsearch/snapshots/SnapshotInfo.java | 2 +- .../rest-api-spec/test/snapshot.get/10_basic.yaml | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/snapshots/SnapshotInfo.java b/core/src/main/java/org/elasticsearch/snapshots/SnapshotInfo.java index 433a631821319..994d5d2a9affb 100644 --- a/core/src/main/java/org/elasticsearch/snapshots/SnapshotInfo.java +++ b/core/src/main/java/org/elasticsearch/snapshots/SnapshotInfo.java @@ -70,7 +70,7 @@ public final class SnapshotInfo implements Comparable, ToXContent, private static final String SUCCESSFUL_SHARDS = "successful_shards"; private static final Version VERSION_INCOMPATIBLE_INTRODUCED = Version.V_5_2_0_UNRELEASED; - public static final Version VERBOSE_INTRODUCED = Version.V_5_5_0_UNRELEASED; + public static final Version VERBOSE_INTRODUCED = Version.V_6_0_0_alpha1_UNRELEASED; private static final Comparator COMPARATOR = Comparator.comparing(SnapshotInfo::startTime).thenComparing(SnapshotInfo::snapshotId); diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.get/10_basic.yaml b/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.get/10_basic.yaml index f9da8fab1e8ac..515662355da3e 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.get/10_basic.yaml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.get/10_basic.yaml @@ -33,6 +33,11 @@ setup: - is_true: snapshots + - do: + snapshot.delete: + repository: test_repo_get_1 + snapshot: test_snapshot + --- "Get missing snapshot info throws an exception": @@ -56,8 +61,8 @@ setup: --- "Get snapshot info when verbose is false": - skip: - version: " - 5.4.99" - reason: verbose mode was introduced in 5.5 + version: " - 5.99.99" + reason: verbose mode was introduced in 6.0 - do: indices.create: @@ -83,3 +88,8 @@ setup: - match: { snapshots.0.snapshot: test_snapshot } - match: { snapshots.0.state: SUCCESS } - is_false: snapshots.0.version + + - do: + snapshot.delete: + repository: test_repo_get_1 + snapshot: test_snapshot From 9e298204df5fef2b7c6d3b92b94fa8d14e72579d Mon Sep 17 00:00:00 2001 From: Ali Beyad Date: Tue, 9 May 2017 22:31:15 -0400 Subject: [PATCH 4/5] add bwc test --- .../elasticsearch/bwcompat/RestoreBackwardsCompatIT.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/core/src/test/java/org/elasticsearch/bwcompat/RestoreBackwardsCompatIT.java b/core/src/test/java/org/elasticsearch/bwcompat/RestoreBackwardsCompatIT.java index 228991f787bad..394f09120d344 100644 --- a/core/src/test/java/org/elasticsearch/bwcompat/RestoreBackwardsCompatIT.java +++ b/core/src/test/java/org/elasticsearch/bwcompat/RestoreBackwardsCompatIT.java @@ -161,6 +161,14 @@ private void testOldSnapshot(String version, String repo, String snapshot) throw SnapshotInfo snapshotInfo = getSnapshotsResponse.getSnapshots().get(0); assertThat(snapshotInfo.version().toString(), equalTo(version)); + logger.info("--> get less verbose snapshot info"); + getSnapshotsResponse = client().admin().cluster().prepareGetSnapshots(repo) + .setSnapshots(snapshot).setVerbose(false).get(); + assertEquals(1, getSnapshotsResponse.getSnapshots().size()); + snapshotInfo = getSnapshotsResponse.getSnapshots().get(0); + assertEquals(snapshot, snapshotInfo.snapshotId().getName()); + assertNull(snapshotInfo.version()); // in verbose=false mode, version doesn't exist + logger.info("--> restoring snapshot"); RestoreSnapshotResponse response = client().admin().cluster().prepareRestoreSnapshot(repo, snapshot).setRestoreGlobalState(true).setWaitForCompletion(true).get(); assertThat(response.status(), equalTo(RestStatus.OK)); From c37d9b8f4513da2d2ccad28a1a3622a628fd8bfd Mon Sep 17 00:00:00 2001 From: Ali Beyad Date: Wed, 10 May 2017 13:10:50 -0400 Subject: [PATCH 5/5] simpler remove logic --- .../cluster/snapshots/get/TransportGetSnapshotsAction.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java b/core/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java index 557627f1b10f3..eec218a4119ba 100644 --- a/core/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java +++ b/core/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java @@ -164,9 +164,8 @@ private List buildSimpleSnapshotInfos(final Set toReso final List currentSnapshots) { List snapshotInfos = new ArrayList<>(); for (SnapshotInfo snapshotInfo : currentSnapshots) { - if (toResolve.contains(snapshotInfo.snapshotId())) { + if (toResolve.remove(snapshotInfo.snapshotId())) { snapshotInfos.add(snapshotInfo.basic()); - toResolve.remove(snapshotInfo.snapshotId()); } } Map> snapshotsToIndices = new HashMap<>();