compact: clean up directories thoroughly

pkg/compact/compact.go

@@ -957,17 +957,9 @@ func (c *BucketCompactor) Compact(ctx context.Context) (rerr error) {
 
 		ignoreDirs := []string{}
 		for _, gr := range groups {
-			groupIDs := []string{}
 			for _, grID := range gr.IDs() {
-				groupIDs = append(groupIDs, grID.String())
+				ignoreDirs = append(ignoreDirs, filepath.Join(gr.Key(), grID.String()))
 			}
-
-			groupDir := filepath.Join(c.compactDir, gr.Key())
-			if err := runutil.DeleteAll(groupDir, groupIDs...); err != nil {
-				level.Warn(c.logger).Log("msg", "failed deleting non-compaction group sub-directories/files, some disk space usage might have leaked. Continuing", "err", err, "dir", groupDir)
-			}
-
-			ignoreDirs = append(ignoreDirs, gr.Key())
 		}
 
 		if err := runutil.DeleteAll(c.compactDir, ignoreDirs...); err != nil {

pkg/runutil/runutil.go

@@ -55,6 +55,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"strings"
 	"time"
 
 	"github.com/go-kit/kit/log"
@@ -162,40 +163,53 @@ func ExhaustCloseWithErrCapture(err *error, r io.ReadCloser, format string, a ..
 
 // DeleteAll deletes all files and directories inside the given
 // dir except for the ignoreDirs directories.
+// NOTE: DeleteAll is not idempotent.
 func DeleteAll(dir string, ignoreDirs ...string) error {
 	entries, err := ioutil.ReadDir(dir)
-	if os.IsNotExist(err) {
-		return nil
-	}
 	if err != nil {
 		return errors.Wrap(err, "read dir")
 	}
 	var groupErrs errutil.MultiError
 
+	var matchingIgnores []string
 	for _, d := range entries {
 		if !d.IsDir() {
-			if err := os.RemoveAll(filepath.Join(dir, d.Name())); err != nil && !os.IsNotExist(err) {
+			if err := os.RemoveAll(filepath.Join(dir, d.Name())); err != nil {
 				groupErrs.Add(err)
 			}
 			continue
 		}
 
-		var found bool
-		for _, id := range ignoreDirs {
-			if id == d.Name() {
-				found = true
-				break
+		// ignoreDirs might be multi-directory paths.
+		matchingIgnores = matchingIgnores[:0]
+		ignore := false
+		for _, ignoreDir := range ignoreDirs {
+			id := strings.Split(ignoreDir, "/")
+			if id[0] == d.Name() {
+				if len(id) == 1 {
+					ignore = true
+					break
+				}
+				matchingIgnores = append(matchingIgnores, filepath.Join(id[1:]...))
 			}
 		}
 
-		if !found {
-			if err := os.RemoveAll(filepath.Join(dir, d.Name())); err != nil && !os.IsNotExist(err) {
+		if ignore {
+			continue
+		}
+
+		if len(matchingIgnores) == 0 {
+			if err := os.RemoveAll(filepath.Join(dir, d.Name())); err != nil {
 				groupErrs.Add(err)
 			}
+			continue
+		}
+		if err := DeleteAll(filepath.Join(dir, d.Name()), matchingIgnores...); err != nil {
+			groupErrs.Add(err)
 		}
 	}
 
-	if groupErrs != nil {
+	if groupErrs.Err() != nil {
 		return errors.Wrap(groupErrs.Err(), "delete file/dir")
 	}
 	return nil

pkg/runutil/runutil_test.go

@@ -125,35 +125,43 @@ func TestCloseMoreThanOnce(t *testing.T) {
 	testutil.Equals(t, true, lc.WasCalled)
 }
 
-func TestClearsDirectoriesFilesProperly(t *testing.T) {
+func TestDeleteAll(t *testing.T) {
 	dir, err := ioutil.TempDir("", "example")
 	testutil.Ok(t, err)
 
 	t.Cleanup(func() {
-		os.RemoveAll(dir)
+		testutil.Ok(t, os.RemoveAll(dir))
 	})
 
-	f, err := os.Create(filepath.Join(dir, "test123"))
+	f, err := os.Create(filepath.Join(dir, "file1"))
 	testutil.Ok(t, err)
 	testutil.Ok(t, f.Close())
 
-	testutil.Ok(t, os.MkdirAll(filepath.Join(dir, "01EHBQRN4RF0HSRR1772KW0TN8"), os.ModePerm))
-	testutil.Ok(t, os.MkdirAll(filepath.Join(dir, "01EHBQRN4RF0HSRR1772KW1TN8"), os.ModePerm))
-	f, err = os.Create(filepath.Join(dir, "01EHBQRN4RF0HSRR1772KW0TN9"))
+	testutil.Ok(t, os.MkdirAll(filepath.Join(dir, "a"), os.ModePerm))
+	testutil.Ok(t, os.MkdirAll(filepath.Join(dir, "b"), os.ModePerm))
+	testutil.Ok(t, os.MkdirAll(filepath.Join(dir, "c", "innerc"), os.ModePerm))
+	f, err = os.Create(filepath.Join(dir, "a", "file2"))
+	testutil.Ok(t, err)
+	testutil.Ok(t, f.Close())
+	f, err = os.Create(filepath.Join(dir, "c", "file3"))
 	testutil.Ok(t, err)
 	testutil.Ok(t, f.Close())
 
-	testutil.Ok(t, DeleteAll(dir, "01EHBQRN4RF0HSRR1772KW0TN9", "01EHBQRN4RF0HSRR1772KW0TN8"))
+	testutil.Ok(t, DeleteAll(dir, "file1", "a", filepath.Join("c", "innerc")))
 
-	_, err = os.Stat(filepath.Join(dir, "test123"))
+	// Deleted.
+	_, err = os.Stat(filepath.Join(dir, "file1"))
 	testutil.Assert(t, os.IsNotExist(err))
-
-	_, err = os.Stat(filepath.Join(dir, "01EHBQRN4RF0HSRR1772KW0TN9"))
+	_, err = os.Stat(filepath.Join(dir, "b/"))
 	testutil.Assert(t, os.IsNotExist(err))
-
-	_, err = os.Stat(filepath.Join(dir, "01EHBQRN4RF0HSRR1772KW1TN8/"))
+	_, err = os.Stat(filepath.Join(dir, "file3"))
 	testutil.Assert(t, os.IsNotExist(err))
 
-	_, err = os.Stat(filepath.Join(dir, "01EHBQRN4RF0HSRR1772KW0TN8/"))
+	// Exits.
+	_, err = os.Stat(filepath.Join(dir, "a", "file2"))
+	testutil.Ok(t, err)
+	_, err = os.Stat(filepath.Join(dir, "a/"))
+	testutil.Ok(t, err)
+	_, err = os.Stat(filepath.Join(dir, "c", "innerc"))
 	testutil.Ok(t, err)
 }