From d730aeaf762aa11b8ed837f31a7a1e9f2be6deba Mon Sep 17 00:00:00 2001
From: Jacob Vosmaer <jacob@gitlab.com>
Date: Mon, 19 Aug 2019 12:01:57 +0200
Subject: [PATCH 1/3] Start cache cleaner only in main executable (not test)

---
 cmd/gitaly/main.go            | 2 ++
 internal/cache/walker.go      | 8 ++++++--
 internal/cache/walker_test.go | 2 +-
 internal/config/config.go     | 7 +++++--
 4 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/cmd/gitaly/main.go b/cmd/gitaly/main.go
index f3f97413b90..8cebe209293 100644
--- a/cmd/gitaly/main.go
+++ b/cmd/gitaly/main.go
@@ -10,6 +10,7 @@ import (
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 	log "github.com/sirupsen/logrus"
 	"gitlab.com/gitlab-org/gitaly/internal/bootstrap"
+	"gitlab.com/gitlab-org/gitaly/internal/cache"
 	"gitlab.com/gitlab-org/gitaly/internal/config"
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/server"
@@ -102,6 +103,7 @@ func main() {
 	tracing.Initialize(tracing.WithServiceName("gitaly"))
 
 	tempdir.StartCleaning()
+	cache.StartCleaning()
 
 	log.WithError(run(b)).Error("shutting down")
 }
diff --git a/internal/cache/walker.go b/internal/cache/walker.go
index 6e0ba685a7f..5321a67a91f 100644
--- a/internal/cache/walker.go
+++ b/internal/cache/walker.go
@@ -136,14 +136,18 @@ func moveAndClear(storage config.Storage) error {
 	return nil
 }
 
+func StartCleaning() {
+	for _, storage := range config.Config.Storages {
+		startCleanWalker(storage)
+	}
+}
+
 func init() {
 	config.RegisterHook(func() error {
 		for _, storage := range config.Config.Storages {
 			if err := moveAndClear(storage); err != nil {
 				return err
 			}
-
-			startCleanWalker(storage)
 		}
 		return nil
 	})
diff --git a/internal/cache/walker_test.go b/internal/cache/walker_test.go
index b3d3044d602..fa11c4a360e 100644
--- a/internal/cache/walker_test.go
+++ b/internal/cache/walker_test.go
@@ -58,7 +58,7 @@ func TestDiskCacheObjectWalker(t *testing.T) {
 	*cache.ExportDisableMoveAndClear = true
 	defer func() { *cache.ExportDisableMoveAndClear = false }()
 
-	require.NoError(t, config.Validate()) // triggers walker
+	cache.StartCleaning()
 
 	pollCountersUntil(t, expectChecks, expectRemovals)
 
diff --git a/internal/config/config.go b/internal/config/config.go
index fb711adda2c..8b65363ed5d 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -122,12 +122,15 @@ func Load(file io.Reader) error {
 	return nil
 }
 
-// RegisterHook adds a post-validation callback.
+// RegisterHook adds a post-validation callback. Be careful with config
+// data race conditions if you spawn goroutines in a hook: tests may
+// modify configuration data _after_ your hook has run.
 func RegisterHook(f func() error) {
 	hooks = append(hooks, f)
 }
 
-// Validate checks the current Config for sanity.
+// Validate checks the current Config for sanity. It will also run all
+// hooks that have been registered RegisterHook.
 func Validate() error {
 	for _, err := range []error{
 		validateListeners(),
-- 
GitLab


From 27f641e516e9b18d2ed711e687718dc145f89bfa Mon Sep 17 00:00:00 2001
From: Jacob Vosmaer <jacob@gitlab.com>
Date: Mon, 19 Aug 2019 12:21:27 +0200
Subject: [PATCH 2/3] doc comment

---
 internal/cache/walker.go | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/internal/cache/walker.go b/internal/cache/walker.go
index 5321a67a91f..c9f3f13c005 100644
--- a/internal/cache/walker.go
+++ b/internal/cache/walker.go
@@ -136,6 +136,9 @@ func moveAndClear(storage config.Storage) error {
 	return nil
 }
 
+// StartCleaning starts goroutines that will clean up cache directories.
+// These goroutines will query config.Config, so you probably don't want
+// to spawn them when testing.
 func StartCleaning() {
 	for _, storage := range config.Config.Storages {
 		startCleanWalker(storage)
-- 
GitLab


From 045b0a9ed7a12993ee26a55a4dcd0667a89bbdf1 Mon Sep 17 00:00:00 2001
From: Jacob Vosmaer <jacob@gitlab.com>
Date: Mon, 19 Aug 2019 12:24:20 +0200
Subject: [PATCH 3/3] grammar

---
 internal/config/config.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/internal/config/config.go b/internal/config/config.go
index 8b65363ed5d..336a2510d93 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -130,7 +130,7 @@ func RegisterHook(f func() error) {
 }
 
 // Validate checks the current Config for sanity. It will also run all
-// hooks that have been registered RegisterHook.
+// hooks that have been registered with RegisterHook.
 func Validate() error {
 	for _, err := range []error{
 		validateListeners(),
-- 
GitLab