WIP: Poxisfs improvements with ocis bump

.drone.env

@@ -1,4 +1,4 @@
 # The test runner source for API tests
-APITESTS_COMMITID=0dc981160fe4089d5a20761fe1274f8af7f90ed6
+APITESTS_COMMITID=099acb4c26eb3d9f2f9aef4c25e8ffdc857fcd63
 APITESTS_BRANCH=master
 APITESTS_REPO_GIT_URL=https://github.com/owncloud/ocis.git

changelog/unreleased/improve-posixfs.md

@@ -0,0 +1,6 @@
+Enhancement: Improve posixfs stability and performance
+
+The posixfs storage driver saw a number of bugfixes and optimizations.
+
+https://github.com/cs3org/reva/pull/4877
+

pkg/storage/fs/posix/lookup/lookup.go

@@ -116,6 +116,11 @@ func (lu *Lookup) NodeIDFromParentAndName(ctx context.Context, parent *node.Node
 		}
 		return "", err
 	}
+	_, err = os.Stat(filepath.Join(parent.InternalPath(), name))
+	if err != nil {
+		return "", err
+	}
+
 	return string(id), nil
 }
 
@@ -245,8 +250,6 @@ func (lu *Lookup) Path(ctx context.Context, n *node.Node, hasPermission node.Per
 			appctx.GetLogger(ctx).
 				Error().Err(err).
 				Str("path", p).
-				Str("spaceid", n.SpaceID).
-				Str("nodeid", n.ID).
 				Msg("Path()")
 			return
 		}

pkg/storage/fs/posix/lookup/lookup_suite_test.go

@@ -0,0 +1,13 @@
+package lookup_test
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+func TestLookup(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Lookup Suite")
+}

pkg/storage/fs/posix/lookup/store_idcache.go

@@ -24,6 +24,7 @@ import (
 
 	microstore "go-micro.dev/v4/store"
 
+	"github.com/cs3org/reva/v2/pkg/appctx"
 	"github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/options"
 	"github.com/cs3org/reva/v2/pkg/store"
 )
