Skip to content

Commit

Permalink
Merge pull request #3100 from flipt-io/gm/optimize-git-fetch
Browse files Browse the repository at this point in the history
perf(fs/git): only fetch known or newly requested references
  • Loading branch information
GeorgeMac authored May 21, 2024
2 parents 67c18b0 + e1cff0f commit 82fa7a2
Show file tree
Hide file tree
Showing 6 changed files with 161 additions and 58 deletions.
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
109 changes: 84 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,60 @@ 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, fmt.Errorf("performing initial clone: %w", err)
}

// do an initial fetch to setup remote tracking branches
if _, err := store.fetch(ctx, []string{store.baseRef}); err != nil {
return nil, fmt.Errorf("performing initial fetch: %w", 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,
CABundle: store.caBundle,
InsecureSkipTLS: store.insecureSkipTLS,
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 +225,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 +252,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 +273,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

0 comments on commit 82fa7a2

Please sign in to comment.