From d5b4307e0293aebf8fec0665b89b564a91863ff9 Mon Sep 17 00:00:00 2001 From: John Cai Date: Wed, 27 Feb 2019 11:07:37 -0800 Subject: adding feature flag for FindAllTags --- .../jc-add-find-all-tags-feature-flag.yml | 5 +++ internal/service/ref/refs.go | 52 ++++++++++++++++++++-- internal/service/ref/refs_test.go | 50 ++++++++++++--------- 3 files changed, 83 insertions(+), 24 deletions(-) create mode 100644 changelogs/unreleased/jc-add-find-all-tags-feature-flag.yml diff --git a/changelogs/unreleased/jc-add-find-all-tags-feature-flag.yml b/changelogs/unreleased/jc-add-find-all-tags-feature-flag.yml new file mode 100644 index 000000000..f9298b43b --- /dev/null +++ b/changelogs/unreleased/jc-add-find-all-tags-feature-flag.yml @@ -0,0 +1,5 @@ +--- +title: Add feature flag for FindAllTags +merge_request: 1106 +author: +type: other diff --git a/internal/service/ref/refs.go b/internal/service/ref/refs.go index 1aab6f40d..940b69869 100644 --- a/internal/service/ref/refs.go +++ b/internal/service/ref/refs.go @@ -8,6 +8,10 @@ import ( "fmt" "strings" + "github.com/prometheus/client_golang/prometheus" + "gitlab.com/gitlab-org/gitaly/internal/featureflag" + "gitlab.com/gitlab-org/gitaly/internal/rubyserver" + "gitlab.com/gitlab-org/gitaly-proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/catfile" @@ -24,8 +28,22 @@ var ( headReference = _headReference // FindBranchNames is exported to be used in other packages FindBranchNames = _findBranchNames + + findAllTagsFeatureFlag = "go-find-all-tags" + + findAllTagsRequests = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Name: "gitaly_find_all_tags_requests_total", + Help: "Counter of go vs ruby implementation of FindAllTags", + }, + []string{"implementation"}, + ) ) +func init() { + prometheus.Register(findAllTagsRequests) +} + type findRefsOpts struct { cmdArgs []string delim []byte @@ -137,11 +155,39 @@ func (s *server) FindAllTags(in *gitalypb.FindAllTagsRequest, stream gitalypb.Re return helper.ErrInvalidArgument(err) } - if err := parseAndReturnTags(ctx, in.GetRepository(), stream); err != nil { - return helper.ErrInternal(err) + if featureflag.IsEnabled(ctx, findAllTagsFeatureFlag) { + findAllTagsRequests.WithLabelValues("go").Inc() + if err := parseAndReturnTags(ctx, in.GetRepository(), stream); err != nil { + return helper.ErrInternal(err) + } + return nil } - return nil + findAllTagsRequests.WithLabelValues("ruby").Inc() + client, err := s.RefServiceClient(ctx) + if err != nil { + return err + } + + clientCtx, err := rubyserver.SetHeaders(ctx, in.GetRepository()) + if err != nil { + return err + } + + rubyStream, err := client.FindAllTags(clientCtx, in) + if err != nil { + return err + } + + return rubyserver.Proxy(func() error { + resp, err := rubyStream.Recv() + if err != nil { + md := rubyStream.Trailer() + stream.SetTrailer(md) + return err + } + return stream.Send(resp) + }) } func validateFindAllTagsRequest(request *gitalypb.FindAllTagsRequest) error { diff --git a/internal/service/ref/refs_test.go b/internal/service/ref/refs_test.go index c4431c376..5db6a232e 100644 --- a/internal/service/ref/refs_test.go +++ b/internal/service/ref/refs_test.go @@ -13,11 +13,13 @@ import ( "github.com/golang/protobuf/ptypes/timestamp" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly-proto/go/gitalypb" + "gitlab.com/gitlab-org/gitaly/internal/featureflag" "gitlab.com/gitlab-org/gitaly/internal/git/log" "gitlab.com/gitlab-org/gitaly/internal/git/updateref" "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "google.golang.org/grpc/codes" + "google.golang.org/grpc/metadata" ) func containsRef(refs [][]byte, ref string) bool { @@ -489,9 +491,7 @@ func TestSuccessfulFindAllTagsRequest(t *testing.T) { rpcRequest := &gitalypb.FindAllTagsRequest{Repository: testRepoCopy} c, err := client.FindAllTags(ctx, rpcRequest) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) var receivedTags []*gitalypb.Tag for { @@ -499,9 +499,7 @@ func TestSuccessfulFindAllTagsRequest(t *testing.T) { if err == io.EOF { break } - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) receivedTags = append(receivedTags, r.GetTags()...) } @@ -570,26 +568,36 @@ func TestSuccessfulFindAllTagsRequest(t *testing.T) { }, } - if len(receivedTags) < len(expectedTags) { - t.Fatalf("expected at least %d tags, got %d", len(expectedTags), len(receivedTags)) - } + require.Len(t, receivedTags, len(expectedTags)) - for _, expectedTag := range expectedTags { - t.Run(string(expectedTag.Name), func(t *testing.T) { - receivedTag := findTag(receivedTags, expectedTag.Name) - require.NotNil(t, receivedTag, "tag not found") - require.Equal(t, expectedTag, receivedTag) - }) + for i, expectedTag := range expectedTags { + require.Equal(t, expectedTag, receivedTags[i]) } -} -func findTag(tags []*gitalypb.Tag, tagName []byte) *gitalypb.Tag { - for _, t := range tags { - if bytes.Equal(t.Name, tagName) { - return t + expectedNumberOfTags := len(receivedTags) + // using Go implementation + + md := metadata.New(map[string]string{featureflag.HeaderKey(findAllTagsFeatureFlag): "true"}) + ctx = metadata.NewOutgoingContext(context.Background(), md) + + c, err = client.FindAllTags(ctx, rpcRequest) + require.NoError(t, err) + + receivedTags = nil + for { + r, err := c.Recv() + if err == io.EOF { + break } + require.NoError(t, err) + receivedTags = append(receivedTags, r.GetTags()...) + } + + require.Len(t, receivedTags, expectedNumberOfTags, "received wrong number of tags") + + for i, expectedTag := range expectedTags { + require.Equal(t, expectedTag, receivedTags[i]) } - return nil } func TestInvalidFindAllTagsRequest(t *testing.T) { -- cgit v1.2.3