hydrator: Concurrency for the manifest hydration queue

[julianderks]

First off, I'm a big fan of the Source Hydrator, and of the new GitOps Promoter. We're building a hub-spoke architecture across multiple clusters using the rendered-manifest pattern with gated promotions, and the two have been great to build on.

In that setup a single hub Argo CD instance hydrates the source repos for all the spokes, and hydration is starting to become a bottleneck, which is what this is about.

Summary

Hydration seems to process one source repo at a time. When commits land in several repos at once, the hydrations run sequentially rather than in parallel.

Could the concurrency of the hydration queue be made tunable? or, if it is meant to be sequential, what is the intended way to scale hydration as the number of source repos grows?

Motivation

We run a hub-spoke Argo CD topology: one hub Argo CD instance manages Applications across many spoke clusters. Each app has its own source repository, and we use the Source Hydrator together with GitOps Promoter for the rendered-manifest pattern with gated promotions between environments.

When commits land in many of those repos around the same time, for example a platform-wide change, or several teams pushing at once, hydration handles them one repo after another. As the number of apps and repos grows, this is becoming the limiting factor for how quickly changes become deployable.

Proposal

I don't know the Argo CD internals myself. An AI coding agent looked into the code and suggested a possible cause, I'm including it here as a lead for the maintainers, not as a confident diagnosis.

It pointed at the hydration queue being drained by a single goroutine in
controller/appcontroller.go:

go wait.Until(func() {
    for ctrl.processHydrationQueueItem() {}
}, time.Second, ctx.Done())

and noted that the status and operation queues are processed by a configurable number of workers (--status-processors / --operation-processors), while this one does not appear to be.

If that is the cause, a configurable processor count for the hydration queue might be one option. But there may be reasons it is sequential that I'm not aware of. I'd appreciate the maintainers' view on whether the current behaviour is intentional and what the right approach would be. Happy to test anything or help however is useful.

[crenshaw-dev]

Makes perfect sense to make these tuneable. There are definitely some risks of race conditions, but I think we could start with a naiive implementation and iterate from there. Do you have time to open up the initial PR?

[GuruduGanesh]

Thanks @crenshaw-dev — happy to take the initial PR. Since @julianderks mentioned they're not familiar with the internals, glad to pick this up.

I went through the controller path and agree this is a good candidate for a small, conservative first PR.

Current behavior

Area	Current behavior
Application status queue	Configurable workers via --status-processors / ARGOCD_APPLICATION_CONTROLLER_STATUS_PROCESSORS (default 20)
Application operation queue	Configurable workers via --operation-processors / ARGOCD_APPLICATION_CONTROLLER_OPERATION_PROCESSORS (default 10)
Hydration queue	A single hardcoded worker goroutine when the hydrator is enabled

The hydrationQueue is keyed by {SourceRepoURL, SourceTargetRevision, DestinationBranch}, and as a workqueue.TypedRateLimitingInterface it already guarantees the same key is never processed by two workers at once — so adding workers only parallelizes work across distinct keys.

Proposed implementation

Change	Reason
Add a hydrationProcessors controller option	Keeps hydration scaling independent from status/operation reconciliation
Add --hydration-processors and ARGOCD_APPLICATION_CONTROLLER_HYDRATION_PROCESSORS	Matches the existing processor-flag pattern
Default it to 1	Preserves current behavior unless explicitly tuned
Drain processHydrationQueueItem() with hydrationProcessors workers	Lets independent hydration keys progress concurrently
Thread the flag through Run(...) and controller manifests / cmd params	Makes the option usable in real deployments
Document near the status/operation processors in HA/operator docs	Explains when and how to tune it

Concurrency risks I'd validate (you flagged this — here's what I found)

Risk area	Notes / validation
Documented ordering race	hydrator.go already carries a FIXME: ProcessHydrationQueueItem can run before all apps have CurrentOperation set by ProcessAppHydrateQueueItem. More workers widens that window — I'd gate hydration to start only once all apps for the key are in HydrateOperationPhaseHydrating
Same key, multiple apps	Workqueue dedups per key, so no duplicate concurrent processing — will add a test to confirm
Git writes to hydrated branches	Confirm the commit server serializes per-repo so two keys targeting the same repo (different branches) via CommitHydratedManifests don't race; distinct repos are inherently safe
Existing hydrator lifecycle	Keep appHydrateQueue and hydrationQueue behavior unchanged except for the hydration worker count

The safest initial version keeps the default at 1 and only raises concurrency when an operator opts in, with no change to hydration semantics beyond the worker count.

If that sounds aligned, I'll open the conservative initial PR.

[crenshaw-dev]

Thanks!

Confirm the commit server serializes per-repo so two keys targeting the same repo (different branches) via CommitHydratedManifests don't race; distinct repos are inherently safe

We shouldn't serialize within a repo. We should be able to push to two different branches at the same time.

The safest initial version keeps the default at 1

We should go ahead and make it higher than 1 so that tests are more likely to catch bugs.

[GuruduGanesh]

