-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implementing pagination for _cat/indices API #14718
base: main
Are you sure you want to change the base?
Implementing pagination for _cat/indices API #14718
Conversation
❌ Gradle check result for 4924a71: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: Harsh Garg <gkharsh@amazon.com>
4924a71
to
0cfb0e9
Compare
❌ Gradle check result for 0cfb0e9: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
server/src/main/java/org/opensearch/rest/action/cat/RestIndicesAction.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/rest/action/cat/RestIndicesAction.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/rest/pagination/PaginationStrategy.java
Show resolved
Hide resolved
server/src/main/java/org/opensearch/rest/action/cat/RestIndicesAction.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/rest/action/cat/RestIndicesAction.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Harsh Garg <gkharsh@amazon.com>
❌ Gradle check result for d665722: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
This looks like a great start. I would consider (don't feel strongly about it though) wrapping additional metadata into {
"metadata": {
"next_token" : "MSQxNzE5NDI3NDk3NDQ0JDE3MTk0Mjc1NTYzODEkaW5kZXg0LXRlc3Q=",
"total_count": 200
},
"indices" : [
{
"health" : "green",
"status" : "open",
"index" : "index4-test",
"uuid" : "s2AJ5CoYRru-8lbB871pWA",
"pri" : "2",
"rep" : "3",
"docs.count" : "0",
"docs.deleted" : "0",
"store.size" : "1.6kb",
"pri.store.size" : "416b"
}
]
} From the implementation POV, can we build |
final boolean includeUnloadedSegments = request.paramAsBoolean("include_unloaded_segments", false); | ||
final String requestedToken = request.param("next_token"); | ||
final int pageSize = Integer.parseInt(request.param("size", DEFAULT_LIST_INDICES_PAGE_SIZE_STRING)); | ||
final String requestedSortOrder = request.param("sort", "ascending"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the part that I think should be thinned out. Instead of receiving a RestRequest
it should be a PaginatedRestRequest
where a bunch of parameters that are not list index specific (token, etc.) were parsed outside of RestIndicesListAction
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @dblock for the feedback. Makes sense to declutter this and also avoid duplication of code. Let me think through a bit more on, if and how can this be achieved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have moved things to a common place now. RestIndicesList
action is now directly extending RestIndicesAction
and all the existing parameters are hence only getting parsed at one place. While, the pagination related params can be parsed individually by the list/paginated APIs.
server/src/main/java/org/opensearch/rest/action/list/RestIndicesListAction.java
Outdated
Show resolved
Hide resolved
// Fetch all the indices from clusterStateRequest for a paginated query. | ||
RestIndicesAction.RestIndicesActionCommonUtils.sendClusterStateRequest( | ||
indices, | ||
IndicesOptions.lenientExpandHidden(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this one should be from the request as the cluster-state will be source of truth to resolve the user expression.
|
||
@Override | ||
public RestChannelConsumer doListRequest(final RestRequest request, final NodeClient client) { | ||
final String[] indices = Strings.splitStringByCommaToArray(request.param("index")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think the request parameters should be common for cat and list indices and should be parsed in a single place. The only additional request parameter the list action should parse is paginatedToken.
Signed-off-by: Harsh Garg <gkharsh@amazon.com>
Signed-off-by: Harsh Garg <gkharsh@amazon.com>
❌ Gradle check result for 2bc80f0: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
*/ | ||
public class RestIndicesListAction extends RestIndicesAction { | ||
|
||
protected static final int MAX_SUPPORTED_LIST_INDICES_PAGE_SIZE_STRING = 1000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added this as a placeholder for now. Need to decide on whether should we impose such limits, and if yes, what should be the values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can test to see what should be reasonable default in terms of latency,
server/src/main/java/org/opensearch/rest/action/cat/AbstractCatAction.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/rest/action/cat/RestIndicesAction.java
Outdated
Show resolved
Hide resolved
indicesToBeQueried, | ||
paginationMetadata | ||
); | ||
groupedListener.onResponse(clusterStateResponse); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the comment below needs an update
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The thing is GetSettings call will require indicesOptions from the request itself. Other calls aren't resolving the wildcards in a way GetSettings call does. The existing tests start to fail when GetSettings call is made with lenient indicesOptions. Need to check on how to call this behaviour out now
server/src/main/java/org/opensearch/rest/action/cat/AbstractCatAction.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/rest/action/list/RestIndicesListAction.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/rest/action/list/RestIndicesListAction.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/rest/action/list/RestIndicesListAction.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/rest/pagination/IndexBasedPaginationStrategy.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/rest/pagination/IndexBasedPaginationStrategy.java
Outdated
Show resolved
Hide resolved
…oken interface Signed-off-by: Harsh Garg <gkharsh@amazon.com>
❌ Gradle check result for f1b5016: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
private final PaginatedQueryResponse paginatedQueryResponse; | ||
private final List<String> indicesFromRequestedToken; | ||
|
||
public IndexBasedPaginationStrategy(PaginatedQueryRequest paginatedQueryRequest, String paginatedElement, ClusterState clusterState) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Constructor seems to have the core implementation of retrieving the page data. Should we move to a seperate init(clusterState)
method and have construtor for only extracting paginatedQueryRequest and setting up member variables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simplified the logic and moved out the implementation to private methods. Please see if the concern is now addressed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- It might be good to define a static method
retriveIndexPageFromClusterState (clusterState, lastRetrievedPage): IndexPage
given that constructor is processing the core logic paginatedElement
can be a static constant and need not be passed by caller
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in the revision.
server/src/main/java/org/opensearch/rest/pagination/IndexBasedPaginationStrategy.java
Outdated
Show resolved
Hide resolved
|
||
public String generateEncryptedToken() { | ||
return PaginationStrategy.encryptStringToken( | ||
posToStartPage + "$" + creationTimeOfLastRespondedIndex + "$" + queryStartTime + "$" + nameOfLastRespondedIndex |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- could the
nameOfLastRespondedIndex
contain the seperator$
? - can we define constants to map the position of token to fields ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could the nameOfLastRespondedIndex contain the seperator $ ?
Yeah, sorry this was one of the open point. "$" is an allowed index name, and was kept as a placeholder.
server/src/main/java/org/opensearch/rest/pagination/IndexBasedPaginationStrategy.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/rest/pagination/IndexBasedPaginationStrategy.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/rest/pagination/IndexBasedPaginationStrategy.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/rest/pagination/IndexBasedPaginationStrategy.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Harsh Garg <gkharsh@amazon.com>
❌ Gradle check result for 0621975: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
sortedIndicesList.size() | ||
); | ||
return new PaginatedQueryResponse( | ||
positionToStartNextPage >= sortedIndicesList.size() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this check required given that you are performing min above ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revised implementation has a slightly different logic, please check.
private final String sort; | ||
private final int size; | ||
|
||
public PaginatedQueryRequest(String requested_token, String sort, int size) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: underscore in variable name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
*/ | ||
public interface PaginationStrategy<T> { | ||
|
||
String DESCENDING_SORT_PARAM_VALUE = "descending"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should ascending and descending be enum constants ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think just a static string should be sufficient. While defining the specs, we can call this out as an ENUM and provide the values inside specs itself. Please let me know if it looks incorrect.
private final PaginatedQueryResponse paginatedQueryResponse; | ||
private final List<String> indicesFromRequestedToken; | ||
|
||
public IndexBasedPaginationStrategy(PaginatedQueryRequest paginatedQueryRequest, String paginatedElement, ClusterState clusterState) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- It might be good to define a static method
retriveIndexPageFromClusterState (clusterState, lastRetrievedPage): IndexPage
given that constructor is processing the core logic paginatedElement
can be a static constant and need not be passed by caller
paginatedQueryRequest.getSort() | ||
); | ||
this.indicesFromRequestedToken = getIndicesFromRequestedToken(sortedIndicesList, paginatedQueryRequest); | ||
this.paginatedQueryResponse = getPaginatedResponseFromRequestedToken(sortedIndicesList, paginatedQueryRequest, paginatedElement); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it not the case that indicesFromRequestedToken
is the data corresponding to the page. Why are we passing sortedIndicesList
again to getPaginatedResponseFromRequestedToken
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check the new logic to get the paginated response object and let me know if it's still a concern.
* Utility method to get list of indices sorted by their creation time. | ||
*/ | ||
static List<IndexMetadata> getListOfIndicesSortedByCreateTime(final ClusterState clusterState, String sortOrder) { | ||
return clusterState.metadata().indices().values().stream().sorted((metadata1, metadata2) -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we define two comparatorFn -> asc and desc and use it based on DESCENDING_SORT_PARAM_VALUE
. This is to avoid branching inside the comparatorFn.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have defined a new comparator, but still kept only a single method. Since it's just one comparison that changes, thought it's better to avoid code duplication. But still open to defining two separate methods, if you feel that would bring out more clarity
server/src/main/java/org/opensearch/rest/pagination/IndexBasedPaginationStrategy.java
Outdated
Show resolved
Hide resolved
if (!Objects.equals(currentIndexAtLastSentPosition.getIndex().getName(), requestedToken.nameOfLastRespondedIndex)) { | ||
// case denoting already responded index/indices has/have been deleted/added in between the paginated queries. | ||
// find the index whose creation time is just after/before (based on sortOrder) the index which was last responded. | ||
if (!DESCENDING_SORT_PARAM_VALUE.equals(paginatedQueryRequest.getSort())) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets define two different nextPageFn
based on sort order. We can link them with sort enum
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have defined a new predicate for filtering and comparator for sorting, please check again.
while (requestedPageStartIndexNumber > 0 | ||
&& (sortedIndicesList.get(requestedPageStartIndexNumber - 1) | ||
.getCreationDate() > requestedToken.creationTimeOfLastRespondedIndex)) { | ||
requestedPageStartIndexNumber--; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Is it guranteed that the
prevPage
returned had all the elements less than (or) equal tocreationTimeOfLastRespondedIndex
? Can we add it to comment and relevant method in comment.
Signed-off-by: Harsh Garg <gkharsh@amazon.com>
❌ Gradle check result for 3bedc82: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
@@ -57,9 +59,11 @@ | |||
*/ | |||
public abstract class AbstractCatAction extends BaseRestHandler { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we want to change AbstractCatAction at all? we can keep the changes in AbstractListAction?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added AbstractListAction which is now extending AbstractCatAction as discussed, please see if it resolves the concern now.
if (!Objects.equals(paginatedQueryRequest.getSort(), "ascending") | ||
&& !Objects.equals(paginatedQueryRequest.getSort(), "descending")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just use asc or desc and also use constants instead of hard coded values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in the revision.
return sort; | ||
} | ||
|
||
public String getRequestedTokenStr() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getRequestedToken remove Str
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in the revision.
* Class specific to paginated queries, which will contain common query params required by a paginated API. | ||
*/ | ||
@PublicApi(since = "2.17.0") | ||
public class PaginatedQueryRequest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just PaginatedRequestParams
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed to PageParams as discussed.
* @return boolean denoting whether the RestAction will output paginated responses or not. | ||
* Is kept false by default, every paginated action to override and return true. | ||
*/ | ||
public boolean isActionPaginated() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would it make sense to add AbstractListAction instead of adding in AbstractCatAction as list are separate APIs anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in the revision.
this.paginatedQueryResponse = getPaginatedResponseForRequestedToken(paginatedQueryRequest.getSize(), sortedIndicesList); | ||
} | ||
|
||
private static Predicate<IndexMetadata> getMetadataListFilter(String requestedTokenStr, String sortOrder) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getIndicesFilter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in the revision.
return indexMetadata -> { | ||
if (indexMetadata.getIndex().getName().equals(requestedToken.nameOfLastRespondedIndex)) { | ||
return false; | ||
} else if (indexMetadata.getCreationDate() == requestedToken.creationTimeOfLastRespondedIndex) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
creationTimeOfLastRespondedIndex - LastIndexCreationTime
nameOfLastRespondedIndex. - LastIndexName
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in the revision.
}; | ||
} | ||
|
||
private static Comparator<IndexMetadata> getMetadataListComparator(String sortOrder) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getIndexComparator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in the revision.
}; | ||
} | ||
|
||
private List<IndexMetadata> getMetadataListForRequestedToken( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getIndicesForToken
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in the revision.
return sortedIndicesList.subList(0, Math.min(paginatedQueryRequest.getSize(), sortedIndicesList.size())); | ||
} | ||
|
||
private PaginatedQueryResponse getPaginatedResponseForRequestedToken(int pageSize, List<IndexMetadata> sortedIndicesList) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getResponseForToken
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this method is not returning actual response?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed, this is just for returning the token. Renamed appropriately.
Signed-off-by: Harsh Garg <gkharsh@amazon.com>
180811d
to
9aebdce
Compare
Signed-off-by: Harsh Garg <gkharsh@amazon.com>
❌ Gradle check result for 8a39c52: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
clusterManagerNodeTimeout, | ||
client, | ||
ActionListener.wrap(groupedListener::onResponse, groupedListener::onFailure) | ||
new ActionListener<ClusterStateResponse>() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be returned from a seperate private method to avoid indentation.
protected IndexPaginationStrategy getPaginationStrategy(ClusterStateResponse clusterStateResponse) { | ||
return null; | ||
} | ||
|
||
protected PageToken getPageToken(IndexPaginationStrategy paginationStrategy) { | ||
return null; | ||
} | ||
|
||
protected String[] getIndicesToBeQueried(String[] indices, IndexPaginationStrategy paginationStrategy) { | ||
return indices; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the default implementation be moved to AbstractListAction ?
builder.startArray(table.getPageToken().getPaginatedEntity()); | ||
} else { | ||
builder.startArray(); | ||
} | ||
List<Integer> rowOrder = getRowOrder(table, request); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we avoid branching in multiple places and instead create two seperate methods - one for paginated and the other for non-paginated response ?
final String healthParam = request.param("health"); | ||
final Table table = getTableWithHeader(request); | ||
final Table table = getTableWithHeader(request, pageToken); | ||
|
||
indicesSettings.forEach((indexName, settings) -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we also use indicesToBeQueried
@Override | ||
public void onResponse(ClusterStateResponse clusterStateResponse) { | ||
IndexPaginationStrategy paginationStrategy = getPaginationStrategy(clusterStateResponse); | ||
final String[] indicesToBeQueried = getIndicesToBeQueried(indices, paginationStrategy); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the indices parameter is never used. Is it required to be passed ?
List<Integer> pageSizeList = List.of(1, 6, 10, 13); | ||
for (int pageSize : pageSizeList) { | ||
String requestedToken = null; | ||
int totalPagesToFetch = (int) Math.ceil(totalIndices / (pageSize * 1.0)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Can we add test to fetch beyond
totalIndices
null, | ||
null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a test to verify pageToken when the table is built for paginated response ?
paginationStrategy = new IndexPaginationStrategy(pageParams, clusterState); | ||
assertEquals(1, paginationStrategy.getRequestedEntities().size()); | ||
assertEquals("test-index-2", paginationStrategy.getRequestedEntities().get(0)); | ||
assertNotNull(paginationStrategy.getResponseToken().getNextToken()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we fetch all the pages and verify the list ?
paginationStrategy = new IndexPaginationStrategy(pageParams, clusterState); | ||
assertEquals(1, paginationStrategy.getRequestedEntities().size()); | ||
assertEquals("test-index-3", paginationStrategy.getRequestedEntities().get(0)); | ||
assertNotNull(paginationStrategy.getResponseToken().getNextToken()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we fetch all the pages and verify the list ?
|
||
// Adding index5 to clusterState, before executing next query. | ||
clusterState = addIndexToClusterState(clusterState, 5); | ||
pageParams = new PageParams(paginationStrategy.getResponseToken().getNextToken(), "asc", 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we test for pageSize 10 ?
Description
Changes include:
_list/indices
API which will display paginated results.Functional Testing on a local cluster:
_list API:
All the indices in the cluster
Retrieving indices in pages of size=1, using _list API, in plain text format:
Retrieving indices in pages of size=1, using _list API, in JSON format:
Related Issues
Resolves #14258
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.