Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf(fs/git): only fetch known or newly requested references #3100

Merged
merged 5 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions go.work.sum
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ cloud.google.com/go/compute v1.19.3/go.mod h1:qxvISKp/gYnXkSAD1ppcSOveRAmzxicEv/
cloud.google.com/go/compute v1.20.1/go.mod h1:4tCnrn48xsqlwSAiLf1HXMQk8CONslYbdiEZc9FEIbM=
cloud.google.com/go/compute v1.23.4/go.mod h1:/EJMj55asU6kAFnuZET8zqgwgJ9FvXWXOkkfQZa4ioI=
cloud.google.com/go/compute v1.24.0/go.mod h1:kw1/T+h/+tK2LJK0wiPPx1intgdAM3j/g3hFDlscY40=
cloud.google.com/go/compute v1.25.1 h1:ZRpHJedLtTpKgr3RV1Fx23NuaAEN1Zfx9hw1u4aJdjU=
cloud.google.com/go/compute v1.25.1/go.mod h1:oopOIR53ly6viBYxaDhBfJwzUAxf1zE//uf3IB011ls=
cloud.google.com/go/compute/metadata v0.2.0/go.mod h1:zFmK7XCadkQkj6TtorcaGlCW1hT1fIilQDwofLpJ20k=
cloud.google.com/go/contactcenterinsights v1.13.0/go.mod h1:ieq5d5EtHsu8vhe2y3amtZ+BE+AQwX5qAy7cpo0POsI=
cloud.google.com/go/container v1.33.0/go.mod h1:u5QBBv/V9dVNK/NtTppCf6T4P8gzp+dQSwx2DqPnAKc=
Expand Down Expand Up @@ -297,6 +299,7 @@ github.com/cncf/xds/go v0.0.0-20210805033703-aa0b78936158/go.mod h1:eXthEFrGJvWH
github.com/cncf/xds/go v0.0.0-20210922020428-25de7278fc84/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs=
github.com/cncf/xds/go v0.0.0-20211011173535-cb28da3451f1/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs=
github.com/cncf/xds/go v0.0.0-20231128003011-0fa0005c9caa/go.mod h1:x/1Gn8zydmfq8dk6e9PdstVsDgu9RuyIIJqAaF//0IM=
github.com/cncf/xds/go v0.0.0-20240318125728-8a4994d93e50/go.mod h1:5e1+Vvlzido69INQaVO6d87Qn543Xr6nooe9Kz7oBFM=
github.com/cockroachdb/apd v1.1.0 h1:3LFP3629v+1aKXU5Q37mxmRxX/pIu1nijXydLShEq5I=
github.com/cockroachdb/apd/v2 v2.0.2/go.mod h1:DDxRlzC2lo3/vSlmSoS7JkqbbrARPuFOGr0B9pvN3Gw=
github.com/cockroachdb/datadriven v0.0.0-20190809214429-80d97fb3cbaa/go.mod h1:zn76sxSg3SzpJ0PPJaLDCu+Bu0Lg3sKTORVIj19EIF8=
Expand Down Expand Up @@ -1270,8 +1273,6 @@ golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBc
golang.org/x/sys v0.7.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.14.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/sys v0.20.0 h1:Od9JTbYCk261bKm4M/mw7AklTlFYIa0bIp9BgSm1S8Y=
golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/telemetry v0.0.0-20240228155512-f48c80bd79b2/go.mod h1:TeRTkGYfJXctD9OcfyVLyj2J3IxLnKwHJR8f4D8a3YE=
golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw=
golang.org/x/term v0.0.0-20210220032956-6a3ed077a48d/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
Expand Down Expand Up @@ -1358,6 +1359,7 @@ google.golang.org/genproto/googleapis/api v0.0.0-20240227224415-6ceb2ff114de/go.
google.golang.org/genproto/googleapis/api v0.0.0-20240304161311-37d4d3c04a78/go.mod h1:O1cOfN1Cy6QEYr7VxtjOyP5AdAuR0aJ/MYZaaof623Y=
google.golang.org/genproto/googleapis/api v0.0.0-20240311132316-a219d84964c2/go.mod h1:O1cOfN1Cy6QEYr7VxtjOyP5AdAuR0aJ/MYZaaof623Y=
google.golang.org/genproto/googleapis/api v0.0.0-20240314234333-6e1732d8331c/go.mod h1:VQW3tUculP/D4B+xVCo+VgSq8As6wA9ZjHl//pmk+6s=
google.golang.org/genproto/googleapis/api v0.0.0-20240318140521-94a12d6c2237/go.mod h1:Z5Iiy3jtmioajWHDGFk7CeugTyHtPvMHA4UTmUkyalE=
google.golang.org/genproto/googleapis/bytestream v0.0.0-20240304161311-37d4d3c04a78/go.mod h1:vh/N7795ftP0AkN1w8XKqN4w1OdUKXW5Eummda+ofv8=
google.golang.org/genproto/googleapis/bytestream v0.0.0-20240311132316-a219d84964c2/go.mod h1:vh/N7795ftP0AkN1w8XKqN4w1OdUKXW5Eummda+ofv8=
google.golang.org/genproto/googleapis/rpc v0.0.0-20230711160842-782d3b101e98/go.mod h1:TUfxEVdsvPg18p6AslUXFoLdpED4oBnGwyqk3dV1XzM=
Expand All @@ -1375,6 +1377,7 @@ google.golang.org/genproto/googleapis/rpc v0.0.0-20240311132316-a219d84964c2/go.
google.golang.org/genproto/googleapis/rpc v0.0.0-20240318140521-94a12d6c2237/go.mod h1:WtryC6hu0hhx87FDGxWCDptyssuo68sk10vYjF+T9fY=
google.golang.org/genproto/googleapis/rpc v0.0.0-20240401170217-c3f982113cda/go.mod h1:WtryC6hu0hhx87FDGxWCDptyssuo68sk10vYjF+T9fY=
google.golang.org/genproto/googleapis/rpc v0.0.0-20240415141817-7cd4c1c1f9ec/go.mod h1:WtryC6hu0hhx87FDGxWCDptyssuo68sk10vYjF+T9fY=
google.golang.org/genproto/googleapis/rpc v0.0.0-20240509183442-62759503f434/go.mod h1:I7Y+G38R2bu5j1aLzfFmQfTcU/WnFuqDwLZAbvKTKpM=
google.golang.org/grpc v0.0.0-20160317175043-d3ddb4469d5a/go.mod h1:yo6s7OP7yaDglbqo1J04qKzAhqBH6lvTonzMVmEdcZw=
google.golang.org/grpc v1.21.0/go.mod h1:oYelfM1adQP15Ek0mdvEgi9Df8B9CZIaU1084ijfRaM=
google.golang.org/grpc v1.23.1/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyacEbxg=
Expand All @@ -1395,6 +1398,7 @@ google.golang.org/grpc v1.61.0/go.mod h1:VUbo7IFqmF1QtCAstipjG0GIoq49KvMe9+h1jFL
google.golang.org/grpc v1.61.1/go.mod h1:VUbo7IFqmF1QtCAstipjG0GIoq49KvMe9+h1jFLBNJs=
google.golang.org/grpc v1.62.0/go.mod h1:IWTG0VlJLCh1SkC58F7np9ka9mx/WNkjl4PGJaiq+QE=
google.golang.org/grpc v1.63.0/go.mod h1:WAX/8DgncnokcFUldAxq7GeB5DXHDbMF+lLvDomNkRA=
google.golang.org/grpc v1.63.2/go.mod h1:WAX/8DgncnokcFUldAxq7GeB5DXHDbMF+lLvDomNkRA=
google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.1.0/go.mod h1:6Kw0yEErY5E/yWrBtf03jp27GLLJujG4z/JK95pnjjw=
google.golang.org/protobuf v1.28.1/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I=
google.golang.org/protobuf v1.30.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I=
Expand Down
12 changes: 6 additions & 6 deletions internal/storage/fs/git/reference_resolvers.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ import (
"github.com/go-git/go-git/v5/plumbing"
)

