Fix race condition in filesystem bucket client Delete() (thanos-io#5103)

* Fix race condition in filesystem bucket client Delete() Signed-off-by: Marco Pracucci <[email protected]> * Added missing copyright header Signed-off-by: Marco Pracucci <[email protected]> * Addressed review comments Signed-off-by: Marco Pracucci <[email protected]> Signed-off-by: Nicholaswang <[email protected]>
Nicholaswang · Mar 6, 2022 · 9ef3454 · 9ef3454
1 parent d218bdb
commit 9ef3454
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 4 deletions.
diff --git a/pkg/objstore/filesystem/filesystem.go b/pkg/objstore/filesystem/filesystem.go
@@ -11,12 +11,11 @@ import (
 	"os"
 	"path/filepath"
 
-	"github.com/thanos-io/thanos/pkg/objstore"
+	"github.com/pkg/errors"
 	"gopkg.in/yaml.v2"
 
+	"github.com/thanos-io/thanos/pkg/objstore"
 	"github.com/thanos-io/thanos/pkg/runutil"
-
-	"github.com/pkg/errors"
 )
 
 // Config stores the configuration for storing and accessing blobs in filesystem.
@@ -197,12 +196,16 @@ func (b *Bucket) Upload(_ context.Context, name string, r io.Reader) (err error)
 
 func isDirEmpty(name string) (ok bool, err error) {
 	f, err := os.Open(filepath.Clean(name))
+	if os.IsNotExist(err) {
+		// The directory doesn't exist. We don't consider it an error and we treat it like empty.
+		return true, nil
+	}
 	if err != nil {
 		return false, err
 	}
 	defer runutil.CloseWithErrCapture(&err, f, "dir open")
 
-	if _, err = f.Readdir(1); err == io.EOF {
+	if _, err = f.Readdir(1); err == io.EOF || os.IsNotExist(err) {
 		return true, nil
 	}
 	return false, err

diff --git a/pkg/objstore/filesystem/filesystem_test.go b/pkg/objstore/filesystem/filesystem_test.go
@@ -0,0 +1,46 @@
+// Copyright (c) The Thanos Authors.
+// Licensed under the Apache License 2.0.
+
+package filesystem
+
+import (
+	"context"
+	"strings"
+	"sync"
+	"testing"
+
+	"github.com/thanos-io/thanos/pkg/testutil"
+)
+
+func TestDelete_EmptyDirDeletionRaceCondition(t *testing.T) {
+	const runs = 1000
+
+	ctx := context.Background()
+
+	for r := 0; r < runs; r++ {
+		b, err := NewBucket(t.TempDir())
+		testutil.Ok(t, err)
+
+		// Upload 2 objects in a subfolder.
+		testutil.Ok(t, b.Upload(ctx, "subfolder/first", strings.NewReader("first")))
+		testutil.Ok(t, b.Upload(ctx, "subfolder/second", strings.NewReader("second")))
+
+		// Prepare goroutines to concurrently delete the 2 objects (each one deletes a different object)
+		start := make(chan struct{})
+		group := sync.WaitGroup{}
+		group.Add(2)
+
+		for _, object := range []string{"first", "second"} {
+			go func(object string) {
+				defer group.Done()
+
+				<-start
+				testutil.Ok(t, b.Delete(ctx, "subfolder/"+object))
+			}(object)
+		}
+
+		// Go!
+		close(start)
+		group.Wait()
+	}
+}