feat: pagninate status poller lists

karlkfi commented 2 years ago

Use the client-go ListPager to paginate requests made by the CachingClusterReader used by the StatusPoller.
Limit requests to 500 items per response (ListPager default)
Benefits:
- Reduce the length of blocking etcd operations, allowing for more responsiveness and higher throughput
- Reduce risk of request timeout
- Reduce size of response for extra large objects

Liujingfang1 commented 2 years ago

Is it possible to run the stress test check the diff of timing?

karlkfi commented 2 years ago

The only big LIST the stress test does is on the 1k namespaces. Pagination would just turn that into 2 LIST calls. It’s unlikely to have a significant impact for that scenario, especially since it’s a local kind cluster with no network latency and no other load.

Pagination is more likely to matter when there’s thousands of resources of one type in the same namespace or very large CRDs or there’s a lot of other load slowing down etcd. We don’t have a test for that yet.

Liujingfang1 commented 2 years ago

/approve

Pagination is more likely to matter when there’s thousands of resources of one type in the same namespace or very large CRDs or there’s a lot of other load slowing down etcd. We don’t have a test for that yet.

How about adding a test for this? It can be done separately.

k8s-ci-robot commented 2 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: karlkfi, Liujingfang1, mortent

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: - ~~[OWNERS](https://github.com/kubernetes-sigs/cli-utils/blob/master/OWNERS)~~ [Liujingfang1,mortent] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment

karlkfi commented 2 years ago

With Pagination:

• [SLOW TEST:897.772 seconds]
Applier
/home/karlisenberg/workspace/cli-utils/test/stress/stress_test.go:36
  StressTest
  /home/karlisenberg/workspace/cli-utils/test/stress/stress_test.go:77
    Thousand Namespaces
    /home/karlisenberg/workspace/cli-utils/test/stress/stress_test.go:98
------------------------------

Ran 1 of 1 Specs in 897.860 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 0 Skipped
PASS

Ginkgo ran 1 suite in 15m2.181886894s
Test Suite Passed

Without Pagination:

• [SLOW TEST:848.551 seconds]
Applier
/home/karlisenberg/workspace/cli-utils/test/stress/stress_test.go:36
  StressTest
  /home/karlisenberg/workspace/cli-utils/test/stress/stress_test.go:77
    Thousand Namespaces
    /home/karlisenberg/workspace/cli-utils/test/stress/stress_test.go:98
------------------------------

Ran 1 of 1 Specs in 848.618 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 0 Skipped
PASS

Ginkgo ran 1 suite in 14m13.658096361s
Test Suite Passed

5-7% time diff for the whole test may or may not be statistically significant. Could easily have just been another trip through the StatusPoller checking 3k objects. :shrug:

Liujingfang1 commented 2 years ago

Thanks for the timing data.

/lgtm

kubernetes-sigs / cli-utils

feat: pagninate status poller lists #567