diff options
author | Paul Okstad <pokstad@gitlab.com> | 2020-01-07 21:48:07 +0300 |
---|---|---|
committer | John Cai <jcai@gitlab.com> | 2020-01-07 21:48:07 +0300 |
commit | f471b52a805f6c112dab105dcd88c4ac7def41ae (patch) | |
tree | 102611809205e283c6bcad52b0422796e9a3eac8 /internal | |
parent | 57f5c06e85935eb33b0b8a8188e83237234eda7d (diff) |
File walker deletes empty directories
Diffstat (limited to 'internal')
-rw-r--r-- | internal/cache/prometheus.go | 8 | ||||
-rw-r--r-- | internal/cache/walk_test.go | 12 | ||||
-rw-r--r-- | internal/cache/walker.go | 68 | ||||
-rw-r--r-- | internal/cache/walker_internal_test.go | 65 | ||||
-rw-r--r-- | internal/cache/walker_test.go | 13 |
5 files changed, 125 insertions, 41 deletions
diff --git a/internal/cache/prometheus.go b/internal/cache/prometheus.go index 16ce368a9..d25405854 100644 --- a/internal/cache/prometheus.go +++ b/internal/cache/prometheus.go @@ -52,6 +52,12 @@ var ( Help: "Total number of events during diskcache filesystem walks", }, ) + walkerErrorTotal = prometheus.NewCounter( + prometheus.CounterOpts{ + Name: "gitaly_diskcache_walker_error_total", + Help: "Total number of errors during diskcache filesystem walks", + }, + ) ) func init() { @@ -63,6 +69,7 @@ func init() { prometheus.MustRegister(errTotal) prometheus.MustRegister(walkerCheckTotal) prometheus.MustRegister(walkerRemovalTotal) + prometheus.MustRegister(walkerErrorTotal) } func countErr(err error) error { @@ -83,4 +90,5 @@ var ( countLoserBytes = func(n float64) { bytesLoserTotals.Add(n) } countWalkRemoval = func() { walkerRemovalTotal.Inc() } countWalkCheck = func() { walkerCheckTotal.Inc() } + countWalkError = func() { walkerErrorTotal.Inc() } ) diff --git a/internal/cache/walk_test.go b/internal/cache/walk_test.go deleted file mode 100644 index bfe037ff4..000000000 --- a/internal/cache/walk_test.go +++ /dev/null @@ -1,12 +0,0 @@ -package cache - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestcleanWalkDirNotExists(t *testing.T) { - err := cleanWalk("/path/that/does/not/exist") - assert.NoError(t, err, "cleanWalk returned an error for a non existing directory") -} diff --git a/internal/cache/walker.go b/internal/cache/walker.go index 94737fa9f..942d3b958 100644 --- a/internal/cache/walker.go +++ b/internal/cache/walker.go @@ -14,46 +14,72 @@ import ( "github.com/sirupsen/logrus" "gitlab.com/gitlab-org/gitaly/internal/config" "gitlab.com/gitlab-org/gitaly/internal/dontpanic" + "gitlab.com/gitlab-org/gitaly/internal/log" "gitlab.com/gitlab-org/gitaly/internal/tempdir" ) -func cleanWalk(walkPath string) error { - walkErr := filepath.Walk(walkPath, func(path string, info os.FileInfo, err error) error { - if err != nil { - return err - } +func logWalkErr(err error, path, msg string) { + countWalkError() + log.Default(). + WithField("path", path). + WithError(err). + Warn(msg) +} + +func cleanWalk(path string) error { + defer time.Sleep(100 * time.Microsecond) // relieve pressure - if info.IsDir() { + countWalkCheck() + entries, err := ioutil.ReadDir(path) + if err != nil { + if os.IsNotExist(err) { + logWalkErr(err, path, "unable to stat directory") return nil } + return err + } - countWalkCheck() + for _, e := range entries { + ePath := filepath.Join(path, e.Name()) - threshold := time.Now().Add(-1 * staleAge) - if info.ModTime().After(threshold) { - return nil + if e.IsDir() { + if err := cleanWalk(ePath); err != nil { + return err + } + continue + } + + countWalkCheck() + if time.Since(e.ModTime()) >= staleAge { + if err := os.Remove(ePath); err != nil { + if os.IsNotExist(err) { + continue + } + logWalkErr(err, ePath, "unable to remove file") + return err + } + countWalkRemoval() } + } + + files, err := ioutil.ReadDir(path) + if err != nil { + logWalkErr(err, path, "unable to stat directory after walk") + return err + } + if len(files) == 0 { if err := os.Remove(path); err != nil { if os.IsNotExist(err) { - // race condition: another file walker on the same storage may - // have deleted the file already return nil } - + logWalkErr(err, path, "unable to remove empty directory") return err } - countWalkRemoval() - - return nil - }) - - if os.IsNotExist(walkErr) { - return nil } - return walkErr + return nil } const cleanWalkFrequency = 10 * time.Minute diff --git a/internal/cache/walker_internal_test.go b/internal/cache/walker_internal_test.go new file mode 100644 index 000000000..bd5d3e768 --- /dev/null +++ b/internal/cache/walker_internal_test.go @@ -0,0 +1,65 @@ +package cache + +import ( + "io/ioutil" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCleanWalkDirNotExists(t *testing.T) { + err := cleanWalk("/path/that/does/not/exist") + assert.NoError(t, err, "cleanWalk returned an error for a non existing directory") +} + +func TestCleanWalkEmptyDirs(t *testing.T) { + tmp, err := ioutil.TempDir("", t.Name()) + require.NoError(t, err) + defer func() { require.NoError(t, os.RemoveAll(tmp)) }() + + for _, tt := range []struct { + path string + stale bool + }{ + {path: "a/b/c/"}, + {path: "a/b/c/1", stale: true}, + {path: "a/b/c/2", stale: true}, + {path: "a/b/d/"}, + {path: "e/"}, + {path: "e/1"}, + {path: "f/"}, + } { + p := filepath.Join(tmp, tt.path) + if strings.HasSuffix(tt.path, "/") { + require.NoError(t, os.MkdirAll(p, 0755)) + } else { + require.NoError(t, ioutil.WriteFile(p, nil, 0655)) + if tt.stale { + require.NoError(t, os.Chtimes(p, time.Now(), time.Now().Add(-time.Hour))) + } + } + } + + require.NoError(t, cleanWalk(tmp)) + + actual := findFiles(t, tmp) + expect := `. +./e +./e/1 +` + require.Equal(t, expect, actual) +} + +func findFiles(t testing.TB, path string) string { + cmd := exec.Command("find", ".") + cmd.Dir = path + out, err := cmd.Output() + require.NoError(t, err) + return string(out) +} diff --git a/internal/cache/walker_test.go b/internal/cache/walker_test.go index 6433b3535..d18bcf5c3 100644 --- a/internal/cache/walker_test.go +++ b/internal/cache/walker_test.go @@ -49,8 +49,7 @@ func TestDiskCacheObjectWalker(t *testing.T) { } } - expectChecks := cache.ExportMockCheckCounter.Count() + 4 - expectRemovals := cache.ExportMockRemovalCounter.Count() + 2 + expectRemovals := cache.ExportMockRemovalCounter.Count() + 4 // disable the initial move-and-clear function since we are only // evaluating the walker @@ -59,7 +58,7 @@ func TestDiskCacheObjectWalker(t *testing.T) { require.NoError(t, config.Validate()) // triggers walker - pollCountersUntil(t, expectChecks, expectRemovals) + pollCountersUntil(t, expectRemovals) for _, p := range shouldExist { assert.FileExists(t, p) @@ -148,22 +147,20 @@ func satisfyConfigValidation(tmpPath string) error { return nil } -func pollCountersUntil(t testing.TB, expectChecks, expectRemovals int) { +func pollCountersUntil(t testing.TB, expectRemovals int) { // poll injected mock prometheus counters until expected events occur timeout := time.After(time.Second) for { select { case <-timeout: t.Fatalf( - "timed out polling prometheus stats; checks: %d removals: %d", - cache.ExportMockCheckCounter.Count(), + "timed out polling prometheus stats; removals: %d", cache.ExportMockRemovalCounter.Count(), ) default: // keep on truckin' } - if cache.ExportMockCheckCounter.Count() == expectChecks && - cache.ExportMockRemovalCounter.Count() == expectRemovals { + if cache.ExportMockRemovalCounter.Count() == expectRemovals { break } time.Sleep(time.Millisecond) |