Elgato_dark/argo-cd
1
0
Fork 0
mirror of https://github.com/argoproj/argo-cd synced 2026-04-21 08:57:17 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 14

fix(repo-server): optimize repoLock on checkout for manifest-generate-paths (#26468)

Browse source 
Signed-off-by: Artem Vdovin <arte.vdovin@gmail.com>
This commit is contained in:
Artem Vdovin 2026-02-26 22:15:52 +05:00 committed by GitHub
parent af05c56bc4
commit ff8305694f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 124 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

17
reposerver/cache/cache.go vendored
View file

@ -520,6 +520,23 @@ func (c *Cache) GetGitDirectories(repoURL, revision string) ([]string, error) {
return item, err
}
func getGitFilesChangesKey(repoURL, revision, targetRevision string) string {
return fmt.Sprintf("gitFilesChanges|%s|%s|%s", repoURL, revision, targetRevision)
}
func (c *Cache) SetGitFilesChanges(repoURL, revision, targetRevision string, files []string) error {
return c.cache.SetItem(
getGitFilesChangesKey(repoURL, revision, targetRevision),
&files,
&cacheutil.CacheActionOpts{Expiration: c.repoCacheExpiration})
}
func (c *Cache) GetGitFilesChanges(repoURL, revision, targetRevision string) ([]string, error) {
var files []string
err := c.cache.GetItem(getGitFilesChangesKey(repoURL, revision, targetRevision), &files)
return files, err
}
func (cmr *CachedManifestResponse) shallowCopy() *CachedManifestResponse {
if cmr == nil {
return nil

38
reposerver/cache/cache_test.go vendored
View file

@ -750,3 +750,41 @@ func TestGetGitFiles(t *testing.T) {
fixtures.mockCache.AssertCacheCalledTimes(t, &mocks.CacheCallCounts{ExternalGets: 1, ExternalSets: 1})
})
}
func TestGetGitFilesChanges(t *testing.T) {
t.Run("GetGitFilesChanges cache miss", func(t *testing.T) {
fixtures := newFixtures()
t.Cleanup(fixtures.mockCache.StopRedisCallback)
directories, err := fixtures.cache.GetGitFilesChanges("test-repo", "test-revision", "syncedRevision")
require.ErrorIs(t, err, ErrCacheMiss)
assert.Empty(t, directories)
fixtures.mockCache.AssertCacheCalledTimes(t, &mocks.CacheCallCounts{ExternalGets: 1})
})
t.Run("GetGitFilesChanges cache hit", func(t *testing.T) {
fixtures := newFixtures()
t.Cleanup(fixtures.mockCache.StopRedisCallback)
cache := fixtures.cache
expectedItem := []string{"test/path/file-1.yaml", "test/path1/file-2.yaml"}
err := cache.cache.SetItem(
getGitFilesChangesKey("test-repo", "test-revision", "syncedRevision"),
expectedItem,
&cacheutil.CacheActionOpts{Expiration: 30 * time.Second})
require.NoError(t, err)
files, err := fixtures.cache.GetGitFilesChanges("test-repo", "test-revision", "syncedRevision")
require.NoError(t, err)
assert.Equal(t, expectedItem, files)
fixtures.mockCache.AssertCacheCalledTimes(t, &mocks.CacheCallCounts{ExternalGets: 1, ExternalSets: 1})
})
t.Run("SetGitFilesChanges", func(t *testing.T) {
fixtures := newFixtures()
t.Cleanup(fixtures.mockCache.StopRedisCallback)
expectedItem := []string{"test/path/file-1.yaml", "test/path1/file-2.yaml"}
err := fixtures.cache.SetGitFilesChanges("test-repo", "test-revision", "syncedRevision", expectedItem)
require.NoError(t, err)
files, err := fixtures.cache.GetGitFilesChanges("test-repo", "test-revision", "syncedRevision")
require.NoError(t, err)
assert.Equal(t, expectedItem, files)
fixtures.mockCache.AssertCacheCalledTimes(t, &mocks.CacheCallCounts{ExternalGets: 1, ExternalSets: 1})
})
}

43
reposerver/repository/repository.go
View file

@ -3061,24 +3061,41 @@ func (s *Service) gitSourceHasChanges(repo *v1alpha1.Repository, revision, synce
return revision, syncedRevision, false, nil
}
s.metricsServer.IncPendingRepoRequest(repo.Repo)
defer s.metricsServer.DecPendingRepoRequest(repo.Repo)
getGitFilesChanges := func() ([]string, error) {
var files []string
closer, err := s.repoLock.Lock(gitClient.Root(), revision, true, func() (goio.Closer, error) {
return s.checkoutRevision(gitClient, revision, false, 0)
})
if err != nil {
return revision, syncedRevision, true, status.Errorf(codes.Internal, "unable to checkout git repo %s with revision %s: %v", repo.Repo, revision, err)
}
defer utilio.Close(closer)
s.metricsServer.IncPendingRepoRequest(repo.Repo)
defer s.metricsServer.DecPendingRepoRequest(repo.Repo)
if err := s.fetch(gitClient, []string{syncedRevision}); err != nil {
return revision, syncedRevision, true, status.Errorf(codes.Internal, "unable to fetch git repo %s with syncedRevisions %s: %v", repo.Repo, syncedRevision, err)
closer, err := s.repoLock.Lock(gitClient.Root(), revision, true, func() (goio.Closer, error) {
return s.checkoutRevision(gitClient, revision, false, 0)
})
if err != nil {
return files, status.Errorf(codes.Internal, "unable to checkout git repo %s with revision %s: %v", repo.Repo, revision, err)
}
defer utilio.Close(closer)
if err := s.fetch(gitClient, []string{syncedRevision}); err != nil {
return files, status.Errorf(codes.Internal, "unable to fetch git repo %s with syncedRevisions %s: %v", repo.Repo, syncedRevision, err)
}
files, err = gitClient.ChangedFiles(syncedRevision, revision)
if err != nil {
return files, status.Errorf(codes.Internal, "unable to get changed files for repo %s with revision %s: %v", repo.Repo, revision, err)
}
return files, nil
}
files, err := gitClient.ChangedFiles(syncedRevision, revision)
files, err := s.cache.GetGitFilesChanges(repo.Repo, revision, syncedRevision)
if err != nil {
return revision, syncedRevision, true, status.Errorf(codes.Internal, "unable to get changed files for repo %s with revision %s: %v", repo.Repo, revision, err)
files, err = getGitFilesChanges()
if err != nil {
return revision, syncedRevision, true, err
}
}
if err := s.cache.SetGitFilesChanges(repo.Repo, revision, syncedRevision, files); err != nil {
log.Warnf("Failed to store git files changes for `%s` repo in `%s...%s` with: %v", repo.Repo, revision, syncedRevision, err)
}
changed := false

55
reposerver/repository/repository_test.go
View file

@ -3915,12 +3915,13 @@ func TestUpdateRevisionForPaths(t *testing.T) {
previousRevision string
}
tests := []struct {
name string
fields fields
args args
want *apiclient.UpdateRevisionForPathsResponse
wantErr assert.ErrorAssertionFunc
cacheHit *cacheHit
name string
fields fields
args args
want *apiclient.UpdateRevisionForPathsResponse
wantErr assert.ErrorAssertionFunc
cacheHit *cacheHit
cacheCallCount *repositorymocks.CacheCallCounts
}{
{name: "NoPathAbort", fields: func() fields {
s, _, c := newServiceWithOpt(t, func(gitClient *gitmocks.Client, _ *helmmocks.Client, _ *ocimocks.Client, _ *iomocks.TempPaths) {
@ -3961,7 +3962,11 @@ func TestUpdateRevisionForPaths(t *testing.T) {
},
}, want: &apiclient.UpdateRevisionForPathsResponse{
Revision: "632039659e542ed7de0c170a4fcc1c571b288fc0",
}, wantErr: assert.NoError},
}, wantErr: assert.NoError, cacheCallCount: &repositorymocks.CacheCallCounts{
ExternalRenames: 0,
ExternalGets: 0,
ExternalSets: 0,
}},
{name: "ChangedFilesDoNothing", fields: func() fields {
s, _, c := newServiceWithOpt(t, func(gitClient *gitmocks.Client, _ *helmmocks.Client, _ *ocimocks.Client, paths *iomocks.TempPaths) {
gitClient.EXPECT().Init().Return(nil)
@ -3994,7 +3999,11 @@ func TestUpdateRevisionForPaths(t *testing.T) {
}, want: &apiclient.UpdateRevisionForPathsResponse{
Revision: "632039659e542ed7de0c170a4fcc1c571b288fc0",
Changes: true,
}, wantErr: assert.NoError},
}, wantErr: assert.NoError, cacheCallCount: &repositorymocks.CacheCallCounts{
ExternalRenames: 0,
ExternalGets: 1,
ExternalSets: 1,
}},
{name: "NoChangesUpdateCache", fields: func() fields {
s, _, c := newServiceWithOpt(t, func(gitClient *gitmocks.Client, _ *helmmocks.Client, _ *ocimocks.Client, paths *iomocks.TempPaths) {
gitClient.EXPECT().Init().Return(nil)
@ -4036,6 +4045,10 @@ func TestUpdateRevisionForPaths(t *testing.T) {
}, wantErr: assert.NoError, cacheHit: &cacheHit{
previousRevision: "1e67a504d03def3a6a1125d934cb511680f72555",
revision: "632039659e542ed7de0c170a4fcc1c571b288fc0",
}, cacheCallCount: &repositorymocks.CacheCallCounts{
ExternalRenames: 1,
ExternalGets: 1,
ExternalSets: 1,
}},
{name: "NoChangesHelmMultiSourceUpdateCache", fields: func() fields {
s, _, c := newServiceWithOpt(t, func(gitClient *gitmocks.Client, _ *helmmocks.Client, _ *ocimocks.Client, paths *iomocks.TempPaths) {
@ -4078,6 +4091,10 @@ func TestUpdateRevisionForPaths(t *testing.T) {
}, wantErr: assert.NoError, cacheHit: &cacheHit{
previousRevision: "1e67a504d03def3a6a1125d934cb511680f72555",
revision: "632039659e542ed7de0c170a4fcc1c571b288fc0",
}, cacheCallCount: &repositorymocks.CacheCallCounts{
ExternalRenames: 1,
ExternalGets: 1,
ExternalSets: 1,
}},
{name: "NoChangesHelmWithRefMultiSourceUpdateCache", fields: func() fields {
s, _, c := newServiceWithOpt(t, func(gitClient *gitmocks.Client, _ *helmmocks.Client, _ *ocimocks.Client, paths *iomocks.TempPaths) {
@ -4131,6 +4148,10 @@ func TestUpdateRevisionForPaths(t *testing.T) {
}, wantErr: assert.NoError, cacheHit: &cacheHit{
previousRevision: "0.0.1",
revision: "0.0.1",
}, cacheCallCount: &repositorymocks.CacheCallCounts{
ExternalRenames: 1,
ExternalGets: 2,
ExternalSets: 2,
}},
{name: "NoChangesHelmWithRefMultiSource_IgnoreUnusedRef", fields: func() fields {
s, _, c := newServiceWithOpt(t, func(gitClient *gitmocks.Client, _ *helmmocks.Client, _ *ocimocks.Client, paths *iomocks.TempPaths) {
@ -4182,6 +4203,10 @@ func TestUpdateRevisionForPaths(t *testing.T) {
}, wantErr: assert.NoError, cacheHit: &cacheHit{
previousRevision: "0.0.1",
revision: "0.0.1",
}, cacheCallCount: &repositorymocks.CacheCallCounts{
ExternalRenames: 1,
ExternalGets: 1,
ExternalSets: 1,
}},
{name: "NoChangesHelmWithRefMultiSource_UndefinedRef", fields: func() fields {
s, _, c := newServiceWithOpt(t, func(gitClient *gitmocks.Client, _ *helmmocks.Client, _ *ocimocks.Client, paths *iomocks.TempPaths) {
@ -4284,6 +4309,10 @@ func TestUpdateRevisionForPaths(t *testing.T) {
}, wantErr: assert.NoError, cacheHit: &cacheHit{
previousRevision: "632039659e542ed7de0c170a4fcc1c571b288fc0",
revision: "1e67a504d03def3a6a1125d934cb511680f72555",
}, cacheCallCount: &repositorymocks.CacheCallCounts{
ExternalRenames: 1,
ExternalGets: 1,
ExternalSets: 1,
}},
}
for _, tt := range tests {
@ -4301,14 +4330,8 @@ func TestUpdateRevisionForPaths(t *testing.T) {
}
assert.Equalf(t, tt.want, got, "UpdateRevisionForPaths(%v, %v)", tt.args.ctx, tt.args.request)
if tt.cacheHit != nil {
cache.mockCache.AssertCacheCalledTimes(t, &repositorymocks.CacheCallCounts{
ExternalRenames: 1,
})
} else {
cache.mockCache.AssertCacheCalledTimes(t, &repositorymocks.CacheCallCounts{
ExternalRenames: 0,
})
if tt.cacheCallCount != nil {
cache.mockCache.AssertCacheCalledTimes(t, tt.cacheCallCount)
}
})
}