Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

configurator: optimize ConfigMap informer filtering #2000

Merged
merged 1 commit into from
Nov 6, 2020

Conversation

shashankram
Copy link
Member

Description:
This change makes use of the filtering capability in informers
to filter API responses on the server side, by using a FieldSelector
to filter on the name of the ConfigMap as opposed to filtering all
objects on the client (osm-controller) side. The informer is already
namespaced, so the namespace and name should be enough to accurately
only have OSM's ConfigMap in the informer cache and events.

This change will also reduce the number of ConfigMap related events
the controller will receive when there are additional ConfigMaps in
the controller's namespace (ex. Grafana's ConfigMap when Grafana is
deployed as a part of control plane), and thus improving performance
in such scenarios. The informer resyncs are on a per resource basis,
so unnecessary events can take up processing resources.

Affected area:

  • New Functionality [ ]
  • Documentation [ ]
  • Install [ ]
  • Control Plane [X]
  • CLI Tool [ ]
  • Certificate Management [ ]
  • Networking [ ]
  • Metrics [ ]
  • SMI Policy [ ]
  • Security [ ]
  • Tests [ ]
  • CI System [ ]
  • Performance [X]
  • Other [ ]

Please answer the following questions with yes/no.

  • Does this change contain code from or inspired by another project? If so, did you notify the maintainers and provide attribution?
    No

This change makes use of the filtering capability in informers
to filter API responses on the server side, by using a FieldSelector
to filter on the name of the ConfigMap as opposed to filtering all
objects on the client (osm-controller) side. The informer is already
namespaced, so the namespace and name should be enough to accurately
only have OSM's ConfigMap in the informer cache and events.

This change will also reduce the number of ConfigMap related events
the controller will receive when there are additional ConfigMaps in
the controller's namespace (ex. Grafana's ConfigMap when Grafana is
deployed as a part of control plane), and thus improving performance
in such scenarios. The informer resyncs are on a per resource basis,
so unnecessary events can take up processing resources.
@shashankram shashankram requested a review from a team as a code owner November 6, 2020 17:41
@codecov-io
Copy link

Codecov Report

Merging #2000 into main will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2000   +/-   ##
=======================================
  Coverage   56.56%   56.56%           
=======================================
  Files         141      141           
  Lines        5677     5677           
=======================================
  Hits         3211     3211           
  Misses       2463     2463           
  Partials        3        3           
Impacted Files Coverage Δ
pkg/configurator/client.go 89.47% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 49a8099...16a5e80. Read the comment docs.

Copy link
Contributor

@draychev draychev left a comment

Choose a reason for hiding this comment

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

Good stuff!
Thank you @shashankram !

@shashankram shashankram merged commit 1290720 into openservicemesh:main Nov 6, 2020
@shashankram shashankram deleted the cfg-informer-filter branch November 6, 2020 18:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants