Merge branch 'po-optimize-rm-empty-ref-dirs' into 'master'

OptimizeRepository will remove empty ref directories

Closes #2778

See merge request gitlab-org/gitaly!2204

3 files changed, 114 insertions, 0 deletions

diff --git a/changelogs/unreleased/po-optimize-rm-empty-ref-dirs.yml b/changelogs/unreleased/po-optimize-rm-empty-ref-dirs.yml
new file mode 100644
index 000000000..4b5dadea8
--- /dev/null
+++ b/changelogs/unreleased/po-optimize-rm-empty-ref-dirs.yml
@@ -0,0 +1,5 @@
+---
+title: OptimizeRepository will remove empty ref directories
+merge_request: 2204
+author:
+type: performance
diff --git a/internal/service/repository/optimize.go b/internal/service/repository/optimize.go
index dd7576977..5c366e881 100644
--- a/internal/service/repository/optimize.go
+++ b/internal/service/repository/optimize.go
@@ -2,14 +2,106 @@ package repository
 
 import (
 	"context"
+	"fmt"
+	"io/ioutil"
 	"os"
+	"path/filepath"
 
+	"github.com/prometheus/client_golang/prometheus"
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/stats"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 )
 
+var (
+	optimizeEmptyDirRemovalTotals = prometheus.NewCounter(
+		prometheus.CounterOpts{
+			Namespace: "gitaly",
+			Subsystem: "repository",
+			Name:      "optimizerepository_empty_dir_removal_total",
+			Help:      "Total number of empty directories removed by OptimizeRepository RPC",
+		},
+	)
+)
+
+func init() {
+	prometheus.MustRegister(optimizeEmptyDirRemovalTotals)
+}
+
+func removeEmptyDirs(ctx context.Context, target string) error {
+	if err := ctx.Err(); err != nil {
+                return err
+        }
+
+	entries, err := ioutil.ReadDir(target)
+	switch {
+	case os.IsNotExist(err):
+		return nil // race condition: someone else deleted it first
+	case err != nil:
+		return err
+	}
+
+	for _, e := range entries {
+		if !e.IsDir() {
+			continue
+		}
+
+		ePath := filepath.Join(target, e.Name())
+		if err := removeEmptyDirs(ctx, ePath); err != nil {
+			return err
+		}
+	}
+
+	// recheck entries now that we have potentially removed some dirs
+	entries, err = ioutil.ReadDir(target)
+	if err != nil && !os.IsNotExist(err) {
+		return err
+	}
+
+	if len(entries) > 0 {
+		return nil
+	}
+
+	switch err := os.Remove(target); {
+	case os.IsNotExist(err):
+		return nil // race condition: someone else deleted it first
+	case err != nil:
+		return err
+	}
+	optimizeEmptyDirRemovalTotals.Inc()
+
+	return nil
+}
+
+func removeRefEmptyDirs(ctx context.Context, repository *gitalypb.Repository) error {
+	rPath, err := helper.GetRepoPath(repository)
+	if err != nil {
+		return err
+	}
+	repoRefsPath := filepath.Join(rPath, "refs")
+
+	// we never want to delete the actual "refs" directory, so we start the
+	// recursive functions for each subdirectory
+	entries, err := ioutil.ReadDir(repoRefsPath)
+	if err != nil {
+		return err
+	}
+
+	for _, e := range entries {
+		if !e.IsDir() {
+			continue
+		}
+
+		ePath := filepath.Join(repoRefsPath, e.Name())
+		if err := removeEmptyDirs(ctx, ePath); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 func (s *server) optimizeRepository(ctx context.Context, repository *gitalypb.Repository) error {
 	hasBitmap, err := stats.HasBitmap(repository)
 	if err != nil {
@@ -34,6 +126,10 @@ func (s *server) optimizeRepository(ctx context.Context, repository *gitalypb.Re
 		}
 	}
 
+	if err := removeRefEmptyDirs(ctx, repository); err != nil {
+		return fmt.Errorf("OptimizeRepository: remove empty refs: %w", err)
+	}
+
 	return nil
 }
 
diff --git a/internal/service/repository/optimize_test.go b/internal/service/repository/optimize_test.go
index ed04062cf..cba2caa29 100644
--- a/internal/service/repository/optimize_test.go
+++ b/internal/service/repository/optimize_test.go
@@ -83,6 +83,11 @@ func TestOptimizeRepository(t *testing.T) {
 	require.NoError(t, err)
 	require.Empty(t, bitmaps)
 
+	mrRefs := filepath.Join(testRepoPath, "refs/merge-requests")
+	emptyRef := filepath.Join(mrRefs, "1")
+	require.NoError(t, os.MkdirAll(emptyRef, 0755))
+	require.DirExists(t, emptyRef, "sanity check for empty ref dir existence")
+
 	// optimize repository on a repository without a bitmap should call repack full
 	_, err = repoClient.OptimizeRepository(ctx, &gitalypb.OptimizeRepositoryRequest{Repository: testRepo})
 	require.NoError(t, err)
@@ -90,6 +95,14 @@ func TestOptimizeRepository(t *testing.T) {
 	bitmaps, err = filepath.Glob(filepath.Join(testRepoPath, "objects", "pack", "*.bitmap"))
 	require.NoError(t, err)
 	require.NotEmpty(t, bitmaps)
+
+	// All empty directories should be removed
+	testhelper.AssertPathNotExists(t, emptyRef)
+	testhelper.AssertPathNotExists(t, mrRefs)
+	require.FileExists(t,
+		filepath.Join(testRepoPath, "refs/heads", blobIDs[0]),
+		"unpacked refs should never be removed",
+	)
 }
 
 func TestOptimizeRepositoryValidation(t *testing.T) {