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

make Reconcile only when related Resources changed #1373

Closed
spwangxp opened this issue Apr 28, 2023 · 11 comments
Closed

make Reconcile only when related Resources changed #1373

spwangxp opened this issue Apr 28, 2023 · 11 comments
Assignees
Labels
area/performance kind/refactor provider/kubernetes Issues related to the Kubernetes provider
Milestone

Comments

@spwangxp
Copy link
Contributor

Description:

if a cr httproute changed, envoyproxy shouldn't be build and compare, just need sync the rule. it's will make logical clear and easy to maintain.

0414db2f27d90c5ecc5966db17f4c51

@spwangxp
Copy link
Contributor Author

@qicz

@qicz
Copy link
Member

qicz commented Apr 28, 2023

@qicz

thanks @spwangxp I will fix this

@qicz qicz self-assigned this Apr 28, 2023
@qicz qicz added provider/kubernetes Issues related to the Kubernetes provider kind/refactor labels Apr 28, 2023
@arkodg
Copy link
Contributor

arkodg commented Apr 28, 2023

is this a regression ?
imo this shouldnt be happening if the InfraIR content is not changing

gets called (Update) only when the value within InfraIR has changed

@arkodg
Copy link
Contributor

arkodg commented Apr 28, 2023

nvm, this is a performance optimization in the provider layer to reduce the k8s List calls for EnvoyProxy .

if err := r.processParamsRef(ctx, acceptedGC, resourceTree); err != nil {
can be selectively called/computed only if the Name, Namespace & ObservedGeneration within the EnvoyProxy within the resourceTree has changed

@arkodg
Copy link
Contributor

arkodg commented Apr 28, 2023

relates to #1365

@arkodg arkodg added this to the 0.5.0-rc1 milestone Apr 28, 2023
@qicz
Copy link
Member

qicz commented Apr 29, 2023

is this a regression ? imo this shouldnt be happening if the InfraIR content is not changing

gets called (Update) only when the value within InfraIR has changed

It is right, but the controller did more unnecessary processing

@qicz
Copy link
Member

qicz commented Apr 29, 2023

nvm, this is a performance optimization in the provider layer to reduce the k8s List calls for EnvoyProxy .

if err := r.processParamsRef(ctx, acceptedGC, resourceTree); err != nil {

can be selectively called/computed only if the Name, Namespace & ObservedGeneration within the EnvoyProxy within the resourceTree has changed

IMO, we should check all Kind that the controller watched in resourceTree

@arkodg
Copy link
Contributor

arkodg commented Apr 29, 2023

thinking again, this might be a premature optimization because, the contents from List should be cached

@qicz
Copy link
Member

qicz commented Apr 29, 2023

thinking again, this might be a premature optimization because, the contents from List should be cached

yep, the watchable.Map just handle the change. check that all Kinds are compared resourceTree and the k8s stored.

the compare logic is a matter of course. cc @spwangxp

@spwangxp
Copy link
Contributor Author

ok, got it. thx!

@qicz
Copy link
Member

qicz commented May 4, 2023

close this issue now.

if you have another issue pls raise it. thanks @spwangxp

@qicz qicz closed this as completed May 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/performance kind/refactor provider/kubernetes Issues related to the Kubernetes provider
Projects
None yet
Development

No branches or pull requests

3 participants