From 437b87ffd800952a296d35cfb6b058f0a08613b5 Mon Sep 17 00:00:00 2001 From: Harsh Garg Date: Thu, 26 Sep 2024 16:18:40 +0530 Subject: [PATCH 01/11] Adding _list/shards API Signed-off-by: Harsh Garg --- .../org/opensearch/action/ActionModule.java | 14 +- .../cluster/shards/CatShardsRequest.java | 27 ++ .../cluster/shards/CatShardsResponse.java | 33 +++ .../shards/TransportCatShardsAction.java | 26 +- .../indices/stats/IndicesStatsResponse.java | 2 +- .../java/org/opensearch/common/Table.java | 15 ++ .../java/org/opensearch/rest/RestHandler.java | 7 + .../java/org/opensearch/rest/RestRequest.java | 8 + .../rest/action/cat/RestShardsAction.java | 40 ++- .../opensearch/rest/action/cat/RestTable.java | 41 ++- .../rest/action/list/AbstractListAction.java | 76 ++++++ .../rest/action/list/RestListAction.java | 58 +++++ .../action/list/RestShardsListAction.java | 73 ++++++ .../rest/action/list/package-info.java | 12 + .../rest/pagination/PageParams.java | 65 +++++ .../opensearch/rest/pagination/PageToken.java | 58 +++++ .../rest/pagination/PaginationStrategy.java | 75 ++++++ .../pagination/ShardPaginationStrategy.java | 244 ++++++++++++++++++ .../rest/pagination/package-info.java | 12 + .../action/cat/RestShardsActionTests.java | 2 +- .../rest/action/cat/RestTableTests.java | 93 +++++-- 21 files changed, 951 insertions(+), 30 deletions(-) create mode 100644 server/src/main/java/org/opensearch/rest/action/list/AbstractListAction.java create mode 100644 server/src/main/java/org/opensearch/rest/action/list/RestListAction.java create mode 100644 server/src/main/java/org/opensearch/rest/action/list/RestShardsListAction.java create mode 100644 server/src/main/java/org/opensearch/rest/action/list/package-info.java create mode 100644 server/src/main/java/org/opensearch/rest/pagination/PageParams.java create mode 100644 server/src/main/java/org/opensearch/rest/pagination/PageToken.java create mode 100644 server/src/main/java/org/opensearch/rest/pagination/PaginationStrategy.java create mode 100644 server/src/main/java/org/opensearch/rest/pagination/ShardPaginationStrategy.java create mode 100644 server/src/main/java/org/opensearch/rest/pagination/package-info.java diff --git a/server/src/main/java/org/opensearch/action/ActionModule.java b/server/src/main/java/org/opensearch/action/ActionModule.java index 3fe0f1dc7cb83..a1672397f029e 100644 --- a/server/src/main/java/org/opensearch/action/ActionModule.java +++ b/server/src/main/java/org/opensearch/action/ActionModule.java @@ -461,6 +461,9 @@ import org.opensearch.rest.action.ingest.RestGetPipelineAction; import org.opensearch.rest.action.ingest.RestPutPipelineAction; import org.opensearch.rest.action.ingest.RestSimulatePipelineAction; +import org.opensearch.rest.action.list.AbstractListAction; +import org.opensearch.rest.action.list.RestListAction; +import org.opensearch.rest.action.list.RestShardsListAction; import org.opensearch.rest.action.search.RestClearScrollAction; import org.opensearch.rest.action.search.RestCountAction; import org.opensearch.rest.action.search.RestCreatePitAction; @@ -802,9 +805,14 @@ private ActionFilters setupActionFilters(List actionPlugins) { public void initRestHandlers(Supplier nodesInCluster) { List catActions = new ArrayList<>(); + List listActions = new ArrayList<>(); Consumer registerHandler = handler -> { if (handler instanceof AbstractCatAction) { - catActions.add((AbstractCatAction) handler); + if (handler instanceof AbstractListAction && ((AbstractListAction) handler).isActionPaginated()) { + listActions.add((AbstractListAction) handler); + } else { + catActions.add((AbstractCatAction) handler); + } } restController.registerHandler(handler); }; @@ -980,6 +988,9 @@ public void initRestHandlers(Supplier nodesInCluster) { } registerHandler.accept(new RestTemplatesAction()); + // LIST API + registerHandler.accept(new RestShardsListAction()); + // Point in time API registerHandler.accept(new RestCreatePitAction()); registerHandler.accept(new RestDeletePitAction()); @@ -1011,6 +1022,7 @@ public void initRestHandlers(Supplier nodesInCluster) { } } registerHandler.accept(new RestCatAction(catActions)); + registerHandler.accept(new RestListAction(listActions)); registerHandler.accept(new RestDecommissionAction()); registerHandler.accept(new RestGetDecommissionStateAction()); registerHandler.accept(new RestRemoteStoreStatsAction()); diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsRequest.java index 49299777db8ae..4ee3aa1c9a175 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsRequest.java @@ -8,12 +8,15 @@ package org.opensearch.action.admin.cluster.shards; +import org.opensearch.Version; import org.opensearch.action.ActionRequestValidationException; import org.opensearch.action.support.clustermanager.ClusterManagerNodeReadRequest; import org.opensearch.common.unit.TimeValue; import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; import org.opensearch.core.tasks.TaskId; import org.opensearch.rest.action.admin.cluster.ClusterAdminTask; +import org.opensearch.rest.pagination.PageParams; import java.io.IOException; import java.util.Map; @@ -27,11 +30,27 @@ public class CatShardsRequest extends ClusterManagerNodeReadRequest headers) { return new ClusterAdminTask(id, type, action, parentTaskId, headers, this.cancelAfterTimeInterval); diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsResponse.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsResponse.java index 3dd88a2cda037..21007c4e6ce96 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsResponse.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsResponse.java @@ -8,13 +8,18 @@ package org.opensearch.action.admin.cluster.shards; +import org.opensearch.Version; import org.opensearch.action.admin.cluster.state.ClusterStateResponse; import org.opensearch.action.admin.indices.stats.IndicesStatsResponse; +import org.opensearch.cluster.routing.ShardRouting; import org.opensearch.core.action.ActionResponse; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.rest.pagination.PageToken; import java.io.IOException; +import java.util.ArrayList; +import java.util.List; /** * A response of a cat shards request. @@ -26,17 +31,29 @@ public class CatShardsResponse extends ActionResponse { private ClusterStateResponse clusterStateResponse = null; private IndicesStatsResponse indicesStatsResponse = null; + private List responseShards = new ArrayList<>(); + private PageToken pageToken = null; public CatShardsResponse() {} public CatShardsResponse(StreamInput in) throws IOException { super(in); + clusterStateResponse = new ClusterStateResponse(in); + indicesStatsResponse = new IndicesStatsResponse(in); + if (in.getVersion().onOrAfter(Version.V_3_0_0)) { + responseShards = in.readList(ShardRouting::new); + pageToken = PageToken.readPageToken(in); + } } @Override public void writeTo(StreamOutput out) throws IOException { clusterStateResponse.writeTo(out); indicesStatsResponse.writeTo(out); + if (out.getVersion().onOrAfter(Version.V_3_0_0)) { + out.writeList(responseShards); + pageToken.writePageToken(out); + } } public void setClusterStateResponse(ClusterStateResponse clusterStateResponse) { @@ -54,4 +71,20 @@ public void setIndicesStatsResponse(IndicesStatsResponse indicesStatsResponse) { public IndicesStatsResponse getIndicesStatsResponse() { return this.indicesStatsResponse; } + + public void setResponseShards(List responseShards) { + this.responseShards = responseShards; + } + + public List getResponseShards() { + return this.responseShards; + } + + public void setPageToken(PageToken pageToken) { + this.pageToken = pageToken; + } + + public PageToken getPageToken() { + return this.pageToken; + } } diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java index 224d3cbc5f10a..41a4e4936c471 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java @@ -19,10 +19,14 @@ import org.opensearch.common.inject.Inject; import org.opensearch.core.action.ActionListener; import org.opensearch.core.action.NotifyOnceListener; +import org.opensearch.rest.pagination.PageParams; +import org.opensearch.rest.pagination.ShardPaginationStrategy; import org.opensearch.tasks.CancellableTask; import org.opensearch.tasks.Task; import org.opensearch.transport.TransportService; +import java.util.Objects; + /** * Perform cat shards action * @@ -44,7 +48,11 @@ public void doExecute(Task parentTask, CatShardsRequest shardsRequest, ActionLis clusterStateRequest.setShouldCancelOnTimeout(true); clusterStateRequest.local(shardsRequest.local()); clusterStateRequest.clusterManagerNodeTimeout(shardsRequest.clusterManagerNodeTimeout()); - clusterStateRequest.clear().nodes(true).routingTable(true).indices(shardsRequest.getIndices()); + if (Objects.nonNull(shardsRequest.getPageParams())) { + clusterStateRequest.clear().nodes(true).routingTable(true).indices(shardsRequest.getIndices()); + } else { + clusterStateRequest.clear().nodes(true).routingTable(true).indices(shardsRequest.getIndices()).metadata(true); + } assert parentTask instanceof CancellableTask; clusterStateRequest.setParentTask(client.getLocalNodeId(), parentTask.getId()); @@ -73,11 +81,21 @@ protected void innerOnFailure(Exception e) { client.admin().cluster().state(clusterStateRequest, new ActionListener() { @Override public void onResponse(ClusterStateResponse clusterStateResponse) { + ShardPaginationStrategy paginationStrategy = getPaginationStrategy(shardsRequest.getPageParams(), clusterStateResponse); + String[] indices = Objects.isNull(paginationStrategy) + ? shardsRequest.getIndices() + : paginationStrategy.getRequestedIndices().toArray(new String[0]); catShardsResponse.setClusterStateResponse(clusterStateResponse); + catShardsResponse.setResponseShards( + Objects.isNull(paginationStrategy) + ? clusterStateResponse.getState().routingTable().allShards() + : paginationStrategy.getRequestedEntities() + ); + catShardsResponse.setPageToken(Objects.isNull(paginationStrategy) ? null : paginationStrategy.getResponseToken()); IndicesStatsRequest indicesStatsRequest = new IndicesStatsRequest(); indicesStatsRequest.setShouldCancelOnTimeout(true); indicesStatsRequest.all(); - indicesStatsRequest.indices(shardsRequest.getIndices()); + indicesStatsRequest.indices(indices); indicesStatsRequest.setParentTask(client.getLocalNodeId(), parentTask.getId()); try { client.admin().indices().stats(indicesStatsRequest, new ActionListener() { @@ -107,4 +125,8 @@ public void onFailure(Exception e) { } } + + private ShardPaginationStrategy getPaginationStrategy(PageParams pageParams, ClusterStateResponse clusterStateResponse) { + return Objects.isNull(pageParams) ? null : new ShardPaginationStrategy(pageParams, clusterStateResponse.getState()); + } } diff --git a/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsResponse.java b/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsResponse.java index 900a886481fe6..ae989573b39ea 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsResponse.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsResponse.java @@ -64,7 +64,7 @@ public class IndicesStatsResponse extends BroadcastResponse { private Map shardStatsMap; - IndicesStatsResponse(StreamInput in) throws IOException { + public IndicesStatsResponse(StreamInput in) throws IOException { super(in); shards = in.readArray(ShardStats::new, (size) -> new ShardStats[size]); } diff --git a/server/src/main/java/org/opensearch/common/Table.java b/server/src/main/java/org/opensearch/common/Table.java index da14f628efa0f..133ec3052e6c9 100644 --- a/server/src/main/java/org/opensearch/common/Table.java +++ b/server/src/main/java/org/opensearch/common/Table.java @@ -34,6 +34,7 @@ import org.opensearch.common.time.DateFormatter; import org.opensearch.core.common.Strings; +import org.opensearch.rest.pagination.PageToken; import java.time.Instant; import java.time.ZoneOffset; @@ -59,9 +60,19 @@ public class Table { private List currentCells; private boolean inHeaders = false; private boolean withTime = false; + /** + * paginatedQueryResponse if null will imply the Table response is not paginated. + */ + private PageToken pageToken; public static final String EPOCH = "epoch"; public static final String TIMESTAMP = "timestamp"; + public Table() {} + + public Table(@Nullable PageToken pageToken) { + this.pageToken = pageToken; + } + public Table startHeaders() { inHeaders = true; currentCells = new ArrayList<>(); @@ -230,6 +241,10 @@ public Map getAliasMap() { return headerAliasMap; } + public PageToken getPageToken() { + return pageToken; + } + /** * Cell in a table * diff --git a/server/src/main/java/org/opensearch/rest/RestHandler.java b/server/src/main/java/org/opensearch/rest/RestHandler.java index 1139e5fc65f31..143cbd472ed07 100644 --- a/server/src/main/java/org/opensearch/rest/RestHandler.java +++ b/server/src/main/java/org/opensearch/rest/RestHandler.java @@ -125,6 +125,13 @@ default boolean allowSystemIndexAccessByDefault() { return false; } + /** + * Denotes whether the RestHandler will output paginated responses or not. + */ + default boolean isActionPaginated() { + return false; + } + static RestHandler wrapper(RestHandler delegate) { return new Wrapper(delegate); } diff --git a/server/src/main/java/org/opensearch/rest/RestRequest.java b/server/src/main/java/org/opensearch/rest/RestRequest.java index 2c397f7fc6e8e..f241b567c3204 100644 --- a/server/src/main/java/org/opensearch/rest/RestRequest.java +++ b/server/src/main/java/org/opensearch/rest/RestRequest.java @@ -51,6 +51,7 @@ import org.opensearch.core.xcontent.XContentParser; import org.opensearch.http.HttpChannel; import org.opensearch.http.HttpRequest; +import org.opensearch.rest.pagination.PageParams; import java.io.IOException; import java.io.InputStream; @@ -67,6 +68,9 @@ import static org.opensearch.common.unit.TimeValue.parseTimeValue; import static org.opensearch.core.common.unit.ByteSizeValue.parseBytesSizeValue; +import static org.opensearch.rest.pagination.PageParams.PARAM_NEXT_TOKEN; +import static org.opensearch.rest.pagination.PageParams.PARAM_SIZE; +import static org.opensearch.rest.pagination.PageParams.PARAM_SORT; /** * REST Request @@ -591,6 +595,10 @@ public static MediaType parseContentType(List header) { throw new IllegalArgumentException("empty Content-Type header"); } + public PageParams parsePaginatedQueryParams(String defaultSortOrder, int defaultPageSize) { + return new PageParams(param(PARAM_NEXT_TOKEN), param(PARAM_SORT, defaultSortOrder), paramAsInt(PARAM_SIZE, defaultPageSize)); + } + /** * Thrown if there is an error in the content type header. * diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestShardsAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestShardsAction.java index a7ad5fe6c14a3..7d44d70b285f6 100644 --- a/server/src/main/java/org/opensearch/rest/action/cat/RestShardsAction.java +++ b/server/src/main/java/org/opensearch/rest/action/cat/RestShardsAction.java @@ -63,6 +63,8 @@ import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestResponse; import org.opensearch.rest.action.RestResponseListener; +import org.opensearch.rest.action.list.AbstractListAction; +import org.opensearch.rest.pagination.PageToken; import org.opensearch.search.suggest.completion.CompletionStats; import java.time.Instant; @@ -80,7 +82,7 @@ * * @opensearch.api */ -public class RestShardsAction extends AbstractCatAction { +public class RestShardsAction extends AbstractListAction { private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestShardsAction.class); @@ -119,14 +121,27 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli public RestResponse buildResponse(CatShardsResponse catShardsResponse) throws Exception { ClusterStateResponse clusterStateResponse = catShardsResponse.getClusterStateResponse(); IndicesStatsResponse indicesStatsResponse = catShardsResponse.getIndicesStatsResponse(); - return RestTable.buildResponse(buildTable(request, clusterStateResponse, indicesStatsResponse), channel); + return RestTable.buildResponse( + buildTable( + request, + clusterStateResponse, + indicesStatsResponse, + catShardsResponse.getResponseShards(), + catShardsResponse.getPageToken() + ), + channel + ); } }); } @Override protected Table getTableWithHeader(final RestRequest request) { - Table table = new Table(); + return getTableWithHeader(request, null); + } + + protected Table getTableWithHeader(final RestRequest request, final PageToken pageToken) { + Table table = new Table(pageToken); table.startHeaders() .addCell("index", "default:true;alias:i,idx;desc:index name") .addCell("shard", "default:true;alias:s,sh;desc:shard name") @@ -295,10 +310,16 @@ private static Object getOrNull(S stats, Function accessor, Functio } // package private for testing - Table buildTable(RestRequest request, ClusterStateResponse state, IndicesStatsResponse stats) { - Table table = getTableWithHeader(request); - - for (ShardRouting shard : state.getState().routingTable().allShards()) { + Table buildTable( + RestRequest request, + ClusterStateResponse state, + IndicesStatsResponse stats, + List responseShards, + PageToken pageToken + ) { + Table table = getTableWithHeader(request, pageToken); + + for (ShardRouting shard : responseShards) { ShardStats shardStats = stats.asMap().get(shard); CommonStats commonStats = null; CommitStats commitStats = null; @@ -454,4 +475,9 @@ Table buildTable(RestRequest request, ClusterStateResponse state, IndicesStatsRe return table; } + + @Override + public boolean isActionPaginated() { + return false; + } } diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestTable.java b/server/src/main/java/org/opensearch/rest/action/cat/RestTable.java index 4f1090b163ee6..d622dd7a956f4 100644 --- a/server/src/main/java/org/opensearch/rest/action/cat/RestTable.java +++ b/server/src/main/java/org/opensearch/rest/action/cat/RestTable.java @@ -58,8 +58,11 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Objects; import java.util.Set; +import static org.opensearch.rest.pagination.PageToken.PAGINATED_RESPONSE_NEXT_TOKEN_KEY; + /** * a REST table * @@ -87,8 +90,37 @@ public static RestResponse buildXContentBuilder(Table table, RestChannel channel RestRequest request = channel.request(); XContentBuilder builder = channel.newBuilder(); List displayHeaders = buildDisplayHeaders(table, request); + if (Objects.nonNull(table.getPageToken())) { + buildPaginatedXContentBuilder(table, request, builder, displayHeaders); + } else { + builder.startArray(); + addRowsToXContentBuilder(table, request, builder, displayHeaders); + builder.endArray(); + } + return new BytesRestResponse(RestStatus.OK, builder); + } + + private static void buildPaginatedXContentBuilder( + Table table, + RestRequest request, + XContentBuilder builder, + List displayHeaders + ) throws Exception { + assert Objects.nonNull(table.getPageToken().getPaginatedEntity()) : "Paginated element is required in-case of paginated responses"; + builder.startObject(); + builder.field(PAGINATED_RESPONSE_NEXT_TOKEN_KEY, table.getPageToken().getNextToken()); + builder.startArray(table.getPageToken().getPaginatedEntity()); + addRowsToXContentBuilder(table, request, builder, displayHeaders); + builder.endArray(); + builder.endObject(); + } - builder.startArray(); + private static void addRowsToXContentBuilder( + Table table, + RestRequest request, + XContentBuilder builder, + List displayHeaders + ) throws Exception { List rowOrder = getRowOrder(table, request); for (Integer row : rowOrder) { builder.startObject(); @@ -97,8 +129,6 @@ public static RestResponse buildXContentBuilder(Table table, RestChannel channel } builder.endObject(); } - builder.endArray(); - return new BytesRestResponse(RestStatus.OK, builder); } public static RestResponse buildTextPlainResponse(Table table, RestChannel channel) throws IOException { @@ -136,6 +166,11 @@ public static RestResponse buildTextPlainResponse(Table table, RestChannel chann } out.append("\n"); } + // Adding a new row for next_token, in the response if the table is paginated. + if (Objects.nonNull(table.getPageToken())) { + out.append("next_token" + " " + table.getPageToken().getNextToken()); + out.append("\n"); + } out.close(); return new BytesRestResponse(RestStatus.OK, BytesRestResponse.TEXT_CONTENT_TYPE, bytesOut.bytes()); } diff --git a/server/src/main/java/org/opensearch/rest/action/list/AbstractListAction.java b/server/src/main/java/org/opensearch/rest/action/list/AbstractListAction.java new file mode 100644 index 0000000000000..6b34a7585c658 --- /dev/null +++ b/server/src/main/java/org/opensearch/rest/action/list/AbstractListAction.java @@ -0,0 +1,76 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.rest.action.list; + +import org.opensearch.client.node.NodeClient; +import org.opensearch.rest.RestRequest; +import org.opensearch.rest.action.cat.AbstractCatAction; +import org.opensearch.rest.pagination.PageParams; + +import java.io.IOException; +import java.util.Objects; + +import static org.opensearch.rest.pagination.PageParams.PARAM_ASC_SORT_VALUE; +import static org.opensearch.rest.pagination.PageParams.PARAM_DESC_SORT_VALUE; + +/** + * Base Transport action class for _list API. + * + * @opensearch.api + */ +public abstract class AbstractListAction extends AbstractCatAction { + + private static final int DEFAULT_PAGE_SIZE = 100; + protected PageParams pageParams; + + protected abstract void documentation(StringBuilder sb); + + @Override + public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { + boolean helpWanted = request.paramAsBoolean("help", false); + if (helpWanted || isActionPaginated() == false) { + return super.prepareRequest(request, client); + } + this.pageParams = validateAndGetPageParams(request); + assert Objects.nonNull(pageParams) : "pageParams can not be null for paginated queries"; + return doCatRequest(request, client); + } + + @Override + public boolean isActionPaginated() { + return true; + } + + /** + * + * @return Metadata that can be extracted out from the rest request. Query params supported by the action specific + * to pagination along with any respective validations to be added here. + */ + protected PageParams validateAndGetPageParams(RestRequest restRequest) { + PageParams pageParams = restRequest.parsePaginatedQueryParams(defaultSort(), defaultPageSize()); + // validating pageSize + if (pageParams.getSize() <= 0) { + throw new IllegalArgumentException("size must be greater than zero"); + } + // Validating sort order + if (!(PARAM_ASC_SORT_VALUE.equals(pageParams.getSort()) || PARAM_DESC_SORT_VALUE.equals(pageParams.getSort()))) { + throw new IllegalArgumentException("value of sort can either be asc or desc"); + } + return pageParams; + } + + protected int defaultPageSize() { + return DEFAULT_PAGE_SIZE; + } + + protected String defaultSort() { + return PARAM_ASC_SORT_VALUE; + } + +} diff --git a/server/src/main/java/org/opensearch/rest/action/list/RestListAction.java b/server/src/main/java/org/opensearch/rest/action/list/RestListAction.java new file mode 100644 index 0000000000000..4b8551ea7e14a --- /dev/null +++ b/server/src/main/java/org/opensearch/rest/action/list/RestListAction.java @@ -0,0 +1,58 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.rest.action.list; + +import org.opensearch.client.node.NodeClient; +import org.opensearch.core.rest.RestStatus; +import org.opensearch.rest.BaseRestHandler; +import org.opensearch.rest.BytesRestResponse; +import org.opensearch.rest.RestRequest; + +import java.io.IOException; +import java.util.List; + +import static java.util.Collections.singletonList; +import static org.opensearch.rest.RestRequest.Method.GET; + +/** + * Base _list API endpoint + * + * @opensearch.api + */ +public class RestListAction extends BaseRestHandler { + + private static final String LIST = ":‑|"; + private static final String LIST_NL = LIST + "\n"; + private final String HELP; + + public RestListAction(List listActions) { + StringBuilder sb = new StringBuilder(); + sb.append(LIST_NL); + for (AbstractListAction listAction : listActions) { + listAction.documentation(sb); + } + HELP = sb.toString(); + } + + @Override + public List routes() { + return singletonList(new Route(GET, "/_list")); + } + + @Override + public String getName() { + return "list_action"; + } + + @Override + public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { + return channel -> channel.sendResponse(new BytesRestResponse(RestStatus.OK, HELP)); + } + +} diff --git a/server/src/main/java/org/opensearch/rest/action/list/RestShardsListAction.java b/server/src/main/java/org/opensearch/rest/action/list/RestShardsListAction.java new file mode 100644 index 0000000000000..2a7a0c300d9b5 --- /dev/null +++ b/server/src/main/java/org/opensearch/rest/action/list/RestShardsListAction.java @@ -0,0 +1,73 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.rest.action.list; + +import org.opensearch.rest.RestRequest; +import org.opensearch.rest.action.cat.RestShardsAction; +import org.opensearch.rest.pagination.PageParams; +import org.opensearch.rest.pagination.ShardPaginationStrategy; + +import java.util.List; +import java.util.Objects; + +import static java.util.Arrays.asList; +import static java.util.Collections.unmodifiableList; +import static org.opensearch.rest.RestRequest.Method.GET; + +/** + * _list API action to output shards in pages. + * + * @opensearch.api + */ +public class RestShardsListAction extends RestShardsAction { + + private static final int MAX_SUPPORTED_LIST_SHARDS_PAGE_SIZE_STRING = 20000; + private static final int MIN_SUPPORTED_LIST_SHARDS_PAGE_SIZE_STRING = 2000; + + @Override + public List routes() { + return unmodifiableList(asList(new Route(GET, "/_list/shards"), new Route(GET, "/_list/shards/{index}"))); + } + + @Override + public String getName() { + return "list_shards_action"; + } + + @Override + protected void documentation(StringBuilder sb) { + sb.append("/_list/shards\n"); + sb.append("/_list/shards/{index}\n"); + } + + @Override + public boolean isActionPaginated() { + return true; + } + + @Override + protected PageParams validateAndGetPageParams(RestRequest restRequest) { + PageParams pageParams = super.validateAndGetPageParams(restRequest); + // validate max supported pageSize + if (pageParams.getSize() < MIN_SUPPORTED_LIST_SHARDS_PAGE_SIZE_STRING) { + throw new IllegalArgumentException("size should at least be [" + MIN_SUPPORTED_LIST_SHARDS_PAGE_SIZE_STRING + "]"); + } else if (pageParams.getSize() > MAX_SUPPORTED_LIST_SHARDS_PAGE_SIZE_STRING) { + throw new IllegalArgumentException("size should be less than [" + MAX_SUPPORTED_LIST_SHARDS_PAGE_SIZE_STRING + "]"); + } + // Next Token in the request will be validated by the ShardStrategyToken itself. + if (Objects.nonNull(pageParams.getRequestedToken())) { + ShardPaginationStrategy.ShardStrategyToken.validateShardStrategyToken(pageParams.getRequestedToken()); + } + return pageParams; + } + + protected int defaultPageSize() { + return MIN_SUPPORTED_LIST_SHARDS_PAGE_SIZE_STRING; + } +} diff --git a/server/src/main/java/org/opensearch/rest/action/list/package-info.java b/server/src/main/java/org/opensearch/rest/action/list/package-info.java new file mode 100644 index 0000000000000..8d6563ff9b344 --- /dev/null +++ b/server/src/main/java/org/opensearch/rest/action/list/package-info.java @@ -0,0 +1,12 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** + * {@link org.opensearch.rest.RestHandler}s for actions that list out results in chunks of pages. + */ +package org.opensearch.rest.action.list; diff --git a/server/src/main/java/org/opensearch/rest/pagination/PageParams.java b/server/src/main/java/org/opensearch/rest/pagination/PageParams.java new file mode 100644 index 0000000000000..72522700658f2 --- /dev/null +++ b/server/src/main/java/org/opensearch/rest/pagination/PageParams.java @@ -0,0 +1,65 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.rest.pagination; + +import org.opensearch.common.annotation.PublicApi; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; + +import java.io.IOException; + +/** + * + * Class specific to paginated queries, which will contain common query params required by a paginated API. + */ +@PublicApi(since = "3.0.0") +public class PageParams { + + public static final String PARAM_SORT = "sort"; + public static final String PARAM_NEXT_TOKEN = "next_token"; + public static final String PARAM_SIZE = "size"; + public static final String PARAM_ASC_SORT_VALUE = "asc"; + public static final String PARAM_DESC_SORT_VALUE = "desc"; + + private final String requestedTokenStr; + private final String sort; + private final int size; + + public PageParams(String requestedToken, String sort, int size) { + this.requestedTokenStr = requestedToken; + this.sort = sort; + this.size = size; + } + + public String getSort() { + return sort; + } + + public String getRequestedToken() { + return requestedTokenStr; + } + + public int getSize() { + return size; + } + + public void writePageParams(StreamOutput out) throws IOException { + out.writeString(requestedTokenStr); + out.writeString(sort); + out.writeInt(size); + } + + public static PageParams readPageParams(StreamInput in) throws IOException { + String requestedToken = in.readString(); + String sort = in.readString(); + int size = in.readInt(); + return new PageParams(requestedToken, sort, size); + } + +} diff --git a/server/src/main/java/org/opensearch/rest/pagination/PageToken.java b/server/src/main/java/org/opensearch/rest/pagination/PageToken.java new file mode 100644 index 0000000000000..088992579c205 --- /dev/null +++ b/server/src/main/java/org/opensearch/rest/pagination/PageToken.java @@ -0,0 +1,58 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.rest.pagination; + +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; + +import java.io.IOException; + +/** + * Pagination response metadata for a paginated query. + * @opensearch.internal + */ +public class PageToken { + + public static final String PAGINATED_RESPONSE_NEXT_TOKEN_KEY = "next_token"; + + /** + * String denoting the next_token of paginated response, which will be used to fetch next page (if any). + */ + private final String nextToken; + + /** + * String denoting the element which is being paginated (for e.g. shards, indices..). + */ + private final String paginatedEntity; + + public PageToken(String nextToken, String paginatedElement) { + assert paginatedElement != null : "paginatedElement must be specified for a paginated response"; + this.nextToken = nextToken; + this.paginatedEntity = paginatedElement; + } + + public String getNextToken() { + return nextToken; + } + + public String getPaginatedEntity() { + return paginatedEntity; + } + + public void writePageToken(StreamOutput out) throws IOException { + out.writeString(nextToken); + out.writeString(paginatedEntity); + } + + public static PageToken readPageToken(StreamInput in) throws IOException { + String nextToken = in.readString(); + String paginatedEntity = in.readString(); + return new PageToken(nextToken, paginatedEntity); + } +} diff --git a/server/src/main/java/org/opensearch/rest/pagination/PaginationStrategy.java b/server/src/main/java/org/opensearch/rest/pagination/PaginationStrategy.java new file mode 100644 index 0000000000000..7f9825a7cc09b --- /dev/null +++ b/server/src/main/java/org/opensearch/rest/pagination/PaginationStrategy.java @@ -0,0 +1,75 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.rest.pagination; + +import org.opensearch.OpenSearchParseException; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.metadata.IndexMetadata; + +import java.util.Base64; +import java.util.Comparator; +import java.util.List; +import java.util.Objects; +import java.util.function.Predicate; +import java.util.stream.Collectors; + +import static java.nio.charset.StandardCharsets.UTF_8; + +/** + * Interface to be implemented by any strategy getting used for paginating rest responses. + * + * @opensearch.internal + */ +public interface PaginationStrategy { + + String INCORRECT_TAINTED_NEXT_TOKEN_ERROR_MESSAGE = + "Parameter [next_token] has been tainted and is incorrect. Please provide a valid [next_token]."; + + /** + * + * @return Base64 encoded string, which can be used to fetch next page of response. + */ + PageToken getResponseToken(); + + /** + * + * @return List of elements fetched corresponding to the store and token received by the strategy. + */ + List getRequestedEntities(); + + /** + * + * Utility method to get list of indices filtered as per {@param filterPredicate} and the sorted according to {@param comparator}. + */ + static List getSortedIndexMetadata( + final ClusterState clusterState, + Predicate filterPredicate, + Comparator comparator + ) { + return clusterState.metadata().indices().values().stream().filter(filterPredicate).sorted(comparator).collect(Collectors.toList()); + } + + static String encryptStringToken(String tokenString) { + if (Objects.isNull(tokenString)) { + return null; + } + return Base64.getEncoder().encodeToString(tokenString.getBytes(UTF_8)); + } + + static String decryptStringToken(String encTokenString) { + if (Objects.isNull(encTokenString)) { + return null; + } + try { + return new String(Base64.getDecoder().decode(encTokenString), UTF_8); + } catch (IllegalArgumentException exception) { + throw new OpenSearchParseException(INCORRECT_TAINTED_NEXT_TOKEN_ERROR_MESSAGE); + } + } +} diff --git a/server/src/main/java/org/opensearch/rest/pagination/ShardPaginationStrategy.java b/server/src/main/java/org/opensearch/rest/pagination/ShardPaginationStrategy.java new file mode 100644 index 0000000000000..18a28a56175ff --- /dev/null +++ b/server/src/main/java/org/opensearch/rest/pagination/ShardPaginationStrategy.java @@ -0,0 +1,244 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.rest.pagination; + +import org.opensearch.OpenSearchParseException; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.cluster.routing.IndexRoutingTable; +import org.opensearch.cluster.routing.IndexShardRoutingTable; +import org.opensearch.cluster.routing.ShardRouting; +import org.opensearch.common.collect.Tuple; + +import java.util.ArrayList; +import java.util.Comparator; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.function.Predicate; +import java.util.stream.Collectors; + +import static org.opensearch.rest.pagination.PageParams.PARAM_ASC_SORT_VALUE; + + +/** + * This strategy can be used by the Rest APIs wanting to paginate the responses based on Shards. + * The strategy considers create timestamps of indices and shardID as the keys to iterate over pages. + * + * @opensearch.internal + */ +public class ShardPaginationStrategy implements PaginationStrategy { + + private static final String DEFAULT_SHARDS_PAGINATED_ENTITY = "shards"; + private static final Comparator ASC_COMPARATOR = (metadata1, metadata2) -> { + if (metadata1.getCreationDate() == metadata2.getCreationDate()) { + return metadata1.getIndex().getName().compareTo(metadata2.getIndex().getName()); + } + return Long.compare(metadata1.getCreationDate(), metadata2.getCreationDate()); + }; + private static final Comparator DESC_COMPARATOR = (metadata1, metadata2) -> { + if (metadata1.getCreationDate() == metadata2.getCreationDate()) { + return metadata2.getIndex().getName().compareTo(metadata1.getIndex().getName()); + } + return Long.compare(metadata2.getCreationDate(), metadata1.getCreationDate()); + }; + + private PageToken pageToken; + private List requestedShardRoutings = new ArrayList<>(); + private List requestedIndices = new ArrayList<>(); + + public ShardPaginationStrategy(PageParams pageParams, ClusterState clusterState) { + // Get list of indices metadata sorted by their creation time and filtered by the last sent index + List sortedIndices = PaginationStrategy.getSortedIndexMetadata( + clusterState, + getMetadataFilter(pageParams.getRequestedToken(), pageParams.getSort()), + PARAM_ASC_SORT_VALUE.equals(pageParams.getSort()) ? ASC_COMPARATOR : DESC_COMPARATOR + ); + // Get the list of shards and indices belonging to current page. + Tuple, List> tuple = getPageData( + clusterState.getRoutingTable().getIndicesRouting(), + sortedIndices, + pageParams.getSize(), + pageParams.getRequestedToken() + ); + this.requestedShardRoutings = tuple.v1(); + List metadataSublist = tuple.v2(); + // Get list of index names from the trimmed metadataSublist + this.requestedIndices = metadataSublist.stream().map(metadata -> metadata.getIndex().getName()).collect(Collectors.toList()); + this.pageToken = getResponseToken( + pageParams.getSize(), + sortedIndices.size(), + metadataSublist.isEmpty() ? null : metadataSublist.get(metadataSublist.size() - 1), + tuple.v1().isEmpty() ? null : tuple.v1().get(tuple.v1().size() - 1) + ); + } + + private static Predicate getMetadataFilter(String requestedTokenStr, String sortOrder) { + boolean isAscendingSort = sortOrder.equals(PARAM_ASC_SORT_VALUE); + ShardStrategyToken requestedToken = Objects.isNull(requestedTokenStr) || requestedTokenStr.isEmpty() + ? null + : new ShardStrategyToken(requestedTokenStr); + if (Objects.isNull(requestedToken)) { + return indexMetadata -> true; + } + return metadata -> { + if (metadata.getIndex().getName().equals(requestedToken.lastIndexName)) { + return true; + } else if (metadata.getCreationDate() == requestedToken.lastIndexCreationTime) { + return isAscendingSort + ? metadata.getIndex().getName().compareTo(requestedToken.lastIndexName) > 0 + : metadata.getIndex().getName().compareTo(requestedToken.lastIndexName) < 0; + } + return isAscendingSort + ? metadata.getCreationDate() > requestedToken.lastIndexCreationTime + : metadata.getCreationDate() < requestedToken.lastIndexCreationTime; + }; + } + + private Tuple, List> getPageData( + Map indicesRouting, + List sortedIndices, + final int pageSize, + String requestedTokenStr + ) { + List shardRoutings = new ArrayList<>(); + List indexMetadataList = new ArrayList<>(); + ShardStrategyToken requestedToken = Objects.isNull(requestedTokenStr) || requestedTokenStr.isEmpty() + ? null + : new ShardStrategyToken(requestedTokenStr); + int shardCount = 0; + for (IndexMetadata indexMetadata : sortedIndices) { + boolean indexShardsAdded = false; + Map indexShardRoutingTable = indicesRouting.get(indexMetadata.getIndex().getName()) + .getShards(); + int shardId = Objects.isNull(requestedToken) ? 0 + : indexMetadata.getIndex().getName().equals(requestedToken.lastIndexName) ? requestedToken.lastShardId + 1 + : 0; + for (; shardId < indexShardRoutingTable.size(); shardId++) { + shardCount += indexShardRoutingTable.get(shardId).size(); + if (shardCount > pageSize) { + break; + } + shardRoutings.addAll(indexShardRoutingTable.get(shardId).shards()); + indexShardsAdded = true; + } + // Add index to the list if any of its shard was added to the count. + if (indexShardsAdded) { + indexMetadataList.add(indexMetadata); + } + if (shardCount >= pageSize) { + break; + } + } + + return new Tuple<>(shardRoutings, indexMetadataList); + } + + private PageToken getResponseToken(final int pageSize, final int totalIndices, IndexMetadata lastIndex, ShardRouting lastShard) { + if (totalIndices <= pageSize && lastIndex.getNumberOfShards() == lastShard.getId()) { + return new PageToken(null, DEFAULT_SHARDS_PAGINATED_ENTITY); + } + return new PageToken( + new ShardStrategyToken(lastShard.getId(), lastIndex.getCreationDate(), lastIndex.getIndex().getName()).generateEncryptedToken(), + DEFAULT_SHARDS_PAGINATED_ENTITY + ); + } + + @Override + public PageToken getResponseToken() { + return pageToken; + } + + @Override + public List getRequestedEntities() { + return requestedShardRoutings; + } + + public List getRequestedIndices() { + return requestedIndices; + } + + /** + * TokenParser to be used by {@link ShardPaginationStrategy}. + * TToken would look like: LastShardIdOfPage + | + CreationTimeOfLastRespondedIndex + | + NameOfLastRespondedIndex + */ + public static class ShardStrategyToken { + + private static final String JOIN_DELIMITER = "|"; + private static final String SPLIT_REGEX = "\\|"; + private static final int SHARD_ID_POS_IN_TOKEN = 0; + private static final int CREATE_TIME_POS_IN_TOKEN = 1; + private static final int INDEX_NAME_POS_IN_TOKEN = 2; + + /** + * Denotes the shardId of the last shard in the response. + * Will be used to identify the next shard to start the page from, in case the shards of an index + * get split across pages. + */ + private final int lastShardId; + /** + * Represents creation times of last index which was displayed in the page. + * Used to identify the new start point in case the indices get created/deleted while queries are executed. + */ + private final long lastIndexCreationTime; + + /** + * Represents name of the last index which was displayed in the page. + * Used to identify whether the sorted list of indices has changed or not. + */ + private final String lastIndexName; + + public ShardStrategyToken(String requestedTokenString) { + validateShardStrategyToken(requestedTokenString); + String decryptedToken = PaginationStrategy.decryptStringToken(requestedTokenString); + final String[] decryptedTokenElements = decryptedToken.split(SPLIT_REGEX); + this.lastShardId = Integer.parseInt(decryptedTokenElements[SHARD_ID_POS_IN_TOKEN]); + this.lastIndexCreationTime = Long.parseLong(decryptedTokenElements[CREATE_TIME_POS_IN_TOKEN]); + this.lastIndexName = decryptedTokenElements[INDEX_NAME_POS_IN_TOKEN]; + } + + public ShardStrategyToken(int lastShardId, long creationTimeOfLastRespondedIndex, String nameOfLastRespondedIndex) { + this.lastShardId = lastShardId; + Objects.requireNonNull(nameOfLastRespondedIndex, "index name should be provided"); + this.lastIndexCreationTime = creationTimeOfLastRespondedIndex; + this.lastIndexName = nameOfLastRespondedIndex; + } + + public String generateEncryptedToken() { + return PaginationStrategy.encryptStringToken( + String.join(JOIN_DELIMITER, String.valueOf(lastShardId), String.valueOf(lastIndexCreationTime), lastIndexName) + ); + } + + /** + * Will perform simple validations on token received in the request. + * Token should be base64 encoded, and should contain the expected number of elements separated by "|". + * Timestamps should also be a valid long. + * + * @param requestedTokenStr string denoting the encoded token requested by the user. + */ + public static void validateShardStrategyToken(String requestedTokenStr) { + Objects.requireNonNull(requestedTokenStr, "requestedTokenString can not be null"); + String decryptedToken = PaginationStrategy.decryptStringToken(requestedTokenStr); + final String[] decryptedTokenElements = decryptedToken.split(SPLIT_REGEX); + if (decryptedTokenElements.length != 3) { + throw new OpenSearchParseException(INCORRECT_TAINTED_NEXT_TOKEN_ERROR_MESSAGE); + } + try { + int shardId = Integer.parseInt(decryptedTokenElements[SHARD_ID_POS_IN_TOKEN]); + long creationTimeOfLastRespondedIndex = Long.parseLong(decryptedTokenElements[CREATE_TIME_POS_IN_TOKEN]); + if (shardId < 0 || creationTimeOfLastRespondedIndex <= 0) { + throw new OpenSearchParseException(INCORRECT_TAINTED_NEXT_TOKEN_ERROR_MESSAGE); + } + } catch (NumberFormatException exception) { + throw new OpenSearchParseException(INCORRECT_TAINTED_NEXT_TOKEN_ERROR_MESSAGE); + } + } + } +} diff --git a/server/src/main/java/org/opensearch/rest/pagination/package-info.java b/server/src/main/java/org/opensearch/rest/pagination/package-info.java new file mode 100644 index 0000000000000..324b8a6c46f88 --- /dev/null +++ b/server/src/main/java/org/opensearch/rest/pagination/package-info.java @@ -0,0 +1,12 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** + * Exposes utilities for Rest actions to paginate responses. + */ +package org.opensearch.rest.pagination; diff --git a/server/src/test/java/org/opensearch/rest/action/cat/RestShardsActionTests.java b/server/src/test/java/org/opensearch/rest/action/cat/RestShardsActionTests.java index 883df7da5d717..df32e3a3bce00 100644 --- a/server/src/test/java/org/opensearch/rest/action/cat/RestShardsActionTests.java +++ b/server/src/test/java/org/opensearch/rest/action/cat/RestShardsActionTests.java @@ -112,7 +112,7 @@ public void testBuildTable() { when(state.getState()).thenReturn(clusterState); final RestShardsAction action = new RestShardsAction(); - final Table table = action.buildTable(new FakeRestRequest(), state, stats); + final Table table = action.buildTable(new FakeRestRequest(), state, stats, state.getState().routingTable().allShards(), null); // now, verify the table is correct List headers = table.getHeaders(); diff --git a/server/src/test/java/org/opensearch/rest/action/cat/RestTableTests.java b/server/src/test/java/org/opensearch/rest/action/cat/RestTableTests.java index 8183cb1d3b910..a82e563d70273 100644 --- a/server/src/test/java/org/opensearch/rest/action/cat/RestTableTests.java +++ b/server/src/test/java/org/opensearch/rest/action/cat/RestTableTests.java @@ -37,6 +37,7 @@ import org.opensearch.core.xcontent.MediaTypeRegistry; import org.opensearch.rest.AbstractRestChannel; import org.opensearch.rest.RestResponse; +import org.opensearch.rest.pagination.PageToken; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.rest.FakeRestRequest; import org.junit.Before; @@ -64,9 +65,14 @@ public class RestTableTests extends OpenSearchTestCase { private static final String ACCEPT = "Accept"; private static final String TEXT_PLAIN = "text/plain; charset=UTF-8"; private static final String TEXT_TABLE_BODY = "foo foo foo foo foo foo foo foo\n"; + private static final String PAGINATED_TEXT_TABLE_BODY = "foo foo foo foo foo foo foo foo\nnext_token foo\n"; private static final String JSON_TABLE_BODY = "[{\"bulk.foo\":\"foo\",\"bulk.bar\":\"foo\",\"aliasedBulk\":\"foo\"," + "\"aliasedSecondBulk\":\"foo\",\"unmatched\":\"foo\"," + "\"invalidAliasesBulk\":\"foo\",\"timestamp\":\"foo\",\"epoch\":\"foo\"}]"; + private static final String PAGINATED_JSON_TABLE_BODY = + "{\"next_token\":\"foo\",\"entities\":[{\"bulk.foo\":\"foo\",\"bulk.bar\":\"foo\",\"aliasedBulk\":\"foo\"," + + "\"aliasedSecondBulk\":\"foo\",\"unmatched\":\"foo\"," + + "\"invalidAliasesBulk\":\"foo\",\"timestamp\":\"foo\",\"epoch\":\"foo\"}]}"; private static final String YAML_TABLE_BODY = "---\n" + "- bulk.foo: \"foo\"\n" + " bulk.bar: \"foo\"\n" @@ -76,6 +82,17 @@ public class RestTableTests extends OpenSearchTestCase { + " invalidAliasesBulk: \"foo\"\n" + " timestamp: \"foo\"\n" + " epoch: \"foo\"\n"; + private static final String PAGINATED_YAML_TABLE_BODY = "---\n" + + "next_token: \"foo\"\n" + + "entities:\n" + + "- bulk.foo: \"foo\"\n" + + " bulk.bar: \"foo\"\n" + + " aliasedBulk: \"foo\"\n" + + " aliasedSecondBulk: \"foo\"\n" + + " unmatched: \"foo\"\n" + + " invalidAliasesBulk: \"foo\"\n" + + " timestamp: \"foo\"\n" + + " epoch: \"foo\"\n"; private Table table; private FakeRestRequest restRequest; @@ -83,20 +100,7 @@ public class RestTableTests extends OpenSearchTestCase { public void setup() { restRequest = new FakeRestRequest(); table = new Table(); - table.startHeaders(); - table.addCell("bulk.foo", "alias:f;desc:foo"); - table.addCell("bulk.bar", "alias:b;desc:bar"); - // should be matched as well due to the aliases - table.addCell("aliasedBulk", "alias:bulkWhatever;desc:bar"); - table.addCell("aliasedSecondBulk", "alias:foobar,bulkolicious,bulkotastic;desc:bar"); - // no match - table.addCell("unmatched", "alias:un.matched;desc:bar"); - // invalid alias - table.addCell("invalidAliasesBulk", "alias:,,,;desc:bar"); - // timestamp - table.addCell("timestamp", "alias:ts"); - table.addCell("epoch", "alias:t"); - table.endHeaders(); + addHeaders(table); } public void testThatDisplayHeadersSupportWildcards() throws Exception { @@ -121,10 +125,28 @@ public void testThatWeUseTheAcceptHeaderJson() throws Exception { assertResponse(Collections.singletonMap(ACCEPT, Collections.singletonList(APPLICATION_JSON)), APPLICATION_JSON, JSON_TABLE_BODY); } + public void testThatWeUseTheAcceptHeaderJsonForPaginatedTable() throws Exception { + assertResponse( + Collections.singletonMap(ACCEPT, Collections.singletonList(APPLICATION_JSON)), + APPLICATION_JSON, + PAGINATED_JSON_TABLE_BODY, + getPaginatedTable() + ); + } + public void testThatWeUseTheAcceptHeaderYaml() throws Exception { assertResponse(Collections.singletonMap(ACCEPT, Collections.singletonList(APPLICATION_YAML)), APPLICATION_YAML, YAML_TABLE_BODY); } + public void testThatWeUseTheAcceptHeaderYamlForPaginatedTable() throws Exception { + assertResponse( + Collections.singletonMap(ACCEPT, Collections.singletonList(APPLICATION_YAML)), + APPLICATION_YAML, + PAGINATED_YAML_TABLE_BODY, + getPaginatedTable() + ); + } + public void testThatWeUseTheAcceptHeaderSmile() throws Exception { assertResponseContentType(Collections.singletonMap(ACCEPT, Collections.singletonList(APPLICATION_SMILE)), APPLICATION_SMILE); } @@ -137,6 +159,15 @@ public void testThatWeUseTheAcceptHeaderText() throws Exception { assertResponse(Collections.singletonMap(ACCEPT, Collections.singletonList(TEXT_PLAIN)), TEXT_PLAIN, TEXT_TABLE_BODY); } + public void testThatWeUseTheAcceptHeaderTextForPaginatedTable() throws Exception { + assertResponse( + Collections.singletonMap(ACCEPT, Collections.singletonList(TEXT_PLAIN)), + TEXT_PLAIN, + PAGINATED_TEXT_TABLE_BODY, + getPaginatedTable() + ); + } + public void testIgnoreContentType() throws Exception { assertResponse(Collections.singletonMap(CONTENT_TYPE, Collections.singletonList(APPLICATION_JSON)), TEXT_PLAIN, TEXT_TABLE_BODY); } @@ -261,6 +292,10 @@ public void testMultiSort() { } private RestResponse assertResponseContentType(Map> headers, String mediaType) throws Exception { + return assertResponseContentType(headers, mediaType, table); + } + + private RestResponse assertResponseContentType(Map> headers, String mediaType, Table table) throws Exception { FakeRestRequest requestWithAcceptHeader = new FakeRestRequest.Builder(xContentRegistry()).withHeaders(headers).build(); table.startRow(); table.addCell("foo"); @@ -282,7 +317,11 @@ public void sendResponse(RestResponse response) {} } private void assertResponse(Map> headers, String mediaType, String body) throws Exception { - RestResponse response = assertResponseContentType(headers, mediaType); + assertResponse(headers, mediaType, body, table); + } + + private void assertResponse(Map> headers, String mediaType, String body, Table table) throws Exception { + RestResponse response = assertResponseContentType(headers, mediaType, table); assertThat(response.content().utf8ToString(), equalTo(body)); } @@ -294,4 +333,28 @@ private List getHeaderNames(List headers) { return headerNames; } + + private Table getPaginatedTable() { + PageToken pageToken = new PageToken("foo", "entities"); + Table paginatedTable = new Table(pageToken); + addHeaders(paginatedTable); + return paginatedTable; + } + + private void addHeaders(Table table) { + table.startHeaders(); + table.addCell("bulk.foo", "alias:f;desc:foo"); + table.addCell("bulk.bar", "alias:b;desc:bar"); + // should be matched as well due to the aliases + table.addCell("aliasedBulk", "alias:bulkWhatever;desc:bar"); + table.addCell("aliasedSecondBulk", "alias:foobar,bulkolicious,bulkotastic;desc:bar"); + // no match + table.addCell("unmatched", "alias:un.matched;desc:bar"); + // invalid alias + table.addCell("invalidAliasesBulk", "alias:,,,;desc:bar"); + // timestamp + table.addCell("timestamp", "alias:ts"); + table.addCell("epoch", "alias:t"); + table.endHeaders(); + } } From 6c855ce7f9c2c0cc65e47e501dc96974b635942e Mon Sep 17 00:00:00 2001 From: Harsh Garg Date: Fri, 27 Sep 2024 17:01:43 +0530 Subject: [PATCH 02/11] Refactoring and bug fixes for ShardPaginationStrategy Signed-off-by: Harsh Garg --- .../cluster/shards/CatShardsRequest.java | 9 +- .../cluster/shards/CatShardsResponse.java | 9 +- .../shards/TransportCatShardsAction.java | 37 +++--- .../rest/action/cat/RestShardsAction.java | 1 + .../pagination/ShardPaginationStrategy.java | 124 ++++++++++-------- 5 files changed, 106 insertions(+), 74 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsRequest.java index 4ee3aa1c9a175..3524ca38f84f4 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsRequest.java @@ -39,7 +39,9 @@ public CatShardsRequest(StreamInput in) throws IOException { if (in.getVersion().onOrAfter(Version.V_3_0_0)) { indices = in.readStringArray(); cancelAfterTimeInterval = in.readTimeValue(); - pageParams = PageParams.readPageParams(in); + if (in.readBoolean()) { + pageParams = PageParams.readPageParams(in); + } } } @@ -49,7 +51,10 @@ public void writeTo(StreamOutput out) throws IOException { if (out.getVersion().onOrAfter(Version.V_3_0_0)) { out.writeStringArray(indices); out.writeTimeValue(cancelAfterTimeInterval); - pageParams.writePageParams(out); + out.writeBoolean(pageParams != null); + if (pageParams != null) { + pageParams.writePageParams(out); + } } } diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsResponse.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsResponse.java index 21007c4e6ce96..80fec51fd25d6 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsResponse.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsResponse.java @@ -42,7 +42,9 @@ public CatShardsResponse(StreamInput in) throws IOException { indicesStatsResponse = new IndicesStatsResponse(in); if (in.getVersion().onOrAfter(Version.V_3_0_0)) { responseShards = in.readList(ShardRouting::new); - pageToken = PageToken.readPageToken(in); + if (in.readBoolean()) { + pageToken = PageToken.readPageToken(in); + } } } @@ -52,7 +54,10 @@ public void writeTo(StreamOutput out) throws IOException { indicesStatsResponse.writeTo(out); if (out.getVersion().onOrAfter(Version.V_3_0_0)) { out.writeList(responseShards); - pageToken.writePageToken(out); + out.writeBoolean(pageToken != null); + if (pageToken != null) { + pageToken.writePageToken(out); + } } } diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java index 41a4e4936c471..5a5e178eb6a82 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java @@ -48,7 +48,7 @@ public void doExecute(Task parentTask, CatShardsRequest shardsRequest, ActionLis clusterStateRequest.setShouldCancelOnTimeout(true); clusterStateRequest.local(shardsRequest.local()); clusterStateRequest.clusterManagerNodeTimeout(shardsRequest.clusterManagerNodeTimeout()); - if (Objects.nonNull(shardsRequest.getPageParams())) { + if (Objects.isNull(shardsRequest.getPageParams())) { clusterStateRequest.clear().nodes(true).routingTable(true).indices(shardsRequest.getIndices()); } else { clusterStateRequest.clear().nodes(true).routingTable(true).indices(shardsRequest.getIndices()).metadata(true); @@ -81,23 +81,26 @@ protected void innerOnFailure(Exception e) { client.admin().cluster().state(clusterStateRequest, new ActionListener() { @Override public void onResponse(ClusterStateResponse clusterStateResponse) { - ShardPaginationStrategy paginationStrategy = getPaginationStrategy(shardsRequest.getPageParams(), clusterStateResponse); - String[] indices = Objects.isNull(paginationStrategy) - ? shardsRequest.getIndices() - : paginationStrategy.getRequestedIndices().toArray(new String[0]); - catShardsResponse.setClusterStateResponse(clusterStateResponse); - catShardsResponse.setResponseShards( - Objects.isNull(paginationStrategy) - ? clusterStateResponse.getState().routingTable().allShards() - : paginationStrategy.getRequestedEntities() - ); - catShardsResponse.setPageToken(Objects.isNull(paginationStrategy) ? null : paginationStrategy.getResponseToken()); - IndicesStatsRequest indicesStatsRequest = new IndicesStatsRequest(); - indicesStatsRequest.setShouldCancelOnTimeout(true); - indicesStatsRequest.all(); - indicesStatsRequest.indices(indices); - indicesStatsRequest.setParentTask(client.getLocalNodeId(), parentTask.getId()); try { + ShardPaginationStrategy paginationStrategy = getPaginationStrategy( + shardsRequest.getPageParams(), + clusterStateResponse + ); + String[] indices = Objects.isNull(paginationStrategy) + ? shardsRequest.getIndices() + : paginationStrategy.getRequestedIndices().toArray(new String[0]); + catShardsResponse.setClusterStateResponse(clusterStateResponse); + catShardsResponse.setResponseShards( + Objects.isNull(paginationStrategy) + ? clusterStateResponse.getState().routingTable().allShards() + : paginationStrategy.getRequestedEntities() + ); + catShardsResponse.setPageToken(Objects.isNull(paginationStrategy) ? null : paginationStrategy.getResponseToken()); + IndicesStatsRequest indicesStatsRequest = new IndicesStatsRequest(); + indicesStatsRequest.setShouldCancelOnTimeout(true); + indicesStatsRequest.all(); + indicesStatsRequest.indices(indices); + indicesStatsRequest.setParentTask(client.getLocalNodeId(), parentTask.getId()); client.admin().indices().stats(indicesStatsRequest, new ActionListener() { @Override public void onResponse(IndicesStatsResponse indicesStatsResponse) { diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestShardsAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestShardsAction.java index 7d44d70b285f6..ccb5bedf03fad 100644 --- a/server/src/main/java/org/opensearch/rest/action/cat/RestShardsAction.java +++ b/server/src/main/java/org/opensearch/rest/action/cat/RestShardsAction.java @@ -115,6 +115,7 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli shardsRequest.clusterManagerNodeTimeout(request.paramAsTime("cluster_manager_timeout", shardsRequest.clusterManagerNodeTimeout())); shardsRequest.setCancelAfterTimeInterval(request.paramAsTime("cancel_after_time_interval", NO_TIMEOUT)); shardsRequest.setIndices(indices); + shardsRequest.setPageParams(pageParams); parseDeprecatedMasterTimeoutParameter(shardsRequest, request, deprecationLogger, getName()); return channel -> client.execute(CatShardsAction.INSTANCE, shardsRequest, new RestResponseListener(channel) { @Override diff --git a/server/src/main/java/org/opensearch/rest/pagination/ShardPaginationStrategy.java b/server/src/main/java/org/opensearch/rest/pagination/ShardPaginationStrategy.java index 18a28a56175ff..b680012484b83 100644 --- a/server/src/main/java/org/opensearch/rest/pagination/ShardPaginationStrategy.java +++ b/server/src/main/java/org/opensearch/rest/pagination/ShardPaginationStrategy.java @@ -26,7 +26,6 @@ import static org.opensearch.rest.pagination.PageParams.PARAM_ASC_SORT_VALUE; - /** * This strategy can be used by the Rest APIs wanting to paginate the responses based on Shards. * The strategy considers create timestamps of indices and shardID as the keys to iterate over pages. @@ -50,89 +49,92 @@ public class ShardPaginationStrategy implements PaginationStrategy }; private PageToken pageToken; - private List requestedShardRoutings = new ArrayList<>(); - private List requestedIndices = new ArrayList<>(); + private List pageShardRoutings = new ArrayList<>(); + private List pageIndices = new ArrayList<>(); public ShardPaginationStrategy(PageParams pageParams, ClusterState clusterState) { + ShardStrategyToken shardStrategyToken = getShardStrategyToken(pageParams.getRequestedToken()); // Get list of indices metadata sorted by their creation time and filtered by the last sent index - List sortedIndices = PaginationStrategy.getSortedIndexMetadata( + List filteredIndices = PaginationStrategy.getSortedIndexMetadata( clusterState, - getMetadataFilter(pageParams.getRequestedToken(), pageParams.getSort()), + getIndexFilter(shardStrategyToken, pageParams.getSort()), PARAM_ASC_SORT_VALUE.equals(pageParams.getSort()) ? ASC_COMPARATOR : DESC_COMPARATOR ); // Get the list of shards and indices belonging to current page. Tuple, List> tuple = getPageData( clusterState.getRoutingTable().getIndicesRouting(), - sortedIndices, - pageParams.getSize(), - pageParams.getRequestedToken() + filteredIndices, + shardStrategyToken, + pageParams.getSize() ); - this.requestedShardRoutings = tuple.v1(); - List metadataSublist = tuple.v2(); + List pageShardRoutings = tuple.v1(); + List pageIndices = tuple.v2(); + this.pageShardRoutings = pageShardRoutings; // Get list of index names from the trimmed metadataSublist - this.requestedIndices = metadataSublist.stream().map(metadata -> metadata.getIndex().getName()).collect(Collectors.toList()); + this.pageIndices = pageIndices.stream().map(metadata -> metadata.getIndex().getName()).collect(Collectors.toList()); this.pageToken = getResponseToken( - pageParams.getSize(), - sortedIndices.size(), - metadataSublist.isEmpty() ? null : metadataSublist.get(metadataSublist.size() - 1), - tuple.v1().isEmpty() ? null : tuple.v1().get(tuple.v1().size() - 1) + pageIndices.isEmpty() ? null : pageIndices.get(pageIndices.size() - 1), + filteredIndices.isEmpty() ? null : filteredIndices.get(filteredIndices.size() - 1).getIndex().getName(), + pageShardRoutings.isEmpty() ? -1 : pageShardRoutings.get(pageShardRoutings.size() - 1).id() ); } - private static Predicate getMetadataFilter(String requestedTokenStr, String sortOrder) { - boolean isAscendingSort = sortOrder.equals(PARAM_ASC_SORT_VALUE); - ShardStrategyToken requestedToken = Objects.isNull(requestedTokenStr) || requestedTokenStr.isEmpty() - ? null - : new ShardStrategyToken(requestedTokenStr); - if (Objects.isNull(requestedToken)) { + private static Predicate getIndexFilter(ShardStrategyToken token, String sortOrder) { + if (Objects.isNull(token)) { return indexMetadata -> true; } + boolean isAscendingSort = sortOrder.equals(PARAM_ASC_SORT_VALUE); return metadata -> { - if (metadata.getIndex().getName().equals(requestedToken.lastIndexName)) { + if (metadata.getIndex().getName().equals(token.lastIndexName)) { return true; - } else if (metadata.getCreationDate() == requestedToken.lastIndexCreationTime) { + } else if (metadata.getCreationDate() == token.lastIndexCreationTime) { return isAscendingSort - ? metadata.getIndex().getName().compareTo(requestedToken.lastIndexName) > 0 - : metadata.getIndex().getName().compareTo(requestedToken.lastIndexName) < 0; + ? metadata.getIndex().getName().compareTo(token.lastIndexName) > 0 + : metadata.getIndex().getName().compareTo(token.lastIndexName) < 0; } return isAscendingSort - ? metadata.getCreationDate() > requestedToken.lastIndexCreationTime - : metadata.getCreationDate() < requestedToken.lastIndexCreationTime; + ? metadata.getCreationDate() > token.lastIndexCreationTime + : metadata.getCreationDate() < token.lastIndexCreationTime; }; } + /** + * Will be used to get the list of shards and respective indices to which they belong, + * which are to be displayed in a page. + * Note: All shards for a shardID will always be present in the same page. + */ private Tuple, List> getPageData( Map indicesRouting, - List sortedIndices, - final int pageSize, - String requestedTokenStr + List filteredIndices, + final ShardStrategyToken token, + final int numShardsRequired ) { List shardRoutings = new ArrayList<>(); List indexMetadataList = new ArrayList<>(); - ShardStrategyToken requestedToken = Objects.isNull(requestedTokenStr) || requestedTokenStr.isEmpty() - ? null - : new ShardStrategyToken(requestedTokenStr); int shardCount = 0; - for (IndexMetadata indexMetadata : sortedIndices) { + + // iterate over indices until shardCount is less than numShardsRequired + for (IndexMetadata indexMetadata : filteredIndices) { + String indexName = indexMetadata.getIndex().getName(); + int startShardId = getStartShardIdForPageIndex(token, indexName); boolean indexShardsAdded = false; - Map indexShardRoutingTable = indicesRouting.get(indexMetadata.getIndex().getName()) - .getShards(); - int shardId = Objects.isNull(requestedToken) ? 0 - : indexMetadata.getIndex().getName().equals(requestedToken.lastIndexName) ? requestedToken.lastShardId + 1 - : 0; - for (; shardId < indexShardRoutingTable.size(); shardId++) { - shardCount += indexShardRoutingTable.get(shardId).size(); - if (shardCount > pageSize) { + Map indexShardRoutingTable = indicesRouting.get(indexName).getShards(); + for (; startShardId < indexShardRoutingTable.size(); startShardId++) { + if (indexShardRoutingTable.get(startShardId).size() > numShardsRequired) { + throw new IllegalArgumentException("size value should be greater than the replica count of all indices"); + } + shardCount += indexShardRoutingTable.get(startShardId).size(); + if (shardCount > numShardsRequired) { break; } - shardRoutings.addAll(indexShardRoutingTable.get(shardId).shards()); + shardRoutings.addAll(indexShardRoutingTable.get(startShardId).shards()); indexShardsAdded = true; } // Add index to the list if any of its shard was added to the count. if (indexShardsAdded) { indexMetadataList.add(indexMetadata); } - if (shardCount >= pageSize) { + if (shardCount >= numShardsRequired) { break; } } @@ -140,16 +142,32 @@ private Tuple, List> getPageData( return new Tuple<>(shardRoutings, indexMetadataList); } - private PageToken getResponseToken(final int pageSize, final int totalIndices, IndexMetadata lastIndex, ShardRouting lastShard) { - if (totalIndices <= pageSize && lastIndex.getNumberOfShards() == lastShard.getId()) { + private PageToken getResponseToken(IndexMetadata pageEndIndex, final String lastFilteredIndexName, final int pageEndShardId) { + // If all the shards of filtered indices list have been included in pageShardRoutings, then no more + // shards are remaining to be fetched, and the next_token should thus be null. + String pageEndIndexName = Objects.isNull(pageEndIndex) ? null : pageEndIndex.getIndex().getName(); + if (Objects.isNull(pageEndIndexName) + || (pageEndIndexName.equals(lastFilteredIndexName) && pageEndIndex.getNumberOfShards() == pageEndShardId + 1)) { return new PageToken(null, DEFAULT_SHARDS_PAGINATED_ENTITY); } return new PageToken( - new ShardStrategyToken(lastShard.getId(), lastIndex.getCreationDate(), lastIndex.getIndex().getName()).generateEncryptedToken(), + new ShardStrategyToken(pageEndIndexName, pageEndShardId, pageEndIndex.getCreationDate()).generateEncryptedToken(), DEFAULT_SHARDS_PAGINATED_ENTITY ); } + private ShardStrategyToken getShardStrategyToken(String requestedToken) { + return Objects.isNull(requestedToken) || requestedToken.isEmpty() ? null : new ShardStrategyToken(requestedToken); + } + + /** + * Provides the shardId to start an index which is to be included in the page. If the index is same as + * lastIndex, start from the shardId next to lastShardId, else always start from 0. + */ + private int getStartShardIdForPageIndex(ShardStrategyToken token, final String pageIndexName) { + return Objects.isNull(token) ? 0 : token.lastIndexName.equals(pageIndexName) ? token.lastShardId + 1 : 0; + } + @Override public PageToken getResponseToken() { return pageToken; @@ -157,11 +175,11 @@ public PageToken getResponseToken() { @Override public List getRequestedEntities() { - return requestedShardRoutings; + return pageShardRoutings; } public List getRequestedIndices() { - return requestedIndices; + return pageIndices; } /** @@ -203,11 +221,11 @@ public ShardStrategyToken(String requestedTokenString) { this.lastIndexName = decryptedTokenElements[INDEX_NAME_POS_IN_TOKEN]; } - public ShardStrategyToken(int lastShardId, long creationTimeOfLastRespondedIndex, String nameOfLastRespondedIndex) { + public ShardStrategyToken(String lastIndexName, int lastShardId, long lastIndexCreationTime) { + Objects.requireNonNull(lastIndexName, "index name should be provided"); + this.lastIndexName = lastIndexName; this.lastShardId = lastShardId; - Objects.requireNonNull(nameOfLastRespondedIndex, "index name should be provided"); - this.lastIndexCreationTime = creationTimeOfLastRespondedIndex; - this.lastIndexName = nameOfLastRespondedIndex; + this.lastIndexCreationTime = lastIndexCreationTime; } public String generateEncryptedToken() { From cbc4c06abf0de5c5186d08dd5d69d41c91264ec8 Mon Sep 17 00:00:00 2001 From: Harsh Garg Date: Mon, 30 Sep 2024 05:37:53 +0530 Subject: [PATCH 03/11] Adding UTs for shard based pagination Signed-off-by: Harsh Garg --- .../action/list/RestShardsListAction.java | 14 +- .../action/cat/RestShardsActionTests.java | 40 +- .../list/RestShardsListActionTests.java | 63 +++ .../ShardPaginationStrategyTests.java | 400 ++++++++++++++++++ .../opensearch/test/rest/FakeRestRequest.java | 9 + 5 files changed, 512 insertions(+), 14 deletions(-) create mode 100644 server/src/test/java/org/opensearch/rest/action/list/RestShardsListActionTests.java create mode 100644 server/src/test/java/org/opensearch/rest/pagination/ShardPaginationStrategyTests.java diff --git a/server/src/main/java/org/opensearch/rest/action/list/RestShardsListAction.java b/server/src/main/java/org/opensearch/rest/action/list/RestShardsListAction.java index 2a7a0c300d9b5..9f14eaebc4309 100644 --- a/server/src/main/java/org/opensearch/rest/action/list/RestShardsListAction.java +++ b/server/src/main/java/org/opensearch/rest/action/list/RestShardsListAction.java @@ -27,8 +27,8 @@ */ public class RestShardsListAction extends RestShardsAction { - private static final int MAX_SUPPORTED_LIST_SHARDS_PAGE_SIZE_STRING = 20000; - private static final int MIN_SUPPORTED_LIST_SHARDS_PAGE_SIZE_STRING = 2000; + protected static final int MAX_SUPPORTED_LIST_SHARDS_PAGE_SIZE = 20000; + protected static final int MIN_SUPPORTED_LIST_SHARDS_PAGE_SIZE = 2000; @Override public List routes() { @@ -55,10 +55,10 @@ public boolean isActionPaginated() { protected PageParams validateAndGetPageParams(RestRequest restRequest) { PageParams pageParams = super.validateAndGetPageParams(restRequest); // validate max supported pageSize - if (pageParams.getSize() < MIN_SUPPORTED_LIST_SHARDS_PAGE_SIZE_STRING) { - throw new IllegalArgumentException("size should at least be [" + MIN_SUPPORTED_LIST_SHARDS_PAGE_SIZE_STRING + "]"); - } else if (pageParams.getSize() > MAX_SUPPORTED_LIST_SHARDS_PAGE_SIZE_STRING) { - throw new IllegalArgumentException("size should be less than [" + MAX_SUPPORTED_LIST_SHARDS_PAGE_SIZE_STRING + "]"); + if (pageParams.getSize() < MIN_SUPPORTED_LIST_SHARDS_PAGE_SIZE) { + throw new IllegalArgumentException("size should at least be [" + MIN_SUPPORTED_LIST_SHARDS_PAGE_SIZE + "]"); + } else if (pageParams.getSize() > MAX_SUPPORTED_LIST_SHARDS_PAGE_SIZE) { + throw new IllegalArgumentException("size should be less than [" + MAX_SUPPORTED_LIST_SHARDS_PAGE_SIZE + "]"); } // Next Token in the request will be validated by the ShardStrategyToken itself. if (Objects.nonNull(pageParams.getRequestedToken())) { @@ -68,6 +68,6 @@ protected PageParams validateAndGetPageParams(RestRequest restRequest) { } protected int defaultPageSize() { - return MIN_SUPPORTED_LIST_SHARDS_PAGE_SIZE_STRING; + return MIN_SUPPORTED_LIST_SHARDS_PAGE_SIZE; } } diff --git a/server/src/test/java/org/opensearch/rest/action/cat/RestShardsActionTests.java b/server/src/test/java/org/opensearch/rest/action/cat/RestShardsActionTests.java index df32e3a3bce00..30f2799a94bd5 100644 --- a/server/src/test/java/org/opensearch/rest/action/cat/RestShardsActionTests.java +++ b/server/src/test/java/org/opensearch/rest/action/cat/RestShardsActionTests.java @@ -48,8 +48,10 @@ import org.opensearch.common.Table; import org.opensearch.index.shard.DocsStats; import org.opensearch.index.shard.ShardPath; +import org.opensearch.rest.pagination.PageToken; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.rest.FakeRestRequest; +import org.junit.Before; import java.nio.file.Path; import java.util.ArrayList; @@ -64,14 +66,18 @@ public class RestShardsActionTests extends OpenSearchTestCase { - public void testBuildTable() { + private final DiscoveryNode localNode = new DiscoveryNode("local", buildNewFakeTransportAddress(), Version.CURRENT); + private List shardRoutings = new ArrayList<>(); + private Map shardStatsMap = new HashMap<>(); + private ClusterStateResponse state; + private IndicesStatsResponse stats; + + @Before + public void setup() { final int numShards = randomIntBetween(1, 5); long numDocs = randomLongBetween(0, 10000); long numDeletedDocs = randomLongBetween(0, 100); - DiscoveryNode localNode = new DiscoveryNode("local", buildNewFakeTransportAddress(), Version.CURRENT); - List shardRoutings = new ArrayList<>(numShards); - Map shardStatsMap = new HashMap<>(); String index = "index"; for (int i = 0; i < numShards; i++) { ShardRoutingState shardRoutingState = ShardRoutingState.fromValue((byte) randomIntBetween(2, 3)); @@ -97,23 +103,43 @@ public void testBuildTable() { when(indexStats.getPrimaries()).thenReturn(new CommonStats()); when(indexStats.getTotal()).thenReturn(new CommonStats()); - IndicesStatsResponse stats = mock(IndicesStatsResponse.class); + stats = mock(IndicesStatsResponse.class); when(stats.asMap()).thenReturn(shardStatsMap); DiscoveryNodes discoveryNodes = mock(DiscoveryNodes.class); when(discoveryNodes.get(localNode.getId())).thenReturn(localNode); - ClusterStateResponse state = mock(ClusterStateResponse.class); + state = mock(ClusterStateResponse.class); RoutingTable routingTable = mock(RoutingTable.class); when(routingTable.allShards()).thenReturn(shardRoutings); ClusterState clusterState = mock(ClusterState.class); when(clusterState.routingTable()).thenReturn(routingTable); when(clusterState.nodes()).thenReturn(discoveryNodes); when(state.getState()).thenReturn(clusterState); + } + public void testBuildTable() { final RestShardsAction action = new RestShardsAction(); final Table table = action.buildTable(new FakeRestRequest(), state, stats, state.getState().routingTable().allShards(), null); + assertTable(table); + } + + public void testBuildTableWithPageToken() { + final RestShardsAction action = new RestShardsAction(); + final Table table = action.buildTable( + new FakeRestRequest(), + state, + stats, + state.getState().routingTable().allShards(), + new PageToken("foo", "test") + ); + assertTable(table); + assertNotNull(table.getPageToken()); + assertEquals("foo", table.getPageToken().getNextToken()); + assertEquals("test", table.getPageToken().getPaginatedEntity()); + } + private void assertTable(Table table) { // now, verify the table is correct List headers = table.getHeaders(); assertThat(headers.get(0).value, equalTo("index")); @@ -128,7 +154,7 @@ public void testBuildTable() { assertThat(headers.get(79).value, equalTo("docs.deleted")); final List> rows = table.getRows(); - assertThat(rows.size(), equalTo(numShards)); + assertThat(rows.size(), equalTo(shardRoutings.size())); Iterator shardRoutingsIt = shardRoutings.iterator(); for (final List row : rows) { diff --git a/server/src/test/java/org/opensearch/rest/action/list/RestShardsListActionTests.java b/server/src/test/java/org/opensearch/rest/action/list/RestShardsListActionTests.java new file mode 100644 index 0000000000000..088e7eb68a3dd --- /dev/null +++ b/server/src/test/java/org/opensearch/rest/action/list/RestShardsListActionTests.java @@ -0,0 +1,63 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.rest.action.list; + +import org.opensearch.OpenSearchParseException; +import org.opensearch.rest.RestRequest; +import org.opensearch.rest.pagination.PageParams; +import org.opensearch.rest.pagination.PaginationStrategy; +import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.test.rest.FakeRestRequest; + +import java.util.HashMap; +import java.util.Map; + +import static org.opensearch.rest.action.list.RestShardsListAction.MAX_SUPPORTED_LIST_SHARDS_PAGE_SIZE; +import static org.opensearch.rest.action.list.RestShardsListAction.MIN_SUPPORTED_LIST_SHARDS_PAGE_SIZE; +import static org.opensearch.rest.pagination.PageParams.PARAM_ASC_SORT_VALUE; + +public class RestShardsListActionTests extends OpenSearchTestCase { + + private final RestShardsListAction action = new RestShardsListAction(); + + public void testShardsListActionIsPaginated() { + assertTrue(action.isActionPaginated()); + } + + public void testValidateAndGetPageParamsWithDefaultParams() { + Map params = new HashMap<>(); + RestRequest restRequest = new FakeRestRequest(params); + PageParams pageParams = action.validateAndGetPageParams(restRequest); + assertEquals(MIN_SUPPORTED_LIST_SHARDS_PAGE_SIZE, pageParams.getSize()); + assertEquals(PARAM_ASC_SORT_VALUE, pageParams.getSort()); + assertNull(pageParams.getRequestedToken()); + } + + public void testValidateAndGetPageParamsWithSizeBelowMin() { + Map params = new HashMap<>(); + params.put("size", String.valueOf(MIN_SUPPORTED_LIST_SHARDS_PAGE_SIZE - 1)); + RestRequest restRequest = new FakeRestRequest(params); + assertThrows(IllegalArgumentException.class, () -> action.validateAndGetPageParams(restRequest)); + } + + public void testValidateAndGetPageParamsWithSizeAboveRange() { + Map params = new HashMap<>(); + params.put("size", String.valueOf(MAX_SUPPORTED_LIST_SHARDS_PAGE_SIZE * 10)); + RestRequest restRequest = new FakeRestRequest(params); + assertThrows(IllegalArgumentException.class, () -> action.validateAndGetPageParams(restRequest)); + } + + public void testValidateAndGetPageParamsWithInvalidRequestToken() { + Map params = new HashMap<>(); + params.put("next_token", PaginationStrategy.encryptStringToken("1|-1|test")); + RestRequest restRequest = new FakeRestRequest(params); + assertThrows(OpenSearchParseException.class, () -> action.validateAndGetPageParams(restRequest)); + } + +} diff --git a/server/src/test/java/org/opensearch/rest/pagination/ShardPaginationStrategyTests.java b/server/src/test/java/org/opensearch/rest/pagination/ShardPaginationStrategyTests.java new file mode 100644 index 0000000000000..8104d33394256 --- /dev/null +++ b/server/src/test/java/org/opensearch/rest/pagination/ShardPaginationStrategyTests.java @@ -0,0 +1,400 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.rest.pagination; + +import org.opensearch.OpenSearchParseException; +import org.opensearch.Version; +import org.opensearch.cluster.ClusterName; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.cluster.metadata.Metadata; +import org.opensearch.cluster.routing.IndexRoutingTable; +import org.opensearch.cluster.routing.RoutingTable; +import org.opensearch.cluster.routing.ShardRouting; +import org.opensearch.test.OpenSearchTestCase; + +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Objects; +import java.util.Set; + +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_CREATION_DATE; +import static org.opensearch.rest.pagination.PageParams.PARAM_ASC_SORT_VALUE; +import static org.opensearch.rest.pagination.PageParams.PARAM_DESC_SORT_VALUE; +import static org.opensearch.rest.pagination.PaginationStrategy.INCORRECT_TAINTED_NEXT_TOKEN_ERROR_MESSAGE; +import static com.carrotsearch.randomizedtesting.RandomizedTest.getRandom; + +public class ShardPaginationStrategyTests extends OpenSearchTestCase { + + private static final String TEST_INDEX_PREFIX = "test-index-"; + private static final int DEFAULT_NUMBER_OF_SHARDS = 5; + private static final int DEFAULT_NUMBER_OF_REPLICAS = 2; + + /** + * Test validates fetching all the shards for 100 indices in a paginated fashion using {@link ShardPaginationStrategy}, + * while no indices are created or deleted during the fetch. + */ + public void testRetrieveAllShardsWithVaryingPageSize() { + List indexNumberList = new ArrayList<>(); + final int totalIndices = 100; + final int totalShards = totalIndices * DEFAULT_NUMBER_OF_SHARDS * (DEFAULT_NUMBER_OF_REPLICAS + 1); + for (int indexNumber = 1; indexNumber <= 100; indexNumber++) { + indexNumberList.add(indexNumber); + } + // creating a cluster state with 100 indices + Collections.shuffle(indexNumberList, getRandom()); + ClusterState clusterState = getRandomClusterState(indexNumberList); + + // Checking pagination response for different pageSizes, which has a mix of even and odd numbers + // to ensure number of shards in last page is not always equal to pageSize. + List pageSizeList = List.of(3, 7, 10, 13); + + List sortOrderList = List.of(PARAM_ASC_SORT_VALUE, PARAM_DESC_SORT_VALUE); + for (String sortOrder : sortOrderList) { + for (int pageSize : pageSizeList) { + List shardRoutings = new ArrayList<>(); + Set indices = new HashSet<>(); + String requestedToken = null; + // Since each shardID will have number of (replicas + 1) shards, thus number + // of shards in a page should always be a multiple of (replicas + 1). + int totalPagesToFetch = (int) Math.ceil(totalShards * 1.0 / (pageSize - pageSize % (DEFAULT_NUMBER_OF_REPLICAS + 1))); + + long lastPageIndexTime = 0; + for (int pageNum = 1; pageNum <= totalPagesToFetch; pageNum++) { + PageParams pageParams = new PageParams(requestedToken, sortOrder, pageSize); + ShardPaginationStrategy strategy = new ShardPaginationStrategy(pageParams, clusterState); + List pageShards = strategy.getRequestedEntities(); + List pageIndices = strategy.getRequestedIndices(); + if (pageNum < totalPagesToFetch) { + assertNotNull(strategy.getResponseToken().getNextToken()); + } else { + assertNull(strategy.getResponseToken().getNextToken()); + } + shardRoutings.addAll(pageShards); + indices.addAll(pageIndices); + long currentLastIndexTime = clusterState.metadata() + .indices() + .get(pageShards.get(pageShards.size() - 1).getIndexName()) + .getCreationDate(); + if (lastPageIndexTime > 0) { + if (sortOrder.equals(PARAM_ASC_SORT_VALUE)) { + assertTrue(lastPageIndexTime <= currentLastIndexTime); + } else { + assertTrue(lastPageIndexTime >= currentLastIndexTime); + } + } + lastPageIndexTime = currentLastIndexTime; + requestedToken = strategy.getResponseToken().getNextToken(); + } + assertEquals(totalShards, shardRoutings.size()); + assertEquals(totalIndices, indices.size()); + } + } + } + + /** + * Test validates fetching all the shards (in asc order) for 100 indices in a paginated fashion using {@link ShardPaginationStrategy}. + * While executing queries, it tries to delete an index for which some of the shards have already been fetched along + * with some other indices which are yet to be fetched. It also creates new indices in between the queries. + * Shards corresponding to indices deleted, should not be fetched while for the indices which were newly created, + * should appear in the response. + */ + public void testRetrieveAllShardsInAscOrderWhileIndicesGetCreatedAndDeleted() { + List indexNumberList = new ArrayList<>(); + List deletedIndices = new ArrayList<>(); + final int totalIndices = 100; + final int numIndicesToDelete = 10; + final int numIndicesToCreate = 5; + final int fixedIndexNumToDelete = 7; + final int numShardsForNewIndices = 4; + final int numReplicasForNewIndices = 5; + final int totalInitialShards = totalIndices * DEFAULT_NUMBER_OF_SHARDS * (DEFAULT_NUMBER_OF_REPLICAS + 1); + for (int indexNumber = 1; indexNumber <= 100; indexNumber++) { + indexNumberList.add(indexNumber); + } + ClusterState clusterState = getRandomClusterState(indexNumberList); + + int pageSize = 10; + String requestedToken = null; + int numPages = 0; + List shardRoutings = new ArrayList<>(); + Set indicesFetched = new HashSet<>(); + do { + numPages++; + PageParams pageParams = new PageParams(requestedToken, PARAM_ASC_SORT_VALUE, pageSize); + ShardPaginationStrategy paginationStrategy = new ShardPaginationStrategy(pageParams, clusterState); + assertNotNull(paginationStrategy); + assertNotNull(paginationStrategy.getResponseToken()); + requestedToken = paginationStrategy.getResponseToken().getNextToken(); + // deleting test-index-7 & 10 more random indices after 11th call. By that time, shards for first 6 + // indices would have been completely fetched. 11th call, would fetch first 3 shardsIDs for test-index-7 and + // if it gets deleted, rest 2 shardIDs for it should not appear. + if (numPages == 11) { + clusterState = deleteIndexFromClusterState(clusterState, fixedIndexNumToDelete); + + deletedIndices = indexNumberList.subList(20, indexNumberList.size()); + Collections.shuffle(deletedIndices, getRandom()); + for (int pos = 0; pos < numIndicesToDelete; pos++) { + clusterState = deleteIndexFromClusterState(clusterState, deletedIndices.get(pos)); + } + } + // creating 5 indices after 5th call + if (numPages == 5) { + for (int indexNumber = totalIndices + 1; indexNumber <= totalIndices + numIndicesToCreate; indexNumber++) { + clusterState = addIndexToClusterState(clusterState, indexNumber, numShardsForNewIndices, numReplicasForNewIndices); + } + } + assertTrue(paginationStrategy.getRequestedEntities().size() <= pageSize); + shardRoutings.addAll(paginationStrategy.getRequestedEntities()); + indicesFetched.addAll(paginationStrategy.getRequestedIndices()); + } while (Objects.nonNull(requestedToken)); + + assertEquals(totalIndices + numIndicesToCreate - numIndicesToDelete, indicesFetched.size()); + + // finalTotalShards = InitialTotal - 2ShardIdsForIndex7 - ShardsFor10RandomlyDeletedIndices + ShardsForNewlyCreatedIndices + final int totalShards = totalInitialShards - 2 * (DEFAULT_NUMBER_OF_REPLICAS + 1) - numIndicesToDelete * DEFAULT_NUMBER_OF_SHARDS + * (DEFAULT_NUMBER_OF_REPLICAS + 1) + numIndicesToCreate * numShardsForNewIndices * (numReplicasForNewIndices + 1); + assertEquals(totalShards, shardRoutings.size()); + + // deleted test-index-7, should appear in the response shards and indices + assertTrue(indicesFetched.contains(TEST_INDEX_PREFIX + 7)); + assertEquals( + shardRoutings.stream().filter(shard -> shard.getIndexName().equals(TEST_INDEX_PREFIX + 7)).count(), + 3 * (DEFAULT_NUMBER_OF_REPLICAS + 1) + ); + // none of the randomly deleted index should appear in the list of fetched indices + for (int deletedIndexPos = 0; deletedIndexPos < numIndicesToDelete; deletedIndexPos++) { + String indexName = TEST_INDEX_PREFIX + deletedIndices.get(deletedIndexPos); + assertFalse(indicesFetched.contains(indexName)); + assertEquals(shardRoutings.stream().filter(shard -> shard.getIndexName().equals(indexName)).count(), 0); + + } + + // all the newly created indices should be present in the list of fetched indices + for (int indexNumber = totalIndices + 1; indexNumber <= totalIndices + numIndicesToCreate; indexNumber++) { + String indexName = TEST_INDEX_PREFIX + indexNumber; + assertTrue(indicesFetched.contains(indexName)); + assertEquals( + numShardsForNewIndices * (numReplicasForNewIndices + 1), + shardRoutings.stream().filter(shard -> shard.getIndexName().equals(indexName)).count() + ); + } + } + + /** + * Test validates fetching all the shards (in desc order) for 100 indices in a paginated fashion using {@link ShardPaginationStrategy}. + * While executing queries, it tries to delete an index for which some of the shards have already been fetched along + * with some other indices which are yet to be fetched. It also creates new indices in between the queries. + * Shards corresponding to indices deleted, should not be fetched while for the indices which were newly created, + * should appear in the response. + */ + public void testRetrieveAllShardsInDescOrderWhileIndicesGetCreatedAndDeleted() { + List indexNumberList = new ArrayList<>(); + List deletedIndices = new ArrayList<>(); + final int totalIndices = 100; + final int numIndicesToDelete = 10; + final int numIndicesToCreate = 5; + final int fixedIndexNumToDelete = 94; + final int numShardsForNewIndices = 4; + final int numReplicasForNewIndices = 5; + final int totalInitialShards = totalIndices * DEFAULT_NUMBER_OF_SHARDS * (DEFAULT_NUMBER_OF_REPLICAS + 1); + for (int indexNumber = 1; indexNumber <= 100; indexNumber++) { + indexNumberList.add(indexNumber); + } + ClusterState clusterState = getRandomClusterState(indexNumberList); + + int pageSize = 10; + String requestedToken = null; + int numPages = 0; + List shardRoutings = new ArrayList<>(); + Set indicesFetched = new HashSet<>(); + do { + PageParams pageParams = new PageParams(requestedToken, PARAM_DESC_SORT_VALUE, pageSize); + ShardPaginationStrategy paginationStrategy = new ShardPaginationStrategy(pageParams, clusterState); + numPages++; + assertNotNull(paginationStrategy); + assertNotNull(paginationStrategy.getResponseToken()); + requestedToken = paginationStrategy.getResponseToken().getNextToken(); + // deleting test-index-94 & 10 more random indices after 11th call. By that time, shards for last 6 + // indices would have been completely fetched. 11th call, would fetch first 3 shardsIDs for test-index-94 and + // if it gets deleted, rest 2 shardIDs for it should not appear. + if (numPages == 11) { + clusterState = deleteIndexFromClusterState(clusterState, fixedIndexNumToDelete); + + deletedIndices = indexNumberList.subList(0, 80); + Collections.shuffle(deletedIndices, getRandom()); + for (int pos = 0; pos < numIndicesToDelete; pos++) { + clusterState = deleteIndexFromClusterState(clusterState, deletedIndices.get(pos)); + } + } + // creating 5 indices after 5th call + if (numPages == 5) { + for (int indexNumber = totalIndices + 1; indexNumber <= totalIndices + numIndicesToCreate; indexNumber++) { + clusterState = addIndexToClusterState(clusterState, indexNumber, numShardsForNewIndices, numReplicasForNewIndices); + } + } + assertTrue(paginationStrategy.getRequestedEntities().size() <= pageSize); + shardRoutings.addAll(paginationStrategy.getRequestedEntities()); + indicesFetched.addAll(paginationStrategy.getRequestedIndices()); + } while (Objects.nonNull(requestedToken)); + assertEquals(totalIndices - numIndicesToDelete, indicesFetched.size()); + // finalTotalShards = InitialTotal - 2ShardIdsForIndex7 - ShardsFor10RandomlyDeletedIndices + final int totalShards = totalInitialShards - 2 * (DEFAULT_NUMBER_OF_REPLICAS + 1) - numIndicesToDelete * DEFAULT_NUMBER_OF_SHARDS + * (DEFAULT_NUMBER_OF_REPLICAS + 1); + assertEquals(totalShards, shardRoutings.size()); + + // deleted test-index-94, should appear in the response shards and indices + assertTrue(indicesFetched.contains(TEST_INDEX_PREFIX + fixedIndexNumToDelete)); + assertEquals( + shardRoutings.stream().filter(shard -> shard.getIndexName().equals(TEST_INDEX_PREFIX + fixedIndexNumToDelete)).count(), + 3 * (DEFAULT_NUMBER_OF_REPLICAS + 1) + ); + + // none of the randomly deleted index should appear in the list of fetched indices + for (int deletedIndexPos = 0; deletedIndexPos < numIndicesToDelete; deletedIndexPos++) { + String indexName = TEST_INDEX_PREFIX + deletedIndices.get(deletedIndexPos); + assertFalse(indicesFetched.contains(indexName)); + assertEquals(shardRoutings.stream().filter(shard -> shard.getIndexName().equals(indexName)).count(), 0); + } + + // none of the newly created indices should be present in the list of fetched indices + for (int indexNumber = totalIndices + 1; indexNumber <= totalIndices + numIndicesToCreate; indexNumber++) { + String indexName = TEST_INDEX_PREFIX + indexNumber; + assertFalse(indicesFetched.contains(indexName)); + assertEquals(0, shardRoutings.stream().filter(shard -> shard.getIndexName().equals(indexName)).count()); + } + } + + /** + * Validates strategy fails with IllegalArgumentException when requests size in pageParam is smaller + * than #(replicas + 1) for any of the index. + */ + public void testIllegalSizeArgumentRequestedFromStrategy() { + int numIndices = 6; + int numShards = 5; + int numReplicas = 8; + int pageSize = numReplicas + 1; + ClusterState clusterState = getRandomClusterState(Collections.emptyList()); + for (int index = 1; index < numIndices; index++) { + clusterState = addIndexToClusterState(clusterState, index, numShards, numReplicas); + } + clusterState = addIndexToClusterState(clusterState, numIndices, numShards + 1, numReplicas + 1); + + try { + String requestedToken = null; + ShardPaginationStrategy strategy; + do { + PageParams pageParams = new PageParams(requestedToken, PARAM_ASC_SORT_VALUE, pageSize); + strategy = new ShardPaginationStrategy(pageParams, clusterState); + requestedToken = strategy.getResponseToken().getNextToken(); + } while (requestedToken != null); + fail("expected exception"); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains("size value should be greater than the replica count of all indices")); + } + } + + public void testCreatingShardStrategyPageTokenWithRequestedTokenNull() { + try { + new ShardPaginationStrategy.ShardStrategyToken(null); + fail("expected exception"); + } catch (Exception e) { + assert e.getMessage().contains("requestedTokenString can not be null"); + } + } + + public void testIndexStrategyPageTokenWithWronglyEncryptedRequestToken() { + assertThrows(OpenSearchParseException.class, () -> new ShardPaginationStrategy.ShardStrategyToken("3%4%5")); + } + + public void testIndexStrategyPageTokenWithIncorrectNumberOfElementsInRequestedToken() { + assertThrows( + OpenSearchParseException.class, + () -> new ShardPaginationStrategy.ShardStrategyToken(PaginationStrategy.encryptStringToken("1|1725361543")) + ); + assertThrows( + OpenSearchParseException.class, + () -> new ShardPaginationStrategy.ShardStrategyToken(PaginationStrategy.encryptStringToken("1|1725361543|index|12345")) + ); + } + + public void testIndexStrategyPageTokenWithInvalidValuesInRequestedToken() { + assertThrows( + OpenSearchParseException.class, + () -> new ShardPaginationStrategy.ShardStrategyToken(PaginationStrategy.encryptStringToken("-1725361543|1725361543|index")) + ); + } + + public void testCreatingIndexStrategyPageTokenWithNameOfLastRespondedIndexNull() { + try { + new ShardPaginationStrategy.ShardStrategyToken(null, 0, 1234l); + fail("expected exception"); + } catch (Exception e) { + assert e.getMessage().contains("index name should be provided"); + } + } + + public void testCreatingIndexStrategyPageTokenWithNonParseableShardID() { + try { + new ShardPaginationStrategy.ShardStrategyToken(PaginationStrategy.encryptStringToken("shardID|1725361543|index")); + fail("expected exception"); + } catch (Exception e) { + assert e.getMessage().contains(INCORRECT_TAINTED_NEXT_TOKEN_ERROR_MESSAGE); + } + } + + /** + * @param indexNumbers would be used to create indices having names with integer appended after foo, like foo1, foo2. + * @return random clusterState consisting of indices having their creation times set to the integer used to name them. + */ + private ClusterState getRandomClusterState(List indexNumbers) { + ClusterState clusterState = ClusterState.builder(new ClusterName("test")) + .metadata(Metadata.builder().build()) + .routingTable(RoutingTable.builder().build()) + .build(); + for (Integer indexNumber : indexNumbers) { + clusterState = addIndexToClusterState(clusterState, indexNumber, DEFAULT_NUMBER_OF_SHARDS, DEFAULT_NUMBER_OF_REPLICAS); + } + return clusterState; + } + + private ClusterState addIndexToClusterState( + ClusterState clusterState, + final int indexNumber, + final int numShards, + final int numReplicas + ) { + IndexMetadata indexMetadata = IndexMetadata.builder(TEST_INDEX_PREFIX + indexNumber) + .settings( + settings(Version.CURRENT).put(SETTING_CREATION_DATE, Instant.now().plus(indexNumber, ChronoUnit.SECONDS).toEpochMilli()) + ) + .numberOfShards(numShards) + .numberOfReplicas(numReplicas) + .build(); + IndexRoutingTable.Builder indexRoutingTableBuilder = new IndexRoutingTable.Builder(indexMetadata.getIndex()).initializeAsNew( + indexMetadata + ); + return ClusterState.builder(clusterState) + .metadata(Metadata.builder(clusterState.metadata()).put(indexMetadata, true).build()) + .routingTable(RoutingTable.builder(clusterState.routingTable()).add(indexRoutingTableBuilder).build()) + .build(); + } + + private ClusterState deleteIndexFromClusterState(ClusterState clusterState, int indexNumber) { + return ClusterState.builder(clusterState) + .metadata(Metadata.builder(clusterState.metadata()).remove(TEST_INDEX_PREFIX + indexNumber)) + .routingTable(RoutingTable.builder(clusterState.routingTable()).remove(TEST_INDEX_PREFIX + indexNumber).build()) + .build(); + } + +} diff --git a/test/framework/src/main/java/org/opensearch/test/rest/FakeRestRequest.java b/test/framework/src/main/java/org/opensearch/test/rest/FakeRestRequest.java index e7810ae4c8f1c..1880977c2d35d 100644 --- a/test/framework/src/main/java/org/opensearch/test/rest/FakeRestRequest.java +++ b/test/framework/src/main/java/org/opensearch/test/rest/FakeRestRequest.java @@ -60,6 +60,15 @@ public FakeRestRequest() { ); } + public FakeRestRequest(Map params) { + this( + NamedXContentRegistry.EMPTY, + new FakeHttpRequest(Method.GET, "", BytesArray.EMPTY, new HashMap<>()), + params, + new FakeHttpChannel(null) + ); + } + private FakeRestRequest( NamedXContentRegistry xContentRegistry, HttpRequest httpRequest, From ee923607baf762588caa88c50b471d5c4d402ae5 Mon Sep 17 00:00:00 2001 From: Harsh Garg Date: Mon, 30 Sep 2024 12:02:38 +0530 Subject: [PATCH 04/11] Spotless Apply Signed-off-by: Harsh Garg --- server/src/main/java/org/opensearch/action/ActionModule.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/action/ActionModule.java b/server/src/main/java/org/opensearch/action/ActionModule.java index ab6e3e7cc081d..35246d7e8d4fc 100644 --- a/server/src/main/java/org/opensearch/action/ActionModule.java +++ b/server/src/main/java/org/opensearch/action/ActionModule.java @@ -462,9 +462,9 @@ import org.opensearch.rest.action.ingest.RestPutPipelineAction; import org.opensearch.rest.action.ingest.RestSimulatePipelineAction; import org.opensearch.rest.action.list.AbstractListAction; +import org.opensearch.rest.action.list.RestIndicesListAction; import org.opensearch.rest.action.list.RestListAction; import org.opensearch.rest.action.list.RestShardsListAction; -import org.opensearch.rest.action.list.RestIndicesListAction; import org.opensearch.rest.action.search.RestClearScrollAction; import org.opensearch.rest.action.search.RestCountAction; import org.opensearch.rest.action.search.RestCreatePitAction; From fab3d1e58a30cd0e43209cc39cfb8d275bbe8dcf Mon Sep 17 00:00:00 2001 From: Harsh Garg Date: Mon, 30 Sep 2024 12:09:44 +0530 Subject: [PATCH 05/11] Added changeLog Signed-off-by: Harsh Garg --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b628e9277959d..2273443e213a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Remove identity-related feature flagged code from the RestController ([#15430](https://github.com/opensearch-project/OpenSearch/pull/15430)) - Add support for msearch API to pass search pipeline name - ([#15923](https://github.com/opensearch-project/OpenSearch/pull/15923)) - Add _list/indices API as paginated alternate to _cat/indices ([#14718](https://github.com/opensearch-project/OpenSearch/pull/14718)) +- Add _list/shards API as paginated alternate to _cat/shards ([#14641](https://github.com/opensearch-project/OpenSearch/pull/14641)) ### Dependencies - Bump `com.azure:azure-identity` from 1.13.0 to 1.13.2 ([#15578](https://github.com/opensearch-project/OpenSearch/pull/15578)) From 84e6e3335f58f4c457622ca7a0d3fd49f14a9aea Mon Sep 17 00:00:00 2001 From: Harsh Garg Date: Tue, 1 Oct 2024 10:47:17 +0530 Subject: [PATCH 06/11] Refactoring pagination classes and adding ser/de UTs Signed-off-by: Harsh Garg --- .../shards/TransportCatShardsActionIT.java | 5 +- .../cluster/shards/CatShardsRequest.java | 16 ++- .../cluster/shards/CatShardsResponse.java | 27 ++-- .../shards/TransportCatShardsAction.java | 6 +- .../pagination/IndexPaginationStrategy.java | 49 ++++--- .../pagination/PageParams.java | 26 ++-- .../pagination/PageToken.java | 22 +-- .../pagination/PaginationStrategy.java | 2 +- .../pagination/ShardPaginationStrategy.java | 122 ++++++++--------- .../pagination/package-info.java | 2 +- .../java/org/opensearch/common/Table.java | 2 +- .../java/org/opensearch/rest/RestRequest.java | 8 +- .../rest/action/cat/RestIndicesAction.java | 4 +- .../rest/action/cat/RestShardsAction.java | 18 +-- .../opensearch/rest/action/cat/RestTable.java | 2 +- .../rest/action/list/AbstractListAction.java | 6 +- .../action/list/RestIndicesListAction.java | 4 +- .../action/list/RestShardsListAction.java | 4 +- .../cluster/shards/CatShardsRequestTests.java | 119 ++++++++++++++++ .../shards/CatShardsResponseTests.java | 129 ++++++++++++++++-- .../IndexPaginationStrategyTests.java | 6 +- .../action/pagination/PageParamsTests.java | 42 ++++++ .../action/pagination/PageTokenTests.java | 40 ++++++ .../ShardPaginationStrategyTests.java | 8 +- .../action/cat/RestIndicesActionTests.java | 2 +- .../action/cat/RestShardsActionTests.java | 12 +- .../rest/action/cat/RestTableTests.java | 2 +- .../list/RestShardsListActionTests.java | 19 ++- 28 files changed, 522 insertions(+), 182 deletions(-) rename server/src/main/java/org/opensearch/{rest => action}/pagination/IndexPaginationStrategy.java (81%) rename server/src/main/java/org/opensearch/{rest => action}/pagination/PageParams.java (72%) rename server/src/main/java/org/opensearch/{rest => action}/pagination/PageToken.java (75%) rename server/src/main/java/org/opensearch/{rest => action}/pagination/PaginationStrategy.java (98%) rename server/src/main/java/org/opensearch/{rest => action}/pagination/ShardPaginationStrategy.java (71%) rename server/src/main/java/org/opensearch/{rest => action}/pagination/package-info.java (86%) create mode 100644 server/src/test/java/org/opensearch/action/admin/cluster/shards/CatShardsRequestTests.java rename server/src/test/java/org/opensearch/{rest => action}/pagination/IndexPaginationStrategyTests.java (99%) create mode 100644 server/src/test/java/org/opensearch/action/pagination/PageParamsTests.java create mode 100644 server/src/test/java/org/opensearch/action/pagination/PageTokenTests.java rename server/src/test/java/org/opensearch/{rest => action}/pagination/ShardPaginationStrategyTests.java (98%) diff --git a/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java b/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java index b86521dedf739..32d5b3db85629 100644 --- a/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java @@ -8,7 +8,6 @@ package org.opensearch.action.admin.cluster.shards; -import org.opensearch.action.admin.cluster.state.ClusterStateResponse; import org.opensearch.action.admin.indices.stats.IndicesStatsResponse; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.routing.ShardRouting; @@ -51,9 +50,9 @@ public void testCatShardsWithSuccessResponse() throws InterruptedException { client().execute(CatShardsAction.INSTANCE, shardsRequest, new ActionListener() { @Override public void onResponse(CatShardsResponse catShardsResponse) { - ClusterStateResponse clusterStateResponse = catShardsResponse.getClusterStateResponse(); + List shardRoutings = catShardsResponse.getResponseShards(); IndicesStatsResponse indicesStatsResponse = catShardsResponse.getIndicesStatsResponse(); - for (ShardRouting shard : clusterStateResponse.getState().routingTable().allShards()) { + for (ShardRouting shard : shardRoutings) { assertEquals("test", shard.getIndexName()); assertNotNull(indicesStatsResponse.asMap().get(shard)); } diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsRequest.java index 3524ca38f84f4..8f7acee9cf95b 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsRequest.java @@ -10,13 +10,13 @@ import org.opensearch.Version; import org.opensearch.action.ActionRequestValidationException; +import org.opensearch.action.pagination.PageParams; import org.opensearch.action.support.clustermanager.ClusterManagerNodeReadRequest; import org.opensearch.common.unit.TimeValue; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; import org.opensearch.core.tasks.TaskId; import org.opensearch.rest.action.admin.cluster.ClusterAdminTask; -import org.opensearch.rest.pagination.PageParams; import java.io.IOException; import java.util.Map; @@ -38,9 +38,9 @@ public CatShardsRequest(StreamInput in) throws IOException { super(in); if (in.getVersion().onOrAfter(Version.V_3_0_0)) { indices = in.readStringArray(); - cancelAfterTimeInterval = in.readTimeValue(); + cancelAfterTimeInterval = in.readOptionalTimeValue(); if (in.readBoolean()) { - pageParams = PageParams.readPageParams(in); + pageParams = new PageParams(in); } } } @@ -49,11 +49,15 @@ public CatShardsRequest(StreamInput in) throws IOException { public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); if (out.getVersion().onOrAfter(Version.V_3_0_0)) { - out.writeStringArray(indices); - out.writeTimeValue(cancelAfterTimeInterval); + if (indices == null) { + out.writeVInt(0); + } else { + out.writeStringArray(indices); + } + out.writeOptionalTimeValue(cancelAfterTimeInterval); out.writeBoolean(pageParams != null); if (pageParams != null) { - pageParams.writePageParams(out); + pageParams.writeTo(out); } } } diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsResponse.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsResponse.java index 80fec51fd25d6..c2499ab190ded 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsResponse.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsResponse.java @@ -9,13 +9,13 @@ package org.opensearch.action.admin.cluster.shards; import org.opensearch.Version; -import org.opensearch.action.admin.cluster.state.ClusterStateResponse; import org.opensearch.action.admin.indices.stats.IndicesStatsResponse; +import org.opensearch.action.pagination.PageToken; +import org.opensearch.cluster.node.DiscoveryNodes; import org.opensearch.cluster.routing.ShardRouting; import org.opensearch.core.action.ActionResponse; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; -import org.opensearch.rest.pagination.PageToken; import java.io.IOException; import java.util.ArrayList; @@ -28,45 +28,44 @@ */ public class CatShardsResponse extends ActionResponse { - private ClusterStateResponse clusterStateResponse = null; - - private IndicesStatsResponse indicesStatsResponse = null; + private IndicesStatsResponse indicesStatsResponse; + private DiscoveryNodes nodes = DiscoveryNodes.EMPTY_NODES; private List responseShards = new ArrayList<>(); - private PageToken pageToken = null; + private PageToken pageToken; public CatShardsResponse() {} public CatShardsResponse(StreamInput in) throws IOException { super(in); - clusterStateResponse = new ClusterStateResponse(in); indicesStatsResponse = new IndicesStatsResponse(in); if (in.getVersion().onOrAfter(Version.V_3_0_0)) { + nodes = DiscoveryNodes.readFrom(in, null); responseShards = in.readList(ShardRouting::new); if (in.readBoolean()) { - pageToken = PageToken.readPageToken(in); + pageToken = new PageToken(in); } } } @Override public void writeTo(StreamOutput out) throws IOException { - clusterStateResponse.writeTo(out); indicesStatsResponse.writeTo(out); if (out.getVersion().onOrAfter(Version.V_3_0_0)) { + nodes.writeToWithAttribute(out); out.writeList(responseShards); out.writeBoolean(pageToken != null); if (pageToken != null) { - pageToken.writePageToken(out); + pageToken.writeTo(out); } } } - public void setClusterStateResponse(ClusterStateResponse clusterStateResponse) { - this.clusterStateResponse = clusterStateResponse; + public void setNodes(DiscoveryNodes nodes) { + this.nodes = nodes; } - public ClusterStateResponse getClusterStateResponse() { - return this.clusterStateResponse; + public DiscoveryNodes getNodes() { + return this.nodes; } public void setIndicesStatsResponse(IndicesStatsResponse indicesStatsResponse) { diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java index 5a5e178eb6a82..a9f3e76417ede 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java @@ -12,6 +12,8 @@ import org.opensearch.action.admin.cluster.state.ClusterStateResponse; import org.opensearch.action.admin.indices.stats.IndicesStatsRequest; import org.opensearch.action.admin.indices.stats.IndicesStatsResponse; +import org.opensearch.action.pagination.PageParams; +import org.opensearch.action.pagination.ShardPaginationStrategy; import org.opensearch.action.support.ActionFilters; import org.opensearch.action.support.HandledTransportAction; import org.opensearch.action.support.TimeoutTaskCancellationUtility; @@ -19,8 +21,6 @@ import org.opensearch.common.inject.Inject; import org.opensearch.core.action.ActionListener; import org.opensearch.core.action.NotifyOnceListener; -import org.opensearch.rest.pagination.PageParams; -import org.opensearch.rest.pagination.ShardPaginationStrategy; import org.opensearch.tasks.CancellableTask; import org.opensearch.tasks.Task; import org.opensearch.transport.TransportService; @@ -89,7 +89,7 @@ public void onResponse(ClusterStateResponse clusterStateResponse) { String[] indices = Objects.isNull(paginationStrategy) ? shardsRequest.getIndices() : paginationStrategy.getRequestedIndices().toArray(new String[0]); - catShardsResponse.setClusterStateResponse(clusterStateResponse); + catShardsResponse.setNodes(clusterStateResponse.getState().getNodes()); catShardsResponse.setResponseShards( Objects.isNull(paginationStrategy) ? clusterStateResponse.getState().routingTable().allShards() diff --git a/server/src/main/java/org/opensearch/rest/pagination/IndexPaginationStrategy.java b/server/src/main/java/org/opensearch/action/pagination/IndexPaginationStrategy.java similarity index 81% rename from server/src/main/java/org/opensearch/rest/pagination/IndexPaginationStrategy.java rename to server/src/main/java/org/opensearch/action/pagination/IndexPaginationStrategy.java index f89ab14e4b24d..e67310fc60809 100644 --- a/server/src/main/java/org/opensearch/rest/pagination/IndexPaginationStrategy.java +++ b/server/src/main/java/org/opensearch/action/pagination/IndexPaginationStrategy.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.rest.pagination; +package org.opensearch.action.pagination; import org.opensearch.OpenSearchParseException; import org.opensearch.cluster.ClusterState; @@ -20,8 +20,6 @@ import java.util.function.Predicate; import java.util.stream.Collectors; -import static org.opensearch.rest.pagination.PageParams.PARAM_ASC_SORT_VALUE; - /** * This strategy can be used by the Rest APIs wanting to paginate the responses based on Indices. * The strategy considers create timestamps of indices as the keys to iterate over pages. @@ -31,13 +29,13 @@ public class IndexPaginationStrategy implements PaginationStrategy { private static final String DEFAULT_INDICES_PAGINATED_ENTITY = "indices"; - private static final Comparator ASC_COMPARATOR = (metadata1, metadata2) -> { + protected static final Comparator ASC_COMPARATOR = (metadata1, metadata2) -> { if (metadata1.getCreationDate() == metadata2.getCreationDate()) { return metadata1.getIndex().getName().compareTo(metadata2.getIndex().getName()); } return Long.compare(metadata1.getCreationDate(), metadata2.getCreationDate()); }; - private static final Comparator DESC_COMPARATOR = (metadata1, metadata2) -> { + protected static final Comparator DESC_COMPARATOR = (metadata1, metadata2) -> { if (metadata1.getCreationDate() == metadata2.getCreationDate()) { return metadata2.getIndex().getName().compareTo(metadata1.getIndex().getName()); } @@ -48,11 +46,20 @@ public class IndexPaginationStrategy implements PaginationStrategy { private final List requestedIndices; public IndexPaginationStrategy(PageParams pageParams, ClusterState clusterState) { + + IndexStrategyToken requestedToken = Objects.isNull(pageParams.getRequestedToken()) || pageParams.getRequestedToken().isEmpty() + ? null + : new IndexStrategyToken(pageParams.getRequestedToken()); // Get list of indices metadata sorted by their creation time and filtered by the last sent index List sortedIndices = PaginationStrategy.getSortedIndexMetadata( clusterState, - getMetadataFilter(pageParams.getRequestedToken(), pageParams.getSort()), - PARAM_ASC_SORT_VALUE.equals(pageParams.getSort()) ? ASC_COMPARATOR : DESC_COMPARATOR + getMetadataFilter( + pageParams.getSort(), + Objects.isNull(requestedToken) ? null : requestedToken.lastIndexName, + Objects.isNull(requestedToken) ? null : requestedToken.lastIndexCreationTime, + false + ), + PageParams.PARAM_ASC_SORT_VALUE.equals(pageParams.getSort()) ? ASC_COMPARATOR : DESC_COMPARATOR ); // Trim sortedIndicesList to get the list of indices metadata to be sent as response List metadataSublist = getMetadataSubList(sortedIndices, pageParams.getSize()); @@ -65,25 +72,27 @@ public IndexPaginationStrategy(PageParams pageParams, ClusterState clusterState) ); } - private static Predicate getMetadataFilter(String requestedTokenStr, String sortOrder) { - boolean isAscendingSort = sortOrder.equals(PARAM_ASC_SORT_VALUE); - IndexStrategyToken requestedToken = Objects.isNull(requestedTokenStr) || requestedTokenStr.isEmpty() - ? null - : new IndexStrategyToken(requestedTokenStr); - if (Objects.isNull(requestedToken)) { + protected static Predicate getMetadataFilter( + String sortOrder, + String lastIndexName, + Long lastIndexCreationTime, + boolean includeLastIndex + ) { + if (Objects.isNull(lastIndexName) || Objects.isNull(lastIndexCreationTime)) { return indexMetadata -> true; } + boolean isAscendingSort = sortOrder.equals(PageParams.PARAM_ASC_SORT_VALUE); return metadata -> { - if (metadata.getIndex().getName().equals(requestedToken.lastIndexName)) { - return false; - } else if (metadata.getCreationDate() == requestedToken.lastIndexCreationTime) { + if (metadata.getIndex().getName().equals(lastIndexName)) { + return includeLastIndex; + } else if (metadata.getCreationDate() == lastIndexCreationTime) { return isAscendingSort - ? metadata.getIndex().getName().compareTo(requestedToken.lastIndexName) > 0 - : metadata.getIndex().getName().compareTo(requestedToken.lastIndexName) < 0; + ? metadata.getIndex().getName().compareTo(lastIndexName) > 0 + : metadata.getIndex().getName().compareTo(lastIndexName) < 0; } return isAscendingSort - ? metadata.getCreationDate() > requestedToken.lastIndexCreationTime - : metadata.getCreationDate() < requestedToken.lastIndexCreationTime; + ? metadata.getCreationDate() > lastIndexCreationTime + : metadata.getCreationDate() < lastIndexCreationTime; }; } diff --git a/server/src/main/java/org/opensearch/rest/pagination/PageParams.java b/server/src/main/java/org/opensearch/action/pagination/PageParams.java similarity index 72% rename from server/src/main/java/org/opensearch/rest/pagination/PageParams.java rename to server/src/main/java/org/opensearch/action/pagination/PageParams.java index 72522700658f2..631fa8c80c726 100644 --- a/server/src/main/java/org/opensearch/rest/pagination/PageParams.java +++ b/server/src/main/java/org/opensearch/action/pagination/PageParams.java @@ -6,11 +6,12 @@ * compatible open source license. */ -package org.opensearch.rest.pagination; +package org.opensearch.action.pagination; import org.opensearch.common.annotation.PublicApi; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.core.common.io.stream.Writeable; import java.io.IOException; @@ -19,7 +20,7 @@ * Class specific to paginated queries, which will contain common query params required by a paginated API. */ @PublicApi(since = "3.0.0") -public class PageParams { +public class PageParams implements Writeable { public static final String PARAM_SORT = "sort"; public static final String PARAM_NEXT_TOKEN = "next_token"; @@ -31,6 +32,12 @@ public class PageParams { private final String sort; private final int size; + public PageParams(StreamInput in) throws IOException { + this.requestedTokenStr = in.readOptionalString(); + this.sort = in.readOptionalString(); + this.size = in.readInt(); + } + public PageParams(String requestedToken, String sort, int size) { this.requestedTokenStr = requestedToken; this.sort = sort; @@ -49,17 +56,10 @@ public int getSize() { return size; } - public void writePageParams(StreamOutput out) throws IOException { - out.writeString(requestedTokenStr); - out.writeString(sort); + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeOptionalString(requestedTokenStr); + out.writeOptionalString(sort); out.writeInt(size); } - - public static PageParams readPageParams(StreamInput in) throws IOException { - String requestedToken = in.readString(); - String sort = in.readString(); - int size = in.readInt(); - return new PageParams(requestedToken, sort, size); - } - } diff --git a/server/src/main/java/org/opensearch/rest/pagination/PageToken.java b/server/src/main/java/org/opensearch/action/pagination/PageToken.java similarity index 75% rename from server/src/main/java/org/opensearch/rest/pagination/PageToken.java rename to server/src/main/java/org/opensearch/action/pagination/PageToken.java index c9779c2763e07..2bbe75cc3848e 100644 --- a/server/src/main/java/org/opensearch/rest/pagination/PageToken.java +++ b/server/src/main/java/org/opensearch/action/pagination/PageToken.java @@ -6,10 +6,11 @@ * compatible open source license. */ -package org.opensearch.rest.pagination; +package org.opensearch.action.pagination; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.core.common.io.stream.Writeable; import java.io.IOException; @@ -17,7 +18,7 @@ * Pagination response metadata for a paginated query. * @opensearch.internal */ -public class PageToken { +public class PageToken implements Writeable { public static final String PAGINATED_RESPONSE_NEXT_TOKEN_KEY = "next_token"; @@ -37,6 +38,11 @@ public PageToken(String nextToken, String paginatedElement) { this.paginatedEntity = paginatedElement; } + public PageToken(StreamInput in) throws IOException { + this.nextToken = in.readOptionalString(); + this.paginatedEntity = in.readString(); + } + public String getNextToken() { return nextToken; } @@ -45,15 +51,9 @@ public String getPaginatedEntity() { return paginatedEntity; } - public void writePageToken(StreamOutput out) throws IOException { - out.writeString(nextToken); + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeOptionalString(nextToken); out.writeString(paginatedEntity); } - - public static PageToken readPageToken(StreamInput in) throws IOException { - String nextToken = in.readString(); - String paginatedEntity = in.readString(); - return new PageToken(nextToken, paginatedEntity); - } - } diff --git a/server/src/main/java/org/opensearch/rest/pagination/PaginationStrategy.java b/server/src/main/java/org/opensearch/action/pagination/PaginationStrategy.java similarity index 98% rename from server/src/main/java/org/opensearch/rest/pagination/PaginationStrategy.java rename to server/src/main/java/org/opensearch/action/pagination/PaginationStrategy.java index 7f9825a7cc09b..0b79496d6a039 100644 --- a/server/src/main/java/org/opensearch/rest/pagination/PaginationStrategy.java +++ b/server/src/main/java/org/opensearch/action/pagination/PaginationStrategy.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.rest.pagination; +package org.opensearch.action.pagination; import org.opensearch.OpenSearchParseException; import org.opensearch.cluster.ClusterState; diff --git a/server/src/main/java/org/opensearch/rest/pagination/ShardPaginationStrategy.java b/server/src/main/java/org/opensearch/action/pagination/ShardPaginationStrategy.java similarity index 71% rename from server/src/main/java/org/opensearch/rest/pagination/ShardPaginationStrategy.java rename to server/src/main/java/org/opensearch/action/pagination/ShardPaginationStrategy.java index b680012484b83..fe067018d1711 100644 --- a/server/src/main/java/org/opensearch/rest/pagination/ShardPaginationStrategy.java +++ b/server/src/main/java/org/opensearch/action/pagination/ShardPaginationStrategy.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.rest.pagination; +package org.opensearch.action.pagination; import org.opensearch.OpenSearchParseException; import org.opensearch.cluster.ClusterState; @@ -14,17 +14,16 @@ import org.opensearch.cluster.routing.IndexRoutingTable; import org.opensearch.cluster.routing.IndexShardRoutingTable; import org.opensearch.cluster.routing.ShardRouting; -import org.opensearch.common.collect.Tuple; import java.util.ArrayList; -import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.function.Predicate; -import java.util.stream.Collectors; -import static org.opensearch.rest.pagination.PageParams.PARAM_ASC_SORT_VALUE; +import static org.opensearch.action.pagination.IndexPaginationStrategy.ASC_COMPARATOR; +import static org.opensearch.action.pagination.IndexPaginationStrategy.DESC_COMPARATOR; +import static org.opensearch.action.pagination.IndexPaginationStrategy.getMetadataFilter; +import static org.opensearch.action.pagination.PageParams.PARAM_ASC_SORT_VALUE; /** * This strategy can be used by the Rest APIs wanting to paginate the responses based on Shards. @@ -35,18 +34,6 @@ public class ShardPaginationStrategy implements PaginationStrategy { private static final String DEFAULT_SHARDS_PAGINATED_ENTITY = "shards"; - private static final Comparator ASC_COMPARATOR = (metadata1, metadata2) -> { - if (metadata1.getCreationDate() == metadata2.getCreationDate()) { - return metadata1.getIndex().getName().compareTo(metadata2.getIndex().getName()); - } - return Long.compare(metadata1.getCreationDate(), metadata2.getCreationDate()); - }; - private static final Comparator DESC_COMPARATOR = (metadata1, metadata2) -> { - if (metadata1.getCreationDate() == metadata2.getCreationDate()) { - return metadata2.getIndex().getName().compareTo(metadata1.getIndex().getName()); - } - return Long.compare(metadata2.getCreationDate(), metadata1.getCreationDate()); - }; private PageToken pageToken; private List pageShardRoutings = new ArrayList<>(); @@ -57,45 +44,24 @@ public ShardPaginationStrategy(PageParams pageParams, ClusterState clusterState) // Get list of indices metadata sorted by their creation time and filtered by the last sent index List filteredIndices = PaginationStrategy.getSortedIndexMetadata( clusterState, - getIndexFilter(shardStrategyToken, pageParams.getSort()), + getMetadataFilter( + pageParams.getSort(), + Objects.isNull(shardStrategyToken) ? null : shardStrategyToken.lastIndexName, + Objects.isNull(shardStrategyToken) ? null : shardStrategyToken.lastIndexCreationTime, + true + ), PARAM_ASC_SORT_VALUE.equals(pageParams.getSort()) ? ASC_COMPARATOR : DESC_COMPARATOR ); // Get the list of shards and indices belonging to current page. - Tuple, List> tuple = getPageData( - clusterState.getRoutingTable().getIndicesRouting(), + PageData pageData = getPageData( filteredIndices, + clusterState.getRoutingTable().getIndicesRouting(), shardStrategyToken, pageParams.getSize() ); - List pageShardRoutings = tuple.v1(); - List pageIndices = tuple.v2(); - this.pageShardRoutings = pageShardRoutings; - // Get list of index names from the trimmed metadataSublist - this.pageIndices = pageIndices.stream().map(metadata -> metadata.getIndex().getName()).collect(Collectors.toList()); - this.pageToken = getResponseToken( - pageIndices.isEmpty() ? null : pageIndices.get(pageIndices.size() - 1), - filteredIndices.isEmpty() ? null : filteredIndices.get(filteredIndices.size() - 1).getIndex().getName(), - pageShardRoutings.isEmpty() ? -1 : pageShardRoutings.get(pageShardRoutings.size() - 1).id() - ); - } - - private static Predicate getIndexFilter(ShardStrategyToken token, String sortOrder) { - if (Objects.isNull(token)) { - return indexMetadata -> true; - } - boolean isAscendingSort = sortOrder.equals(PARAM_ASC_SORT_VALUE); - return metadata -> { - if (metadata.getIndex().getName().equals(token.lastIndexName)) { - return true; - } else if (metadata.getCreationDate() == token.lastIndexCreationTime) { - return isAscendingSort - ? metadata.getIndex().getName().compareTo(token.lastIndexName) > 0 - : metadata.getIndex().getName().compareTo(token.lastIndexName) < 0; - } - return isAscendingSort - ? metadata.getCreationDate() > token.lastIndexCreationTime - : metadata.getCreationDate() < token.lastIndexCreationTime; - }; + this.pageToken = pageData.pageToken; + this.pageShardRoutings = pageData.shardRoutings; + this.pageIndices = pageData.indices; } /** @@ -103,21 +69,24 @@ private static Predicate getIndexFilter(ShardStrategyToken token, * which are to be displayed in a page. * Note: All shards for a shardID will always be present in the same page. */ - private Tuple, List> getPageData( - Map indicesRouting, + private PageData getPageData( List filteredIndices, + Map indicesRouting, final ShardStrategyToken token, final int numShardsRequired ) { List shardRoutings = new ArrayList<>(); - List indexMetadataList = new ArrayList<>(); + List indices = new ArrayList<>(); int shardCount = 0; + IndexMetadata lastAddedIndex = null; // iterate over indices until shardCount is less than numShardsRequired for (IndexMetadata indexMetadata : filteredIndices) { String indexName = indexMetadata.getIndex().getName(); - int startShardId = getStartShardIdForPageIndex(token, indexName); boolean indexShardsAdded = false; + // Always start from shardID 0 for all indices except for the first one which might be same as the last sent + // index. To identify if an index is same as last sent index, verify both the index name and creaton time. + int startShardId = shardCount == 0 ? getStartShardIdForPageIndex(token, indexName, indexMetadata.getCreationDate()) : 0; Map indexShardRoutingTable = indicesRouting.get(indexName).getShards(); for (; startShardId < indexShardRoutingTable.size(); startShardId++) { if (indexShardRoutingTable.get(startShardId).size() > numShardsRequired) { @@ -130,24 +99,35 @@ private Tuple, List> getPageData( shardRoutings.addAll(indexShardRoutingTable.get(startShardId).shards()); indexShardsAdded = true; } - // Add index to the list if any of its shard was added to the count. + if (indexShardsAdded) { - indexMetadataList.add(indexMetadata); + lastAddedIndex = indexMetadata; + indices.add(indexName); } if (shardCount >= numShardsRequired) { break; } } - return new Tuple<>(shardRoutings, indexMetadataList); + if (filteredIndices.isEmpty() || shardRoutings.isEmpty()) { + return new PageData(shardRoutings, indices, new PageToken(null, DEFAULT_SHARDS_PAGINATED_ENTITY)); + } + return new PageData( + shardRoutings, + indices, + getResponseToken( + lastAddedIndex, + filteredIndices.get(filteredIndices.size() - 1).getIndex().getName(), + shardRoutings.get(shardRoutings.size() - 1).id() + ) + ); } private PageToken getResponseToken(IndexMetadata pageEndIndex, final String lastFilteredIndexName, final int pageEndShardId) { // If all the shards of filtered indices list have been included in pageShardRoutings, then no more // shards are remaining to be fetched, and the next_token should thus be null. - String pageEndIndexName = Objects.isNull(pageEndIndex) ? null : pageEndIndex.getIndex().getName(); - if (Objects.isNull(pageEndIndexName) - || (pageEndIndexName.equals(lastFilteredIndexName) && pageEndIndex.getNumberOfShards() == pageEndShardId + 1)) { + String pageEndIndexName = pageEndIndex.getIndex().getName(); + if (pageEndIndexName.equals(lastFilteredIndexName) && pageEndIndex.getNumberOfShards() == pageEndShardId + 1) { return new PageToken(null, DEFAULT_SHARDS_PAGINATED_ENTITY); } return new PageToken( @@ -164,8 +144,10 @@ private ShardStrategyToken getShardStrategyToken(String requestedToken) { * Provides the shardId to start an index which is to be included in the page. If the index is same as * lastIndex, start from the shardId next to lastShardId, else always start from 0. */ - private int getStartShardIdForPageIndex(ShardStrategyToken token, final String pageIndexName) { - return Objects.isNull(token) ? 0 : token.lastIndexName.equals(pageIndexName) ? token.lastShardId + 1 : 0; + private int getStartShardIdForPageIndex(ShardStrategyToken token, final String pageIndexName, final long pageIndexCreationTime) { + return Objects.isNull(token) ? 0 + : token.lastIndexName.equals(pageIndexName) && token.lastIndexCreationTime == pageIndexCreationTime ? token.lastShardId + 1 + : 0; } @Override @@ -259,4 +241,20 @@ public static void validateShardStrategyToken(String requestedTokenStr) { } } } + + /** + * Private utility class to consolidate the page details of shard strategy. Will be used to set all the details at once, + * to avoid parsing collections multiple times. + */ + private static class PageData { + private final List shardRoutings; + private final List indices; + private final PageToken pageToken; + + PageData(List shardRoutings, List indices, PageToken pageToken) { + this.shardRoutings = shardRoutings; + this.indices = indices; + this.pageToken = pageToken; + } + } } diff --git a/server/src/main/java/org/opensearch/rest/pagination/package-info.java b/server/src/main/java/org/opensearch/action/pagination/package-info.java similarity index 86% rename from server/src/main/java/org/opensearch/rest/pagination/package-info.java rename to server/src/main/java/org/opensearch/action/pagination/package-info.java index 324b8a6c46f88..4cf4c7d05adaf 100644 --- a/server/src/main/java/org/opensearch/rest/pagination/package-info.java +++ b/server/src/main/java/org/opensearch/action/pagination/package-info.java @@ -9,4 +9,4 @@ /** * Exposes utilities for Rest actions to paginate responses. */ -package org.opensearch.rest.pagination; +package org.opensearch.action.pagination; diff --git a/server/src/main/java/org/opensearch/common/Table.java b/server/src/main/java/org/opensearch/common/Table.java index 133ec3052e6c9..c2dd770da845b 100644 --- a/server/src/main/java/org/opensearch/common/Table.java +++ b/server/src/main/java/org/opensearch/common/Table.java @@ -32,9 +32,9 @@ package org.opensearch.common; +import org.opensearch.action.pagination.PageToken; import org.opensearch.common.time.DateFormatter; import org.opensearch.core.common.Strings; -import org.opensearch.rest.pagination.PageToken; import java.time.Instant; import java.time.ZoneOffset; diff --git a/server/src/main/java/org/opensearch/rest/RestRequest.java b/server/src/main/java/org/opensearch/rest/RestRequest.java index f241b567c3204..4d5bad2e7fd49 100644 --- a/server/src/main/java/org/opensearch/rest/RestRequest.java +++ b/server/src/main/java/org/opensearch/rest/RestRequest.java @@ -33,6 +33,7 @@ package org.opensearch.rest; import org.opensearch.OpenSearchParseException; +import org.opensearch.action.pagination.PageParams; import org.opensearch.common.Booleans; import org.opensearch.common.CheckedConsumer; import org.opensearch.common.Nullable; @@ -51,7 +52,6 @@ import org.opensearch.core.xcontent.XContentParser; import org.opensearch.http.HttpChannel; import org.opensearch.http.HttpRequest; -import org.opensearch.rest.pagination.PageParams; import java.io.IOException; import java.io.InputStream; @@ -66,11 +66,11 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; +import static org.opensearch.action.pagination.PageParams.PARAM_NEXT_TOKEN; +import static org.opensearch.action.pagination.PageParams.PARAM_SIZE; +import static org.opensearch.action.pagination.PageParams.PARAM_SORT; import static org.opensearch.common.unit.TimeValue.parseTimeValue; import static org.opensearch.core.common.unit.ByteSizeValue.parseBytesSizeValue; -import static org.opensearch.rest.pagination.PageParams.PARAM_NEXT_TOKEN; -import static org.opensearch.rest.pagination.PageParams.PARAM_SIZE; -import static org.opensearch.rest.pagination.PageParams.PARAM_SORT; /** * REST Request diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestIndicesAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestIndicesAction.java index 1e76008ff8c64..9d7564a001e22 100644 --- a/server/src/main/java/org/opensearch/rest/action/cat/RestIndicesAction.java +++ b/server/src/main/java/org/opensearch/rest/action/cat/RestIndicesAction.java @@ -43,6 +43,8 @@ import org.opensearch.action.admin.indices.stats.IndexStats; import org.opensearch.action.admin.indices.stats.IndicesStatsRequest; import org.opensearch.action.admin.indices.stats.IndicesStatsResponse; +import org.opensearch.action.pagination.IndexPaginationStrategy; +import org.opensearch.action.pagination.PageToken; import org.opensearch.action.support.GroupedActionListener; import org.opensearch.action.support.IndicesOptions; import org.opensearch.client.node.NodeClient; @@ -63,8 +65,6 @@ import org.opensearch.rest.RestResponse; import org.opensearch.rest.action.RestResponseListener; import org.opensearch.rest.action.list.AbstractListAction; -import org.opensearch.rest.pagination.IndexPaginationStrategy; -import org.opensearch.rest.pagination.PageToken; import java.time.Instant; import java.time.ZoneOffset; diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestShardsAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestShardsAction.java index ccb5bedf03fad..f1cb7a72d23b5 100644 --- a/server/src/main/java/org/opensearch/rest/action/cat/RestShardsAction.java +++ b/server/src/main/java/org/opensearch/rest/action/cat/RestShardsAction.java @@ -35,11 +35,12 @@ import org.opensearch.action.admin.cluster.shards.CatShardsAction; import org.opensearch.action.admin.cluster.shards.CatShardsRequest; import org.opensearch.action.admin.cluster.shards.CatShardsResponse; -import org.opensearch.action.admin.cluster.state.ClusterStateResponse; import org.opensearch.action.admin.indices.stats.CommonStats; import org.opensearch.action.admin.indices.stats.IndicesStatsResponse; import org.opensearch.action.admin.indices.stats.ShardStats; +import org.opensearch.action.pagination.PageToken; import org.opensearch.client.node.NodeClient; +import org.opensearch.cluster.node.DiscoveryNodes; import org.opensearch.cluster.routing.ShardRouting; import org.opensearch.cluster.routing.UnassignedInfo; import org.opensearch.common.Table; @@ -64,7 +65,6 @@ import org.opensearch.rest.RestResponse; import org.opensearch.rest.action.RestResponseListener; import org.opensearch.rest.action.list.AbstractListAction; -import org.opensearch.rest.pagination.PageToken; import org.opensearch.search.suggest.completion.CompletionStats; import java.time.Instant; @@ -120,12 +120,12 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli return channel -> client.execute(CatShardsAction.INSTANCE, shardsRequest, new RestResponseListener(channel) { @Override public RestResponse buildResponse(CatShardsResponse catShardsResponse) throws Exception { - ClusterStateResponse clusterStateResponse = catShardsResponse.getClusterStateResponse(); + DiscoveryNodes nodes = catShardsResponse.getNodes(); IndicesStatsResponse indicesStatsResponse = catShardsResponse.getIndicesStatsResponse(); return RestTable.buildResponse( buildTable( request, - clusterStateResponse, + nodes, indicesStatsResponse, catShardsResponse.getResponseShards(), catShardsResponse.getPageToken() @@ -313,7 +313,7 @@ private static Object getOrNull(S stats, Function accessor, Functio // package private for testing Table buildTable( RestRequest request, - ClusterStateResponse state, + DiscoveryNodes nodes, IndicesStatsResponse stats, List responseShards, PageToken pageToken @@ -347,13 +347,13 @@ Table buildTable( table.addCell(getOrNull(commonStats, CommonStats::getDocs, DocsStats::getCount)); table.addCell(getOrNull(commonStats, CommonStats::getStore, StoreStats::getSize)); if (shard.assignedToNode()) { - String ip = state.getState().nodes().get(shard.currentNodeId()).getHostAddress(); + String ip = nodes.get(shard.currentNodeId()).getHostAddress(); String nodeId = shard.currentNodeId(); StringBuilder name = new StringBuilder(); - name.append(state.getState().nodes().get(shard.currentNodeId()).getName()); + name.append(nodes.get(shard.currentNodeId()).getName()); if (shard.relocating()) { - String reloIp = state.getState().nodes().get(shard.relocatingNodeId()).getHostAddress(); - String reloNme = state.getState().nodes().get(shard.relocatingNodeId()).getName(); + String reloIp = nodes.get(shard.relocatingNodeId()).getHostAddress(); + String reloNme = nodes.get(shard.relocatingNodeId()).getName(); String reloNodeId = shard.relocatingNodeId(); name.append(" -> "); name.append(reloIp); diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestTable.java b/server/src/main/java/org/opensearch/rest/action/cat/RestTable.java index d622dd7a956f4..e27c8212e16a0 100644 --- a/server/src/main/java/org/opensearch/rest/action/cat/RestTable.java +++ b/server/src/main/java/org/opensearch/rest/action/cat/RestTable.java @@ -61,7 +61,7 @@ import java.util.Objects; import java.util.Set; -import static org.opensearch.rest.pagination.PageToken.PAGINATED_RESPONSE_NEXT_TOKEN_KEY; +import static org.opensearch.action.pagination.PageToken.PAGINATED_RESPONSE_NEXT_TOKEN_KEY; /** * a REST table diff --git a/server/src/main/java/org/opensearch/rest/action/list/AbstractListAction.java b/server/src/main/java/org/opensearch/rest/action/list/AbstractListAction.java index f3d6d6653a550..5bdee750fd96f 100644 --- a/server/src/main/java/org/opensearch/rest/action/list/AbstractListAction.java +++ b/server/src/main/java/org/opensearch/rest/action/list/AbstractListAction.java @@ -8,16 +8,16 @@ package org.opensearch.rest.action.list; +import org.opensearch.action.pagination.PageParams; import org.opensearch.client.node.NodeClient; import org.opensearch.rest.RestRequest; import org.opensearch.rest.action.cat.AbstractCatAction; -import org.opensearch.rest.pagination.PageParams; import java.io.IOException; import java.util.Objects; -import static org.opensearch.rest.pagination.PageParams.PARAM_ASC_SORT_VALUE; -import static org.opensearch.rest.pagination.PageParams.PARAM_DESC_SORT_VALUE; +import static org.opensearch.action.pagination.PageParams.PARAM_ASC_SORT_VALUE; +import static org.opensearch.action.pagination.PageParams.PARAM_DESC_SORT_VALUE; /** * Base Transport action class for _list API. diff --git a/server/src/main/java/org/opensearch/rest/action/list/RestIndicesListAction.java b/server/src/main/java/org/opensearch/rest/action/list/RestIndicesListAction.java index ad5c58c86ce90..66d112cf9d21b 100644 --- a/server/src/main/java/org/opensearch/rest/action/list/RestIndicesListAction.java +++ b/server/src/main/java/org/opensearch/rest/action/list/RestIndicesListAction.java @@ -9,12 +9,12 @@ package org.opensearch.rest.action.list; import org.opensearch.action.admin.cluster.state.ClusterStateResponse; +import org.opensearch.action.pagination.IndexPaginationStrategy; +import org.opensearch.action.pagination.PageParams; import org.opensearch.common.collect.Tuple; import org.opensearch.common.settings.Settings; import org.opensearch.rest.RestRequest; import org.opensearch.rest.action.cat.RestIndicesAction; -import org.opensearch.rest.pagination.IndexPaginationStrategy; -import org.opensearch.rest.pagination.PageParams; import java.util.Iterator; import java.util.List; diff --git a/server/src/main/java/org/opensearch/rest/action/list/RestShardsListAction.java b/server/src/main/java/org/opensearch/rest/action/list/RestShardsListAction.java index 9f14eaebc4309..6fdf07e8ce9f4 100644 --- a/server/src/main/java/org/opensearch/rest/action/list/RestShardsListAction.java +++ b/server/src/main/java/org/opensearch/rest/action/list/RestShardsListAction.java @@ -8,10 +8,10 @@ package org.opensearch.rest.action.list; +import org.opensearch.action.pagination.PageParams; +import org.opensearch.action.pagination.ShardPaginationStrategy; import org.opensearch.rest.RestRequest; import org.opensearch.rest.action.cat.RestShardsAction; -import org.opensearch.rest.pagination.PageParams; -import org.opensearch.rest.pagination.ShardPaginationStrategy; import java.util.List; import java.util.Objects; diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/shards/CatShardsRequestTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/shards/CatShardsRequestTests.java new file mode 100644 index 0000000000000..2279b9e1c6897 --- /dev/null +++ b/server/src/test/java/org/opensearch/action/admin/cluster/shards/CatShardsRequestTests.java @@ -0,0 +1,119 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.action.admin.cluster.shards; + +import org.opensearch.Version; +import org.opensearch.action.pagination.PageParams; +import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.common.unit.TimeValue; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.test.VersionUtils; + +public class CatShardsRequestTests extends OpenSearchTestCase { + + public void testSerializationWithDefaultParameters() throws Exception { + CatShardsRequest request = new CatShardsRequest(); + Version version = Version.CURRENT; + try (BytesStreamOutput out = new BytesStreamOutput()) { + out.setVersion(version); + request.writeTo(out); + try (StreamInput in = out.bytes().streamInput()) { + in.setVersion(version); + CatShardsRequest deserialized = new CatShardsRequest(in); + assertNull(deserialized.getPageParams()); + assertNull(deserialized.getCancelAfterTimeInterval()); + assertEquals(0, deserialized.getIndices().length); + } + } + } + + public void testSerializationWithStringPageParamsNull() throws Exception { + CatShardsRequest catShardsRequest = new CatShardsRequest(); + catShardsRequest.setPageParams(new PageParams(null, null, randomIntBetween(1, 5))); + int numIndices = randomIntBetween(1, 5); + String[] indices = new String[numIndices]; + for (int i = 0; i < numIndices; i++) { + indices[i] = randomAlphaOfLengthBetween(3, 10); + } + catShardsRequest.setIndices(indices); + catShardsRequest.setCancelAfterTimeInterval(TimeValue.timeValueMillis(randomIntBetween(1, 5))); + + Version version = Version.CURRENT; + try (BytesStreamOutput out = new BytesStreamOutput()) { + out.setVersion(version); + catShardsRequest.writeTo(out); + try (StreamInput in = out.bytes().streamInput()) { + in.setVersion(version); + CatShardsRequest deserialized = new CatShardsRequest(in); + + // asserting pageParams of deserialized request + assertNotNull(deserialized.getPageParams()); + assertNull(deserialized.getPageParams().getRequestedToken()); + assertNull(deserialized.getPageParams().getSort()); + assertEquals(catShardsRequest.getPageParams().getSize(), deserialized.getPageParams().getSize()); + + // assert indices + assertArrayEquals(catShardsRequest.getIndices(), deserialized.getIndices()); + + // assert timeout + assertEquals(catShardsRequest.getCancelAfterTimeInterval(), deserialized.getCancelAfterTimeInterval()); + } + } + } + + public void testSerializationWithPageParamsSet() throws Exception { + CatShardsRequest catShardsRequest = new CatShardsRequest(); + catShardsRequest.setPageParams( + new PageParams(randomAlphaOfLengthBetween(3, 10), randomAlphaOfLengthBetween(3, 10), randomIntBetween(1, 5)) + ); + Version version = Version.CURRENT; + try (BytesStreamOutput out = new BytesStreamOutput()) { + out.setVersion(version); + catShardsRequest.writeTo(out); + try (StreamInput in = out.bytes().streamInput()) { + in.setVersion(version); + CatShardsRequest deserialized = new CatShardsRequest(in); + + // asserting pageParams of deserialized request + assertNotNull(deserialized.getPageParams()); + assertEquals(catShardsRequest.getPageParams().getRequestedToken(), deserialized.getPageParams().getRequestedToken()); + assertEquals(catShardsRequest.getPageParams().getSort(), deserialized.getPageParams().getSort()); + assertEquals(catShardsRequest.getPageParams().getSize(), deserialized.getPageParams().getSize()); + + assertEquals(0, deserialized.getIndices().length); + assertNull(deserialized.getCancelAfterTimeInterval()); + } + } + } + + public void testSerializationWithOlderVersionsParametersNotSerialized() throws Exception { + CatShardsRequest catShardsRequest = new CatShardsRequest(); + catShardsRequest.setPageParams( + new PageParams(randomAlphaOfLengthBetween(3, 10), randomAlphaOfLengthBetween(3, 10), randomIntBetween(1, 5)) + ); + catShardsRequest.setCancelAfterTimeInterval(TimeValue.timeValueMillis(randomIntBetween(1, 5))); + catShardsRequest.setIndices(new String[2]); + + Version version = VersionUtils.getPreviousVersion(Version.CURRENT); + try (BytesStreamOutput out = new BytesStreamOutput()) { + out.setVersion(version); + catShardsRequest.writeTo(out); + try (StreamInput in = out.bytes().streamInput()) { + in.setVersion(version); + CatShardsRequest deserialized = new CatShardsRequest(in); + + // asserting pageParams of deserialized request + assertNull(deserialized.getPageParams()); + assertNull(deserialized.getIndices()); + assertNull(deserialized.getCancelAfterTimeInterval()); + } + } + } +} diff --git a/server/src/test/java/org/opensearch/action/admin/indices/shards/CatShardsResponseTests.java b/server/src/test/java/org/opensearch/action/admin/indices/shards/CatShardsResponseTests.java index d0b98f4d286ae..bf86a030c38b0 100644 --- a/server/src/test/java/org/opensearch/action/admin/indices/shards/CatShardsResponseTests.java +++ b/server/src/test/java/org/opensearch/action/admin/indices/shards/CatShardsResponseTests.java @@ -32,24 +32,46 @@ package org.opensearch.action.admin.indices.shards; +import org.opensearch.Version; import org.opensearch.action.admin.cluster.shards.CatShardsResponse; -import org.opensearch.action.admin.cluster.state.ClusterStateResponse; +import org.opensearch.action.admin.indices.stats.CommonStats; import org.opensearch.action.admin.indices.stats.IndicesStatsResponse; -import org.opensearch.cluster.ClusterName; +import org.opensearch.action.admin.indices.stats.ShardStats; +import org.opensearch.action.pagination.PageToken; +import org.opensearch.cluster.node.DiscoveryNode; +import org.opensearch.cluster.node.DiscoveryNodes; +import org.opensearch.cluster.routing.ShardRouting; +import org.opensearch.cluster.routing.ShardRoutingState; +import org.opensearch.cluster.routing.TestShardRouting; +import org.opensearch.common.UUIDs; +import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.transport.TransportAddress; +import org.opensearch.core.index.Index; +import org.opensearch.core.index.shard.ShardId; +import org.opensearch.index.shard.ShardPath; import org.opensearch.test.OpenSearchTestCase; -import static org.junit.Assert.assertEquals; +import java.io.IOException; +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; + +import static java.util.Collections.emptyMap; +import static java.util.Collections.emptySet; +import static org.mockito.Mockito.mock; public class CatShardsResponseTests extends OpenSearchTestCase { + private static final String TEST_NODE_ID = "test_node_id"; private final CatShardsResponse catShardsResponse = new CatShardsResponse(); - public void testGetAndSetClusterStateResponse() { - ClusterName clusterName = new ClusterName("1"); - ClusterStateResponse clusterStateResponse = new ClusterStateResponse(clusterName, null, false); - catShardsResponse.setClusterStateResponse(clusterStateResponse); - - assertEquals(clusterStateResponse, catShardsResponse.getClusterStateResponse()); + public void testGetAndSetNodes() { + DiscoveryNodes discoveryNodes = mock(DiscoveryNodes.class); + catShardsResponse.setNodes(discoveryNodes); + assertEquals(discoveryNodes, catShardsResponse.getNodes()); } public void testGetAndSetIndicesStatsResponse() { @@ -58,4 +80,93 @@ public void testGetAndSetIndicesStatsResponse() { assertEquals(indicesStatsResponse, catShardsResponse.getIndicesStatsResponse()); } + + public void testSerialization() throws IOException { + CatShardsResponse response = new CatShardsResponse(); + response.setIndicesStatsResponse(getIndicesStatsResponse()); + response.setNodes(getDiscoveryNodes()); + response.setPageToken(new PageToken("token", "test")); + Version version = Version.CURRENT; + try (BytesStreamOutput out = new BytesStreamOutput()) { + out.setVersion(version); + response.writeTo(out); + try (StreamInput in = out.bytes().streamInput()) { + in.setVersion(version); + CatShardsResponse deserialized = new CatShardsResponse(in); + + assertNotNull(deserialized.getPageToken()); + assertEquals(response.getPageToken().getNextToken(), deserialized.getPageToken().getNextToken()); + assertEquals(response.getPageToken().getPaginatedEntity(), deserialized.getPageToken().getPaginatedEntity()); + assertNotNull(deserialized.getNodes()); + assertEquals(1, deserialized.getNodes().getNodes().size()); + assertNotNull(deserialized.getNodes().getNodes().get(TEST_NODE_ID)); + assertNotNull(deserialized.getIndicesStatsResponse()); + assertEquals( + response.getIndicesStatsResponse().getShards().length, + deserialized.getIndicesStatsResponse().getShards().length + ); + assertTrue(deserialized.getResponseShards().isEmpty()); + } + } + } + + public void testSerializationWithOnlyStatsAndNodesInResponse() throws IOException { + CatShardsResponse response = new CatShardsResponse(); + response.setIndicesStatsResponse(getIndicesStatsResponse()); + response.setNodes(getDiscoveryNodes()); + Version version = Version.CURRENT; + try (BytesStreamOutput out = new BytesStreamOutput()) { + out.setVersion(version); + response.writeTo(out); + try (StreamInput in = out.bytes().streamInput()) { + in.setVersion(version); + CatShardsResponse deserialized = new CatShardsResponse(in); + + assertNull(deserialized.getPageToken()); + assertNotNull(deserialized.getNodes()); + assertEquals(1, deserialized.getNodes().getNodes().size()); + assertNotNull(deserialized.getNodes().getNodes().get(TEST_NODE_ID)); + assertNotNull(deserialized.getIndicesStatsResponse()); + assertEquals( + response.getIndicesStatsResponse().getShards().length, + deserialized.getIndicesStatsResponse().getShards().length + ); + assertTrue(deserialized.getResponseShards().isEmpty()); + } + } + } + + private DiscoveryNodes getDiscoveryNodes() throws UnknownHostException { + DiscoveryNodes.Builder dnBuilder = new DiscoveryNodes.Builder(); + InetAddress inetAddress = InetAddress.getByAddress(new byte[] { (byte) 192, (byte) 168, (byte) 0, (byte) 1 }); + TransportAddress transportAddress = new TransportAddress(inetAddress, randomIntBetween(0, 65535)); + dnBuilder.add(new DiscoveryNode(TEST_NODE_ID, TEST_NODE_ID, transportAddress, emptyMap(), emptySet(), Version.CURRENT)); + return dnBuilder.build(); + } + + private IndicesStatsResponse getIndicesStatsResponse() { + List shards = new ArrayList<>(); + int noOfIndexes = randomIntBetween(2, 5); + + for (int indCnt = 0; indCnt < noOfIndexes; indCnt++) { + Index index = createIndex(randomAlphaOfLength(9)); + int numShards = randomIntBetween(1, 5); + for (int shardId = 0; shardId < numShards; shardId++) { + ShardId shId = new ShardId(index, shardId); + Path path = createTempDir().resolve("indices").resolve(index.getUUID()).resolve(String.valueOf(shardId)); + ShardPath shardPath = new ShardPath(false, path, path, shId); + ShardRouting routing = createShardRouting(shId, (shardId == 0)); + shards.add(new ShardStats(routing, shardPath, new CommonStats(), null, null, null)); + } + } + return new IndicesStatsResponse(shards.toArray(new ShardStats[0]), 0, 0, 0, null); + } + + private ShardRouting createShardRouting(ShardId shardId, boolean isPrimary) { + return TestShardRouting.newShardRouting(shardId, randomAlphaOfLength(4), isPrimary, ShardRoutingState.STARTED); + } + + private Index createIndex(String indexName) { + return new Index(indexName, UUIDs.base64UUID()); + } } diff --git a/server/src/test/java/org/opensearch/rest/pagination/IndexPaginationStrategyTests.java b/server/src/test/java/org/opensearch/action/pagination/IndexPaginationStrategyTests.java similarity index 99% rename from server/src/test/java/org/opensearch/rest/pagination/IndexPaginationStrategyTests.java rename to server/src/test/java/org/opensearch/action/pagination/IndexPaginationStrategyTests.java index 01464b489e26e..9900810bf3eb9 100644 --- a/server/src/test/java/org/opensearch/rest/pagination/IndexPaginationStrategyTests.java +++ b/server/src/test/java/org/opensearch/action/pagination/IndexPaginationStrategyTests.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.rest.pagination; +package org.opensearch.action.pagination; import org.opensearch.OpenSearchParseException; import org.opensearch.Version; @@ -27,9 +27,9 @@ import java.util.Objects; import static java.nio.charset.StandardCharsets.UTF_8; +import static org.opensearch.action.pagination.PageParams.PARAM_ASC_SORT_VALUE; +import static org.opensearch.action.pagination.PageParams.PARAM_DESC_SORT_VALUE; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_CREATION_DATE; -import static org.opensearch.rest.pagination.PageParams.PARAM_ASC_SORT_VALUE; -import static org.opensearch.rest.pagination.PageParams.PARAM_DESC_SORT_VALUE; import static com.carrotsearch.randomizedtesting.RandomizedTest.getRandom; public class IndexPaginationStrategyTests extends OpenSearchTestCase { diff --git a/server/src/test/java/org/opensearch/action/pagination/PageParamsTests.java b/server/src/test/java/org/opensearch/action/pagination/PageParamsTests.java new file mode 100644 index 0000000000000..63cd5d6b3de1a --- /dev/null +++ b/server/src/test/java/org/opensearch/action/pagination/PageParamsTests.java @@ -0,0 +1,42 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.action.pagination; + +import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.test.OpenSearchTestCase; + +public class PageParamsTests extends OpenSearchTestCase { + + public void testSerialization() throws Exception { + PageParams pageParams = new PageParams("foo", "foo", 1); + try (BytesStreamOutput out = new BytesStreamOutput()) { + pageParams.writeTo(out); + try (StreamInput in = out.bytes().streamInput()) { + assertDeserializedPageParams(pageParams, new PageParams(in)); + } + } + } + + public void testSerializationWithRequestedTokenAndSortAbsent() throws Exception { + PageParams pageParams = new PageParams(null, null, 1); + try (BytesStreamOutput out = new BytesStreamOutput()) { + pageParams.writeTo(out); + try (StreamInput in = out.bytes().streamInput()) { + assertDeserializedPageParams(pageParams, new PageParams(in)); + } + } + } + + private void assertDeserializedPageParams(PageParams pageParams, PageParams deserialized) { + assertEquals(pageParams.getSort(), deserialized.getSort()); + assertEquals(pageParams.getSize(), deserialized.getSize()); + assertEquals(pageParams.getRequestedToken(), deserialized.getRequestedToken()); + } +} diff --git a/server/src/test/java/org/opensearch/action/pagination/PageTokenTests.java b/server/src/test/java/org/opensearch/action/pagination/PageTokenTests.java new file mode 100644 index 0000000000000..34388e93a44f3 --- /dev/null +++ b/server/src/test/java/org/opensearch/action/pagination/PageTokenTests.java @@ -0,0 +1,40 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.action.pagination; + +import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.test.OpenSearchTestCase; + +public class PageTokenTests extends OpenSearchTestCase { + + public void testSerialization() throws Exception { + PageToken pageToken = new PageToken("foo", "test"); + try (BytesStreamOutput out = new BytesStreamOutput()) { + pageToken.writeTo(out); + try (StreamInput in = out.bytes().streamInput()) { + PageToken deserialized = new PageToken(in); + assertEquals(pageToken.getNextToken(), deserialized.getNextToken()); + assertEquals(pageToken.getPaginatedEntity(), deserialized.getPaginatedEntity()); + } + } + } + + public void testSerializationWithNextTokenAbsent() throws Exception { + PageToken pageToken = new PageToken(null, "test"); + try (BytesStreamOutput out = new BytesStreamOutput()) { + pageToken.writeTo(out); + try (StreamInput in = out.bytes().streamInput()) { + PageToken deserialized = new PageToken(in); + assertNull(deserialized.getNextToken()); + assertEquals(pageToken.getPaginatedEntity(), deserialized.getPaginatedEntity()); + } + } + } +} diff --git a/server/src/test/java/org/opensearch/rest/pagination/ShardPaginationStrategyTests.java b/server/src/test/java/org/opensearch/action/pagination/ShardPaginationStrategyTests.java similarity index 98% rename from server/src/test/java/org/opensearch/rest/pagination/ShardPaginationStrategyTests.java rename to server/src/test/java/org/opensearch/action/pagination/ShardPaginationStrategyTests.java index 8104d33394256..a596fce21f7f7 100644 --- a/server/src/test/java/org/opensearch/rest/pagination/ShardPaginationStrategyTests.java +++ b/server/src/test/java/org/opensearch/action/pagination/ShardPaginationStrategyTests.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.rest.pagination; +package org.opensearch.action.pagination; import org.opensearch.OpenSearchParseException; import org.opensearch.Version; @@ -28,10 +28,10 @@ import java.util.Objects; import java.util.Set; +import static org.opensearch.action.pagination.PageParams.PARAM_ASC_SORT_VALUE; +import static org.opensearch.action.pagination.PageParams.PARAM_DESC_SORT_VALUE; +import static org.opensearch.action.pagination.PaginationStrategy.INCORRECT_TAINTED_NEXT_TOKEN_ERROR_MESSAGE; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_CREATION_DATE; -import static org.opensearch.rest.pagination.PageParams.PARAM_ASC_SORT_VALUE; -import static org.opensearch.rest.pagination.PageParams.PARAM_DESC_SORT_VALUE; -import static org.opensearch.rest.pagination.PaginationStrategy.INCORRECT_TAINTED_NEXT_TOKEN_ERROR_MESSAGE; import static com.carrotsearch.randomizedtesting.RandomizedTest.getRandom; public class ShardPaginationStrategyTests extends OpenSearchTestCase { diff --git a/server/src/test/java/org/opensearch/rest/action/cat/RestIndicesActionTests.java b/server/src/test/java/org/opensearch/rest/action/cat/RestIndicesActionTests.java index 1d1b509ae94e5..613a2f6724a71 100644 --- a/server/src/test/java/org/opensearch/rest/action/cat/RestIndicesActionTests.java +++ b/server/src/test/java/org/opensearch/rest/action/cat/RestIndicesActionTests.java @@ -35,6 +35,7 @@ import org.opensearch.Version; import org.opensearch.action.admin.indices.stats.CommonStats; import org.opensearch.action.admin.indices.stats.IndexStats; +import org.opensearch.action.pagination.PageToken; import org.opensearch.cluster.health.ClusterHealthStatus; import org.opensearch.cluster.health.ClusterIndexHealth; import org.opensearch.cluster.metadata.IndexMetadata; @@ -48,7 +49,6 @@ import org.opensearch.core.index.shard.ShardId; import org.opensearch.index.IndexSettings; import org.opensearch.rest.action.list.RestIndicesListAction; -import org.opensearch.rest.pagination.PageToken; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.rest.FakeRestRequest; import org.junit.Before; diff --git a/server/src/test/java/org/opensearch/rest/action/cat/RestShardsActionTests.java b/server/src/test/java/org/opensearch/rest/action/cat/RestShardsActionTests.java index 30f2799a94bd5..c412167a10c75 100644 --- a/server/src/test/java/org/opensearch/rest/action/cat/RestShardsActionTests.java +++ b/server/src/test/java/org/opensearch/rest/action/cat/RestShardsActionTests.java @@ -38,6 +38,7 @@ import org.opensearch.action.admin.indices.stats.IndexStats; import org.opensearch.action.admin.indices.stats.IndicesStatsResponse; import org.opensearch.action.admin.indices.stats.ShardStats; +import org.opensearch.action.pagination.PageToken; import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.node.DiscoveryNodes; @@ -48,7 +49,6 @@ import org.opensearch.common.Table; import org.opensearch.index.shard.DocsStats; import org.opensearch.index.shard.ShardPath; -import org.opensearch.rest.pagination.PageToken; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.rest.FakeRestRequest; import org.junit.Before; @@ -120,7 +120,13 @@ public void setup() { public void testBuildTable() { final RestShardsAction action = new RestShardsAction(); - final Table table = action.buildTable(new FakeRestRequest(), state, stats, state.getState().routingTable().allShards(), null); + final Table table = action.buildTable( + new FakeRestRequest(), + state.getState().nodes(), + stats, + state.getState().routingTable().allShards(), + null + ); assertTable(table); } @@ -128,7 +134,7 @@ public void testBuildTableWithPageToken() { final RestShardsAction action = new RestShardsAction(); final Table table = action.buildTable( new FakeRestRequest(), - state, + state.getState().nodes(), stats, state.getState().routingTable().allShards(), new PageToken("foo", "test") diff --git a/server/src/test/java/org/opensearch/rest/action/cat/RestTableTests.java b/server/src/test/java/org/opensearch/rest/action/cat/RestTableTests.java index a82e563d70273..f475d0ff1efda 100644 --- a/server/src/test/java/org/opensearch/rest/action/cat/RestTableTests.java +++ b/server/src/test/java/org/opensearch/rest/action/cat/RestTableTests.java @@ -32,12 +32,12 @@ package org.opensearch.rest.action.cat; +import org.opensearch.action.pagination.PageToken; import org.opensearch.common.Table; import org.opensearch.common.xcontent.XContentType; import org.opensearch.core.xcontent.MediaTypeRegistry; import org.opensearch.rest.AbstractRestChannel; import org.opensearch.rest.RestResponse; -import org.opensearch.rest.pagination.PageToken; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.rest.FakeRestRequest; import org.junit.Before; diff --git a/server/src/test/java/org/opensearch/rest/action/list/RestShardsListActionTests.java b/server/src/test/java/org/opensearch/rest/action/list/RestShardsListActionTests.java index 088e7eb68a3dd..cebcee05ca69b 100644 --- a/server/src/test/java/org/opensearch/rest/action/list/RestShardsListActionTests.java +++ b/server/src/test/java/org/opensearch/rest/action/list/RestShardsListActionTests.java @@ -9,18 +9,18 @@ package org.opensearch.rest.action.list; import org.opensearch.OpenSearchParseException; +import org.opensearch.action.pagination.PageParams; +import org.opensearch.action.pagination.PaginationStrategy; import org.opensearch.rest.RestRequest; -import org.opensearch.rest.pagination.PageParams; -import org.opensearch.rest.pagination.PaginationStrategy; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.rest.FakeRestRequest; import java.util.HashMap; import java.util.Map; +import static org.opensearch.action.pagination.PageParams.PARAM_ASC_SORT_VALUE; import static org.opensearch.rest.action.list.RestShardsListAction.MAX_SUPPORTED_LIST_SHARDS_PAGE_SIZE; import static org.opensearch.rest.action.list.RestShardsListAction.MIN_SUPPORTED_LIST_SHARDS_PAGE_SIZE; -import static org.opensearch.rest.pagination.PageParams.PARAM_ASC_SORT_VALUE; public class RestShardsListActionTests extends OpenSearchTestCase { @@ -60,4 +60,17 @@ public void testValidateAndGetPageParamsWithInvalidRequestToken() { assertThrows(OpenSearchParseException.class, () -> action.validateAndGetPageParams(restRequest)); } + public void testValidateAndGetPageParamsWithValidPageParams() { + Map params = new HashMap<>(); + params.put("next_token", PaginationStrategy.encryptStringToken("1|1|test")); + params.put("sort", "asc"); + params.put("size", "3000"); + RestRequest restRequest = new FakeRestRequest(params); + PageParams pageParams = action.validateAndGetPageParams(restRequest); + + assertEquals(PaginationStrategy.encryptStringToken("1|1|test"), pageParams.getRequestedToken()); + assertEquals(3000, pageParams.getSize()); + assertEquals("asc", pageParams.getSort()); + } + } From 30ea52ae76478e9aaec7bf3f808f0e207ce50dd2 Mon Sep 17 00:00:00 2001 From: Harsh Garg Date: Thu, 3 Oct 2024 11:38:24 +0530 Subject: [PATCH 07/11] Refactoring tests Signed-off-by: Harsh Garg --- .../pagination/IndexPaginationStrategy.java | 36 ++-- .../action/pagination/PageParams.java | 17 ++ .../action/pagination/PageToken.java | 15 ++ .../action/pagination/PaginationStrategy.java | 8 + .../pagination/ShardPaginationStrategy.java | 41 ++--- .../rest/action/cat/RestShardsAction.java | 6 +- .../action/list/RestShardsListAction.java | 1 + .../cluster/shards/CatShardsRequestTests.java | 16 +- .../shards/CatShardsResponseTests.java | 8 +- .../action/pagination/PageParamsTests.java | 9 +- .../action/pagination/PageTokenTests.java | 6 +- .../ShardPaginationStrategyTests.java | 157 ++++++++++++++++-- 12 files changed, 233 insertions(+), 87 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/pagination/IndexPaginationStrategy.java b/server/src/main/java/org/opensearch/action/pagination/IndexPaginationStrategy.java index e67310fc60809..869ae736296d8 100644 --- a/server/src/main/java/org/opensearch/action/pagination/IndexPaginationStrategy.java +++ b/server/src/main/java/org/opensearch/action/pagination/IndexPaginationStrategy.java @@ -11,7 +11,6 @@ import org.opensearch.OpenSearchParseException; import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.metadata.IndexMetadata; -import org.opensearch.common.Nullable; import java.util.ArrayList; import java.util.Comparator; @@ -51,16 +50,14 @@ public IndexPaginationStrategy(PageParams pageParams, ClusterState clusterState) ? null : new IndexStrategyToken(pageParams.getRequestedToken()); // Get list of indices metadata sorted by their creation time and filtered by the last sent index - List sortedIndices = PaginationStrategy.getSortedIndexMetadata( + List sortedIndices = getEligibleIndices( clusterState, - getMetadataFilter( - pageParams.getSort(), - Objects.isNull(requestedToken) ? null : requestedToken.lastIndexName, - Objects.isNull(requestedToken) ? null : requestedToken.lastIndexCreationTime, - false - ), - PageParams.PARAM_ASC_SORT_VALUE.equals(pageParams.getSort()) ? ASC_COMPARATOR : DESC_COMPARATOR + pageParams.getSort(), + Objects.isNull(requestedToken) ? null : requestedToken.lastIndexName, + Objects.isNull(requestedToken) ? null : requestedToken.lastIndexCreationTime, + false ); + // Trim sortedIndicesList to get the list of indices metadata to be sent as response List metadataSublist = getMetadataSubList(sortedIndices, pageParams.getSize()); // Get list of index names from the trimmed metadataSublist @@ -72,6 +69,26 @@ public IndexPaginationStrategy(PageParams pageParams, ClusterState clusterState) ); } + protected static List getEligibleIndices( + ClusterState clusterState, + String sortOrder, + String lastIndexName, + Long lastIndexCreationTime, + boolean includeLastIndex + ) { + if (Objects.isNull(lastIndexName) || Objects.isNull(lastIndexCreationTime)) { + return PaginationStrategy.getSortedIndexMetadata( + clusterState, + PageParams.PARAM_ASC_SORT_VALUE.equals(sortOrder) ? ASC_COMPARATOR : DESC_COMPARATOR + ); + } + return PaginationStrategy.getSortedIndexMetadata( + clusterState, + getMetadataFilter(sortOrder, lastIndexName, lastIndexCreationTime, includeLastIndex), + PageParams.PARAM_ASC_SORT_VALUE.equals(sortOrder) ? ASC_COMPARATOR : DESC_COMPARATOR + ); + } + protected static Predicate getMetadataFilter( String sortOrder, String lastIndexName, @@ -114,7 +131,6 @@ private PageToken getResponseToken(final int pageSize, final int totalIndices, I } @Override - @Nullable public PageToken getResponseToken() { return pageToken; } diff --git a/server/src/main/java/org/opensearch/action/pagination/PageParams.java b/server/src/main/java/org/opensearch/action/pagination/PageParams.java index 631fa8c80c726..03de1aa465a15 100644 --- a/server/src/main/java/org/opensearch/action/pagination/PageParams.java +++ b/server/src/main/java/org/opensearch/action/pagination/PageParams.java @@ -14,6 +14,7 @@ import org.opensearch.core.common.io.stream.Writeable; import java.io.IOException; +import java.util.Objects; /** * @@ -62,4 +63,20 @@ public void writeTo(StreamOutput out) throws IOException { out.writeOptionalString(sort); out.writeInt(size); } + + // Overriding equals and hashcode for tests + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + PageParams that = (PageParams) o; + return this.size == that.size + && Objects.equals(this.requestedTokenStr, that.requestedTokenStr) + && Objects.equals(this.sort, that.sort); + } + + @Override + public int hashCode() { + return Objects.hash(requestedTokenStr, sort, size); + } } diff --git a/server/src/main/java/org/opensearch/action/pagination/PageToken.java b/server/src/main/java/org/opensearch/action/pagination/PageToken.java index 2bbe75cc3848e..44d4079681501 100644 --- a/server/src/main/java/org/opensearch/action/pagination/PageToken.java +++ b/server/src/main/java/org/opensearch/action/pagination/PageToken.java @@ -13,6 +13,7 @@ import org.opensearch.core.common.io.stream.Writeable; import java.io.IOException; +import java.util.Objects; /** * Pagination response metadata for a paginated query. @@ -56,4 +57,18 @@ public void writeTo(StreamOutput out) throws IOException { out.writeOptionalString(nextToken); out.writeString(paginatedEntity); } + + // Overriding equals and hashcode for testing + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + PageToken that = (PageToken) o; + return Objects.equals(this.nextToken, that.nextToken) && Objects.equals(this.paginatedEntity, that.paginatedEntity); + } + + @Override + public int hashCode() { + return Objects.hash(nextToken, paginatedEntity); + } } diff --git a/server/src/main/java/org/opensearch/action/pagination/PaginationStrategy.java b/server/src/main/java/org/opensearch/action/pagination/PaginationStrategy.java index 0b79496d6a039..edb2489b1e4f0 100644 --- a/server/src/main/java/org/opensearch/action/pagination/PaginationStrategy.java +++ b/server/src/main/java/org/opensearch/action/pagination/PaginationStrategy.java @@ -55,6 +55,14 @@ static List getSortedIndexMetadata( return clusterState.metadata().indices().values().stream().filter(filterPredicate).sorted(comparator).collect(Collectors.toList()); } + /** + * + * Utility method to get list of indices sorted as per {@param comparator}. + */ + static List getSortedIndexMetadata(final ClusterState clusterState, Comparator comparator) { + return clusterState.metadata().indices().values().stream().sorted(comparator).collect(Collectors.toList()); + } + static String encryptStringToken(String tokenString) { if (Objects.isNull(tokenString)) { return null; diff --git a/server/src/main/java/org/opensearch/action/pagination/ShardPaginationStrategy.java b/server/src/main/java/org/opensearch/action/pagination/ShardPaginationStrategy.java index fe067018d1711..2b62baf3d6b9f 100644 --- a/server/src/main/java/org/opensearch/action/pagination/ShardPaginationStrategy.java +++ b/server/src/main/java/org/opensearch/action/pagination/ShardPaginationStrategy.java @@ -20,11 +20,6 @@ import java.util.Map; import java.util.Objects; -import static org.opensearch.action.pagination.IndexPaginationStrategy.ASC_COMPARATOR; -import static org.opensearch.action.pagination.IndexPaginationStrategy.DESC_COMPARATOR; -import static org.opensearch.action.pagination.IndexPaginationStrategy.getMetadataFilter; -import static org.opensearch.action.pagination.PageParams.PARAM_ASC_SORT_VALUE; - /** * This strategy can be used by the Rest APIs wanting to paginate the responses based on Shards. * The strategy considers create timestamps of indices and shardID as the keys to iterate over pages. @@ -35,33 +30,25 @@ public class ShardPaginationStrategy implements PaginationStrategy private static final String DEFAULT_SHARDS_PAGINATED_ENTITY = "shards"; - private PageToken pageToken; - private List pageShardRoutings = new ArrayList<>(); - private List pageIndices = new ArrayList<>(); + private PageData pageData; public ShardPaginationStrategy(PageParams pageParams, ClusterState clusterState) { ShardStrategyToken shardStrategyToken = getShardStrategyToken(pageParams.getRequestedToken()); // Get list of indices metadata sorted by their creation time and filtered by the last sent index - List filteredIndices = PaginationStrategy.getSortedIndexMetadata( + List filteredIndices = IndexPaginationStrategy.getEligibleIndices( clusterState, - getMetadataFilter( - pageParams.getSort(), - Objects.isNull(shardStrategyToken) ? null : shardStrategyToken.lastIndexName, - Objects.isNull(shardStrategyToken) ? null : shardStrategyToken.lastIndexCreationTime, - true - ), - PARAM_ASC_SORT_VALUE.equals(pageParams.getSort()) ? ASC_COMPARATOR : DESC_COMPARATOR + pageParams.getSort(), + Objects.isNull(shardStrategyToken) ? null : shardStrategyToken.lastIndexName, + Objects.isNull(shardStrategyToken) ? null : shardStrategyToken.lastIndexCreationTime, + true ); // Get the list of shards and indices belonging to current page. - PageData pageData = getPageData( + this.pageData = getPageData( filteredIndices, clusterState.getRoutingTable().getIndicesRouting(), shardStrategyToken, pageParams.getSize() ); - this.pageToken = pageData.pageToken; - this.pageShardRoutings = pageData.shardRoutings; - this.pageIndices = pageData.indices; } /** @@ -123,15 +110,15 @@ private PageData getPageData( ); } - private PageToken getResponseToken(IndexMetadata pageEndIndex, final String lastFilteredIndexName, final int pageEndShardId) { + private PageToken getResponseToken(IndexMetadata pageEndIndexMetadata, final String lastFilteredIndexName, final int pageEndShardId) { // If all the shards of filtered indices list have been included in pageShardRoutings, then no more // shards are remaining to be fetched, and the next_token should thus be null. - String pageEndIndexName = pageEndIndex.getIndex().getName(); - if (pageEndIndexName.equals(lastFilteredIndexName) && pageEndIndex.getNumberOfShards() == pageEndShardId + 1) { + String pageEndIndexName = pageEndIndexMetadata.getIndex().getName(); + if (pageEndIndexName.equals(lastFilteredIndexName) && pageEndIndexMetadata.getNumberOfShards() == pageEndShardId + 1) { return new PageToken(null, DEFAULT_SHARDS_PAGINATED_ENTITY); } return new PageToken( - new ShardStrategyToken(pageEndIndexName, pageEndShardId, pageEndIndex.getCreationDate()).generateEncryptedToken(), + new ShardStrategyToken(pageEndIndexName, pageEndShardId, pageEndIndexMetadata.getCreationDate()).generateEncryptedToken(), DEFAULT_SHARDS_PAGINATED_ENTITY ); } @@ -152,16 +139,16 @@ private int getStartShardIdForPageIndex(ShardStrategyToken token, final String p @Override public PageToken getResponseToken() { - return pageToken; + return pageData.pageToken; } @Override public List getRequestedEntities() { - return pageShardRoutings; + return pageData.shardRoutings; } public List getRequestedIndices() { - return pageIndices; + return pageData.indices; } /** diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestShardsAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestShardsAction.java index f1cb7a72d23b5..2df23138f244f 100644 --- a/server/src/main/java/org/opensearch/rest/action/cat/RestShardsAction.java +++ b/server/src/main/java/org/opensearch/rest/action/cat/RestShardsAction.java @@ -120,13 +120,11 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli return channel -> client.execute(CatShardsAction.INSTANCE, shardsRequest, new RestResponseListener(channel) { @Override public RestResponse buildResponse(CatShardsResponse catShardsResponse) throws Exception { - DiscoveryNodes nodes = catShardsResponse.getNodes(); - IndicesStatsResponse indicesStatsResponse = catShardsResponse.getIndicesStatsResponse(); return RestTable.buildResponse( buildTable( request, - nodes, - indicesStatsResponse, + catShardsResponse.getNodes(), + catShardsResponse.getIndicesStatsResponse(), catShardsResponse.getResponseShards(), catShardsResponse.getPageToken() ), diff --git a/server/src/main/java/org/opensearch/rest/action/list/RestShardsListAction.java b/server/src/main/java/org/opensearch/rest/action/list/RestShardsListAction.java index 6fdf07e8ce9f4..ba11592398174 100644 --- a/server/src/main/java/org/opensearch/rest/action/list/RestShardsListAction.java +++ b/server/src/main/java/org/opensearch/rest/action/list/RestShardsListAction.java @@ -67,6 +67,7 @@ protected PageParams validateAndGetPageParams(RestRequest restRequest) { return pageParams; } + @Override protected int defaultPageSize() { return MIN_SUPPORTED_LIST_SHARDS_PAGE_SIZE; } diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/shards/CatShardsRequestTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/shards/CatShardsRequestTests.java index 2279b9e1c6897..3d03d590f87c5 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/shards/CatShardsRequestTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/shards/CatShardsRequestTests.java @@ -52,16 +52,10 @@ public void testSerializationWithStringPageParamsNull() throws Exception { try (StreamInput in = out.bytes().streamInput()) { in.setVersion(version); CatShardsRequest deserialized = new CatShardsRequest(in); - // asserting pageParams of deserialized request - assertNotNull(deserialized.getPageParams()); - assertNull(deserialized.getPageParams().getRequestedToken()); - assertNull(deserialized.getPageParams().getSort()); - assertEquals(catShardsRequest.getPageParams().getSize(), deserialized.getPageParams().getSize()); - + assertEquals(catShardsRequest.getPageParams(), deserialized.getPageParams()); // assert indices assertArrayEquals(catShardsRequest.getIndices(), deserialized.getIndices()); - // assert timeout assertEquals(catShardsRequest.getCancelAfterTimeInterval(), deserialized.getCancelAfterTimeInterval()); } @@ -82,11 +76,7 @@ public void testSerializationWithPageParamsSet() throws Exception { CatShardsRequest deserialized = new CatShardsRequest(in); // asserting pageParams of deserialized request - assertNotNull(deserialized.getPageParams()); - assertEquals(catShardsRequest.getPageParams().getRequestedToken(), deserialized.getPageParams().getRequestedToken()); - assertEquals(catShardsRequest.getPageParams().getSort(), deserialized.getPageParams().getSort()); - assertEquals(catShardsRequest.getPageParams().getSize(), deserialized.getPageParams().getSize()); - + assertEquals(catShardsRequest.getPageParams(), deserialized.getPageParams()); assertEquals(0, deserialized.getIndices().length); assertNull(deserialized.getCancelAfterTimeInterval()); } @@ -108,8 +98,6 @@ public void testSerializationWithOlderVersionsParametersNotSerialized() throws E try (StreamInput in = out.bytes().streamInput()) { in.setVersion(version); CatShardsRequest deserialized = new CatShardsRequest(in); - - // asserting pageParams of deserialized request assertNull(deserialized.getPageParams()); assertNull(deserialized.getIndices()); assertNull(deserialized.getCancelAfterTimeInterval()); diff --git a/server/src/test/java/org/opensearch/action/admin/indices/shards/CatShardsResponseTests.java b/server/src/test/java/org/opensearch/action/admin/indices/shards/CatShardsResponseTests.java index bf86a030c38b0..11b1d5567d9fb 100644 --- a/server/src/test/java/org/opensearch/action/admin/indices/shards/CatShardsResponseTests.java +++ b/server/src/test/java/org/opensearch/action/admin/indices/shards/CatShardsResponseTests.java @@ -93,10 +93,7 @@ public void testSerialization() throws IOException { try (StreamInput in = out.bytes().streamInput()) { in.setVersion(version); CatShardsResponse deserialized = new CatShardsResponse(in); - - assertNotNull(deserialized.getPageToken()); - assertEquals(response.getPageToken().getNextToken(), deserialized.getPageToken().getNextToken()); - assertEquals(response.getPageToken().getPaginatedEntity(), deserialized.getPageToken().getPaginatedEntity()); + assertEquals(response.getPageToken(), deserialized.getPageToken()); assertNotNull(deserialized.getNodes()); assertEquals(1, deserialized.getNodes().getNodes().size()); assertNotNull(deserialized.getNodes().getNodes().get(TEST_NODE_ID)); @@ -121,8 +118,7 @@ public void testSerializationWithOnlyStatsAndNodesInResponse() throws IOExceptio try (StreamInput in = out.bytes().streamInput()) { in.setVersion(version); CatShardsResponse deserialized = new CatShardsResponse(in); - - assertNull(deserialized.getPageToken()); + assertEquals(response.getPageToken(), deserialized.getPageToken()); assertNotNull(deserialized.getNodes()); assertEquals(1, deserialized.getNodes().getNodes().size()); assertNotNull(deserialized.getNodes().getNodes().get(TEST_NODE_ID)); diff --git a/server/src/test/java/org/opensearch/action/pagination/PageParamsTests.java b/server/src/test/java/org/opensearch/action/pagination/PageParamsTests.java index 63cd5d6b3de1a..66459ad473a49 100644 --- a/server/src/test/java/org/opensearch/action/pagination/PageParamsTests.java +++ b/server/src/test/java/org/opensearch/action/pagination/PageParamsTests.java @@ -19,7 +19,7 @@ public void testSerialization() throws Exception { try (BytesStreamOutput out = new BytesStreamOutput()) { pageParams.writeTo(out); try (StreamInput in = out.bytes().streamInput()) { - assertDeserializedPageParams(pageParams, new PageParams(in)); + assertEquals(pageParams, new PageParams(in)); } } } @@ -29,14 +29,9 @@ public void testSerializationWithRequestedTokenAndSortAbsent() throws Exception try (BytesStreamOutput out = new BytesStreamOutput()) { pageParams.writeTo(out); try (StreamInput in = out.bytes().streamInput()) { - assertDeserializedPageParams(pageParams, new PageParams(in)); + assertEquals(pageParams, new PageParams(in)); } } } - private void assertDeserializedPageParams(PageParams pageParams, PageParams deserialized) { - assertEquals(pageParams.getSort(), deserialized.getSort()); - assertEquals(pageParams.getSize(), deserialized.getSize()); - assertEquals(pageParams.getRequestedToken(), deserialized.getRequestedToken()); - } } diff --git a/server/src/test/java/org/opensearch/action/pagination/PageTokenTests.java b/server/src/test/java/org/opensearch/action/pagination/PageTokenTests.java index 34388e93a44f3..7c24092dea64c 100644 --- a/server/src/test/java/org/opensearch/action/pagination/PageTokenTests.java +++ b/server/src/test/java/org/opensearch/action/pagination/PageTokenTests.java @@ -20,8 +20,7 @@ public void testSerialization() throws Exception { pageToken.writeTo(out); try (StreamInput in = out.bytes().streamInput()) { PageToken deserialized = new PageToken(in); - assertEquals(pageToken.getNextToken(), deserialized.getNextToken()); - assertEquals(pageToken.getPaginatedEntity(), deserialized.getPaginatedEntity()); + assertEquals(pageToken, deserialized); } } } @@ -32,8 +31,7 @@ public void testSerializationWithNextTokenAbsent() throws Exception { pageToken.writeTo(out); try (StreamInput in = out.bytes().streamInput()) { PageToken deserialized = new PageToken(in); - assertNull(deserialized.getNextToken()); - assertEquals(pageToken.getPaginatedEntity(), deserialized.getPaginatedEntity()); + assertEquals(pageToken, deserialized); } } } diff --git a/server/src/test/java/org/opensearch/action/pagination/ShardPaginationStrategyTests.java b/server/src/test/java/org/opensearch/action/pagination/ShardPaginationStrategyTests.java index a596fce21f7f7..aed7315660378 100644 --- a/server/src/test/java/org/opensearch/action/pagination/ShardPaginationStrategyTests.java +++ b/server/src/test/java/org/opensearch/action/pagination/ShardPaginationStrategyTests.java @@ -27,6 +27,7 @@ import java.util.List; import java.util.Objects; import java.util.Set; +import java.util.stream.Collectors; import static org.opensearch.action.pagination.PageParams.PARAM_ASC_SORT_VALUE; import static org.opensearch.action.pagination.PageParams.PARAM_DESC_SORT_VALUE; @@ -98,6 +99,9 @@ public void testRetrieveAllShardsWithVaryingPageSize() { } assertEquals(totalShards, shardRoutings.size()); assertEquals(totalIndices, indices.size()); + + // verifying the order of shards in the response. + verifyOrderForAllShards(clusterState, shardRoutings, sortOrder); } } } @@ -112,6 +116,7 @@ public void testRetrieveAllShardsWithVaryingPageSize() { public void testRetrieveAllShardsInAscOrderWhileIndicesGetCreatedAndDeleted() { List indexNumberList = new ArrayList<>(); List deletedIndices = new ArrayList<>(); + List newIndices = new ArrayList<>(); final int totalIndices = 100; final int numIndicesToDelete = 10; final int numIndicesToCreate = 5; @@ -140,17 +145,28 @@ public void testRetrieveAllShardsInAscOrderWhileIndicesGetCreatedAndDeleted() { // indices would have been completely fetched. 11th call, would fetch first 3 shardsIDs for test-index-7 and // if it gets deleted, rest 2 shardIDs for it should not appear. if (numPages == 11) { - clusterState = deleteIndexFromClusterState(clusterState, fixedIndexNumToDelete); + // asserting last elements of current response list, which should be the last shard of test-index-6 + assertEquals(TEST_INDEX_PREFIX + 6, shardRoutings.get(shardRoutings.size() - 1).getIndexName()); + assertEquals(DEFAULT_NUMBER_OF_SHARDS - 1, shardRoutings.get(shardRoutings.size() - 1).getId()); + + // asserting the result of 11th query, which should only contain 3 shardIDs belonging to test-index-7 + assertEquals(3 * (DEFAULT_NUMBER_OF_REPLICAS + 1), paginationStrategy.getRequestedEntities().size()); + assertEquals(1, paginationStrategy.getRequestedIndices().size()); + assertEquals(TEST_INDEX_PREFIX + 7, paginationStrategy.getRequestedIndices().get(0)); + assertEquals(2, paginationStrategy.getRequestedEntities().get(8).id()); + clusterState = deleteIndexFromClusterState(clusterState, fixedIndexNumToDelete); deletedIndices = indexNumberList.subList(20, indexNumberList.size()); Collections.shuffle(deletedIndices, getRandom()); - for (int pos = 0; pos < numIndicesToDelete; pos++) { - clusterState = deleteIndexFromClusterState(clusterState, deletedIndices.get(pos)); + deletedIndices = deletedIndices.subList(0, numIndicesToDelete); + for (Integer index : deletedIndices) { + clusterState = deleteIndexFromClusterState(clusterState, index); } } // creating 5 indices after 5th call if (numPages == 5) { for (int indexNumber = totalIndices + 1; indexNumber <= totalIndices + numIndicesToCreate; indexNumber++) { + newIndices.add(TEST_INDEX_PREFIX + indexNumber); clusterState = addIndexToClusterState(clusterState, indexNumber, numShardsForNewIndices, numReplicasForNewIndices); } } @@ -160,7 +176,6 @@ public void testRetrieveAllShardsInAscOrderWhileIndicesGetCreatedAndDeleted() { } while (Objects.nonNull(requestedToken)); assertEquals(totalIndices + numIndicesToCreate - numIndicesToDelete, indicesFetched.size()); - // finalTotalShards = InitialTotal - 2ShardIdsForIndex7 - ShardsFor10RandomlyDeletedIndices + ShardsForNewlyCreatedIndices final int totalShards = totalInitialShards - 2 * (DEFAULT_NUMBER_OF_REPLICAS + 1) - numIndicesToDelete * DEFAULT_NUMBER_OF_SHARDS * (DEFAULT_NUMBER_OF_REPLICAS + 1) + numIndicesToCreate * numShardsForNewIndices * (numReplicasForNewIndices + 1); @@ -172,15 +187,20 @@ public void testRetrieveAllShardsInAscOrderWhileIndicesGetCreatedAndDeleted() { shardRoutings.stream().filter(shard -> shard.getIndexName().equals(TEST_INDEX_PREFIX + 7)).count(), 3 * (DEFAULT_NUMBER_OF_REPLICAS + 1) ); + // none of the randomly deleted index should appear in the list of fetched indices - for (int deletedIndexPos = 0; deletedIndexPos < numIndicesToDelete; deletedIndexPos++) { - String indexName = TEST_INDEX_PREFIX + deletedIndices.get(deletedIndexPos); + for (Integer index : deletedIndices) { + String indexName = TEST_INDEX_PREFIX + index; assertFalse(indicesFetched.contains(indexName)); assertEquals(shardRoutings.stream().filter(shard -> shard.getIndexName().equals(indexName)).count(), 0); - } // all the newly created indices should be present in the list of fetched indices + verifyOrderForAllShards( + clusterState, + shardRoutings.stream().filter(shard -> newIndices.contains(shard.getIndexName())).collect(Collectors.toList()), + PARAM_ASC_SORT_VALUE + ); for (int indexNumber = totalIndices + 1; indexNumber <= totalIndices + numIndicesToCreate; indexNumber++) { String indexName = TEST_INDEX_PREFIX + indexNumber; assertTrue(indicesFetched.contains(indexName)); @@ -229,12 +249,22 @@ public void testRetrieveAllShardsInDescOrderWhileIndicesGetCreatedAndDeleted() { // indices would have been completely fetched. 11th call, would fetch first 3 shardsIDs for test-index-94 and // if it gets deleted, rest 2 shardIDs for it should not appear. if (numPages == 11) { - clusterState = deleteIndexFromClusterState(clusterState, fixedIndexNumToDelete); + // asserting last elements of current response list, which should be the last shard of test-index-95 + assertEquals(TEST_INDEX_PREFIX + 95, shardRoutings.get(shardRoutings.size() - 1).getIndexName()); + assertEquals(DEFAULT_NUMBER_OF_SHARDS - 1, shardRoutings.get(shardRoutings.size() - 1).getId()); + + // asserting the result of 11th query, which should only contain 3 shardIDs belonging to test-index-7 + assertEquals(3 * (DEFAULT_NUMBER_OF_REPLICAS + 1), paginationStrategy.getRequestedEntities().size()); + assertEquals(1, paginationStrategy.getRequestedIndices().size()); + assertEquals(TEST_INDEX_PREFIX + 94, paginationStrategy.getRequestedIndices().get(0)); + assertEquals(2, paginationStrategy.getRequestedEntities().get(8).id()); + clusterState = deleteIndexFromClusterState(clusterState, fixedIndexNumToDelete); deletedIndices = indexNumberList.subList(0, 80); Collections.shuffle(deletedIndices, getRandom()); - for (int pos = 0; pos < numIndicesToDelete; pos++) { - clusterState = deleteIndexFromClusterState(clusterState, deletedIndices.get(pos)); + deletedIndices = deletedIndices.subList(0, numIndicesToDelete); + for (Integer index : deletedIndices) { + clusterState = deleteIndexFromClusterState(clusterState, index); } } // creating 5 indices after 5th call @@ -261,8 +291,8 @@ public void testRetrieveAllShardsInDescOrderWhileIndicesGetCreatedAndDeleted() { ); // none of the randomly deleted index should appear in the list of fetched indices - for (int deletedIndexPos = 0; deletedIndexPos < numIndicesToDelete; deletedIndexPos++) { - String indexName = TEST_INDEX_PREFIX + deletedIndices.get(deletedIndexPos); + for (int deletedIndex : deletedIndices) { + String indexName = TEST_INDEX_PREFIX + deletedIndex; assertFalse(indicesFetched.contains(indexName)); assertEquals(shardRoutings.stream().filter(shard -> shard.getIndexName().equals(indexName)).count(), 0); } @@ -304,6 +334,65 @@ public void testIllegalSizeArgumentRequestedFromStrategy() { } } + public void testRetrieveShardsWhenLastIndexGetsDeletedAndReCreated() { + final int totalIndices = 3; + final int numShards = 3; + final int numReplicas = 3; + final int pageSize = (numReplicas + 1) * 2; + final int totalPages = (int) Math.ceil((totalIndices * numShards * (numReplicas + 1) * 1.0) / pageSize); + ClusterState clusterState = getRandomClusterState(Collections.emptyList()); + for (int indexNum = 1; indexNum <= totalIndices; indexNum++) { + clusterState = addIndexToClusterState(clusterState, indexNum, numShards, numReplicas); + } + + String requestedToken = null; + ShardPaginationStrategy strategy = null; + for (int page = 1; page < totalPages; page++) { + PageParams pageParams = new PageParams(requestedToken, PARAM_ASC_SORT_VALUE, pageSize); + strategy = new ShardPaginationStrategy(pageParams, clusterState); + requestedToken = strategy.getResponseToken().getNextToken(); + } + // The last page was supposed to fetch the remaining two indices for test-index-3 + assertEquals(8, strategy.getRequestedEntities().size()); + assertEquals(1, strategy.getRequestedIndices().size()); + assertEquals(TEST_INDEX_PREFIX + 3, strategy.getRequestedIndices().get(0)); + + // Delete the index and re-create + clusterState = deleteIndexFromClusterState(clusterState, 3); + clusterState = addIndexToClusterState( + clusterState, + 3, + numShards, + numReplicas, + Instant.now().plus(4, ChronoUnit.SECONDS).toEpochMilli() + ); + + // since test-index-3 should now be considered a new index, all shards for it should appear + PageParams pageParams = new PageParams(requestedToken, PARAM_ASC_SORT_VALUE, pageSize); + strategy = new ShardPaginationStrategy(pageParams, clusterState); + assertEquals(8, strategy.getRequestedEntities().size()); + for (ShardRouting shardRouting : strategy.getRequestedEntities()) { + assertTrue( + (shardRouting.getId() == 0 || shardRouting.getId() == 1) + && Objects.equals(shardRouting.getIndexName(), TEST_INDEX_PREFIX + 3) + ); + } + assertEquals(1, strategy.getRequestedIndices().size()); + assertEquals(TEST_INDEX_PREFIX + 3, strategy.getRequestedIndices().get(0)); + assertNotNull(strategy.getResponseToken().getNextToken()); + + requestedToken = strategy.getResponseToken().getNextToken(); + pageParams = new PageParams(requestedToken, PARAM_ASC_SORT_VALUE, pageSize); + strategy = new ShardPaginationStrategy(pageParams, clusterState); + assertEquals(4, strategy.getRequestedEntities().size()); + for (ShardRouting shardRouting : strategy.getRequestedEntities()) { + assertTrue(shardRouting.getId() == 2 && Objects.equals(shardRouting.getIndexName(), TEST_INDEX_PREFIX + 3)); + } + assertEquals(1, strategy.getRequestedIndices().size()); + assertEquals(TEST_INDEX_PREFIX + 3, strategy.getRequestedIndices().get(0)); + assertNull(strategy.getResponseToken().getNextToken()); + } + public void testCreatingShardStrategyPageTokenWithRequestedTokenNull() { try { new ShardPaginationStrategy.ShardStrategyToken(null); @@ -373,11 +462,25 @@ private ClusterState addIndexToClusterState( final int indexNumber, final int numShards, final int numReplicas + ) { + return addIndexToClusterState( + clusterState, + indexNumber, + numShards, + numReplicas, + Instant.now().plus(indexNumber, ChronoUnit.SECONDS).toEpochMilli() + ); + } + + private ClusterState addIndexToClusterState( + ClusterState clusterState, + final int indexNumber, + final int numShards, + final int numReplicas, + final long creationTime ) { IndexMetadata indexMetadata = IndexMetadata.builder(TEST_INDEX_PREFIX + indexNumber) - .settings( - settings(Version.CURRENT).put(SETTING_CREATION_DATE, Instant.now().plus(indexNumber, ChronoUnit.SECONDS).toEpochMilli()) - ) + .settings(settings(Version.CURRENT).put(SETTING_CREATION_DATE, creationTime)) .numberOfShards(numShards) .numberOfReplicas(numReplicas) .build(); @@ -397,4 +500,28 @@ private ClusterState deleteIndexFromClusterState(ClusterState clusterState, int .build(); } + private void verifyOrderForAllShards(ClusterState clusterState, List shardRoutings, String sortOrder) { + ShardRouting prevShard = shardRoutings.get(0); + for (ShardRouting shard : shardRoutings.subList(1, shardRoutings.size())) { + if (Objects.equals(shard.getIndexName(), prevShard.getIndexName())) { + assertTrue(shard.getId() >= prevShard.getId()); + } else { + if (sortOrder.equals(PARAM_ASC_SORT_VALUE)) { + assertTrue( + clusterState.metadata().index(shard.getIndexName()).getCreationDate() > clusterState.metadata() + .index(prevShard.getIndexName()) + .getCreationDate() + ); + } else { + assertTrue( + clusterState.metadata().index(shard.getIndexName()).getCreationDate() < clusterState.metadata() + .index(prevShard.getIndexName()) + .getCreationDate() + ); + } + prevShard = shard; + } + } + } + } From ede07004b459dfa4bcf1d50e49e88abffeb4f65f Mon Sep 17 00:00:00 2001 From: Harsh Garg Date: Thu, 3 Oct 2024 17:42:56 +0530 Subject: [PATCH 08/11] Changing computation of PageToken in ShardStrategy Signed-off-by: Harsh Garg --- .../cluster/shards/CatShardsRequest.java | 3 +- .../pagination/ShardPaginationStrategy.java | 40 +++++++------------ .../cluster/shards/CatShardsRequestTests.java | 5 +++ 3 files changed, 22 insertions(+), 26 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsRequest.java index 01416daece6f4..0319fa103138f 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsRequest.java @@ -43,6 +43,7 @@ public CatShardsRequest(StreamInput in) throws IOException { if (in.readBoolean()) { pageParams = new PageParams(in); } + requestLimitCheckSupported = in.readBoolean(); } } @@ -60,8 +61,8 @@ public void writeTo(StreamOutput out) throws IOException { if (pageParams != null) { pageParams.writeTo(out); } + out.writeBoolean(requestLimitCheckSupported); } - this.requestLimitCheckSupported = false; } @Override diff --git a/server/src/main/java/org/opensearch/action/pagination/ShardPaginationStrategy.java b/server/src/main/java/org/opensearch/action/pagination/ShardPaginationStrategy.java index 2b62baf3d6b9f..7201578a87aca 100644 --- a/server/src/main/java/org/opensearch/action/pagination/ShardPaginationStrategy.java +++ b/server/src/main/java/org/opensearch/action/pagination/ShardPaginationStrategy.java @@ -91,36 +91,26 @@ private PageData getPageData( lastAddedIndex = indexMetadata; indices.add(indexName); } - if (shardCount >= numShardsRequired) { - break; + + if (shardCount > numShardsRequired) { + return new PageData( + shardRoutings, + indices, + new PageToken( + new ShardStrategyToken( + lastAddedIndex.getIndex().getName(), + shardRoutings.get(shardRoutings.size() - 1).id(), + lastAddedIndex.getCreationDate() + ).generateEncryptedToken(), + DEFAULT_SHARDS_PAGINATED_ENTITY + ) + ); } } - if (filteredIndices.isEmpty() || shardRoutings.isEmpty()) { - return new PageData(shardRoutings, indices, new PageToken(null, DEFAULT_SHARDS_PAGINATED_ENTITY)); - } - return new PageData( - shardRoutings, - indices, - getResponseToken( - lastAddedIndex, - filteredIndices.get(filteredIndices.size() - 1).getIndex().getName(), - shardRoutings.get(shardRoutings.size() - 1).id() - ) - ); - } - - private PageToken getResponseToken(IndexMetadata pageEndIndexMetadata, final String lastFilteredIndexName, final int pageEndShardId) { // If all the shards of filtered indices list have been included in pageShardRoutings, then no more // shards are remaining to be fetched, and the next_token should thus be null. - String pageEndIndexName = pageEndIndexMetadata.getIndex().getName(); - if (pageEndIndexName.equals(lastFilteredIndexName) && pageEndIndexMetadata.getNumberOfShards() == pageEndShardId + 1) { - return new PageToken(null, DEFAULT_SHARDS_PAGINATED_ENTITY); - } - return new PageToken( - new ShardStrategyToken(pageEndIndexName, pageEndShardId, pageEndIndexMetadata.getCreationDate()).generateEncryptedToken(), - DEFAULT_SHARDS_PAGINATED_ENTITY - ); + return new PageData(shardRoutings, indices, new PageToken(null, DEFAULT_SHARDS_PAGINATED_ENTITY)); } private ShardStrategyToken getShardStrategyToken(String requestedToken) { diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/shards/CatShardsRequestTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/shards/CatShardsRequestTests.java index 3d03d590f87c5..f4215f54c1e21 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/shards/CatShardsRequestTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/shards/CatShardsRequestTests.java @@ -30,6 +30,7 @@ public void testSerializationWithDefaultParameters() throws Exception { assertNull(deserialized.getPageParams()); assertNull(deserialized.getCancelAfterTimeInterval()); assertEquals(0, deserialized.getIndices().length); + assertFalse(deserialized.isRequestLimitCheckSupported()); } } } @@ -44,6 +45,7 @@ public void testSerializationWithStringPageParamsNull() throws Exception { } catShardsRequest.setIndices(indices); catShardsRequest.setCancelAfterTimeInterval(TimeValue.timeValueMillis(randomIntBetween(1, 5))); + catShardsRequest.setRequestLimitCheckSupported(true); Version version = Version.CURRENT; try (BytesStreamOutput out = new BytesStreamOutput()) { @@ -58,6 +60,7 @@ public void testSerializationWithStringPageParamsNull() throws Exception { assertArrayEquals(catShardsRequest.getIndices(), deserialized.getIndices()); // assert timeout assertEquals(catShardsRequest.getCancelAfterTimeInterval(), deserialized.getCancelAfterTimeInterval()); + assertTrue(deserialized.isRequestLimitCheckSupported()); } } } @@ -79,6 +82,7 @@ public void testSerializationWithPageParamsSet() throws Exception { assertEquals(catShardsRequest.getPageParams(), deserialized.getPageParams()); assertEquals(0, deserialized.getIndices().length); assertNull(deserialized.getCancelAfterTimeInterval()); + assertFalse(deserialized.isRequestLimitCheckSupported()); } } } @@ -101,6 +105,7 @@ public void testSerializationWithOlderVersionsParametersNotSerialized() throws E assertNull(deserialized.getPageParams()); assertNull(deserialized.getIndices()); assertNull(deserialized.getCancelAfterTimeInterval()); + assertFalse(deserialized.isRequestLimitCheckSupported()); } } } From 757b837100f2e0d6d096acf585527f91ae542a05 Mon Sep 17 00:00:00 2001 From: Harsh Garg Date: Thu, 3 Oct 2024 23:36:46 +0530 Subject: [PATCH 09/11] Separating metadata filtering for pagination strategies Signed-off-by: Harsh Garg --- .../pagination/IndexPaginationStrategy.java | 34 +++++++-------- .../pagination/ShardPaginationStrategy.java | 42 +++++++++++++++++-- 2 files changed, 54 insertions(+), 22 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/pagination/IndexPaginationStrategy.java b/server/src/main/java/org/opensearch/action/pagination/IndexPaginationStrategy.java index 869ae736296d8..6180d098a851d 100644 --- a/server/src/main/java/org/opensearch/action/pagination/IndexPaginationStrategy.java +++ b/server/src/main/java/org/opensearch/action/pagination/IndexPaginationStrategy.java @@ -54,8 +54,7 @@ public IndexPaginationStrategy(PageParams pageParams, ClusterState clusterState) clusterState, pageParams.getSort(), Objects.isNull(requestedToken) ? null : requestedToken.lastIndexName, - Objects.isNull(requestedToken) ? null : requestedToken.lastIndexCreationTime, - false + Objects.isNull(requestedToken) ? null : requestedToken.lastIndexCreationTime ); // Trim sortedIndicesList to get the list of indices metadata to be sent as response @@ -69,40 +68,37 @@ public IndexPaginationStrategy(PageParams pageParams, ClusterState clusterState) ); } - protected static List getEligibleIndices( + private static List getEligibleIndices( ClusterState clusterState, String sortOrder, String lastIndexName, - Long lastIndexCreationTime, - boolean includeLastIndex + Long lastIndexCreationTime ) { if (Objects.isNull(lastIndexName) || Objects.isNull(lastIndexCreationTime)) { return PaginationStrategy.getSortedIndexMetadata( clusterState, PageParams.PARAM_ASC_SORT_VALUE.equals(sortOrder) ? ASC_COMPARATOR : DESC_COMPARATOR ); + } else { + return PaginationStrategy.getSortedIndexMetadata( + clusterState, + getMetadataFilter(sortOrder, lastIndexName, lastIndexCreationTime), + PageParams.PARAM_ASC_SORT_VALUE.equals(sortOrder) ? ASC_COMPARATOR : DESC_COMPARATOR + ); } - return PaginationStrategy.getSortedIndexMetadata( - clusterState, - getMetadataFilter(sortOrder, lastIndexName, lastIndexCreationTime, includeLastIndex), - PageParams.PARAM_ASC_SORT_VALUE.equals(sortOrder) ? ASC_COMPARATOR : DESC_COMPARATOR - ); } - protected static Predicate getMetadataFilter( - String sortOrder, - String lastIndexName, - Long lastIndexCreationTime, - boolean includeLastIndex - ) { + private static Predicate getMetadataFilter(String sortOrder, String lastIndexName, Long lastIndexCreationTime) { if (Objects.isNull(lastIndexName) || Objects.isNull(lastIndexCreationTime)) { return indexMetadata -> true; } + return getIndexCreateTimeFilter(sortOrder, lastIndexName, lastIndexCreationTime); + } + + protected static Predicate getIndexCreateTimeFilter(String sortOrder, String lastIndexName, Long lastIndexCreationTime) { boolean isAscendingSort = sortOrder.equals(PageParams.PARAM_ASC_SORT_VALUE); return metadata -> { - if (metadata.getIndex().getName().equals(lastIndexName)) { - return includeLastIndex; - } else if (metadata.getCreationDate() == lastIndexCreationTime) { + if (metadata.getCreationDate() == lastIndexCreationTime) { return isAscendingSort ? metadata.getIndex().getName().compareTo(lastIndexName) > 0 : metadata.getIndex().getName().compareTo(lastIndexName) < 0; diff --git a/server/src/main/java/org/opensearch/action/pagination/ShardPaginationStrategy.java b/server/src/main/java/org/opensearch/action/pagination/ShardPaginationStrategy.java index 7201578a87aca..9a73f31a144e8 100644 --- a/server/src/main/java/org/opensearch/action/pagination/ShardPaginationStrategy.java +++ b/server/src/main/java/org/opensearch/action/pagination/ShardPaginationStrategy.java @@ -19,6 +19,10 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.function.Predicate; + +import static org.opensearch.action.pagination.IndexPaginationStrategy.ASC_COMPARATOR; +import static org.opensearch.action.pagination.IndexPaginationStrategy.DESC_COMPARATOR; /** * This strategy can be used by the Rest APIs wanting to paginate the responses based on Shards. @@ -35,12 +39,11 @@ public class ShardPaginationStrategy implements PaginationStrategy public ShardPaginationStrategy(PageParams pageParams, ClusterState clusterState) { ShardStrategyToken shardStrategyToken = getShardStrategyToken(pageParams.getRequestedToken()); // Get list of indices metadata sorted by their creation time and filtered by the last sent index - List filteredIndices = IndexPaginationStrategy.getEligibleIndices( + List filteredIndices = getEligibleIndices( clusterState, pageParams.getSort(), Objects.isNull(shardStrategyToken) ? null : shardStrategyToken.lastIndexName, - Objects.isNull(shardStrategyToken) ? null : shardStrategyToken.lastIndexCreationTime, - true + Objects.isNull(shardStrategyToken) ? null : shardStrategyToken.lastIndexCreationTime ); // Get the list of shards and indices belonging to current page. this.pageData = getPageData( @@ -51,6 +54,39 @@ public ShardPaginationStrategy(PageParams pageParams, ClusterState clusterState) ); } + private static List getEligibleIndices( + ClusterState clusterState, + String sortOrder, + String lastIndexName, + Long lastIndexCreationTime + ) { + if (Objects.isNull(lastIndexName) || Objects.isNull(lastIndexCreationTime)) { + return PaginationStrategy.getSortedIndexMetadata( + clusterState, + PageParams.PARAM_ASC_SORT_VALUE.equals(sortOrder) ? ASC_COMPARATOR : DESC_COMPARATOR + ); + } else { + return PaginationStrategy.getSortedIndexMetadata( + clusterState, + getMetadataFilter(sortOrder, lastIndexName, lastIndexCreationTime), + PageParams.PARAM_ASC_SORT_VALUE.equals(sortOrder) ? ASC_COMPARATOR : DESC_COMPARATOR + ); + } + } + + private static Predicate getMetadataFilter(String sortOrder, String lastIndexName, Long lastIndexCreationTime) { + if (Objects.isNull(lastIndexName) || Objects.isNull(lastIndexCreationTime)) { + return indexMetadata -> true; + } + return indexNameFilter(lastIndexName).or( + IndexPaginationStrategy.getIndexCreateTimeFilter(sortOrder, lastIndexName, lastIndexCreationTime) + ); + } + + private static Predicate indexNameFilter(String lastIndexName) { + return metadata -> metadata.getIndex().getName().equals(lastIndexName); + } + /** * Will be used to get the list of shards and respective indices to which they belong, * which are to be displayed in a page. From 46988583ae22dbe92e4158a984b9959a27f84c52 Mon Sep 17 00:00:00 2001 From: Harsh Garg Date: Fri, 4 Oct 2024 01:55:42 +0530 Subject: [PATCH 10/11] Retry Build Signed-off-by: Harsh Garg From 10f82d2e9f2a96f714c66a4ba3b4cdc44752e743 Mon Sep 17 00:00:00 2001 From: Harsh Garg Date: Mon, 7 Oct 2024 11:34:13 +0530 Subject: [PATCH 11/11] Updating exception message in ShardPaginationStrategy Signed-off-by: Harsh Garg --- .../opensearch/action/pagination/ShardPaginationStrategy.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/action/pagination/ShardPaginationStrategy.java b/server/src/main/java/org/opensearch/action/pagination/ShardPaginationStrategy.java index 9a73f31a144e8..1eb364c883e60 100644 --- a/server/src/main/java/org/opensearch/action/pagination/ShardPaginationStrategy.java +++ b/server/src/main/java/org/opensearch/action/pagination/ShardPaginationStrategy.java @@ -113,7 +113,9 @@ private PageData getPageData( Map indexShardRoutingTable = indicesRouting.get(indexName).getShards(); for (; startShardId < indexShardRoutingTable.size(); startShardId++) { if (indexShardRoutingTable.get(startShardId).size() > numShardsRequired) { - throw new IllegalArgumentException("size value should be greater than the replica count of all indices"); + throw new IllegalArgumentException( + "size value should be greater than the replica count of all indices, so that all primary and replicas of a shard show up in single page" + ); } shardCount += indexShardRoutingTable.get(startShardId).size(); if (shardCount > numShardsRequired) {