From 89c0a9ccf1b45fa2e8f78cce57b20abd1ee92604 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 22 Mar 2019 15:29:54 -0400 Subject: [PATCH 1/2] Add start and stop time to cat recovery API The cat recovery API is incredibly useful. Yet it is missing the start and stop time as an option from the output. This commit adds these as options to the cat recovery API. We elect to make these not visible by default to avoid breaking the output that users might rely on. --- .../elasticsearch/action/ActionModule.java | 3 +- ...Action.java => RestCatRecoveryAction.java} | 13 +- .../action/cat/RestRecoveryActionTests.java | 133 ++++++++++-------- 3 files changed, 91 insertions(+), 58 deletions(-) rename server/src/main/java/org/elasticsearch/rest/action/cat/{RestRecoveryAction.java => RestCatRecoveryAction.java} (89%) diff --git a/server/src/main/java/org/elasticsearch/action/ActionModule.java b/server/src/main/java/org/elasticsearch/action/ActionModule.java index 83e1e01614435..2cfe66372115f 100644 --- a/server/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/server/src/main/java/org/elasticsearch/action/ActionModule.java @@ -289,6 +289,7 @@ import org.elasticsearch.rest.action.cat.RestAliasAction; import org.elasticsearch.rest.action.cat.RestAllocationAction; import org.elasticsearch.rest.action.cat.RestCatAction; +import org.elasticsearch.rest.action.cat.RestCatRecoveryAction; import org.elasticsearch.rest.action.cat.RestFielddataAction; import org.elasticsearch.rest.action.cat.RestHealthAction; import org.elasticsearch.rest.action.cat.RestIndicesAction; @@ -665,7 +666,7 @@ public void initRestHandlers(Supplier nodesInCluster) { // Fully qualified to prevent interference with rest.action.count.RestCountAction registerHandler.accept(new org.elasticsearch.rest.action.cat.RestCountAction(settings, restController)); // Fully qualified to prevent interference with rest.action.indices.RestRecoveryAction - registerHandler.accept(new org.elasticsearch.rest.action.cat.RestRecoveryAction(settings, restController)); + registerHandler.accept(new RestCatRecoveryAction(settings, restController)); registerHandler.accept(new RestHealthAction(settings, restController)); registerHandler.accept(new org.elasticsearch.rest.action.cat.RestPendingClusterTasksAction(settings, restController)); registerHandler.accept(new RestAliasAction(settings, restController)); diff --git a/server/src/main/java/org/elasticsearch/rest/action/cat/RestRecoveryAction.java b/server/src/main/java/org/elasticsearch/rest/action/cat/RestCatRecoveryAction.java similarity index 89% rename from server/src/main/java/org/elasticsearch/rest/action/cat/RestRecoveryAction.java rename to server/src/main/java/org/elasticsearch/rest/action/cat/RestCatRecoveryAction.java index a66741c2d9411..0cea93e4e7ee7 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/cat/RestRecoveryAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/cat/RestCatRecoveryAction.java @@ -30,6 +30,7 @@ import org.elasticsearch.common.Table; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.common.xcontent.XContentElasticsearchExtension; import org.elasticsearch.indices.recovery.RecoveryState; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestRequest; @@ -47,8 +48,8 @@ * in a string format, designed to be used at the command line. An Index can * be specified to limit output to a particular index or indices. */ -public class RestRecoveryAction extends AbstractCatAction { - public RestRecoveryAction(Settings settings, RestController restController) { +public class RestCatRecoveryAction extends AbstractCatAction { + public RestCatRecoveryAction(Settings settings, RestController restController) { super(settings); restController.registerHandler(GET, "/_cat/recovery", this); restController.registerHandler(GET, "/_cat/recovery/{index}", this); @@ -86,6 +87,10 @@ protected Table getTableWithHeader(RestRequest request) { t.startHeaders() .addCell("index", "alias:i,idx;desc:index name") .addCell("shard", "alias:s,sh;desc:shard name") + .addCell("start_time", "default:false;alias:start;desc:recovery start time") + .addCell("start_time_millis", "default:false;alias:start_millis;desc:recovery start time in epoch milliseconds") + .addCell("stop_time", "default:false;alias:stop;desc:recovery stop time") + .addCell("stop_time_millis", "default:false;alias:stop_millis;desc:recovery stop time in epoch milliseconds") .addCell("time", "alias:t,ti;desc:recovery time") .addCell("type", "alias:ty;desc:recovery type") .addCell("stage", "alias:st;desc:recovery stage") @@ -149,6 +154,10 @@ public int compare(RecoveryState o1, RecoveryState o2) { t.startRow(); t.addCell(index); t.addCell(state.getShardId().id()); + t.addCell(XContentElasticsearchExtension.DEFAULT_DATE_PRINTER.print(state.getTimer().startTime())); + t.addCell(state.getTimer().startTime()); + t.addCell(XContentElasticsearchExtension.DEFAULT_DATE_PRINTER.print(state.getTimer().stopTime())); + t.addCell(state.getTimer().stopTime()); t.addCell(new TimeValue(state.getTimer().time())); t.addCell(state.getRecoverySource().getType().toString().toLowerCase(Locale.ROOT)); t.addCell(state.getStage().toString().toLowerCase(Locale.ROOT)); diff --git a/server/src/test/java/org/elasticsearch/rest/action/cat/RestRecoveryActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/cat/RestRecoveryActionTests.java index 25f04532ac8ce..e01a05aaa7980 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/cat/RestRecoveryActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/cat/RestRecoveryActionTests.java @@ -29,6 +29,7 @@ import org.elasticsearch.common.Table; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.common.xcontent.XContentElasticsearchExtension; import org.elasticsearch.index.Index; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.indices.recovery.RecoveryState; @@ -37,7 +38,9 @@ import org.elasticsearch.usage.UsageService; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; +import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.Locale; @@ -53,7 +56,7 @@ public void testRestRecoveryAction() { final Settings settings = Settings.EMPTY; UsageService usageService = new UsageService(); final RestController restController = new RestController(Collections.emptySet(), null, null, null, usageService); - final RestRecoveryAction action = new RestRecoveryAction(settings, restController); + final RestCatRecoveryAction action = new RestCatRecoveryAction(settings, restController); final int totalShards = randomIntBetween(1, 32); final int successfulShards = Math.max(0, totalShards - randomIntBetween(1, 2)); final int failedShards = totalShards - successfulShards; @@ -64,7 +67,11 @@ public void testRestRecoveryAction() { final RecoveryState state = mock(RecoveryState.class); when(state.getShardId()).thenReturn(new ShardId(new Index("index", "_na_"), i)); final RecoveryState.Timer timer = mock(RecoveryState.Timer.class); - when(timer.time()).thenReturn((long)randomIntBetween(1000000, 10 * 1000000)); + final long startTime = randomLongBetween(0, new Date().getTime()); + when(timer.startTime()).thenReturn(startTime); + final long time = randomLongBetween(1000000, 10 * 1000000); + when(timer.time()).thenReturn(time); + when(timer.stopTime()).thenReturn(startTime + time); when(state.getTimer()).thenReturn(timer); when(state.getRecoverySource()).thenReturn(TestShardRouting.randomRecoverySource()); when(state.getStage()).thenReturn(randomFrom(RecoveryState.Stage.values())); @@ -122,63 +129,79 @@ public void testRestRecoveryAction() { List headers = table.getHeaders(); - assertThat(headers.get(0).value, equalTo("index")); - assertThat(headers.get(1).value, equalTo("shard")); - assertThat(headers.get(2).value, equalTo("time")); - assertThat(headers.get(3).value, equalTo("type")); - assertThat(headers.get(4).value, equalTo("stage")); - assertThat(headers.get(5).value, equalTo("source_host")); - assertThat(headers.get(6).value, equalTo("source_node")); - assertThat(headers.get(7).value, equalTo("target_host")); - assertThat(headers.get(8).value, equalTo("target_node")); - assertThat(headers.get(9).value, equalTo("repository")); - assertThat(headers.get(10).value, equalTo("snapshot")); - assertThat(headers.get(11).value, equalTo("files")); - assertThat(headers.get(12).value, equalTo("files_recovered")); - assertThat(headers.get(13).value, equalTo("files_percent")); - assertThat(headers.get(14).value, equalTo("files_total")); - assertThat(headers.get(15).value, equalTo("bytes")); - assertThat(headers.get(16).value, equalTo("bytes_recovered")); - assertThat(headers.get(17).value, equalTo("bytes_percent")); - assertThat(headers.get(18).value, equalTo("bytes_total")); - assertThat(headers.get(19).value, equalTo("translog_ops")); - assertThat(headers.get(20).value, equalTo("translog_ops_recovered")); - assertThat(headers.get(21).value, equalTo("translog_ops_percent")); + final List expectedHeaders = Arrays.asList( + "index", + "shard", + "start_time", + "start_time_millis", + "stop_time", + "stop_time_millis", + "time", + "type", + "stage", + "source_host", + "source_node", + "target_host", + "target_node", + "repository", + "snapshot", + "files", + "files_recovered", + "files_percent", + "files_total", + "bytes", + "bytes_recovered", + "bytes_percent", + "bytes_total", + "translog_ops", + "translog_ops_recovered", + "translog_ops_percent"); + + for (int i = 0; i < expectedHeaders.size(); i++) { + assertThat(headers.get(i).value, equalTo(expectedHeaders.get(i))); + } assertThat(table.getRows().size(), equalTo(successfulShards)); + for (int i = 0; i < successfulShards; i++) { final RecoveryState state = recoveryStates.get(i); - List cells = table.getRows().get(i); - assertThat(cells.get(0).value, equalTo("index")); - assertThat(cells.get(1).value, equalTo(i)); - assertThat(cells.get(2).value, equalTo(new TimeValue(state.getTimer().time()))); - assertThat(cells.get(3).value, equalTo(state.getRecoverySource().getType().name().toLowerCase(Locale.ROOT))); - assertThat(cells.get(4).value, equalTo(state.getStage().name().toLowerCase(Locale.ROOT))); - assertThat(cells.get(5).value, equalTo(state.getSourceNode() == null ? "n/a" : state.getSourceNode().getHostName())); - assertThat(cells.get(6).value, equalTo(state.getSourceNode() == null ? "n/a" : state.getSourceNode().getName())); - assertThat(cells.get(7).value, equalTo(state.getTargetNode().getHostName())); - assertThat(cells.get(8).value, equalTo(state.getTargetNode().getName())); - assertThat( - cells.get(9).value, - equalTo(state.getRecoverySource() == null || state.getRecoverySource().getType() != RecoverySource.Type.SNAPSHOT ? - "n/a" : - ((SnapshotRecoverySource) state.getRecoverySource()).snapshot().getRepository())); - assertThat( - cells.get(10).value, - equalTo(state.getRecoverySource() == null || state.getRecoverySource().getType() != RecoverySource.Type.SNAPSHOT ? - "n/a" : - ((SnapshotRecoverySource) state.getRecoverySource()).snapshot().getSnapshotId().getName())); - assertThat(cells.get(11).value, equalTo(state.getIndex().totalRecoverFiles())); - assertThat(cells.get(12).value, equalTo(state.getIndex().recoveredFileCount())); - assertThat(cells.get(13).value, equalTo(percent(state.getIndex().recoveredFilesPercent()))); - assertThat(cells.get(14).value, equalTo(state.getIndex().totalFileCount())); - assertThat(cells.get(15).value, equalTo(state.getIndex().totalRecoverBytes())); - assertThat(cells.get(16).value, equalTo(state.getIndex().recoveredBytes())); - assertThat(cells.get(17).value, equalTo(percent(state.getIndex().recoveredBytesPercent()))); - assertThat(cells.get(18).value, equalTo(state.getIndex().totalBytes())); - assertThat(cells.get(19).value, equalTo(state.getTranslog().totalOperations())); - assertThat(cells.get(20).value, equalTo(state.getTranslog().recoveredOperations())); - assertThat(cells.get(21).value, equalTo(percent(state.getTranslog().recoveredPercent()))); + final List expectedValues = Arrays.asList( + "index", + i, + XContentElasticsearchExtension.DEFAULT_DATE_PRINTER.print(state.getTimer().startTime()), + state.getTimer().startTime(), + XContentElasticsearchExtension.DEFAULT_DATE_PRINTER.print(state.getTimer().stopTime()), + state.getTimer().stopTime(), + new TimeValue(state.getTimer().time()), + state.getRecoverySource().getType().name().toLowerCase(Locale.ROOT), + state.getStage().name().toLowerCase(Locale.ROOT), + state.getSourceNode() == null ? "n/a" : state.getSourceNode().getHostName(), + state.getSourceNode() == null ? "n/a" : state.getSourceNode().getName(), + state.getTargetNode().getHostName(), + state.getTargetNode().getName(), + state.getRecoverySource() == null || state.getRecoverySource().getType() != RecoverySource.Type.SNAPSHOT ? + "n/a" : + ((SnapshotRecoverySource) state.getRecoverySource()).snapshot().getRepository(), + state.getRecoverySource() == null || state.getRecoverySource().getType() != RecoverySource.Type.SNAPSHOT ? + "n/a" : + ((SnapshotRecoverySource) state.getRecoverySource()).snapshot().getSnapshotId().getName(), + state.getIndex().totalRecoverFiles(), + state.getIndex().recoveredFileCount(), + percent(state.getIndex().recoveredFilesPercent()), + state.getIndex().totalFileCount(), + state.getIndex().totalRecoverBytes(), + state.getIndex().recoveredBytes(), + percent(state.getIndex().recoveredBytesPercent()), + state.getIndex().totalBytes(), + state.getTranslog().totalOperations(), + state.getTranslog().recoveredOperations(), + percent(state.getTranslog().recoveredPercent())); + + final List cells = table.getRows().get(i); + for (int j = 0; j < expectedValues.size(); j++) { + System.out.println(j); + assertThat(cells.get(j).value, equalTo(expectedValues.get(j))); + } } } From 7bc78e3c8cc80a4795bcf38424f16e7de7ca876f Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sat, 23 Mar 2019 07:30:43 -0400 Subject: [PATCH 2/2] Update server/src/test/java/org/elasticsearch/rest/action/cat/RestRecoveryActionTests.java --- .../elasticsearch/rest/action/cat/RestRecoveryActionTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/rest/action/cat/RestRecoveryActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/cat/RestRecoveryActionTests.java index e01a05aaa7980..7bfa50ff2b724 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/cat/RestRecoveryActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/cat/RestRecoveryActionTests.java @@ -199,7 +199,6 @@ public void testRestRecoveryAction() { final List cells = table.getRows().get(i); for (int j = 0; j < expectedValues.size(); j++) { - System.out.println(j); assertThat(cells.get(j).value, equalTo(expectedValues.get(j))); } }