@@ -64,15 +65,43 @@ func (c *StoreIDCache) Delete(_ context.Context, spaceID, nodeID string) error {
 func (c *StoreIDCache) DeleteByPath(ctx context.Context, path string) error {
 	spaceID, nodeID, ok := c.GetByPath(ctx, path)
 	if !ok {
-		return nil
+		appctx.GetLogger(ctx).Error().Str("record", path).Msg("could not get spaceID and nodeID from cache")
+	} else {
+		err := c.cache.Delete(reverseCacheKey(path))
+		if err != nil {
+			appctx.GetLogger(ctx).Error().Err(err).Str("record", path).Str("spaceID", spaceID).Str("nodeID", nodeID).Msg("could not get spaceID and nodeID from cache")
+		}
+
+		err = c.cache.Delete(cacheKey(spaceID, nodeID))
+		if err != nil {
+			appctx.GetLogger(ctx).Error().Err(err).Str("record", path).Str("spaceID", spaceID).Str("nodeID", nodeID).Msg("could not get spaceID and nodeID from cache")
+		}
 	}
 
-	err := c.cache.Delete(reverseCacheKey(path))
+	list, err := c.cache.List(
+		microstore.ListPrefix(reverseCacheKey(path) + "/"),
+	)
 	if err != nil {
 		return err
 	}
+	for _, record := range list {
+		spaceID, nodeID, ok := c.GetByPath(ctx, record)
+		if !ok {
+			appctx.GetLogger(ctx).Error().Str("record", record).Msg("could not get spaceID and nodeID from cache")
+			continue
+		}
 
-	return c.cache.Delete(cacheKey(spaceID, nodeID))
+		err := c.cache.Delete(reverseCacheKey(record))
+		if err != nil {
+			appctx.GetLogger(ctx).Error().Err(err).Str("record", record).Str("spaceID", spaceID).Str("nodeID", nodeID).Msg("could not get spaceID and nodeID from cache")
+		}
+
+		err = c.cache.Delete(cacheKey(spaceID, nodeID))
+		if err != nil {
+			appctx.GetLogger(ctx).Error().Err(err).Str("record", record).Str("spaceID", spaceID).Str("nodeID", nodeID).Msg("could not get spaceID and nodeID from cache")
+		}
+	}
+	return nil
 }
 
 // DeletePath removes only the path entry from the cache

pkg/storage/fs/posix/lookup/store_idcache_test.go

@@ -0,0 +1,98 @@
+package lookup_test
+
+import (
+	"context"
+
+	"github.com/cs3org/reva/v2/pkg/storage/cache"
+	"github.com/cs3org/reva/v2/pkg/storage/fs/posix/lookup"
+	"github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/options"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("StoreIdcache", func() {
+	var (
+		storeIdcache *lookup.StoreIDCache
+	)
+
+	BeforeEach(func() {
+		storeIdcache = lookup.NewStoreIDCache(&options.Options{
+			IDCache: cache.Config{
+				Store:    "memory",
+				Database: "idcache",
+				Size:     100,
+				Nodes:    []string{"localhost:2379"},
+			},
+		})
+
+		Expect(storeIdcache.Set(context.TODO(), "spaceID", "nodeID", "path")).To(Succeed())
+	})
+
+	Describe("StoreIdcache", func() {
+		Describe("NewStoreIDCache", func() {
+			It("should return a new StoreIDCache", func() {
+				Expect(storeIdcache).ToNot(BeNil())
+			})
+		})
+
+		Describe("Delete", func() {
+			It("should delete an entry from the cache", func() {
+				v, ok := storeIdcache.Get(context.TODO(), "spaceID", "nodeID")
+				Expect(ok).To(BeTrue())
+				Expect(v).To(Equal("path"))
+
+				err := storeIdcache.Delete(context.TODO(), "spaceID", "nodeID")
+				Expect(err).ToNot(HaveOccurred())
+
+				_, ok = storeIdcache.Get(context.TODO(), "spaceID", "nodeID")
+				Expect(ok).To(BeFalse())
+			})
+		})
+
+		Describe("DeleteByPath", func() {
+			It("should delete an entry from the cache", func() {
+				v, ok := storeIdcache.Get(context.TODO(), "spaceID", "nodeID")
+				Expect(ok).To(BeTrue())
+				Expect(v).To(Equal("path"))
+
+				err := storeIdcache.DeleteByPath(context.TODO(), "path")
+				Expect(err).ToNot(HaveOccurred())
+
+				_, ok = storeIdcache.Get(context.TODO(), "spaceID", "nodeID")
+				Expect(ok).To(BeFalse())
+			})
+
+			It("should not delete an entry from the cache if the path does not exist", func() {
+				err := storeIdcache.DeleteByPath(context.TODO(), "nonexistent")
+				Expect(err).ToNot(HaveOccurred())
+			})
+
+			It("deletes recursively", func() {
+				Expect(storeIdcache.Set(context.TODO(), "spaceID", "nodeID", "path")).To(Succeed())
+				Expect(storeIdcache.Set(context.TODO(), "spaceID", "nodeID2", "path/child")).To(Succeed())
+				Expect(storeIdcache.Set(context.TODO(), "spaceID", "nodeID3", "path/child2")).To(Succeed())
+
+				v, ok := storeIdcache.Get(context.TODO(), "spaceID", "nodeID")
+				Expect(ok).To(BeTrue())
+				Expect(v).To(Equal("path"))
+				v, ok = storeIdcache.Get(context.TODO(), "spaceID", "nodeID2")
+				Expect(ok).To(BeTrue())
+				Expect(v).To(Equal("path/child"))
+				v, ok = storeIdcache.Get(context.TODO(), "spaceID", "nodeID3")
+				Expect(ok).To(BeTrue())
+				Expect(v).To(Equal("path/child2"))
+
+				err := storeIdcache.DeleteByPath(context.TODO(), "path")
+				Expect(err).ToNot(HaveOccurred())
+
+				_, ok = storeIdcache.Get(context.TODO(), "spaceID", "nodeID")
+				Expect(ok).To(BeFalse())
+				_, ok = storeIdcache.Get(context.TODO(), "spaceID", "nodeID2")
+				Expect(ok).To(BeFalse())
+				_, ok = storeIdcache.Get(context.TODO(), "spaceID", "nodeID3")
+				Expect(ok).To(BeFalse())
+			})
+		})
+	})
+})

pkg/storage/fs/posix/timemanager/timemanager_bsd.go

@@ -4,7 +4,7 @@ package timemanager
 
 import "syscall"
 
-// StatCtime returns the change time
+// StatCtime returns the creation time
 func StatCTime(st *syscall.Stat_t) syscall.Timespec {
 	return st.Ctimespec
 }

pkg/storage/fs/posix/timemanager/timemanager_sysv.go

@@ -4,7 +4,7 @@ package timemanager
 
 import "syscall"
 
-// StatCtime returns the change time
+// StatCtime returns the creation time
 func StatCTime(st *syscall.Stat_t) syscall.Timespec {
 	return st.Ctim
 }

pkg/storage/fs/posix/tree/assimilation.go

@@ -66,6 +66,8 @@ type queueItem struct {
 	timer *time.Timer
 }
 
+const dirtyFlag = "user.ocis.dirty"
+
 // NewScanDebouncer returns a new SpaceDebouncer instance
 func NewScanDebouncer(d time.Duration, f func(item scanItem)) *ScanDebouncer {
 	return &ScanDebouncer{
@@ -147,7 +149,7 @@ func (t *Tree) workScanQueue() {
 				}
 
 				if item.Recurse {
-					err = t.WarmupIDCache(item.Path, true)
+					err = t.WarmupIDCache(item.Path, true, false)
 					if err != nil {
 						log.Error().Err(err).Str("path", item.Path).Msg("failed to warmup id cache")
 					}
@@ -172,6 +174,9 @@ func (t *Tree) Scan(path string, action EventAction, isDir bool, recurse bool) e
 					ForceRescan: false,
 				})
 			}
+			if err := t.setDirty(filepath.Dir(path), true); err != nil {
+				return err
+			}
 			t.scanDebouncer.Debounce(scanItem{
 				Path:        filepath.Dir(path),
 				ForceRescan: true,
@@ -180,6 +185,9 @@ func (t *Tree) Scan(path string, action EventAction, isDir bool, recurse bool) e
 		} else {
 			// 2. New directory
 			//  -> scan directory
+			if err := t.setDirty(path, true); err != nil {
+				return err
+			}
 			t.scanDebouncer.Debounce(scanItem{
 				Path:        path,
 				ForceRescan: true,
@@ -368,7 +376,7 @@ func (t *Tree) assimilate(item scanItem) error {
 				}
 				if fi.IsDir() {
 					// if it was moved and it is a directory we need to propagate the move
-					go func() { _ = t.WarmupIDCache(item.Path, false) }()
+					go func() { _ = t.WarmupIDCache(item.Path, false, true) }()
 				}
 
 				parentID, err := t.lookup.MetadataBackend().Get(context.Background(), item.Path, prefixes.ParentidAttr)
@@ -536,7 +544,8 @@ assimilate:
 }
 
 // WarmupIDCache warms up the id cache
-func (t *Tree) WarmupIDCache(root string, assimilate bool) error {
+func (t *Tree) WarmupIDCache(root string, assimilate, onlyDirty bool) error {
+	root = filepath.Clean(root)
 	spaceID := []byte("")
 
 	scopeSpace := func(spaceCandidate string) error {
@@ -557,9 +566,9 @@ func (t *Tree) WarmupIDCache(root string, assimilate bool) error {
 
 	sizes := make(map[string]int64)
 	err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
-		// skip lock files
-		if isLockFile(path) {
-			return nil
+		// skip lock and upload files
+		if isLockFile(path) || isTrash(path) || t.isUpload(path) {
+			return filepath.SkipDir
 		}
 
 		if err != nil {
@@ -568,8 +577,19 @@ func (t *Tree) WarmupIDCache(root string, assimilate bool) error {
 
 		// calculate tree sizes
 		if !info.IsDir() {
-			dir := filepath.Dir(path)
-			sizes[dir] += info.Size()
+			dir := path
+			for dir != root {
+				dir = filepath.Clean(filepath.Dir(dir))
+				sizes[dir] += info.Size()
+			}
+		} else if onlyDirty {
+			dirty, err := t.isDirty(path)
+			if err != nil {
+				return err
+			}
+			if !dirty {
+				return filepath.SkipDir
+			}
 		}
 
 		attribs, err := t.lookup.MetadataBackend().All(context.Background(), path)
@@ -618,10 +638,14 @@ func (t *Tree) WarmupIDCache(root string, assimilate bool) error {
 		} else if assimilate {
 			_ = t.assimilate(scanItem{Path: path, ForceRescan: true})
 		}
-		return nil
+		return t.setDirty(path, false)
 	})
 
 	for dir, size := range sizes {
+		if dir == root {
+			// Propagate the size diff further up the tree
+			_ = t.propagateSizeDiff(dir, size)
+		}
 		_ = t.lookup.MetadataBackend().Set(context.Background(), dir, prefixes.TreesizeAttr, []byte(fmt.Sprintf("%d", size)))
 	}
 
@@ -631,3 +655,40 @@ func (t *Tree) WarmupIDCache(root string, assimilate bool) error {
 
 	return nil
 }
+
+func (t *Tree) propagateSizeDiff(dir string, size int64) error {
+	// First find the space id
+	spaceID, _, err := t.findSpaceId(dir)
+	if err != nil {
+		return err
+	}
+	attrs, err := t.lookup.MetadataBackend().All(context.Background(), dir)
+	if err != nil {
+		return err
+	}
+	n, err := t.lookup.NodeFromID(context.Background(), &provider.ResourceId{
+		StorageId: t.options.MountID,
+		SpaceId:   spaceID,
+		OpaqueId:  string(attrs[prefixes.IDAttr]),
+	})
+	if err != nil {
+		return err
+	}
+	oldSize, err := node.Attributes(attrs).Int64(prefixes.TreesizeAttr)
+	if err != nil {
+		return err
+	}
+	return t.Propagate(context.Background(), n, size-oldSize)
+}
+
+func (t *Tree) setDirty(path string, dirty bool) error {
+	return t.lookup.MetadataBackend().Set(context.Background(), path, dirtyFlag, []byte(fmt.Sprintf("%t", dirty)))
+}
+
+func (t *Tree) isDirty(path string) (bool, error) {
+	dirtyAttr, err := t.lookup.MetadataBackend().Get(context.Background(), path, dirtyFlag)
+	if err != nil {
+		return false, err
+	}
+	return string(dirtyAttr) == "true", nil
+}

pkg/storage/fs/posix/tree/gpfsfilauditloggingwatcher.go

@@ -59,7 +59,7 @@ start:
 			if err != nil {
 				continue
 			}
-			if isLockFile(ev.Path) || isTrash(ev.Path) {
+			if isLockFile(ev.Path) || isTrash(ev.Path) || w.tree.isUpload(ev.Path) {
 				continue
 			}
 			switch ev.Event {
@@ -73,7 +73,7 @@ start:
 			case "RENAME":
 				go func() {
 					_ = w.tree.Scan(ev.Path, ActionMove, false, true)
-					_ = w.tree.WarmupIDCache(ev.Path, false)
+					_ = w.tree.WarmupIDCache(ev.Path, false, false)
 				}()
 			}
 		case io.EOF: