Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitaly.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEric Ju <eju@gitlab.com>2024-01-18 03:37:07 +0300
committerEric Ju <eju@gitlab.com>2024-01-18 03:37:07 +0300
commit7997900a820e07709f644145231cadc799f02c44 (patch)
tree43b0fc470c0a5bc69aeeded299530c6f0e9bb249
parentaa352ff89f9a2aca1496c62f6a6dc8935a2c48b3 (diff)
repository: Make GetInfoAttributes read from HEAD:.gitattributeej-stop-using-info-atrributes-2
This is a solution for N+1 problem we come across, see https://gitlab.com/groups/gitlab-org/-/epics/9006#note_1730745750 for details. In order to remove the reliance of info/attribute and avoid breaking Rails with TooManyInvocationsError. GetInfoAttributes is set to read from HEAD:.gitattribute. It will also try to delete info/attribute before reading. After proper refactor on Rails to solve the N+1 case. GetInfoAttributes can be removed totally. This is my commit message body
-rw-r--r--internal/gitaly/service/repository/info_attributes.go51
-rw-r--r--internal/gitaly/service/repository/info_attributes_test.go12
2 files changed, 49 insertions, 14 deletions
diff --git a/internal/gitaly/service/repository/info_attributes.go b/internal/gitaly/service/repository/info_attributes.go
index 9ddf04fdc..f5a9628b2 100644
--- a/internal/gitaly/service/repository/info_attributes.go
+++ b/internal/gitaly/service/repository/info_attributes.go
@@ -1,16 +1,17 @@
package repository
import (
- "io"
- "os"
- "path/filepath"
-
+ "bufio"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/git"
"gitlab.com/gitlab-org/gitaly/v16/internal/structerr"
"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
"gitlab.com/gitlab-org/gitaly/v16/streamio"
+ "io"
+ "os"
+ "strings"
)
-func (s *server) GetInfoAttributes(in *gitalypb.GetInfoAttributesRequest, stream gitalypb.RepositoryService_GetInfoAttributesServer) error {
+func (s *server) GetInfoAttributes(in *gitalypb.GetInfoAttributesRequest, stream gitalypb.RepositoryService_GetInfoAttributesServer) (returnedErr error) {
repository := in.GetRepository()
if err := s.locator.ValidateRepository(repository); err != nil {
return structerr.NewInvalidArgument("%w", err)
@@ -20,14 +21,40 @@ func (s *server) GetInfoAttributes(in *gitalypb.GetInfoAttributesRequest, stream
return err
}
- attrFile := filepath.Join(repoPath, "info", "attributes")
- f, err := os.Open(attrFile)
- if err != nil {
- if os.IsNotExist(err) {
- return stream.Send(&gitalypb.GetInfoAttributesResponse{})
+ // In git 2.43.0+, gitattributes supports reading from HEAD:.gitattributes,
+ // so info/attributes is no longer needed. To make sure info/attributes file is cleaned up,
+ // we delete it if it exists when reading from HEAD:.gitattributes is called.
+ // This logic can be removed when ApplyGitattributes and GetInfoAttributes PRC are totally removed from
+ // the code base.
+ deletionErr := deleteInfoAttributesFile(repoPath)
+ if !os.IsNotExist(deletionErr) {
+ s.logger.WithError(deletionErr).Error("failed to delete info/gitattributes file at " + repoPath)
+ }
+
+ repo := s.localrepo(in.GetRepository())
+ ctx := stream.Context()
+ var stderr strings.Builder
+ // Call cat-file -p HEAD:.gitattributes instead of cat info/attributes
+ catFileCmd, err := repo.Exec(ctx, git.Command{
+ Name: "cat-file",
+ Flags: []git.Option{
+ git.Flag{Name: "-p"},
+ },
+ Args: []string{"HEAD:.gitattributes"},
+ },
+ git.WithSetupStdout(),
+ git.WithStderr(&stderr),
+ )
+ defer func() {
+ if err := catFileCmd.Wait(); err != nil {
+ s.logger.Error("git cat-file command error: " + stderr.String())
}
+ }()
- return structerr.NewInternal("failure to read info attributes: %w", err)
+ buf := bufio.NewReader(catFileCmd)
+ _, err = buf.Peek(1)
+ if err == io.EOF {
+ return stream.Send(&gitalypb.GetInfoAttributesResponse{})
}
sw := streamio.NewWriter(func(p []byte) error {
@@ -35,7 +62,7 @@ func (s *server) GetInfoAttributes(in *gitalypb.GetInfoAttributesRequest, stream
Attributes: p,
})
})
+ _, err = io.Copy(sw, buf)
- _, err = io.Copy(sw, f)
return err
}
diff --git a/internal/gitaly/service/repository/info_attributes_test.go b/internal/gitaly/service/repository/info_attributes_test.go
index 908761822..7cb1c3e90 100644
--- a/internal/gitaly/service/repository/info_attributes_test.go
+++ b/internal/gitaly/service/repository/info_attributes_test.go
@@ -2,6 +2,7 @@ package repository
import (
"bytes"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/helper/perm"
"io"
"os"
"path/filepath"
@@ -10,7 +11,6 @@ import (
"github.com/stretchr/testify/require"
"gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest"
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
- "gitlab.com/gitlab-org/gitaly/v16/internal/helper/perm"
"gitlab.com/gitlab-org/gitaly/v16/internal/structerr"
"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
@@ -33,6 +33,13 @@ func TestGetInfoAttributesExisting(t *testing.T) {
err := os.WriteFile(attrsPath, data, perm.SharedFile)
require.NoError(t, err)
+ gitattributesContent := "*.go diff=go text\n*.md text\n*.jpg -text"
+ gittest.WriteCommit(t, cfg, repoPath,
+ gittest.WithBranch("main"),
+ gittest.WithTreeEntries(
+ gittest.TreeEntry{Path: ".gitattributes", Mode: "100644", Content: gitattributesContent},
+ ))
+
request := &gitalypb.GetInfoAttributesRequest{Repository: repo}
//nolint:staticcheck
@@ -45,7 +52,8 @@ func TestGetInfoAttributesExisting(t *testing.T) {
}))
require.NoError(t, err)
- require.Equal(t, data, receivedData)
+ require.Equal(t, gitattributesContent, string(receivedData))
+ require.NoFileExists(t, attrsPath)
}
func TestGetInfoAttributesNonExisting(t *testing.T) {