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

Improve Fleet performance #12896

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Conversation

aruiz14
Copy link
Contributor

@aruiz14 aruiz14 commented Dec 17, 2024

Summary

Issue #12895

Performance improvements while keeping the same behavior.

Occurred changes and/or fixed issues

(Changes are logically split into different commits)

  • Standardize on clusterID vs. "clusterLabel" (which normally refers to the management Cluster resource name, while the Fleet Cluster name is usually the same as the "display" name).
    • The clusterId is compound by ${namespace}/${name} of the Cluster resource, making it easier to work with.
    • The management "Cluster" object (clusterLabel) is only really needed for calculating the detailLocation of deployed resouces, as they will link to the Explorer, which uses the management clusters' names.
  • Refactor usages of clusterResourceStatus, which is always combined with a find by clusterLabel (now clusterId).
    • Used to obtain the resource statuses count per cluster.
    • Used to get the cluster state (based in the resource statuses aggregation).
    • This was being performed for every cluster individually, so it impacts performance exponentially to the number downstream clusters.
    • Reworking this allows to finally get rid of this function completely 🎉 .
  • The Continuous Delivery dashboard iterates over all gitrepos/bundles/clusters in order obtain the badges and tooltip for every single entry in the table.
    • I've computed the necessary information in a single place and passed it to the relevant functions so they can easily pick what they need, hence greatly reducing the number of iterations (see screenshot below)
  • Small optimizations (e.g. use a variable to avoid calling the same getters multiple times within the same function).

Technical notes summary

Above changes were driven by capturing and analyzing Performance traces with the Chrome Dev Tools, on a local Rancher installation where many sample workloads were installed.
See a summary of CPU usage over 30s of loading the CD dashboard:

  • Before:
    Screenshot 2024-12-10 at 12 42 50
  • After:
    Screenshot 2024-12-10 at 12 42 59

Areas or cases that should be tested

  • Continuous Delivery:
    • Dashboard
    • Cluster list
    • Cluster details
    • GitRepo list
    • GitRepo details

Areas which could experience regressions

These changes aim for improving performance while not introducing any changes to the information being displayed.
I've performed several tests using a local UI build and couldn't find any difference, but I may have missed something.

Checklist

  • The PR is linked to an issue and the linked issue has a Milestone, or no issue is needed
  • The PR has a Milestone
  • The PR template has been filled out
  • The PR has been self reviewed
  • The PR has a reviewer assigned
  • The PR has automated tests or clear instructions for manual tests and the linked issue has appropriate QA labels, or tests are not needed
  • The PR has reviewed with UX and tested in light and dark mode, or there are no UX changes

@aruiz14 aruiz14 added this to the v2.11.0 milestone Dec 17, 2024
@aruiz14 aruiz14 requested a review from richard-cox December 17, 2024 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant