singleflight go commands (#1877)

* singleflight go commands * Apply suggestions from code review Co-authored-by: Brendan Le Glaunec <brendan@glaulabs.com> --------- Co-authored-by: michael-wozniak <michael.wozniak@snapchat.com> Co-authored-by: Brendan Le Glaunec <brendan@glaulabs.com>
2026-02-03 12:10:32 +00:00 · 2023-12-21 08:37:36 -05:00
parent 640cc67e33
commit 3af0d009a8
2 changed files with 114 additions and 87 deletions
@@ -14,6 +14,7 @@ import (
 	"github.com/gomods/athens/pkg/observ"
 	"github.com/gomods/athens/pkg/storage"
 	"github.com/spf13/afero"
+	"golang.org/x/sync/singleflight"
 )

 type goGetFetcher struct {
@@ -21,6 +22,7 @@ type goGetFetcher struct {
 	goBinaryName string
 	envVars      []string
 	gogetDir     string
+	sfg          *singleflight.Group
 }

 type goModule struct {
@@ -46,6 +48,7 @@ func NewGoGetFetcher(goBinaryName, gogetDir string, envVars []string, fs afero.F
 		goBinaryName: goBinaryName,
 		envVars:      envVars,
 		gogetDir:     gogetDir,
+		sfg:          &singleflight.Group{},
 	}, nil
 }

@@ -56,57 +59,63 @@ func (g *goGetFetcher) Fetch(ctx context.Context, mod, ver string) (*storage.Ver
 	ctx, span := observ.StartSpan(ctx, op.String())
 	defer span.End()

-	// setup the GOPATH
-	goPathRoot, err := afero.TempDir(g.fs, g.gogetDir, "athens")
-	if err != nil {
-		return nil, errors.E(op, err)
-	}
-	sourcePath := filepath.Join(goPathRoot, "src")
-	modPath := filepath.Join(sourcePath, getRepoDirName(mod, ver))
-	if err := g.fs.MkdirAll(modPath, os.ModeDir|os.ModePerm); err != nil {
-		_ = clearFiles(g.fs, goPathRoot)
-		return nil, errors.E(op, err)
-	}
+	resp, err, _ := g.sfg.Do(mod+"###"+ver, func() (any, error) {
+		// setup the GOPATH
+		goPathRoot, err := afero.TempDir(g.fs, g.gogetDir, "athens")
+		if err != nil {
+			return nil, errors.E(op, err)
+		}
+		sourcePath := filepath.Join(goPathRoot, "src")
+		modPath := filepath.Join(sourcePath, getRepoDirName(mod, ver))
+		if err := g.fs.MkdirAll(modPath, os.ModeDir|os.ModePerm); err != nil {
+			_ = clearFiles(g.fs, goPathRoot)
+			return nil, errors.E(op, err)
+		}

-	m, err := downloadModule(
-		ctx,
-		g.goBinaryName,
-		g.envVars,
-		goPathRoot,
-		modPath,
-		mod,
-		ver,
-	)
-	if err != nil {
-		_ = clearFiles(g.fs, goPathRoot)
-		return nil, errors.E(op, err)
-	}
+		m, err := downloadModule(
+			ctx,
+			g.goBinaryName,
+			g.envVars,
+			goPathRoot,
+			modPath,
+			mod,
+			ver,
+		)
+		if err != nil {
+			_ = clearFiles(g.fs, goPathRoot)
+			return nil, errors.E(op, err)
+		}

-	var storageVer storage.Version
-	storageVer.Semver = m.Version
-	info, err := afero.ReadFile(g.fs, m.Info)
-	if err != nil {
-		return nil, errors.E(op, err)
-	}
-	storageVer.Info = info
+		var storageVer storage.Version
+		storageVer.Semver = m.Version
+		info, err := afero.ReadFile(g.fs, m.Info)
+		if err != nil {
+			return nil, errors.E(op, err)
+		}
+		storageVer.Info = info

-	gomod, err := afero.ReadFile(g.fs, m.GoMod)
-	if err != nil {
-		return nil, errors.E(op, err)
-	}
-	storageVer.Mod = gomod
+		gomod, err := afero.ReadFile(g.fs, m.GoMod)
+		if err != nil {
+			return nil, errors.E(op, err)
+		}
+		storageVer.Mod = gomod

-	zip, err := g.fs.Open(m.Zip)
-	if err != nil {
-		return nil, errors.E(op, err)
-	}
-	// note: don't close zip here so that the caller can read directly from disk.
-	//
-	// if we close, then the caller will panic, and the alternative to make this work is
-	// that we read into memory and return an io.ReadCloser that reads out of memory
-	storageVer.Zip = &zipReadCloser{zip, g.fs, goPathRoot}
+		zip, err := g.fs.Open(m.Zip)
+		if err != nil {
+			return nil, errors.E(op, err)
+		}
+		// note: don't close zip here so that the caller can read directly from disk.
+		//
+		// if we close, then the caller will panic, and the alternative to make this work is
+		// that we read into memory and return an io.ReadCloser that reads out of memory
+		storageVer.Zip = &zipReadCloser{zip, g.fs, goPathRoot}

-	return &storageVer, nil
+		return &storageVer, nil
+	})
+	if err != nil {
+		return nil, err
+	}
+	return resp.(*storage.Version), nil
 }

 // given a filesystem, gopath, repository root, module and version, runs 'go mod download -json'
@@ -13,6 +13,7 @@ import (
 	"github.com/gomods/athens/pkg/observ"
 	"github.com/gomods/athens/pkg/storage"
 	"github.com/spf13/afero"
+	"golang.org/x/sync/singleflight"
 )

 type listResp struct {
@@ -26,6 +27,7 @@ type vcsLister struct {
 	goBinPath string
 	env       []string
 	fs        afero.Fs
+	sfg       *singleflight.Group
 }

 // NewVCSLister creates an UpstreamLister which uses VCS to fetch a list of available versions.
@@ -34,58 +36,74 @@ func NewVCSLister(goBinPath string, env []string, fs afero.Fs) UpstreamLister {
 		goBinPath: goBinPath,
 		env:       env,
 		fs:        fs,
+		sfg:       &singleflight.Group{},
 	}
 }

+type listSFResp struct {
+	rev      *storage.RevInfo
+	versions []string
+}
+
 func (l *vcsLister) List(ctx context.Context, module string) (*storage.RevInfo, []string, error) {
 	const op errors.Op = "vcsLister.List"
 	_, span := observ.StartSpan(ctx, op.String())
 	defer span.End()
-	tmpDir, err := afero.TempDir(l.fs, "", "go-list")
-	if err != nil {
-		return nil, nil, errors.E(op, err)
-	}
-	defer func() { _ = l.fs.RemoveAll(tmpDir) }()
+	sfResp, err, _ := l.sfg.Do(module, func() (any, error) {
+		tmpDir, err := afero.TempDir(l.fs, "", "go-list")
+		if err != nil {
+			return nil, errors.E(op, err)
+		}
+		defer func() { _ = l.fs.RemoveAll(tmpDir) }()

-	cmd := exec.Command(
-		l.goBinPath,
-		"list", "-m", "-versions", "-json",
-		config.FmtModVer(module, "latest"),
-	)
-	cmd.Dir = tmpDir
-	stdout := &bytes.Buffer{}
-	stderr := &bytes.Buffer{}
-	cmd.Stdout = stdout
-	cmd.Stderr = stderr
+		cmd := exec.Command(
+			l.goBinPath,
+			"list", "-m", "-versions", "-json",
+			config.FmtModVer(module, "latest"),
+		)
+		cmd.Dir = tmpDir
+		stdout := &bytes.Buffer{}
+		stderr := &bytes.Buffer{}
+		cmd.Stdout = stdout
+		cmd.Stderr = stderr

-	gopath, err := afero.TempDir(l.fs, "", "athens")
-	if err != nil {
-		return nil, nil, errors.E(op, err)
-	}
-	defer func() { _ = clearFiles(l.fs, gopath) }()
-	cmd.Env = prepareEnv(gopath, l.env)
+		gopath, err := afero.TempDir(l.fs, "", "athens")
+		if err != nil {
+			return nil, errors.E(op, err)
+		}
+		defer func() { _ = clearFiles(l.fs, gopath) }()
+		cmd.Env = prepareEnv(gopath, l.env)

-	err = cmd.Run()
-	if err != nil {
-		err = fmt.Errorf("%w: %s", err, stderr)
-		// as of now, we can't recognize between a true NotFound
-		// and an unexpected error, so we choose the more
-		// hopeful path of NotFound. This way the Go command
-		// will not log en error and we still get to log
-		// what happened here if someone wants to dig in more.
-		// Once, https://github.com/golang/go/issues/30134 is
-		// resolved, we can hopefully differentiate.
-		return nil, nil, errors.E(op, err, errors.KindNotFound)
-	}
+		err = cmd.Run()
+		if err != nil {
+			err = fmt.Errorf("%w: %s", err, stderr)
+			// as of now, we can't recognize between a true NotFound
+			// and an unexpected error, so we choose the more
+			// hopeful path of NotFound. This way the Go command
+			// will not log en error and we still get to log
+			// what happened here if someone wants to dig in more.
+			// Once, https://github.com/golang/go/issues/30134 is
+			// resolved, we can hopefully differentiate.
+			return nil, errors.E(op, err, errors.KindNotFound)
+		}

-	var lr listResp
-	err = json.NewDecoder(stdout).Decode(&lr)
+		var lr listResp
+		err = json.NewDecoder(stdout).Decode(&lr)
+		if err != nil {
+			return nil, errors.E(op, err)
+		}
+		rev := storage.RevInfo{
+			Time:    lr.Time,
+			Version: lr.Version,
+		}
+		return listSFResp{
+			rev:      &rev,
+			versions: lr.Versions,
+		}, nil
+	})
 	if err != nil {
-		return nil, nil, errors.E(op, err)
+		return nil, nil, err
 	}
-	rev := storage.RevInfo{
-		Time:    lr.Time,
-		Version: lr.Version,
-	}
-	return &rev, lr.Versions, nil
+	ret := sfResp.(listSFResp)
+	return ret.rev, ret.versions, nil
 }