Skip to content
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

kubectl get doesn't stream results with many results #866

Open
apelisse opened this issue May 4, 2020 · 18 comments
Open

kubectl get doesn't stream results with many results #866

apelisse opened this issue May 4, 2020 · 18 comments
Assignees
Labels
area/kubectl kind/bug Categorizes issue or PR as related to a bug. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/P2 sig/cli Categorizes an issue or PR as relevant to SIG CLI.

Comments

@apelisse
Copy link
Member

apelisse commented May 4, 2020

I've got a cluster here with many many objects of a specific resource type (probably too many), and I'm trying to list them anyway:

$ kubectl get myresourcetype
<hangs for ever, or not but who knows>

If I run the same request with v8, I can see that it's running a lot of requests to the apiserver, with the continue token etc. But it looks like none of these resources are going to be printed before they are all collected.

There is a chunk-size parameter, but that only guides the number of objects returned by each request, not how things are going to be displayed, there's also no limit parameter.

That seems wrong to me.

/assign @soltysh @seans3
cc @lavalamp

@seans3
Copy link
Contributor

seans3 commented May 27, 2020

/sig cli
/area kubectl
/kind bug

@k8s-ci-robot k8s-ci-robot added sig/cli Categorizes an issue or PR as relevant to SIG CLI. area/kubectl kind/bug Categorizes issue or PR as related to a bug. labels May 27, 2020
@apelisse
Copy link
Member Author

I would maybe consider this a feature more than a bug, but it's debatable :-)

@eddiezane
Copy link
Member

I was able to replicate this by creating 100k configmaps. I'll try poking around and see what I can figure out.

@eddiezane
Copy link
Member

I've tracked this down to being a bottleneck here:

https://github.com/kubernetes/kubernetes/blob/e422e9a3f41bfdf27c4bd2ebfabff40fe7a8b1e9/staging/src/k8s.io/kubectl/pkg/cmd/get/get.go#L498

Specifically here:

https://github.com/kubernetes/kubernetes/blob/e422e9a3f41bfdf27c4bd2ebfabff40fe7a8b1e9/staging/src/k8s.io/cli-runtime/pkg/resource/result.go#L122

Issue is in fact the chunk-size. By default it's 500 which means that this loop makes 200 requests to get my 100k configmaps.

Passing --chunk-size 100000 makes the command run nearly instantly because the API server can handle the request.

./_output/bin/kubectl get configmaps  4.98s user 0.98s system 14% cpu 41.915 total

vs

./_output/bin/kubectl get configmaps --chunk-size 100000  3.62s user 0.56s system 78% cpu 5.307 total

Where to go from here I'm unsure. I guess we'd need to refactor the Infos()/Visit() calls to pass results back in a channel. Might not be worth the effort?

Thoughts @apelisse?

cc @soltysh @seans3 @brianpursley

@apelisse
Copy link
Member Author

apelisse commented Jun 3, 2020

I suspect there are two things that are going to make the non-caching behavior happen:

  1. If we have to sort the results, we need to receive them all fist. is there a non-sorting option? Can you look at the options here, maybe we might have an option that would say "don't sort just give me the results as fast as you can". 1
  2. I believe we do something relative to printing the header for the columns based on the content, so we need to know the length of names before we can print the columns. I would look at what is done for multi-gvk 2

And I may be missing other reasons.

Hope that helps, happy to answer further questions

@eddiezane
Copy link
Member

Hmmm

  1. As of now when sorting is enabled we set chunk-size = 0 to fetch in one request.

https://github.com/kubernetes/kubernetes/blob/e422e9a3f41bfdf27c4bd2ebfabff40fe7a8b1e9/staging/src/k8s.io/kubectl/pkg/cmd/get/get.go#L464-L468

  1. I'm not too sure on this one but Infos() is already called to fetch the requests before we reach the multi-gvk stuff.

@apelisse
Copy link
Member Author

apelisse commented Jun 3, 2020

I'm not too sure on this one but Infos() is already called to fetch the requests before we reach the multi-gvk stuff.

Yeah, keep digging there, you might find something useful.

https://github.com/kubernetes/kubernetes/blob/e422e9a3f41bfdf27c4bd2ebfabff40fe7a8b1e9/staging/src/k8s.io/kubectl/pkg/cmd/get/get.go#L464-L468

Isn't it sorting by default? I'm a little confused. I should take a minute to look at the sorting option for kubectl get.

@eddiezane
Copy link
Member

@apelisse
Copy link
Member Author

apelisse commented Jun 4, 2020

Sure, can you dig deeper and make sure that they are not eventually sorted anyway on a simple get? If there is no sorting involved, that would probably make things much simpler.

And also, I would suggest that you keep digging on this multi-gvk thing.

Thanks @eddiezane!

@eddiezane
Copy link
Member

/priority P2

@brianpursley
Copy link
Member

brianpursley commented Jun 24, 2020

@eddiezane @apelisse I think this is a throttling issue. Take a look at this PR and let me know what you think: kubernetes/kubernetes#92483

EDIT: I guess this doesn't fix the original issue, but fixes a related problem of poor performance with chunking large lists. The original issue of not being able to see partial results before all chunks are received remains.

@brianpursley
Copy link
Member

I looked into this a little more and discovered that in order to stream results as they are received, kubectl get needs to be changed to use Visit() instead of Infos()

https://github.com/kubernetes/kubernetes/blob/6d7a9048b67d81e57923892650a23636f1afba42/staging/src/k8s.io/cli-runtime/pkg/resource/result.go#L91-L101

Visit() would allow row-by-row printing, whereas Infos() returns the entire result after it is received.

It isn't too straightforward though because Visit would not work in the case of sorting, so it would still need to keep the current ability to get all results up-front.

@seans3
Copy link
Contributor

seans3 commented Jun 24, 2020

This (sorting) was one of the reasons we changed to Infos(). In addition, we've added pre-processing and post-processing hooks in apply. The sorting/ordering of resources as well as filtering can happen in the pre-processing hook. The post-processing hook is now where the prune functionality lives.

@apelisse
Copy link
Member Author

Agree about the post/pre processing in apply, but I'm hoping this is unrelated because it's get. I agree that sorting will make it difficult, but in that case, would be nice to have an option to disable sorting altogether in order to stream the results

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 22, 2020
@seans3
Copy link
Contributor

seans3 commented Sep 23, 2020

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 23, 2020
@seans3
Copy link
Contributor

seans3 commented Sep 23, 2020

/lifecycle frozen

@k8s-ci-robot k8s-ci-robot added the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Sep 23, 2020
@eddiezane
Copy link
Member

cc @KnVerey

@helayoty helayoty added this to SIG CLI Oct 2, 2023
@github-project-automation github-project-automation bot moved this to Needs Triage in SIG CLI Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/kubectl kind/bug Categorizes issue or PR as related to a bug. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/P2 sig/cli Categorizes an issue or PR as relevant to SIG CLI.
Projects
Status: Needs Triage
Development

No branches or pull requests

7 participants