From 5624136ef16c77cf1b795e5ea658e09991243ae4 Mon Sep 17 00:00:00 2001 From: Vasilii Iakliushin Date: Mon, 18 Jul 2022 20:17:45 +0200 Subject: Remove 'exact_pagination_token_match' feature flag Contributes to https://gitlab.com/gitlab-org/gitaly/-/issues/3817 Feature flag was enabled by default in 15.1: https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4606 Changelog: removed --- internal/gitaly/service/ref/find_all_tags_test.go | 23 +------- internal/gitaly/service/ref/refs.go | 19 +++--- internal/gitaly/service/ref/refs_test.go | 68 +++------------------- .../featureflag/ff_exact_pagination_token_match.go | 10 ---- 4 files changed, 17 insertions(+), 103 deletions(-) delete mode 100644 internal/metadata/featureflag/ff_exact_pagination_token_match.go diff --git a/internal/gitaly/service/ref/find_all_tags_test.go b/internal/gitaly/service/ref/find_all_tags_test.go index 4396c8a99..60e224da9 100644 --- a/internal/gitaly/service/ref/find_all_tags_test.go +++ b/internal/gitaly/service/ref/find_all_tags_test.go @@ -20,7 +20,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/updateref" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" - "gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "google.golang.org/grpc/codes" @@ -508,10 +507,7 @@ func TestFindAllTags_invalidRequest(t *testing.T) { } func TestFindAllTags_pagination(t *testing.T) { - testhelper.NewFeatureSets(featureflag.ExactPaginationTokenMatch).Run(t, testFindAllTagsPagination) -} - -func testFindAllTagsPagination(t *testing.T, ctx context.Context) { + ctx := testhelper.Context(t) cfg, repoProto, repoPath, client := setupRefService(ctx, t) catfileCache := catfile.NewCache(cfg) @@ -565,15 +561,7 @@ func testFindAllTagsPagination(t *testing.T, ctx context.Context) { paginationParams: &gitalypb.PaginationParameter{Limit: 3, PageToken: "refs/tags/v1.0.0"}, sortBy: &gitalypb.FindAllTagsRequest_SortBy{Key: gitalypb.FindAllTagsRequest_SortBy_REFNAME, Direction: gitalypb.SortDirection_DESCENDING}, expected: func(context.Context) ([]string, error) { - if featureflag.ExactPaginationTokenMatch.IsEnabled(ctx) { - return []string{ - annotatedTagID.String(), - }, nil - } - return []string{ - "8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b", - "f4e6814c3e4e7a0de82a9e7cd20c626cc963a2f8", annotatedTagID.String(), }, nil }, @@ -589,14 +577,7 @@ func testFindAllTagsPagination(t *testing.T, ctx context.Context) { desc: "with invalid page token", paginationParams: &gitalypb.PaginationParameter{Limit: 3, PageToken: "refs/tags/invalid"}, expected: func(ctx context.Context) ([]string, error) { - if featureflag.ExactPaginationTokenMatch.IsEnabled(ctx) { - return nil, helper.ErrInvalidArgumentf("could not find page token") - } - - return []string{ - "8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b", - "8f03acbcd11c53d9c9468078f32a2622005a4841", - }, nil + return nil, helper.ErrInvalidArgumentf("could not find page token") }, }, } { diff --git a/internal/gitaly/service/ref/refs.go b/internal/gitaly/service/ref/refs.go index 19e532dc0..998415e6d 100644 --- a/internal/gitaly/service/ref/refs.go +++ b/internal/gitaly/service/ref/refs.go @@ -13,7 +13,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/lines" - "gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -296,19 +295,15 @@ func buildPaginationOpts(ctx context.Context, p *gitalypb.PaginationParameter) * } if p.GetPageToken() != "" { - if featureflag.ExactPaginationTokenMatch.IsEnabled(ctx) { - opts.IsPageToken = func(line []byte) bool { - // Only use the first part of the line before \x00 separator - if nullByteIndex := bytes.IndexByte(line, 0); nullByteIndex != -1 { - line = line[:nullByteIndex] - } - - return bytes.Equal(line, []byte(p.GetPageToken())) + opts.IsPageToken = func(line []byte) bool { + // Only use the first part of the line before \x00 separator + if nullByteIndex := bytes.IndexByte(line, 0); nullByteIndex != -1 { + line = line[:nullByteIndex] } - opts.PageTokenError = true - } else { - opts.IsPageToken = func(l []byte) bool { return bytes.Compare(l, []byte(p.GetPageToken())) >= 0 } + + return bytes.Equal(line, []byte(p.GetPageToken())) } + opts.PageTokenError = true } return opts diff --git a/internal/gitaly/service/ref/refs_test.go b/internal/gitaly/service/ref/refs_test.go index aa4e0faff..9e62b580f 100644 --- a/internal/gitaly/service/ref/refs_test.go +++ b/internal/gitaly/service/ref/refs_test.go @@ -5,7 +5,6 @@ package ref import ( "bufio" "bytes" - "context" "fmt" "io" "strings" @@ -18,7 +17,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/git/updateref" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" - "gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "google.golang.org/grpc/codes" @@ -295,10 +293,7 @@ func TestSuccessfulFindLocalBranches(t *testing.T) { } func TestFindLocalBranches_huge_committer(t *testing.T) { - testhelper.NewFeatureSets(featureflag.ExactPaginationTokenMatch).Run(t, testFindLocalBranchesHugeCommitter) -} - -func testFindLocalBranchesHugeCommitter(t *testing.T, ctx context.Context) { + ctx := testhelper.Context(t) cfg, repo, repoPath, client := setupRefService(ctx, t) gittest.WriteCommit(t, cfg, repoPath, @@ -321,10 +316,7 @@ func testFindLocalBranchesHugeCommitter(t *testing.T, ctx context.Context) { } func TestFindLocalBranchesPagination(t *testing.T) { - testhelper.NewFeatureSets(featureflag.ExactPaginationTokenMatch).Run(t, testFindLocalBranchesPagination) -} - -func testFindLocalBranchesPagination(t *testing.T, ctx context.Context) { + ctx := testhelper.Context(t) _, repo, _, client := setupRefService(ctx, t) limit := 1 @@ -373,10 +365,7 @@ func testFindLocalBranchesPagination(t *testing.T, ctx context.Context) { } func TestFindLocalBranchesPaginationSequence(t *testing.T) { - testhelper.NewFeatureSets(featureflag.ExactPaginationTokenMatch).Run(t, testFindLocalBranchesPaginationSequence) -} - -func testFindLocalBranchesPaginationSequence(t *testing.T, ctx context.Context) { + ctx := testhelper.Context(t) _, repo, _, client := setupRefService(ctx, t) limit := 2 @@ -426,10 +415,7 @@ func testFindLocalBranchesPaginationSequence(t *testing.T, ctx context.Context) } func TestFindLocalBranchesPaginationWithIncorrectToken(t *testing.T) { - testhelper.NewFeatureSets(featureflag.ExactPaginationTokenMatch).Run(t, testFindLocalBranchesPaginationWithIncorrectToken) -} - -func testFindLocalBranchesPaginationWithIncorrectToken(t *testing.T, ctx context.Context) { + ctx := testhelper.Context(t) _, repo, _, client := setupRefService(ctx, t) limit := 1 @@ -443,44 +429,9 @@ func testFindLocalBranchesPaginationWithIncorrectToken(t *testing.T, ctx context c, err := client.FindLocalBranches(ctx, rpcRequest) require.NoError(t, err) - if featureflag.ExactPaginationTokenMatch.IsEnabled(ctx) { - _, err = c.Recv() - require.NotEqual(t, err, io.EOF) - testhelper.RequireGrpcError(t, helper.ErrInternalf("could not find page token"), err) - } else { - require.NoError(t, err) - - var branches []*gitalypb.FindLocalBranchResponse - for { - r, err := c.Recv() - if err == io.EOF { - break - } - require.NoError(t, err) - branches = append(branches, r.GetBranches()...) - } - - testhelper.ProtoEqual(t, []*gitalypb.FindLocalBranchResponse{ - { - Name: []byte("refs/heads/rebase-encoding-failure-trigger"), - Commit: gittest.CommitsByID["ca47bfd5e930148c42ed74c3b561a8783e381f7f"], - CommitId: "ca47bfd5e930148c42ed74c3b561a8783e381f7f", - CommitSubject: []byte("Add Modula-2 source file for language detection"), - CommitAuthor: &gitalypb.FindLocalBranchCommitAuthor{ - Name: []byte("Jacob Vosmaer"), - Email: []byte("jacob@gitlab.com"), - Date: ×tamppb.Timestamp{Seconds: 1501503403}, - Timezone: []byte("+0200"), - }, - CommitCommitter: &gitalypb.FindLocalBranchCommitAuthor{ - Name: []byte("Ahmad Sherif"), - Email: []byte("me@ahmadsherif.com"), - Date: ×tamppb.Timestamp{Seconds: 1521033060}, - Timezone: []byte("+0100"), - }, - }, - }, branches) - } + _, err = c.Recv() + require.NotEqual(t, err, io.EOF) + testhelper.RequireGrpcError(t, helper.ErrInternalf("could not find page token"), err) } // Test that `s` contains the elements in `relativeOrder` in that order @@ -502,10 +453,7 @@ func isOrderedSubset(subset, set []string) bool { } func TestFindLocalBranchesSort(t *testing.T) { - testhelper.NewFeatureSets(featureflag.ExactPaginationTokenMatch).Run(t, testFindLocalBranchesSort) -} - -func testFindLocalBranchesSort(t *testing.T, ctx context.Context) { + ctx := testhelper.Context(t) testCases := []struct { desc string relativeOrder []string diff --git a/internal/metadata/featureflag/ff_exact_pagination_token_match.go b/internal/metadata/featureflag/ff_exact_pagination_token_match.go deleted file mode 100644 index ad9392f80..000000000 --- a/internal/metadata/featureflag/ff_exact_pagination_token_match.go +++ /dev/null @@ -1,10 +0,0 @@ -package featureflag - -// ExactPaginationTokenMatch enables exact matching for provided pagination tokens and -// returns an error if the match is not found. -var ExactPaginationTokenMatch = NewFeatureFlag( - "exact_pagination_token_match", - "v14.10.0", - "https://gitlab.com/gitlab-org/gitaly/-/issues/3817", - true, -) -- cgit v1.2.3