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

feat: reconcile pod ephemeral metadata in parallel #4130

Merged
merged 2 commits into from
Feb 13, 2025

Conversation

com6056
Copy link
Contributor

@com6056 com6056 commented Feb 12, 2025

Right now, every pod has it's ephemeral updated serially. When dealing with a large number of pods, this can get to be quite slow, especially when you have things like validating/mutating webhooks that have to run for every request.

This updates it to simply use an errgroup with a default concurrency of 10 to update the pods in parallel, which should greatly speed things up 🎉

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this is a chore.
  • The title of the PR is (a) conventional with a list of types and scopes found here, (b) states what changed, and (c) suffixes the related issues number. E.g. "fix(controller): Updates such and such. Fixes #1234".
  • I've signed my commits with DCO
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My builds are green. Try syncing with master if they are not.
  • My organization is added to USERS.md.

Signed-off-by: Jordan Rodgers <jrodgers@figma.com>
@com6056 com6056 force-pushed the parallel-ephemeral-metadata branch from fe13e00 to 4595bcb Compare February 12, 2025 21:21
appsv1 "k8s.io/api/apps/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
replicasetutil "github.com/argoproj/argo-rollouts/utils/replicaset"
)

// DefaultEphemeralMetadataThreads is the default number of worker threads to run when reconciling ephemeral metadata
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copied verbiage from

// DefaultRolloutThreads is the default number of rollout worker threads to start with the controller
DefaultRolloutThreads = 10
// DefaultExperimentThreads is the default number of experiment worker threads to start with the controller
DefaultExperimentThreads = 10
// DefaultAnalysisThreads is the default number of analysis worker threads to start with the controller
DefaultAnalysisThreads = 30
// DefaultServiceThreads is the default number of service worker threads to start with the controller
DefaultServiceThreads = 10
// DefaultIngressThreads is the default number of ingress worker threads to start with the controller
DefaultIngressThreads = 10

@@ -299,6 +302,7 @@ func newCommand() *cobra.Command {
command.Flags().IntVar(&analysisThreads, "analysis-threads", controller.DefaultAnalysisThreads, "Set the number of worker threads for the Experiment controller")
command.Flags().IntVar(&serviceThreads, "service-threads", controller.DefaultServiceThreads, "Set the number of worker threads for the Service controller")
command.Flags().IntVar(&ingressThreads, "ingress-threads", controller.DefaultIngressThreads, "Set the number of worker threads for the Ingress controller")
command.Flags().IntVar(&ephemeralMetadataThreads, "ephemeral-metadata-threads", rollout.DefaultEphemeralMetadataThreads, "Set the number of worker threads for the Ephemeral Metadata reconciler")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copied verbiage from

command.Flags().IntVar(&rolloutThreads, "rollout-threads", controller.DefaultRolloutThreads, "Set the number of worker threads for the Rollout controller")
command.Flags().IntVar(&experimentThreads, "experiment-threads", controller.DefaultExperimentThreads, "Set the number of worker threads for the Experiment controller")
command.Flags().IntVar(&analysisThreads, "analysis-threads", controller.DefaultAnalysisThreads, "Set the number of worker threads for the Experiment controller")
command.Flags().IntVar(&serviceThreads, "service-threads", controller.DefaultServiceThreads, "Set the number of worker threads for the Service controller")
command.Flags().IntVar(&ingressThreads, "ingress-threads", controller.DefaultIngressThreads, "Set the number of worker threads for the Ingress controller")

Copy link
Contributor

github-actions bot commented Feb 12, 2025

Published E2E Test Results

  4 files    4 suites   3h 11m 1s ⏱️
113 tests 101 ✅  7 💤 5 ❌
460 runs  424 ✅ 28 💤 8 ❌

For more details on these failures, see this check.

Results for commit da425f4.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Feb 12, 2025

Published Unit Test Results

2 295 tests   2 295 ✅  2m 59s ⏱️
  128 suites      0 💤
    1 files        0 ❌

Results for commit da425f4.

♻️ This comment has been updated with latest results.

@com6056 com6056 marked this pull request as ready for review February 13, 2025 01:07
Signed-off-by: Jordan Rodgers <jrodgers@figma.com>
@com6056 com6056 force-pushed the parallel-ephemeral-metadata branch from 7023e70 to da425f4 Compare February 13, 2025 01:08
@zachaller zachaller added this to the v1.9 milestone Feb 13, 2025
@zachaller zachaller changed the title perf: reconcile pod ephemeral metadata in parallel feat: reconcile pod ephemeral metadata in parallel Feb 13, 2025
@zachaller zachaller merged commit d8994c0 into argoproj:master Feb 13, 2025
25 checks passed
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.

3 participants