Merge branch 'sk/2756-fix-temp-dir-cleanup' into 'master'

Fix hourly tmp folder cleanup

See merge request gitlab-org/gitaly!2212

3 files changed, 47 insertions, 19 deletions

diff --git a/cmd/gitaly/main.go b/cmd/gitaly/main.go
index 535095a5b..e4853f313 100644
--- a/cmd/gitaly/main.go
+++ b/cmd/gitaly/main.go
@@ -4,6 +4,7 @@ import (
 	"flag"
 	"fmt"
 	"os"
+	"time"
 
 	log "github.com/sirupsen/logrus"
 	"gitlab.com/gitlab-org/gitaly/internal/bootstrap"
@@ -77,7 +78,7 @@ func main() {
 	config.ConfigureConcurrencyLimits()
 	tracing.Initialize(tracing.WithServiceName("gitaly"))
 
-	tempdir.StartCleaning()
+	tempdir.StartCleaning(time.Hour)
 
 	log.WithError(run(b)).Error("shutting down")
 }
diff --git a/internal/tempdir/tempdir.go b/internal/tempdir/tempdir.go
index ba2481e85..a9187abd8 100644
--- a/internal/tempdir/tempdir.go
+++ b/internal/tempdir/tempdir.go
@@ -11,6 +11,7 @@ import (
 
 	log "github.com/sirupsen/logrus"
 	"gitlab.com/gitlab-org/gitaly/internal/config"
+	"gitlab.com/gitlab-org/gitaly/internal/dontpanic"
 	"gitlab.com/gitlab-org/gitaly/internal/helper/housekeeping"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 )
@@ -113,24 +114,29 @@ func newAsRepository(ctx context.Context, storageName string, prefix string) (*g
 	return newAsRepo, tempDir, err
 }
 
-// StartCleaning starts tempdir cleanup goroutines.
-func StartCleaning() {
-	for _, st := range config.Config.Storages {
-		go func(storage config.Storage) {
-			start := time.Now()
-			err := clean(TempDir(storage))
-
-			entry := log.WithFields(log.Fields{
-				"time_ms": int(1000 * time.Since(start).Seconds()),
-				"storage": storage.Name,
-			})
-			if err != nil {
-				entry = entry.WithError(err)
-			}
-			entry.Info("finished tempdir cleaner walk")
-
-			time.Sleep(1 * time.Hour)
-		}(st)
+// StartCleaning starts tempdir cleanup in a goroutine.
+func StartCleaning(d time.Duration) {
+	dontpanic.Go(func() {
+		for {
+			cleanTempDir()
+			time.Sleep(d)
+		}
+	})
+}
+
+func cleanTempDir() {
+	for _, storage := range config.Config.Storages {
+		start := time.Now()
+		err := clean(TempDir(storage))
+
+		entry := log.WithFields(log.Fields{
+			"time_ms": time.Since(start).Milliseconds(),
+			"storage": storage.Name,
+		})
+		if err != nil {
+			entry = entry.WithError(err)
+		}
+		entry.Info("finished tempdir cleaner walk")
 	}
 }
 
diff --git a/internal/tempdir/tempdir_test.go b/internal/tempdir/tempdir_test.go
index 8c62a3515..3e184f67e 100644
--- a/internal/tempdir/tempdir_test.go
+++ b/internal/tempdir/tempdir_test.go
@@ -7,7 +7,9 @@ import (
 	"testing"
 	"time"
 
+	"github.com/sirupsen/logrus/hooks/test"
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/config"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
@@ -99,6 +101,25 @@ func TestCleanSuccess(t *testing.T) {
 	assertEntries(t, "c", "e")
 }
 
+func TestCleanTempDir(t *testing.T) {
+	hook := test.NewGlobal()
+
+	oldStorages := config.Config.Storages
+	defer func() {
+		config.Config.Storages = oldStorages
+	}()
+
+	config.Config.Storages = append(config.Config.Storages, config.Storage{
+		Name: "default",
+		Path: "testdata/clean",
+	})
+
+	cleanTempDir()
+
+	require.Equal(t, 2, len(hook.Entries))
+	require.Equal(t, "finished tempdir cleaner walk", hook.LastEntry().Message)
+}
+
 func chmod(p string, mode os.FileMode) error {
 	return os.Chmod(path.Join(cleanRoot, p), mode)
 }