Skip to content

feat(source/istio): reduce cpu cycles with indexers#5708

Closed
ivankatliarchuk wants to merge 2 commits intokubernetes-sigs:masterfrom
gofogo:istio-indexing-5458
Closed

feat(source/istio): reduce cpu cycles with indexers#5708
ivankatliarchuk wants to merge 2 commits intokubernetes-sigs:masterfrom
gofogo:istio-indexing-5458

Conversation

@ivankatliarchuk
Copy link
Copy Markdown
Member

@ivankatliarchuk ivankatliarchuk commented Aug 1, 2025

What does it do ?

TODO:

  • results from real clusters

This is a follow-up PR for #5536

  • Added indexers to speed up lookups
  • Added transfomers to reduce memory footprint

Before

BenchmarkEndpointTargetsFromServicesMedium-16                       	   11194	    105419 ns/op
BenchmarkEndpointTargetsFromServicesMediumIterateOverGateways
BenchmarkEndpointTargetsFromServicesMediumIterateOverGateways-16    	     290	   4070484 ns/op
BenchmarkEndpointTargetsFromServicesHigh
BenchmarkEndpointTargetsFromServicesHigh-16                         	      88	  12669344 ns/op
BenchmarkEndpointTargetsFromServicesHighIterateOverGateways
BenchmarkEndpointTargetsFromServicesHighIterateOverGateways-16      	       1	13425263466 ns/op

With the change

BenchmarkEndpointTargetsFromServicesMedium
BenchmarkEndpointTargetsFromServicesMedium-14                       	  586600	      2069 ns/op
BenchmarkEndpointTargetsFromServicesMediumIterateOverGateways
BenchmarkEndpointTargetsFromServicesMediumIterateOverGateways-14    	    5715	    210000 ns/op
BenchmarkEndpointTargetsFromServicesHigh
BenchmarkEndpointTargetsFromServicesHigh-14                         	  540793	      1881 ns/op
BenchmarkEndpointTargetsFromServicesHighIterateOverGateways
BenchmarkEndpointTargetsFromServicesHighIterateOverGateways-14      	       6	 192442562 ns/op

This are syntetic benchmarks. Same time the difference is 5x. So it should provide a potential improvement in terms of cpu cycles and processing times

Follow-up:

  • move optimisations for istio source; example from pod service
    err := podInformer.Informer().AddIndexers(informers.IndexerWithOptions[*corev1.Pod](
    .
  • do not make API call for ingresses, instead use caching/indexing/transformations

Motivation

PR #5473
Issue #5458
Potential why performance is poor description #5473 (comment)

More

  • Yes, this PR title follows Conventional Commits
  • Yes, I added unit tests
  • Yes, I updated end user documentation accordingly

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign raffo for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 1, 2025
@k8s-ci-robot k8s-ci-robot requested a review from szuecs August 1, 2025 12:22
@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Aug 1, 2025
@ivankatliarchuk ivankatliarchuk force-pushed the istio-indexing-5458 branch 2 times, most recently from 68af795 to 5187aa7 Compare August 1, 2025 12:31
@ivankatliarchuk ivankatliarchuk changed the title feat(source/istio): speed up processing with indexers feat(source/istio): reduce cpu cycles with indexers Aug 1, 2025
Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
Copy link
Copy Markdown
Collaborator

@mloiseleur mloiseleur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 1, 2025
@ivankatliarchuk
Copy link
Copy Markdown
Member Author

I'll block this. I think found a bug or issue that this approach may not work as expected

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 2, 2025
@ivankatliarchuk
Copy link
Copy Markdown
Member Author

Great example of unit-tests all green, but reality is different. The case I found did not have any tests coverage.

@ivankatliarchuk ivankatliarchuk marked this pull request as draft August 8, 2025 15:48
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 8, 2025
@ivankatliarchuk
Copy link
Copy Markdown
Member Author

Closing this for now. As current approach has some bugs

/close

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

@ivankatliarchuk: Closed this PR.

Details

In response to this:

Closing this for now. As current approach has some bugs

/close

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-sigs/prow repository.

@ivankatliarchuk ivankatliarchuk deleted the istio-indexing-5458 branch March 23, 2026 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. source

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants