-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[Paginating ClusterManager Read APIs] Paginate _cat/indices API. #14258
Comments
Curious to know how are we solving it.
This makes sense, helps to identify the newly created indices with descending time stamp. |
asc/ desc sorting will definitely be helpful. |
Introducing nextToken in the responseBody will change how response is parsed today |
@gargharsh3134 Regarding the question of returning JSON, the "cat" in "_cat" literally means "compact aligned text". If the proposed API is going to return JSON then it really sounds like a different API that isn't "cat". Is there an example today of a _cat API that returns something other than compact aligned text? I'm not totally opposed to changing the meaning of "cat", but you're already proposing to stick a new "v2" in the URL so maybe all that suggests this really should just be a new API all together? |
@andrross Sorry if I misunderstood your comment, but JSON format is an existing common url parameter for _cat APIs. Plain text format is however the default one, but if specified, the JSON response as of today looks like this:
The proposal is to add new keys to the JSON object in case requested format is JSON (will edit the issue to bring this out clearly). Please do let me know if you still see this as a concern or otherwise. Thanks! |
@gargharsh3134 - Given that we are proposing to sort on indices creation time ( In table format, would the last two rows returned always (even if there no tokens) ? Just thinking as to how clients will adapt to the table parsing. |
@rajiv-kv latest_indices_first and previous_token are being introduced as two independent features. latest_indices_first would be just used to define the order in which the indices are to be returned, while previous_token can be used to move between the pages (i.e. say moving from page3 back to page2).
Given that the default behaviour of V2 API will be paginated, they will always be returned (will be null if doesn't exist). Please let me know, if this behaviour seems correct to you. |
Thanks @gargharsh3134! I didn't realize the _cat APIs could already return formats other than compact aligned text. Ignore my comment. |
My two cents lets start with nextToken and avoid a previous token since it's quite possible to see inconsistent results when user performs prev and then next when the backing data/metadata has changed. Keeping it consistent would require a local copy in some cases, and since there is no keep_alive we might be holding or would require to hold certain resources for long. |
@Bukhtawar Agree, this is prone to introducing some inconsistency (as is already called out in the issue) if the already displayed indices/elements are deleted, then the previous page will not be the same as it was when last queried. I was thinking to clarify and explicitly call this behaviour out in the documentation. Please let me know if you still think that we should avoid providing previous_token. |
@gargharsh3134 : Adding Rest API versioning with v2 at the server side needs a more wider discussion. I don't want this decision to be taken in the context of limited APIs (_cat APIs here) rather think more holistically so that in future if versioning is introduced for any other API, it can follow it consistently. |
@gargharsh3134 @shwetathareja Agreed. There was some initial discussion in #3035 for this. |
Thanks @andrross #3035 this issue is more from client perspective. On the Server side to add versioning in routing path, the different proposals are:
I am in favor of Option 3 (Option 4 is also acceptable), but open for feedback if you think any other option is better. Tagging : @Bukhtawar @backslasht @andrross @dblock @reta @rajiv-kv @rwali-aws |
Thanks @shwetathareja do we also want to consider versioning using custom headers |
Thanks @Bukhtawar . I did think about header for versioning, just that it is not so obvious or visible. Also for certain APIs (like here where next_token will be required to get next pages after first page), it may require request params or body to be different so just the setting header may not be sufficient as well. Updated above as well. |
The header only serves to remove the URI clutter, while I agree it is not so obvious and that's also what de-clutters the URI. Yes it would require the params to be different but thats with all the other options too, as all of them require client side code changes. I am good with 3, 4 and 6 |
I prefer option 3, the most cleanest and less ambiguous of all. |
Thanks @shwetathareja , I would side with @Bukhtawar here and the (early) conclusions from #3035 as @andrross mentioned. We already use some versioning as part of content type (fe Alternatively, we could keep the same API endpoint
This will not introduce any changes in the payload / URL but only use hypermedia ( |
While header solves the problem of not modifying the request/response structure (directly), it is not very apparent from the user perspective. Also, high level clients (across all languages) does need to introduce new request/response structures to support pagination. |
Yes, but this is the approach OS (and ES) already offers for quite a while, this not something new we introducing
Depending on the approach, but not at all when using
breaks all command line tooling (notably, |
Thanks @shwetathareja . I think option-3 is better as we can set expectation to users that the response is partial and user needs to make follow-up calls to fetch additional pages. I think command line tooling would anyways require a change as they would have to interpret the token and accumulate responses across multiple calls. |
Thanks @Bukhtawar @backslasht @reta @rajiv-kv for the feedback. @reta I do agree with the pain of changing the custom scripts written for _cat API to handle pagination. But, the current _cat APIs were not designed considering large cluster setup (both in terms of nodes and indices/shards) in perspective. That, we all agree we need to fix. Now, the reason to keep the routing same much as possible is the familiarity of users on how they interact _cat APIs and adding headers is also a friction to users on how they interact with these APIs e.g. users using OpenSearch Dashboards to perform GET _cat/shards, doesn't look for response headers by default. I do agree with @rajiv-kv , in OpenSearch we don't use response headers to store part of the response. This is not something OpenSearch users are familiar with. It will make their diagnostics even more difficult. |
Thanks @shwetathareja , I will very much disagree with you here, we do store the information in response headers (deprecation warnings is one of them, there are more). In my opinion, the goal to keep tooling intact and change non-breaking is very important. In any case, I share just my opinion on the implementation. Thanks! |
Thanks @reta for sharing your opinion. |
+1, completely agree here.
IMHO, this is a very clean option. This also provides enough time for users to switch to new API as and when it is required (when they start using large number of indices) while the existing _cat API will work as is. |
Using new Link header ls 100% non-breaking change: existing clients are not forced to consume it, there is no any user facing change in request or/and response.
Agree with @backslasht , yet another good option to consider |
Just to clarify, existing _cat APIs wouldn't have been removed even in 3.0 (in case this caused the confusion). So we have alignment on introducing users to _list/* APIs. We are aiming to add this support for selected APIs in 2.17 itself. |
So the I do like
I am not attached to the HAL format at all, OpenSearch probably wants to develop its own page envelope. If any API can be paginated this way then clients can implement something generic for pagination too. In ruby for example we can make this very elegant and easy to paginate over a large volume of data (example from slack-ruby-client). client.cat_aliases(size: 10) # returns the first page of aliases from /_cat/aliases
client.cat_aliases(size: 10) do |alias|
# automatically paginates underneath, yields aliases one by one
end I want to call out that it's important the embedded items be in the identical format to what |
I very like this direction but it is difficult to replicate it for the
💯 |
Is your feature request related to a problem? Please describe
As the number of indices grow in an opensearch cluster, the response size and the latency of the default
_cat/indices
API increases which makes it difficult not only for the client to consume such large responses, but also stresses out the cluster by making it accumulate stats across all the indices.Thus, pagination will not only help in limiting the size of response for a single query but will also prevent the cluster from accumulating indices stats for all the indices. So, this issue tracks the approaches that can be used to paginate the response.
Describe the solution you'd like
Paginated API would require new query params and response formats to be introduced, thus it is being proposed to add new
_list/indices
API.As for the high level approach, for paginating the response, a pagination key would be required for which a deterministic order is/can be maintained/generated in the cluster. Deterministic order is required for starting a new response page from the point where the last page left. Index creation timestamps will thus be used as pagination keys.
Overview
Each index has a creation timestamp stored in IndexMetadata which is part of Metadata object of ClusterState. These creation timestamps can act as sort/pagination key using which list of indices, sorted as per their respective creation timestamps, can be generated. The generated sorted list can then be used to prepare a list of indices to be sent as a response as per the page size.
Proposed User Experience
New API Path/URL:
curl "localhost:9200/_list/indices?___
curl "localhost:9200/_list/indices/{indices}?___
where{indices}
is a comma separated list of indices.New Query Parameters:
Sample Query :
curl "localhost:9200/_list/indices?next_token=<nextToken>&size=1000&sort=asc"
curl "localhost:9200/_list/indices/{indices}?next_token=<nextToken>&size=1000&sort=asc"
New Response Parameters:
Note: The next_token would be Base64 encoded.
New Response Formats:
format=JSON: next_token, and indices will be new keys of the JSON response object.
Plain text format (or table format): next_token will be present as the last row of the table.
Proposed Pagination Behaviour
Note: The indices which might get created while the paginated queries are being executed, will be referred to as newly created indices for the following points.
Number of indices in a page will always be less than or equal to the user provided size query parameter.
Displaying newly created indices will depend on the requested sort type.
If sort is specified as "asc", then the newly created indices will be shown as part of rear end of the response pages. However, for sort specified as "desc", because the subsequent pages will only contain the indices which are older than the already displayed ones, newly created indices will be filtered out.
Any index yet to be displayed, if deleted will NOT be a part of response.
Implementation Details:
Common PageParams class which would contain parameters required for paginating APIs, and the underlying implementation to paginate would thus interact with it:
Common PageToken class which would contain response body parameters for paginated APIs, which the pagination logic should output:
As, parsing request is the responsibility of Rest Layer, the implementation to do so for paginated APIs can be abstracted out. A new AbstractListAction would be introduced, and the API to be paginated (
_list/indices
in this case) can thus extend it:Since paginating an API would require hold of clusterState to fetch metadata for indices, the core logic to paginate can only be executed after a TransportClusterStateAction has been completed. Each API has its own way of calling Transport Actions. APIs would either be making the TransportAction calls directly from the Rest layer or would have a wrapper Transport Action which would call required actions. Given these behaviours co-exist, the approach has to be agnostic of the way TransportClusterState call is being made.
Thus, the proposal is to have PaginationStrategies implementing a common Strategy interface, which the onboarding APIs can instantiate:
Related component
Cluster Manager
Describe alternatives you've considered
Re-using existing APIs by introducing versioning in Opensearch APIs.
Following URLs were taken into consideration:
_cat/v2/indices
- This messes up the API routing for all _cat APIs - Not Preferred_cat/indices/v2
- v2 will conflict with index names which are supported as filters - Not Preferredv2/_cat/indices
- More rest friendly but it would impact fine grained access control users may have configured on their clusters. Most of the APIs will still be without version in the path and specifics one will have v2 in the path - Preferred_cat/indicesV2
- This will be less friction but v2 has to be appended to every API being changed - This might also be okAlternate implementation details which were tried out:
Generating nextToken and previousToken.
During the first paginated query (when nextToken in request is null), generate the following:
queryStartTime = CreationTimeOfMostRecentlyCreatedIndex
The queryStartTime will be used to filter out any newly created indices & then passed along in the next & previous tokens for subsequent page requests. Creation time of most recently created index is being chosen to avoid using coordinator node's clock which might be skewed.
nextToken = base64Encoded("PositionOfCurrentPage'sLastIndexInSortedListOfIndices" + "$ " + "CreationTimeStampOfCurrentPage'sLastIndex" + "$" + "queryStartTime" + "NameOfCurrentPage'sLastIndex")
For previousToken, need to first get the:
previousPage'sStartingIndexPosition = max(PositionOfCurrentPage'sLastIndexInSortedListOfIndices - maxPageSize, 0)
then:
previousToken= base64Encoded("previousPage'sStartingIndexPosition" + "$ " + "CreationTimeStampOfPreviousPage'sStartingIndex" + "$" + "queryStartTime" + "NameOfPreviousPage'sStartingIndex")
Generating sorted list of indices based on creation time. To generate a sorted list, the following approaches need to be compared:
(RECOMMENDED) Sorting the list obtained from clusterState metadata at the rest layer each time a paginated query is obtained.
Pros:
* Lesser refactoring and cleaner.
Cons:
* Performance/Latency overhead of sorting list of indices for each paginated query.
Making the
indices()
map of theMetadata
object itself deterministic/sorted. The currentHashmap
implementation can be replaced with sayLinkedHashMap
and each time a new entry (index) is put/inserted into the map, a sorted map will be maintained.Pros:
* Metadata will always remain sorted and can then be directly used at the rest layer.
Cons:
Needs significant refactoring & will introduce branching because of the following issues:
* The order needs to maintained while publishing and applying the clusterState's diff.
* RemoteState has a different diff logic and does't make use of the common diff utility.
* Handling multi version nodes in a cluster will be required. While reading the state from a node on an older version, a new sorting layer would be required in
readFrom
.* Handling case when clusterState is built from state stored in Lucene index on the disc.
Maintaining a cache on each node which can be updated as part of appliers and listeners being run after each clusterState update. Also, a new TransportAction (with a term version check) might also be required for rest layer to fetch that cache.
Additional context
No response
The text was updated successfully, but these errors were encountered: