diff --git a/docs/operator-manual/high_availability.md b/docs/operator-manual/high_availability.md index f8a86a7a39..c5970ef124 100644 --- a/docs/operator-manual/high_availability.md +++ b/docs/operator-manual/high_availability.md @@ -265,6 +265,42 @@ manifest generation requires to change a file in the local repository clone then per server instance is allowed. This limitation might significantly slow down Argo CD if you have a monorepo with multiple applications (50+). +### Use Fully Qualified Git References + +When specifying the `targetRevision` in your Application manifests, using fully qualified Git reference paths instead of short names can significantly improve repo-server performance, especially in large monorepos with hundreds of thousands of commits and tags. + +**Performance Impact:** + +When resolving a Git reference (e.g., converting `main` to a commit SHA), Argo CD needs to: + +1. Load all Git references (branches and tags) +2. Iterate through all references to find a match +3. Resolve symbolic references if needed + +For repositories with many references, this process is CPU and memory intensive. Using fully qualified references allows Argo CD to optimize the resolution process and leverage caching more effectively. + +**Recommended approach:** + +```yaml +# ❌ Less efficient - requires iteration through all refs +spec: + source: + targetRevision: main + +# ✅ More efficient - directly identifies the reference type +spec: + source: + targetRevision: refs/heads/main +``` + +**Common fully qualified reference formats:** + +* **Branches**: `refs/heads/` (e.g., `refs/heads/main`, `refs/heads/develop`) +* **Tags**: `refs/tags/` (e.g., `refs/tags/v1.0.0`) +* **Pull requests** (GitHub): `refs/pull//head` (e.g., `refs/pull/123/head`) +* **Merge requests** (GitLab): `refs/merge-requests//head` + + ### Enable Concurrent Processing Argo CD determines if manifest generation might change local files in the local repository clone based on the config diff --git a/util/git/client.go b/util/git/client.go index ed4629b683..3ee2171bbb 100644 --- a/util/git/client.go +++ b/util/git/client.go @@ -776,6 +776,9 @@ func (m *nativeGitClient) lsRemote(revision string) (string, error) { // symbolic reference (like HEAD), in which case we will resolve it from the refToHash map refToResolve := "" + isShortRef := IsShortRef(revision) + log.Debugf("Attempting to resolve revision '%s' (is short ref: %t)", revision, isShortRef) + for _, ref := range refs { refName := ref.Name().String() hash := ref.Hash().String() @@ -783,7 +786,7 @@ func (m *nativeGitClient) lsRemote(revision string) (string, error) { refToHash[refName] = hash } // log.Debugf("%s\t%s", hash, refName) - if ref.Name().Short() == revision || refName == revision { + if (isShortRef && ref.Name().Short() == revision) || refName == revision { if ref.Type() == plumbing.HashReference { log.Debugf("revision '%s' resolved to '%s'", revision, hash) return hash, nil diff --git a/util/git/git.go b/util/git/git.go index 65c6ea9b17..9cf37e9edf 100644 --- a/util/git/git.go +++ b/util/git/git.go @@ -5,6 +5,8 @@ import ( "net/url" "regexp" "strings" + + "github.com/go-git/go-git/v5/plumbing" ) // EnsurePrefix idempotently ensures that a base string has a given prefix. @@ -105,3 +107,15 @@ func TestRepo(repo string, creds Creds, insecure bool, enableLfs bool, proxy str } return nil } + +// IsShortRef determines if the supplied revision is a short ref (e.g. "master" instead of "refs/heads/master"). +// ref.Name().Short() is an expensive call to be performed in a loop over all refs in a repository, so we want to avoid calling it if we can determine up front that the supplied revision is not a short ref. +// The intention is to optimize for a case where the full ref string is supplied, as comparing the full ref string is cheaper than calling Short() on every ref in the loop. +// If the supplied revision is a short ref, we will compare it with the short version of each ref in the loop. +// If the supplied revision is not a short ref, we will compare it with the full ref string in the loop, which is cheaper than calling Short() on every ref. +// This performance optimization is based on the observation coming from larger repositories where the number of refs can be in the order of tens of thousands, +// and we want to avoid calling Short() on every ref if we can determine up front that the supplied revision is not a short ref. +func IsShortRef(revision string) bool { + refTentative := plumbing.NewReferenceFromStrings(revision, "dummyHash") + return refTentative.Name().Short() == revision +} diff --git a/util/git/git_test.go b/util/git/git_test.go index 6a844b326f..cde7dfd1c4 100644 --- a/util/git/git_test.go +++ b/util/git/git_test.go @@ -722,3 +722,122 @@ func TestAnnotatedTagHandling(t *testing.T) { // Verify tag exists in the list and points to a valid commit SHA assert.Contains(t, refs.Tags, "v1.0.0", "Tag v1.0.0 should exist in refs") } + +func TestIsShortRef(t *testing.T) { + tests := []struct { + name string + revision string + expected bool + }{ + // Short refs (should return true) + { + name: "short branch name", + revision: "master", + expected: true, + }, + { + name: "short branch name with slashes", + revision: "feature/my-feature", + expected: true, + }, + { + name: "short branch name with multiple slashes", + revision: "release/v1.0/bugfix", + expected: true, + }, + { + name: "short tag name", + revision: "v1.0.0", + expected: true, + }, + { + name: "short tag name without version prefix", + revision: "release-1.0", + expected: true, + }, + { + name: "simple branch name", + revision: "main", + expected: true, + }, + { + name: "branch with hyphens", + revision: "my-feature-branch", + expected: true, + }, + { + name: "branch with underscores", + revision: "my_feature_branch", + expected: true, + }, + // Full refs (should return false) + { + name: "full branch ref", + revision: "refs/heads/master", + expected: false, + }, + { + name: "full tag ref", + revision: "refs/tags/v1.0.0", + expected: false, + }, + { + name: "full remote branch ref", + revision: "refs/remotes/origin/master", + expected: false, + }, + { + name: "full branch ref with slashes in name", + revision: "refs/heads/feature/my-feature", + expected: false, + }, + { + name: "full tag ref with slashes in name", + revision: "refs/tags/release/v1.0.0", + expected: false, + }, + { + name: "full remote branch ref with slashes", + revision: "refs/remotes/origin/feature/branch", + expected: false, + }, + { + name: "full pull request ref", + revision: "refs/pull/123/head", + expected: false, + }, + { + name: "full notes ref", + revision: "refs/notes/commits", + expected: false, + }, + // Special cases + { + name: "HEAD", + revision: "HEAD", + expected: true, + }, + { + name: "commit SHA", + revision: "9d921f65f3c5373b682e2eb4b37afba6592e8f8b", + expected: true, + }, + { + name: "truncated commit SHA", + revision: "9d921f6", + expected: true, + }, + { + name: "empty string", + revision: "", + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := IsShortRef(tt.revision) + assert.Equal(t, tt.expected, result, "IsShortRef(%q) = %v, expected %v", tt.revision, result, tt.expected) + }) + } +}