diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-09-28 16:06:43 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-09-29 12:23:46 +0300 |
commit | 0a27d921759b461094458219ccc55012a0856b33 (patch) | |
tree | 60d25b5cc9c855e20a5a134422f112b2aaedb0d4 | |
parent | 15fb0dcbc902e88589084577896fd59ddb61b058 (diff) |
stats: Convert `GetPackfiles()` to use `os.ReadDir()`
Go 1.15 has deprecated the `ioutil` package and made replacements
available in the `io` or `os` packages. While we have already migrated
most callsites away, one notable omission was `ioutil.ReadDir()` as this
function doesn't have an exact replacement: the new `os.ReadDir()`
function does not stat all directory entries anymore.
Refactor `stats.GetPackfiles()` to use the new function and manually
stat the packfiles at callsites as required.
-rw-r--r-- | internal/git/stats/profile.go | 9 | ||||
-rw-r--r-- | internal/gitaly/service/repository/midx.go | 27 | ||||
-rw-r--r-- | internal/gitaly/service/repository/midx_test.go | 18 |
3 files changed, 36 insertions, 18 deletions
diff --git a/internal/git/stats/profile.go b/internal/git/stats/profile.go index 68d4e0991..85c4015f8 100644 --- a/internal/git/stats/profile.go +++ b/internal/git/stats/profile.go @@ -1,10 +1,9 @@ package stats -//nolint:depguard import ( "context" "errors" - "io/ioutil" + "io/fs" "os" "path/filepath" "strconv" @@ -34,13 +33,13 @@ func PackfilesCount(repoPath string) (int, error) { } // GetPackfiles returns the FileInfo of packfiles inside a repository. -func GetPackfiles(repoPath string) ([]os.FileInfo, error) { - files, err := ioutil.ReadDir(filepath.Join(repoPath, "objects/pack/")) +func GetPackfiles(repoPath string) ([]fs.DirEntry, error) { + files, err := os.ReadDir(filepath.Join(repoPath, "objects/pack/")) if err != nil { return nil, err } - var packFiles []os.FileInfo + var packFiles []fs.DirEntry for _, f := range files { if filepath.Ext(f.Name()) == ".pack" { packFiles = append(packFiles, f) diff --git a/internal/gitaly/service/repository/midx.go b/internal/gitaly/service/repository/midx.go index 7691c0680..85c0f8809 100644 --- a/internal/gitaly/service/repository/midx.go +++ b/internal/gitaly/service/repository/midx.go @@ -2,7 +2,9 @@ package repository import ( "context" + "errors" "fmt" + "io/fs" "os" "path/filepath" "strconv" @@ -209,28 +211,39 @@ func (s *server) midxRepack(ctx context.Context, repo repository.GitRepo) error // Reference: // - https://public-inbox.org/git/f3b25a9927fe560b764850ea880a71932ec2af32.1598380599.git.gitgitgadget@gmail.com/ func calculateBatchSize(repoPath string) (int64, error) { - files, err := stats.GetPackfiles(repoPath) + packfiles, err := stats.GetPackfiles(repoPath) if err != nil { return 0, err } // In case of 2 or less packs, // batch size should be 0 for a full repack - if len(files) <= 2 { + if len(packfiles) <= 2 { return 0, nil } var biggestSize int64 var secondBiggestSize int64 - for _, f := range files { - if f.Size() > biggestSize { + for _, packfile := range packfiles { + info, err := packfile.Info() + if err != nil { + // It's fine if the entry has disappeared meanwhile, we just don't account + // for its size. + if errors.Is(err, fs.ErrNotExist) { + continue + } + + return 0, fmt.Errorf("statting packfile: %w", err) + } + + if info.Size() > biggestSize { secondBiggestSize = biggestSize - biggestSize = f.Size() + biggestSize = info.Size() continue } - if f.Size() > secondBiggestSize { - secondBiggestSize = f.Size() + if info.Size() > secondBiggestSize { + secondBiggestSize = info.Size() } } diff --git a/internal/gitaly/service/repository/midx_test.go b/internal/gitaly/service/repository/midx_test.go index ea358bf36..5057fbd37 100644 --- a/internal/gitaly/service/repository/midx_test.go +++ b/internal/gitaly/service/repository/midx_test.go @@ -5,6 +5,7 @@ package repository import ( "context" "fmt" + "io/fs" "os" "path/filepath" "testing" @@ -111,8 +112,8 @@ func TestMidxRepack(t *testing.T) { "At least 1 pack file should have been created", ) - newPackFile := findNewestPackFile(t, repoPath) - assert.True(t, newPackFile.ModTime().After(time.Time{})) + _, newPackFileInfo := findNewestPackFile(t, repoPath) + assert.True(t, newPackFileInfo.ModTime().After(time.Time{})) } func TestMidxRepack_transactional(t *testing.T) { @@ -214,21 +215,26 @@ func TestMidxRepackExpire(t *testing.T) { } // findNewestPackFile returns the latest created pack file in repo's odb -func findNewestPackFile(t *testing.T, repoPath string) os.FileInfo { +func findNewestPackFile(t *testing.T, repoPath string) (fs.DirEntry, fs.FileInfo) { t.Helper() files, err := stats.GetPackfiles(repoPath) require.NoError(t, err) - var newestPack os.FileInfo + var newestPack fs.DirEntry + var newestPackInfo fs.FileInfo for _, f := range files { - if newestPack == nil || f.ModTime().After(newestPack.ModTime()) { + info, err := f.Info() + require.NoError(t, err) + + if newestPack == nil || info.ModTime().After(newestPackInfo.ModTime()) { newestPack = f + newestPackInfo = info } } require.NotNil(t, newestPack) - return newestPack + return newestPack, newestPackInfo } // addPackFiles creates some packfiles by |