-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
Updating the api chunking proposal to add remainingItemCount #3710
Conversation
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.
Adding some detail in use cases of what this is for, how exact it has to be, and why we should add it would be my request.
@@ -78,6 +80,7 @@ The server **may** limit the amount of time a continue token is valid for. Clien | |||
|
|||
The server **must** support `continue` tokens that are valid across multiple API servers. The server **must** support a mechanism for rolling restart such that continue tokens are valid after one or all API servers have been restarted. | |||
|
|||
The `remainingItemCount` returned by the server is the number of subsequent items in the list which are not included in this list response. If the list request contained label or field selectors, then the number of remaining items is unknown and this field will be unset and omitted during serialization. If the list is complete (either because the list request is not a chunking one or because this is the last chunk), then there are no more remaining items and this field is unset and is omitted during serialization. Servers older than v1.15 omit this field in their response to any list 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.
Is it required to be exact? What’s the use case?
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.
What happens when I’m halfway through a list?
@@ -91,6 +94,8 @@ Implementations that cannot offer consistent ranging (returning a set of results | |||
|
|||
For etcd3 the continue token would contain a resource version (the snapshot that we are reading that is consistent across the entire LIST) and the start key for the next set of results. Upon receiving a valid continue token the apiserver would instruct etcd3 to retrieve the set of results at a given resource version, beginning at the provided start key, limited by the maximum number of requests provided by the continue token (or optionally, by a different limit specified by the client). If more results remain after reading up to the limit, the storage should calculate a continue token that would begin at the next possible key, and the continue token set on the returned list. | |||
|
|||
etcd3 returns the total number of keys within the range as `response.Count` if the read is a range read. The storage checks if the list request contained label or field selector. If not, the storage calculates the `remainingItemCount` as `response.Count-limit`. |
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.
If so, what happens?
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: caesarxuchao If they are not already assigned, you can assign the PR to them by writing The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
5716c31
to
58aa279
Compare
@smarterclayton I added use cases and why we needed the field. PTAL. The remainingItemCount is exact. The doc never suggests that the count is an approximation, so I think readers will get it. |
That's actually my concern, that we enshrine an exact count as part of the API. I would generally want to be as vague as possible to leave open the door for either relaxed semantics (performance) or alternate backends (proxies that can't necessarily know). |
Also add what feature flag gate this will be under and what release it will be promoted (1.16 is ok for promotion to GA if we get good data in 1.15). |
|
||
For example, assuming there are 1450 pods in the cluster when the client sends a chunking list request for pods, with `limit` set to 500. The first chunked list response contains 500 pods, a `continue` token, and with `metadata.remainingItemCount` set to 950. If the client use the `continue` token to continue listing, the server returns the second chunked list response containing the next 500 pods, another `continue` token, and with `metadata.remainingItemCount` set to 450. Note that the `remainingItemCount` is calculated based on the consistent list taken at the first chunking list request, that is, no matter if pods are created or deleted between the first and the second chunking list requests, the `metadata.remainingItemCount` in the second list response is always set to 450. | ||
|
||
The `remainingItemCount` offers a simple and efficient way to get the count of objects of a resource type. For example, to get the total count of all pods in the cluster, simply sends `GET /api/v1/pods?limit=1` to the apiserver. Similarly, one can get the total count of all pods in a namespace. Note that the `remainingItemCount` is set to 0 when the list request contains any label or field selector, so this feature cannot be used to count the number of objects matching specific label or field selectors. Without the `remainingItemCount` feature, one needs to list all pods to get a count. |
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.
Since old servers do not support this, and I don't think we can require this value in all possible future implementations, you need to specifically describe to a client how they can tell when the server doesn't support this field, and instruct them how to correctly detect it (as other sections do).
Effectively the algorithm is:
- Make a limit request
- Observe whether continue is set (if not, you have only one or zero items)
- Observe whether remainingItemCount is set (if not, remainingItemCount isn't supported)
Hrm, I didn't see a description of the use cases in the form I would expect. I.e. "we want to let people estimate the size of the collection", "we want to make this part of a UI" (I hope it's the former rather than the latter, because we discouraged use of chunking for human UI viewing). |
Yeah, it's the former, specifically we want to estimate when the storage
migrator will finish a migration.
*From: *Clayton Coleman <notifications@github.com>
*Date: *Tue, May 14, 2019 at 7:56 AM
*To: *kubernetes/community
*Cc: *Daniel Smith, Mention
Hrm, I didn't see a description of the use cases in the form I would
… expect. I.e. "we want to let people estimate the size of the collection",
"we want to make this part of a UI" (I hope it's the former rather than the
latter, because we discouraged use of chunking for human UI viewing).
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3710?email_source=notifications&email_token=AAE6BFTIC6WM77AAYMEV7DTPVLHIDA5CNFSM4HMU6A3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVLX5LA#issuecomment-492273324>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAE6BFWPBD4GPFOEPSJIHALPVLHIDANCNFSM4HMU6A3A>
.
|
RE. concerns over the "exact count", I added "The intended use of the RE. detecting if RE. feature flag, I'll wait for you and Daniel to converge. See Daniel's comment at kubernetes/kubernetes#75993 (comment). Starting the field as beta sounds more useful to me. RE. use cases, perhaps I didn't understand your request, let me know if the current wording is still too vague. |
Added feature flag. @smarterclayton PTAL. Thank you. |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
Stale issues rot after 30d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
Rotten issues close after 30d of inactivity. Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
@fejta-bot: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/sig api-machinery
Retrospectively updating the proposal for api-chunking for the remainingItemCount API change introduced in kubernetes/kubernetes#75993.
/assign @smarterclayton
cc @lavalamp