Skip to content

Commit d9d35a6

Browse files
jbaierajbaiera
committed
Address reviewer comments.
Convert the stats objects to objects instead of fragments. Simplify unit tests. Add more response uses to the documentation Rename api method to 'status'
1 parent 9024aad commit d9d35a6

File tree

11 files changed

+84
-100
lines changed

11 files changed

+84
-100
lines changed

‎client/rest-high-level/src/main/java/org/elasticsearch/client/SnapshotClient.java‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -167,32 +167,30 @@ public void verifyRepositoryAsync(VerifyRepositoryRequest verifyRepositoryReques
167167
}
168168

169169
/**
170-
* Gets the status of any snapshots currently in progress. If snapshot names are provided, this will return detailed status information
171-
* for them even if they are not currently running.
170+
* Gets the status of requested snapshots.
172171
* See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/modules-snapshots.html"> Snapshot and Restore
173172
* API on elastic.co</a>
174173
* @param snapshotsStatusRequest the request
175174
* @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
176175
* @return the response
177176
* @throws IOException in case there is a problem sending the request or parsing back the response
178177
*/
179-
public SnapshotsStatusResponse snapshotsStatus(SnapshotsStatusRequest snapshotsStatusRequest, RequestOptions options)
178+
public SnapshotsStatusResponse status(SnapshotsStatusRequest snapshotsStatusRequest, RequestOptions options)
180179
throws IOException {
181180
return restHighLevelClient.performRequestAndParseEntity(snapshotsStatusRequest, RequestConverters::snapshotsStatus, options,
182181
SnapshotsStatusResponse::fromXContent, emptySet());
183182
}
184183

185184
/**
186-
* Asynchronously gets the status of any snapshots currently in progress. If snapshot names are provided, this will return detailed
187-
* status information for them even if they are not currently running.
185+
* Asynchronously gets the status of requested snapshots.
188186
* See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/modules-snapshots.html"> Snapshot and Restore
189187
* API on elastic.co</a>
190188
* @param snapshotsStatusRequest the request
191189
* @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
192190
* @param listener the listener to be notified upon request completion
193191
*/
194-
public void snapshotsStatusAsync(SnapshotsStatusRequest snapshotsStatusRequest, RequestOptions options,
195-
ActionListener<SnapshotsStatusResponse> listener) {
192+
public void statusAsync(SnapshotsStatusRequest snapshotsStatusRequest, RequestOptions options,
193+
ActionListener<SnapshotsStatusResponse> listener) {
196194
restHighLevelClient.performRequestAsyncAndParseEntity(snapshotsStatusRequest, RequestConverters::snapshotsStatus, options,
197195
SnapshotsStatusResponse::fromXContent, listener, emptySet());
198196
}

‎client/rest-high-level/src/test/java/org/elasticsearch/client/SnapshotIT.java‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@
1919

2020
package org.elasticsearch.client;
2121

22-
import org.apache.http.entity.ContentType;
23-
import org.apache.http.nio.entity.NStringEntity;
2422
import org.elasticsearch.ElasticsearchException;
2523
import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteRepositoryRequest;
2624
import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteRepositoryResponse;
@@ -58,7 +56,7 @@ private PutRepositoryResponse createTestRepository(String repository, String typ
5856
private Response createTestSnapshot(String repository, String snapshot, String index) throws IOException {
5957
Request createSnapshot = new Request("put", String.format(Locale.ROOT, "_snapshot/%s/%s", repository, snapshot));
6058
createSnapshot.addParameter("wait_for_completion", "true");
61-
createSnapshot.setEntity(new NStringEntity("{\"indices\":\""+index+"\"}", ContentType.APPLICATION_JSON));
59+
createSnapshot.setJsonEntity("{\"indices\":\""+index+"\"}");
6260
return highLevelClient().getLowLevelClient().performRequest(createSnapshot);
6361
}
6462

@@ -141,8 +139,8 @@ public void testSnapshotsStatus() throws IOException {
141139
SnapshotsStatusRequest request = new SnapshotsStatusRequest();
142140
request.repository(testRepository);
143141
request.snapshots(new String[]{testSnapshot});
144-
SnapshotsStatusResponse response = execute(request, highLevelClient().snapshot()::snapshotsStatus,
145-
highLevelClient().snapshot()::snapshotsStatusAsync);
142+
SnapshotsStatusResponse response = execute(request, highLevelClient().snapshot()::status,
143+
highLevelClient().snapshot()::statusAsync);
146144
assertThat(response.getSnapshots().size(), equalTo(1));
147145
assertThat(response.getSnapshots().get(0).getSnapshot().getRepository(), equalTo(testRepository));
148146
assertThat(response.getSnapshots().get(0).getSnapshot().getSnapshotId().getName(), equalTo(testSnapshot));

‎client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SnapshotClientDocumentationIT.java‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import org.elasticsearch.action.admin.cluster.repositories.verify.VerifyRepositoryResponse;
3232
import org.elasticsearch.action.admin.cluster.snapshots.delete.DeleteSnapshotRequest;
3333
import org.elasticsearch.action.admin.cluster.snapshots.delete.DeleteSnapshotResponse;
34+
import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotStats;
3435
import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotStatus;
3536
import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotsStatusRequest;
3637
import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotsStatusResponse;
@@ -39,6 +40,7 @@
3940
import org.elasticsearch.client.RequestOptions;
4041
import org.elasticsearch.client.Response;
4142
import org.elasticsearch.client.RestHighLevelClient;
43+
import org.elasticsearch.cluster.SnapshotsInProgress;
4244
import org.elasticsearch.cluster.metadata.RepositoryMetaData;
4345
import org.elasticsearch.common.settings.Settings;
4446
import org.elasticsearch.common.unit.TimeValue;
@@ -77,6 +79,7 @@ public class SnapshotClientDocumentationIT extends ESRestHighLevelClientTestCase
7779

7880
private static final String repositoryName = "test_repository";
7981
private static final String snapshotName = "test_snapshot";
82+
private static final String indexName = "test_index";
8083

8184
public void testSnapshotCreateRepository() throws IOException {
8285
RestHighLevelClient client = highLevelClient();
@@ -372,6 +375,7 @@ public void onFailure(Exception e) {
372375
public void testSnapshotSnapshotsStatus() throws IOException {
373376
RestHighLevelClient client = highLevelClient();
374377
createTestRepositories();
378+
createTestIndex();
375379
createTestSnapshots();
376380

377381
// tag::snapshots-status-request
@@ -394,15 +398,19 @@ public void testSnapshotSnapshotsStatus() throws IOException {
394398
// end::snapshots-status-request-masterTimeout
395399

396400
// tag::snapshots-status-execute
397-
SnapshotsStatusResponse response = client.snapshot().snapshotsStatus(request, RequestOptions.DEFAULT);
401+
SnapshotsStatusResponse response = client.snapshot().status(request, RequestOptions.DEFAULT);
398402
// end::snapshots-status-execute
399403

400404
// tag::snapshots-status-response
401405
List<SnapshotStatus> snapshotStatusesResponse = response.getSnapshots();
406+
SnapshotStatus snapshotStatus = snapshotStatusesResponse.get(0); // <1>
407+
SnapshotsInProgress.State snapshotState = snapshotStatus.getState(); // <2>
408+
SnapshotStats shardStats = snapshotStatus.getIndices().get(indexName).getShards().get(0).getStats(); // <3>
402409
// end::snapshots-status-response
403410
assertThat(snapshotStatusesResponse.size(), equalTo(1));
404-
assertThat(snapshotStatusesResponse.get(0).getSnapshot().getRepository(), equalTo(repositoryName));
411+
assertThat(snapshotStatusesResponse.get(0).getSnapshot().getRepository(), equalTo(SnapshotClientDocumentationIT.repositoryName));
405412
assertThat(snapshotStatusesResponse.get(0).getSnapshot().getSnapshotId().getName(), equalTo(snapshotName));
413+
assertThat(snapshotState.completed(), equalTo(true));
406414
}
407415

408416
public void testSnapshotSnapshotsStatusAsync() throws InterruptedException {
@@ -430,7 +438,7 @@ public void onFailure(Exception e) {
430438
listener = new LatchedActionListener<>(listener, latch);
431439

432440
// tag::snapshots-status-execute-async
433-
client.snapshot().snapshotsStatusAsync(request, RequestOptions.DEFAULT, listener); // <1>
441+
client.snapshot().statusAsync(request, RequestOptions.DEFAULT, listener); // <1>
434442
// end::snapshots-status-execute-async
435443

436444
assertTrue(latch.await(30L, TimeUnit.SECONDS));
@@ -441,6 +449,7 @@ public void testSnapshotDeleteSnapshot() throws IOException {
441449
RestHighLevelClient client = highLevelClient();
442450

443451
createTestRepositories();
452+
createTestIndex();
444453
createTestSnapshots();
445454

446455
// tag::delete-snapshot-request
@@ -502,9 +511,14 @@ private void createTestRepositories() throws IOException {
502511
assertTrue(highLevelClient().snapshot().createRepository(request, RequestOptions.DEFAULT).isAcknowledged());
503512
}
504513

514+
private void createTestIndex() throws IOException {
515+
createIndex(indexName, Settings.EMPTY);
516+
}
517+
505518
private void createTestSnapshots() throws IOException {
506519
Request createSnapshot = new Request("put", String.format(Locale.ROOT, "_snapshot/%s/%s", repositoryName, snapshotName));
507520
createSnapshot.addParameter("wait_for_completion", "true");
521+
createSnapshot.setJsonEntity("{\"indices\":\"" + indexName + "\"}");
508522
Response response = highLevelClient().getLowLevelClient().performRequest(createSnapshot);
509523
// check that the request went ok without parsing JSON here. When using the high level client, check acknowledgement instead.
510524
assertEquals(200, response.getStatusLine().getStatusCode());

‎docs/java-rest/high-level/snapshot/snapshots_status.asciidoc‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,21 +13,23 @@ A `SnapshotsStatusRequest`:
1313
include-tagged::{doc-tests}/SnapshotClientDocumentationIT.java[snapshots-status-request]
1414
--------------------------------------------------
1515

16-
==== Optional Arguments
17-
The following arguments can optionally be provided:
16+
==== Required Arguments
17+
The following arguments must be provided:
1818

1919
["source","java",subs="attributes,callouts,macros"]
2020
--------------------------------------------------
2121
include-tagged::{doc-tests}/SnapshotClientDocumentationIT.java[snapshots-status-request-repository]
2222
--------------------------------------------------
23-
<1> Sets the repository to check for snapshot statuses currently in progress
23+
<1> Sets the repository to check for snapshot statuses
2424

2525
["source","java",subs="attributes,callouts,macros"]
2626
--------------------------------------------------
2727
include-tagged::{doc-tests}/SnapshotClientDocumentationIT.java[snapshots-status-request-snapshots]
2828
--------------------------------------------------
29-
<1> The list of snapshot names to check the status of. If this is set, the status for the snapshots
30-
is returned, even if they are not currently in progress
29+
<1> The list of snapshot names to check the status of
30+
31+
==== Optional Arguments
32+
The following arguments can optionally be provided:
3133

3234
["source","java",subs="attributes,callouts,macros"]
3335
--------------------------------------------------
@@ -90,3 +92,6 @@ executed operation as follows:
9092
--------------------------------------------------
9193
include-tagged::{doc-tests}/SnapshotClientDocumentationIT.java[snapshots-status-response]
9294
--------------------------------------------------
95+
<1> Request contains a list of snapshot statuses
96+
<2> Each status contains information about the snapshot
97+
<3> Example of reading snapshot statistics about a specific index and shard

‎server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/status/SnapshotIndexShardStatus.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ static final class Fields {
162162
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
163163
builder.startObject(Integer.toString(getShardId().getId()));
164164
builder.field(Fields.STAGE, getStage());
165-
stats.toXContent(builder, params);
165+
builder.field(SnapshotStats.Fields.STATS, stats, params);
166166
if (getNodeId() != null) {
167167
builder.field(Fields.NODE, getNodeId());
168168
}

‎server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/status/SnapshotIndexStatus.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,8 @@ static final class Fields {
112112
@Override
113113
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
114114
builder.startObject(getIndex());
115-
shardsStats.toXContent(builder, params);
116-
stats.toXContent(builder, params);
115+
builder.field(SnapshotShardsStats.Fields.SHARDS_STATS, shardsStats, params);
116+
builder.field(SnapshotStats.Fields.STATS, stats, params);
117117
builder.startObject(Fields.SHARDS);
118118
for (SnapshotIndexShardStatus shard : indexShards.values()) {
119119
shard.toXContent(builder, params);

‎server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/status/SnapshotShardsStats.java‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
import org.elasticsearch.common.ParseField;
2323
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
2424
import org.elasticsearch.common.xcontent.ToXContent;
25-
import org.elasticsearch.common.xcontent.ToXContentFragment;
25+
import org.elasticsearch.common.xcontent.ToXContentObject;
2626
import org.elasticsearch.common.xcontent.XContentBuilder;
2727
import org.elasticsearch.common.xcontent.XContentParser;
2828

@@ -34,7 +34,7 @@
3434
/**
3535
* Status of a snapshot shards
3636
*/
37-
public class SnapshotShardsStats implements ToXContentFragment {
37+
public class SnapshotShardsStats implements ToXContentObject {
3838

3939
private int initializingShards;
4040
private int startedShards;
@@ -132,13 +132,15 @@ static final class Fields {
132132

133133
@Override
134134
public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException {
135-
builder.startObject(Fields.SHARDS_STATS);
136-
builder.field(Fields.INITIALIZING, getInitializingShards());
137-
builder.field(Fields.STARTED, getStartedShards());
138-
builder.field(Fields.FINALIZING, getFinalizingShards());
139-
builder.field(Fields.DONE, getDoneShards());
140-
builder.field(Fields.FAILED, getFailedShards());
141-
builder.field(Fields.TOTAL, getTotalShards());
135+
builder.startObject();
136+
{
137+
builder.field(Fields.INITIALIZING, getInitializingShards());
138+
builder.field(Fields.STARTED, getStartedShards());
139+
builder.field(Fields.FINALIZING, getFinalizingShards());
140+
builder.field(Fields.DONE, getDoneShards());
141+
builder.field(Fields.FAILED, getFailedShards());
142+
builder.field(Fields.TOTAL, getTotalShards());
143+
}
142144
builder.endObject();
143145
return builder;
144146
}

‎server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/status/SnapshotStats.java‎

Lines changed: 30 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,14 @@
2626
import org.elasticsearch.common.unit.ByteSizeValue;
2727
import org.elasticsearch.common.unit.TimeValue;
2828
import org.elasticsearch.common.xcontent.ToXContent;
29-
import org.elasticsearch.common.xcontent.ToXContentFragment;
29+
import org.elasticsearch.common.xcontent.ToXContentObject;
3030
import org.elasticsearch.common.xcontent.XContentBuilder;
3131
import org.elasticsearch.common.xcontent.XContentParser;
3232
import org.elasticsearch.common.xcontent.XContentParserUtils;
3333

3434
import java.io.IOException;
3535

36-
public class SnapshotStats implements Streamable, ToXContentFragment {
36+
public class SnapshotStats implements Streamable, ToXContentObject {
3737

3838
private long startTime;
3939
private long time;
@@ -178,32 +178,35 @@ static final class Fields {
178178

179179
@Override
180180
public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException {
181-
builder.startObject(Fields.STATS)
182-
// incremental starts
183-
.startObject(Fields.INCREMENTAL)
184-
.field(Fields.FILE_COUNT, getIncrementalFileCount())
185-
.humanReadableField(Fields.SIZE_IN_BYTES, Fields.SIZE, new ByteSizeValue(getIncrementalSize()))
186-
// incremental ends
187-
.endObject();
188-
189-
if (getProcessedFileCount() != getIncrementalFileCount()) {
190-
// processed starts
191-
builder.startObject(Fields.PROCESSED)
192-
.field(Fields.FILE_COUNT, getProcessedFileCount())
193-
.humanReadableField(Fields.SIZE_IN_BYTES, Fields.SIZE, new ByteSizeValue(getProcessedSize()))
194-
// processed ends
195-
.endObject();
196-
}
197-
// total starts
198-
builder.startObject(Fields.TOTAL)
199-
.field(Fields.FILE_COUNT, getTotalFileCount())
200-
.humanReadableField(Fields.SIZE_IN_BYTES, Fields.SIZE, new ByteSizeValue(getTotalSize()))
201-
// total ends
202-
.endObject();
203-
// timings stats
204-
builder.field(Fields.START_TIME_IN_MILLIS, getStartTime())
205-
.humanReadableField(Fields.TIME_IN_MILLIS, Fields.TIME, new TimeValue(getTime()));
181+
builder.startObject();
182+
{
183+
builder.startObject(Fields.INCREMENTAL);
184+
{
185+
builder.field(Fields.FILE_COUNT, getIncrementalFileCount());
186+
builder.humanReadableField(Fields.SIZE_IN_BYTES, Fields.SIZE, new ByteSizeValue(getIncrementalSize()));
187+
}
188+
builder.endObject();
189+
190+
if (getProcessedFileCount() != getIncrementalFileCount()) {
191+
builder.startObject(Fields.PROCESSED);
192+
{
193+
builder.field(Fields.FILE_COUNT, getProcessedFileCount());
194+
builder.humanReadableField(Fields.SIZE_IN_BYTES, Fields.SIZE, new ByteSizeValue(getProcessedSize()));
195+
}
196+
builder.endObject();
197+
}
198+
199+
builder.startObject(Fields.TOTAL);
200+
{
201+
builder.field(Fields.FILE_COUNT, getTotalFileCount());
202+
builder.humanReadableField(Fields.SIZE_IN_BYTES, Fields.SIZE, new ByteSizeValue(getTotalSize()));
203+
}
204+
builder.endObject();
206205

206+
// timings stats
207+
builder.field(Fields.START_TIME_IN_MILLIS, getStartTime());
208+
builder.humanReadableField(Fields.TIME_IN_MILLIS, Fields.TIME, new TimeValue(getTime()));
209+
}
207210
return builder.endObject();
208211
}
209212

‎server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/status/SnapshotStatus.java‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -230,8 +230,8 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
230230
if (includeGlobalState != null) {
231231
builder.field(INCLUDE_GLOBAL_STATE, includeGlobalState);
232232
}
233-
shardsStats.toXContent(builder, params);
234-
stats.toXContent(builder, params);
233+
builder.field(SnapshotShardsStats.Fields.SHARDS_STATS, shardsStats, params);
234+
builder.field(SnapshotStats.Fields.STATS, stats, params);
235235
builder.startObject(INDICES);
236236
for (SnapshotIndexStatus indexStatus : getIndices().values()) {
237237
indexStatus.toXContent(builder, params);
@@ -255,12 +255,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
255255
@SuppressWarnings("unchecked") List<SnapshotIndexStatus> indices = ((List<SnapshotIndexStatus>) parsedObjects[i]);
256256

257257
Snapshot snapshot = new Snapshot(repository, new SnapshotId(name, uuid));
258-
SnapshotsInProgress.State state;
259-
try {
260-
state = SnapshotsInProgress.State.valueOf(rawState);
261-
} catch (IllegalArgumentException iae) {
262-
throw new ElasticsearchParseException("failed to parse snapshot status, unknown state value [{}]", iae, rawState);
263-
}
258+
SnapshotsInProgress.State state = SnapshotsInProgress.State.valueOf(rawState);
264259
Map<String, SnapshotIndexStatus> indicesStatus;
265260
List<SnapshotIndexShardStatus> shards;
266261
if (indices == null || indices.isEmpty()) {

0 commit comments

Comments
 (0)