Skip to content

Commit

Permalink
Output all empty snapshot info fields if in verbose mode
Browse files Browse the repository at this point in the history
In #24477, a less verbose option was added to retrieve snapshot info via
GET /_snapshot/{repo}/{snapshots}.  The point of adding this less
verbose option was so that if the repository is a cloud based one, and
there are many snapshots for which the snapshot info needed to be
retrieved, then each snapshot would require reading a separate snapshot
metadata file to pull out the necessary information.  This can be costly
(performance and cost) on cloud based repositories, so a less verbose
option was added that only retrieves very basic information about each
snapshot that is all available in the index-N blob - requiring only one
read!

In order to display this less verbose snapshot info appropriately, logic
was added to not display those fields which could not be populated.
However, this broke integrators (e.g. ECE) that required these fields to
be present, even if empty.  This commit is to return these fields in the
response, even if empty, if the verbose option is set.
  • Loading branch information
Ali Beyad committed Jun 28, 2017
1 parent a156ccd commit c148601
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,26 @@
package org.elasticsearch.rest.action.admin.cluster;

import org.elasticsearch.action.admin.cluster.snapshots.get.GetSnapshotsRequest;
import org.elasticsearch.action.admin.cluster.snapshots.get.GetSnapshotsResponse;
import org.elasticsearch.client.node.NodeClient;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.rest.BaseRestHandler;
import org.elasticsearch.rest.BytesRestResponse;
import org.elasticsearch.rest.RestController;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.action.RestToXContentListener;
import org.elasticsearch.rest.RestResponse;
import org.elasticsearch.rest.action.RestBuilderListener;

import java.io.IOException;
import java.util.Collections;
import java.util.Map;

import static org.elasticsearch.client.Requests.getSnapshotsRequest;
import static org.elasticsearch.rest.RestRequest.Method.GET;
import static org.elasticsearch.rest.RestStatus.OK;

/**
* Returns information about snapshot
Expand All @@ -52,10 +60,19 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
String repository = request.param("repository");
String[] snapshots = request.paramAsStringArray("snapshot", Strings.EMPTY_ARRAY);

GetSnapshotsRequest getSnapshotsRequest = getSnapshotsRequest(repository).snapshots(snapshots);
final 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));
return channel -> client.admin().cluster().getSnapshots(getSnapshotsRequest,
new RestBuilderListener<GetSnapshotsResponse>(channel) {
@Override
public RestResponse buildResponse(GetSnapshotsResponse response, XContentBuilder builder) throws Exception {
Map<String, String> params = Collections.singletonMap("verbose", Boolean.toString(getSnapshotsRequest.verbose()));
ToXContent.MapParams xContentParams = new ToXContent.MapParams(params);
response.toXContent(builder, xContentParams);
return new BytesRestResponse(OK, builder);
}
});
}
}
11 changes: 6 additions & 5 deletions core/src/main/java/org/elasticsearch/snapshots/SnapshotInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,7 @@ public XContentBuilder toXContent(final XContentBuilder builder, final Params pa
return toXContentSnapshot(builder, params);
}

final boolean verbose = params.paramAsBoolean("verbose", false);
// write snapshot info for the API and any other situations
builder.startObject();
builder.field(SNAPSHOT, snapshotId.getName());
Expand All @@ -359,22 +360,22 @@ public XContentBuilder toXContent(final XContentBuilder builder, final Params pa
builder.value(index);
}
builder.endArray();
if (state != null) {
if (verbose || state != null) {
builder.field(STATE, state);
}
if (reason != null) {
builder.field(REASON, reason);
}
if (startTime != 0) {
if (verbose || startTime != 0) {
builder.field(START_TIME, DATE_TIME_FORMATTER.printer().print(startTime));
builder.field(START_TIME_IN_MILLIS, startTime);
}
if (endTime != 0) {
if (verbose || endTime != 0) {
builder.field(END_TIME, DATE_TIME_FORMATTER.printer().print(endTime));
builder.field(END_TIME_IN_MILLIS, endTime);
builder.timeValueField(DURATION_IN_MILLIS, DURATION, endTime - startTime);
}
if (!shardFailures.isEmpty()) {
if (verbose || !shardFailures.isEmpty()) {
builder.startArray(FAILURES);
for (SnapshotShardFailure shardFailure : shardFailures) {
builder.startObject();
Expand All @@ -383,7 +384,7 @@ public XContentBuilder toXContent(final XContentBuilder builder, final Params pa
}
builder.endArray();
}
if (totalShards != 0) {
if (verbose || totalShards != 0) {
builder.startObject(SHARDS);
builder.field(TOTAL, totalShards);
builder.field(FAILED, failedShards());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ setup:
snapshot: test_snapshot

- is_true: snapshots
- is_true: snapshots.0.failures

- do:
snapshot.delete:
Expand Down Expand Up @@ -87,6 +88,8 @@ setup:
- is_true: snapshots
- match: { snapshots.0.snapshot: test_snapshot }
- match: { snapshots.0.state: SUCCESS }
- is_false: snapshots.0.failures
- is_false: snapshots.0.shards
- is_false: snapshots.0.version

- do:
Expand Down

0 comments on commit c148601

Please sign in to comment.