Merge 7374b5110b into b91a772d71

internal/resolver/resolver.go

@@ -122,7 +122,7 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
 		var ok bool
 		found := true
 		if !registry.IsOCI(d.Repository) {
-			repoIndex, err := repo.LoadIndexFile(filepath.Join(r.cachepath, helmpath.CacheIndexFile(repoName)))
+			repoIndex, err := repo.LoadIndexFileWithCaching(filepath.Join(r.cachepath, helmpath.CacheIndexFile(repoName)))
 			if err != nil {
 				return nil, errors.Wrapf(err, "no cached repository for %s found. (try 'helm repo update')", repoName)
 			}

pkg/downloader/chart_downloader.go

@@ -281,7 +281,7 @@ func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, er
 
 	// Next, we need to load the index, and actually look up the chart.
 	idxFile := filepath.Join(c.RepositoryCache, helmpath.CacheIndexFile(r.Config.Name))
-	i, err := repo.LoadIndexFile(idxFile)
+	i, err := repo.LoadIndexFileWithCaching(idxFile)
 	if err != nil {
 		return u, errors.Wrap(err, "no cached repo found. (try 'helm repo update')")
 	}
@@ -380,7 +380,7 @@ func (c *ChartDownloader) scanReposForURL(u string, rf *repo.File) (*repo.Entry,
 		}
 
 		idxFile := filepath.Join(c.RepositoryCache, helmpath.CacheIndexFile(r.Config.Name))
-		i, err := repo.LoadIndexFile(idxFile)
+		i, err := repo.LoadIndexFileWithCaching(idxFile)
 		if err != nil {
 			return nil, errors.Wrap(err, "no cached repo found. (try 'helm repo update')")
 		}

pkg/downloader/manager.go

@@ -826,7 +826,7 @@ func (m *Manager) loadChartRepositories() (map[string]*repo.ChartRepository, err
 	for _, re := range rf.Repositories {
 		lname := re.Name
 		idxFile := filepath.Join(m.RepositoryCache, helmpath.CacheIndexFile(lname))
-		index, err := repo.LoadIndexFile(idxFile)
+		index, err := repo.LoadIndexFileWithCaching(idxFile)
 		if err != nil {
 			return indices, err
 		}

pkg/repo/chartrepo.go

@@ -99,7 +99,7 @@ func (r *ChartRepository) Load() error {
 	filepath.Walk(r.Config.Name, func(path string, f os.FileInfo, _ error) error {
 		if !f.IsDir() {
 			if strings.Contains(f.Name(), "-index.yaml") {
-				i, err := LoadIndexFile(path)
+				i, err := LoadIndexFileWithCaching(path)
 				if err != nil {
 					return err
 				}
@@ -254,7 +254,7 @@ func FindChartInAuthAndTLSAndPassRepoURL(repoURL, username, password, chartName,
 	}()
 
 	// Read the index file for the repository to get chart information and return chart URL
-	repoIndex, err := LoadIndexFile(idx)
+	repoIndex, err := LoadIndexFileWithCaching(idx)
 	if err != nil {
 		return "", err
 	}

pkg/repo/chartrepo_test.go

@@ -92,7 +92,7 @@ func TestIndex(t *testing.T) {
 	}
 
 	tempIndexPath := filepath.Join(testRepository, indexPath)
-	actual, err := LoadIndexFile(tempIndexPath)
+	actual, err := LoadIndexFileWithCaching(tempIndexPath)
 	defer os.Remove(tempIndexPath) // clean up
 	if err != nil {
 		t.Errorf("Error loading index file %v", err)
@@ -104,7 +104,7 @@ func TestIndex(t *testing.T) {
 	if err != nil {
 		t.Errorf("Error performing re-index: %s\n", err)
 	}
-	second, err := LoadIndexFile(tempIndexPath)
+	second, err := LoadIndexFileWithCaching(tempIndexPath)
 	if err != nil {
 		t.Errorf("Error re-loading index file %v", err)
 	}

pkg/repo/index.go

@@ -25,6 +25,7 @@ import (
 	"path/filepath"
 	"sort"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/Masterminds/semver/v3"
@@ -115,6 +116,40 @@ func LoadIndexFile(path string) (*IndexFile, error) {
 	return i, nil
 }
 
+type cacheEntry struct {
+	indexFile *IndexFile
+	err       error
+	lock      sync.RWMutex
+}
+
+var cache sync.Map
+
+// LoadIndexFileWithCaching is a wrapper around LoadIndexFile
+// it adds an internal global cache to avoid reading the same file multiple times
+func LoadIndexFileWithCaching(path string) (*IndexFile, error) {
+	// assume the entry is not cached, prepare a newEntry
+	newEntry := &cacheEntry{}
+	// lock to avoid threading issues in case of multiple loads in parallel
+	newEntry.lock.Lock()
+
+	// check if index already in cache, add newEntry atomically if not-cached
+	if v, loaded := cache.LoadOrStore(path, newEntry); loaded {
+		// We hit the cache. newEntry is not needed anymore
+		newEntry.lock.Unlock()
+		entry := v.(*cacheEntry)
+		// wait for RLock on cached entry
+		// LoadIndexFile might not have completed yet in another go-routine
+		entry.lock.RLock()
+		defer entry.lock.RUnlock()
+		return entry.indexFile, entry.err
+	}
+	// no entry found in cache
+	// LoadIndexFile while holding the write-lock
+	newEntry.indexFile, newEntry.err = LoadIndexFile(path)
+	defer newEntry.lock.Unlock()
+	return newEntry.indexFile, newEntry.err
+}
+
 // MustAdd adds a file to the index
 // This can leave the index in an unsorted state
 func (i IndexFile) MustAdd(md *chart.Metadata, filename, baseURL, digest string) error {

pkg/repo/index_test.go

@@ -28,6 +28,8 @@ import (
 	"strings"
 	"testing"
 
+	"golang.org/x/sync/errgroup"
+
 	"helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/cli"
 	"helm.sh/helm/v3/pkg/getter"
@@ -171,6 +173,45 @@ func TestLoadIndex(t *testing.T) {
 	}
 }
 
+func TestLoadIndexWithCaching(t *testing.T) {
+	var eg errgroup.Group
+
+	const nrParallelTests = 10
+
+	var indexFiles [nrParallelTests]*IndexFile
+
+	// Load same index multiple times in parallel
+	for i := 0; i < nrParallelTests; i++ {
+		indexNr := i
+		eg.Go(func() (err error) {
+			indexFiles[indexNr], err = LoadIndexFileWithCaching(chartmuseumtestfile)
+			return err
+		})
+	}
+
+	if err := eg.Wait(); err != nil {
+		t.Fatal(err)
+	}
+
+	// we check if the same cache entry is used by comparing pointers to indexFiles
+	// if the pointers are the same, we assume the same cache entry is used.
+	for i := 1; i < nrParallelTests; i++ {
+		if indexFiles[i] != indexFiles[0] {
+			t.Fatal("not all indices retrieved from same cache entry")
+		}
+	}
+
+	// load another index, and check if new_entry is used
+	otherFile, err := LoadIndexFileWithCaching(testfile)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if otherFile == indexFiles[0] {
+		t.Fatal("same index used for different files")
+	}
+}
+
 // TestLoadIndex_Duplicates is a regression to make sure that we don't non-deterministically allow duplicate packages.
 func TestLoadIndex_Duplicates(t *testing.T) {
 	if _, err := loadIndex([]byte(indexWithDuplicates), "indexWithDuplicates"); err == nil {