Quick update — I've opened the initial PR (#27948). Summarizing what's included, the decisions that came out of this thread, and (flagged loudly) the extra correctness fixes beyond the original ask.

Thanks @julianderks for the detailed report and @crenshaw-dev for the steer to start with a tunable, naive implementation.

What this implements (the issue's ask)

Following the existing --status-processors / --operation-processors pattern:

• New --hydration-processors flag and ARGOCD_APPLICATION_CONTROLLER_HYDRATION_PROCESSORS env var.
• The manifest hydration queue (hydrationQueue) is now drained by N workers instead of a single goroutine. appHydrateQueue stays single-worker — it only marks each app's phase before enqueuing the deduplicated key, so it is cheap and ordering-sensitive.
• Threaded through Run(...), the application-controller StatefulSet/Deployment manifests, argocd-cmd-params-cm (controller.hydration.processors), and the HA/operator docs (plus the regenerated CLI-reference doc).

Decisions from this thread

• Default is > 1 (set to 5). Per @crenshaw-dev's point that the default should be above 1 so the default deployment exercises concurrency and tests are more likely to catch races.
• No per-repo serialization. This change does not add per-repo serialization; commit requests use isolated temp dirs, which aligns with the requested ability to push different branches of the same repo concurrently. Nothing was added here.
• Because hydration keys are {SourceRepoURL, SourceTargetRevision, DestinationBranch} on a rate-limiting workqueue, the same key is never processed by two workers at once; additional workers only parallelize hydration across distinct keys.

⚠️ Extra fixes beyond the original scope (flagging loudly)

Adding concurrency makes the pre-existing race that the FIXME in hydrator.go warned about materially more likely: a hydration item can be picked up before ProcessAppHydrateQueueItem has marked every app in the group as Hydrating, so an app can still have a nil CurrentOperation. With a single worker this was rare; with multiple workers it is common, and the original code would panic dereferencing the nil op and could overwrite stale status. So this fix is in scope for landing concurrency safely, not gold-plating.

The safe design (and the subtle part):

• Always commit the COMPLETE app set for a hydration key in one pass. The commit server records a git note per dry SHA and short-circuits any later commit for the same dry SHA before writing manifests (IsHydrated in commitserver/commit/commit.go). If we committed only the apps already marked Hydrating, a later commit for the remaining apps would be skipped by that note — they would be marked hydrated while their manifests were never written. So partial hydration must be avoided.
• Guard only the per-app status writes. Apps not yet in the Hydrating phase are skipped (their CurrentOperation may be nil, which would panic, or stale, which we must not overwrite). Their manifests are still committed; their status finalizes on a later hydration pass via the existing refresh/resync path (an app left Hydrating is re-queued once its operation is older than statusRefreshTimeout), which hits the dry-SHA note and returns the existing hydrated SHA without re-committing.
• Fixed a pre-existing data race in hydrate(): the concurrent manifest-generation goroutines wrote function-scoped pathDetails/err variables (and could append the wrong path under the mutex). They are now goroutine-local. This was caught by the race detector in make test-race-local after the multi-app hydration coverage was added.
• I also clamp --hydration-processors to a minimum of 1 — a 0/negative value on the CLI would otherwise start zero workers and silently stall hydration.

I intentionally did not implement "requeue until all apps are Hydrating": apps can share a hydration key while hydrating different paths, and appNeedsHydration is per-app, so an app may legitimately never enter Hydrating in a given cycle — waiting for all of them would loop forever.

Tests added

• Race: hydration item processed before an app is Hydrating → no panic, no status write, manifests still committed.
• Complete-path-set regression: mixed-phase apps sharing a key → asserts the single dry-SHA commit contains every app's path (the partial-hydration/git-note hazard).
• Skipped app finalizes its status on a later pass.
• Mixed-phase at scale (20 apps) → only Hydrating apps get status writes; no panics.
• Concurrency: distinct keys are processed in parallel with multiple workers; the same key is never processed concurrently (dedup guarantee).
• Worker-count clamp and the flag-default contract.

Local checks

go build, go vet, gofmt -l, and go test -race (the same race detector make test-race-local runs in CI) pass for ./controller, ./controller/hydrator, and ./cmd/argocd-application-controller/commands. Manifests/cmd-params/docs are updated and the CLI-reference doc regenerated; the full make codegen should still be confirmed in the CI environment.

Happy to tune the default value or split the race-condition handling into a follow-up if you'd prefer a more minimal initial PR — otherwise everything above is in #27948.

[GuruduGanesh]

@crenshaw-dev — when you have a chance, would appreciate eyes on #27948. No rush, just wanted to make sure it had not slipped through.

The PR implements the tunable hydration concurrency we discussed here (--hydration-processors, default 5), plus the safety fixes needed around the existing hydrator race once multiple workers are enabled.

All current CI checks are green now, including Go unit tests, race tests, e2e, generated-code checks, lint, docs, and UI.

Happy to adjust the default, split any of the safety fixes out, or rework the approach based on what you would prefer.

Thanks again for the guidance.