// ReferenceResolver is a function type used to describe reference resolver functions.
type ReferenceResolver func(repo *git.Repository, ref string) (plumbing.Hash, error)
// referenceResolver is a function type used to describe reference resolver functions.
type referenceResolver func(repo *git.Repository, ref string) (plumbing.Hash, error)

// StaticResolver is a resolver which just resolve static references.
func StaticResolver() ReferenceResolver {
// staticResolver is a resolver which just resolve static references.
func staticResolver() referenceResolver {
return func(repo *git.Repository, ref string) (plumbing.Hash, error) {
if plumbing.IsHash(ref) {
return plumbing.NewHash(ref), nil
Expand All @@ -27,8 +27,8 @@ func StaticResolver() ReferenceResolver {
}
}

// SemverResolver is a resolver which resolver semantic versioning references for tags.
func SemverResolver() ReferenceResolver {
// semverResolver is a resolver which resolver semantic versioning references for tags.
func semverResolver() referenceResolver {
return func(repo *git.Repository, ref string) (plumbing.Hash, error) {
constraint, err := semver.NewConstraint(ref)
if err != nil {
Expand Down
10 changes: 5 additions & 5 deletions internal/storage/fs/git/reference_resolvers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
func TestStaticResolver(t *testing.T) {
t.Run("should resolve static references correctly", func(t *testing.T) {
repo := newGitRepo(t)
resolver := StaticResolver()
resolver := staticResolver()

commitHash := repo.createCommit(t)
resolvedHash, err := resolver(repo.repo, "main")
Expand All @@ -38,7 +38,7 @@ func TestStaticResolver(t *testing.T) {
func TestSemverResolver(t *testing.T) {
t.Run("should resolve semver tags correctly when the reference is a constraint", func(t *testing.T) {
repo := newGitRepo(t)
resolver := SemverResolver()
resolver := semverResolver()
constraint := "v0.1.*"

commitHash := repo.createCommit(t)
Expand All @@ -61,7 +61,7 @@ func TestSemverResolver(t *testing.T) {

t.Run("should resolve semver tags correctly when the reference is not a constraint", func(t *testing.T) {
repo := newGitRepo(t)
resolver := SemverResolver()
resolver := semverResolver()

commitHash := repo.createCommit(t)
repo.createTag(t, "v0.1.0", commitHash)
Expand All @@ -74,7 +74,7 @@ func TestSemverResolver(t *testing.T) {

t.Run("should resolve semver tags correctly when there is non compliant semver tags", func(t *testing.T) {
repo := newGitRepo(t)
resolver := SemverResolver()
resolver := semverResolver()

commitHash := repo.createCommit(t)
repo.createTag(t, "non-semver-tag", commitHash)
Expand All @@ -90,7 +90,7 @@ func TestSemverResolver(t *testing.T) {

t.Run("should return an error when no matching tag was found", func(t *testing.T) {
repo := newGitRepo(t)
resolver := SemverResolver()
resolver := semverResolver()

commitHash := repo.createCommit(t)
repo.createTag(t, "v0.1.0", commitHash)
Expand Down
107 changes: 82 additions & 25 deletions internal/storage/fs/git/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ package git
import (
"context"
"errors"
"fmt"
"io/fs"
"slices"
"sync"

"github.com/go-git/go-git/v5"
Expand Down Expand Up @@ -35,7 +37,8 @@ type SnapshotStore struct {
logger *zap.Logger
url string
baseRef string
referenceResolver ReferenceResolver
refTypeTag bool
referenceResolver referenceResolver
directory string
auth transport.AuthMethod
insecureSkipTLS bool
Expand All @@ -58,10 +61,11 @@ func WithRef(ref string) containers.Option[SnapshotStore] {
}
}

// WithRefResolver configures how the reference will be resolved for the repository.
func WithRefResolver(resolver ReferenceResolver) containers.Option[SnapshotStore] {
// WithSemverResolver configures how the reference will be resolved for the repository.
func WithSemverResolver() containers.Option[SnapshotStore] {
return func(s *SnapshotStore) {
s.referenceResolver = resolver
s.refTypeTag = true
s.referenceResolver = semverResolver()
}
}

Expand Down Expand Up @@ -114,7 +118,7 @@ func NewSnapshotStore(ctx context.Context, logger *zap.Logger, url string, opts
logger: logger.With(zap.String("repository", url)),
url: url,
baseRef: "main",
referenceResolver: StaticResolver(),
referenceResolver: staticResolver(),
}
containers.ApplyAll(store, opts...)

Expand All @@ -125,19 +129,58 @@ func NewSnapshotStore(ctx context.Context, logger *zap.Logger, url string, opts
return nil, err
}

store.repo, err = git.Clone(memory.NewStorage(), nil, &git.CloneOptions{
Auth: store.auth,
URL: store.url,
CABundle: store.caBundle,
InsecureSkipTLS: store.insecureSkipTLS,
})
if err != nil {
return nil, err
}
if !plumbing.IsHash(store.baseRef) {
// if the base ref is not an explicit SHA then
// attempt to clone either the explicit branch
// or all references for tag based semver
cloneOpts := &git.CloneOptions{
Auth: store.auth,
URL: store.url,
CABundle: store.caBundle,
InsecureSkipTLS: store.insecureSkipTLS,
}

// do an initial fetch to setup remote tracking branches
if _, err := store.fetch(ctx); err != nil {
return nil, err
// if our reference is a branch type then we can assume it exists
// and attempt to only clone from this branch initially
if !store.refTypeTag {
cloneOpts.ReferenceName = plumbing.NewBranchReferenceName(store.baseRef)
cloneOpts.SingleBranch = true
}

store.repo, err = git.Clone(memory.NewStorage(), nil, cloneOpts)
if err != nil {
return nil, err
}

// do an initial fetch to setup remote tracking branches
if _, err := store.fetch(ctx, store.snaps.References()); err != nil {
return nil, err
}
} else {
// fetch single reference
store.repo, err = git.InitWithOptions(memory.NewStorage(), nil, git.InitOptions{
DefaultBranch: plumbing.Main,
})
if err != nil {
return nil, err
}

if _, err = store.repo.CreateRemote(&config.RemoteConfig{
Name: "origin",
URLs: []string{store.url},
}); err != nil {
return nil, err
}

if err := store.repo.FetchContext(ctx, &git.FetchOptions{
Auth: store.auth,
Depth: 1,
RefSpecs: []config.RefSpec{
config.RefSpec(fmt.Sprintf("%[1]s:%[1]s", store.baseRef)),
},
}); err != nil {
return nil, err
}
}

// fetch base ref snapshot at-least once before returning store
Expand Down Expand Up @@ -180,8 +223,13 @@ func (s *SnapshotStore) View(ctx context.Context, storeRef storage.Reference, fn
return fn(snap)
}

refs := s.snaps.References()
if !slices.Contains(refs, ref) {
refs = append(refs, ref)
}

// force attempt a fetch to get the latest references
if _, err := s.fetch(ctx); err != nil {
if _, err := s.fetch(ctx, refs); err != nil {
return err
}

Expand All @@ -202,7 +250,7 @@ func (s *SnapshotStore) View(ctx context.Context, storeRef storage.Reference, fn
// HEAD updates to a new revision, it builds a snapshot and updates it
// on the store.
func (s *SnapshotStore) update(ctx context.Context) (bool, error) {
if updated, err := s.fetch(ctx); !(err == nil && updated) {
if updated, err := s.fetch(ctx, s.snaps.References()); !(err == nil && updated) {
// either nothing updated or err != nil
return updated, err
}
Expand All @@ -223,16 +271,25 @@ func (s *SnapshotStore) update(ctx context.Context) (bool, error) {
return true, errors.Join(errs...)
}

func (s *SnapshotStore) fetch(ctx context.Context) (bool, error) {
func (s *SnapshotStore) fetch(ctx context.Context, heads []string) (bool, error) {
s.mu.Lock()
defer s.mu.Unlock()

refSpecs := []config.RefSpec{}

if s.refTypeTag {
refSpecs = append(refSpecs, "+refs/tags/*:refs/tags/*")
}

for _, head := range heads {
refSpecs = append(refSpecs,
config.RefSpec(fmt.Sprintf("+refs/heads/%[1]s:refs/heads/%[1]s", head)),
)
}

if err := s.repo.FetchContext(ctx, &git.FetchOptions{
Auth: s.auth,
RefSpecs: []config.RefSpec{
"+refs/heads/*:refs/heads/*",
"+refs/tags/*:refs/tags/*",
},
Auth: s.auth,
RefSpecs: refSpecs,
}); err != nil {
if !errors.Is(err, git.NoErrAlreadyUpToDate) {
return false, err
Expand Down
70 changes: 56 additions & 14 deletions internal/storage/fs/git/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ flags:
require.NoError(t, fi.Close())

// commit changes
_, err = tree.Commit("chore: update features.yml", &git.CommitOptions{
_, err = tree.Commit("chore: update features.yml add foo and bar", &git.CommitOptions{
All: true,
Author: &object.Signature{Email: "[email protected]", Name: "dev"},
})
Expand All @@ -185,18 +185,6 @@ flags:
RefSpecs: []config.RefSpec{"refs/heads/new-branch:refs/heads/new-branch"},
}))

// wait until the snapshot is updated or
// we timeout
select {
case <-ch:
case <-time.After(time.Minute):
t.Fatal("timed out waiting for snapshot")
}

require.NoError(t, err)

t.Log("received new snapshot")

require.NoError(t, store.View(ctx, "", func(s storage.ReadOnlyStore) error {
_, err := s.GetFlag(ctx, storage.NewResource("production", "bar"))
require.Error(t, err, "flag should not be found in default revision")
Expand All @@ -209,11 +197,65 @@ flags:
return nil
}))

// should be able to fetch flag from previously unfetched reference
require.NoError(t, store.View(ctx, "new-branch", func(s storage.ReadOnlyStore) error {
_, err := s.GetFlag(ctx, storage.NewResource("production", "bar"))
require.NoError(t, err, "flag should be present on new-branch")
return nil
}))

// flag bar should not yet be present
require.NoError(t, store.View(ctx, "new-branch", func(s storage.ReadOnlyStore) error {
_, err := s.GetFlag(ctx, storage.NewResource("production", "baz"))
require.Error(t, err, "flag should not be found in explicitly named new-branch revision")
return nil
}))

// update features.yml, now with the bar flag
fi, err = workdir.OpenFile("features.yml", os.O_TRUNC|os.O_RDWR, os.ModePerm)
require.NoError(t, err)

updated = []byte(`namespace: production
flags:
- key: foo
name: Foo
- key: bar
name: Bar
- key: baz
name: Baz`)

_, err = fi.Write(updated)
require.NoError(t, err)
require.NoError(t, fi.Close())

// commit changes
_, err = tree.Commit("chore: update features.yml add baz", &git.CommitOptions{
All: true,
Author: &object.Signature{Email: "[email protected]", Name: "dev"},
})
require.NoError(t, err)

// push new commit
require.NoError(t, repo.Push(&git.PushOptions{
Auth: &http.BasicAuth{Username: "root", Password: "password"},
RemoteName: "origin",
RefSpecs: []config.RefSpec{"refs/heads/new-branch:refs/heads/new-branch"},
}))

// we should expect to see a modified event now because
// the new reference should be tracked
select {
case <-ch:
case <-time.After(time.Minute):
t.Fatal("timed out waiting for fetch")
}

// should be able to fetch flag bar now that it has been pushed
require.NoError(t, store.View(ctx, "new-branch", func(s storage.ReadOnlyStore) error {
_, err := s.GetFlag(ctx, storage.NewResource("production", "baz"))
require.NoError(t, err, "flag should be present on new-branch")
return nil
}))
}

func Test_Store_View_WithSemverRevision(t *testing.T) {
Expand All @@ -232,7 +274,7 @@ func Test_Store_View_WithSemverRevision(t *testing.T) {
ch := make(chan struct{})
store, skip := testStore(t, gitRepoURL,
WithRef("v0.1.*"),
WithRefResolver(SemverResolver()),
WithSemverResolver(),
WithPollOptions(
fs.WithInterval(time.Second),
fs.WithNotify(t, func(modified bool) {
Expand Down
Loading
Loading