Add a xattr write-through cache for nodes

The cache functions as a request-level xattr cache and reduces the number of *xattr syscalls being made.
cs3org · Nov 7, 2022 · 5a6a954 · 5a6a954
1 parent 78bcc2c
commit 5a6a954
Show file tree

Hide file tree

Showing 11 changed files with 240 additions and 179 deletions.
diff --git a/pkg/storage/utils/decomposedfs/decomposedfs.go b/pkg/storage/utils/decomposedfs/decomposedfs.go
@@ -341,7 +341,7 @@ func (fs *Decomposedfs) CreateDir(ctx context.Context, ref *provider.Reference)
 
 	if fs.o.TreeTimeAccounting || fs.o.TreeSizeAccounting {
 		// mark the home node as the end of propagation
-		if err = n.SetMetadata(xattrs.PropagationAttr, "1"); err != nil {
+		if err = n.SetXattr(xattrs.PropagationAttr, "1"); err != nil {
 			appctx.GetLogger(ctx).Error().Err(err).Interface("node", n).Msg("could not mark node to propagate")
 
 			// FIXME: This does not return an error at all, but results in a severe situation that the

diff --git a/pkg/storage/utils/decomposedfs/grants.go b/pkg/storage/utils/decomposedfs/grants.go
@@ -135,15 +135,21 @@ func (fs *Decomposedfs) ListGrants(ctx context.Context, ref *provider.Reference)
 	}
 	log := appctx.GetLogger(ctx)
 	np := node.InternalPath()
-	var attrs []string
-	if attrs, err = xattrs.List(np); err != nil {
+	var attrs map[string]string
+	if attrs, err = node.Xattrs(); err != nil {
 		log.Error().Err(err).Msg("error listing attributes")
 		return nil, err
 	}
+	attrNames := make([]string, len(attrs))
+	i := 0
+	for attr := range attrs {
+		attrNames[i] = attr
+		i++
+	}
 
-	log.Debug().Interface("attrs", attrs).Msg("read attributes")
+	log.Debug().Interface("attrs", attrNames).Msg("read attributes")
 
-	aces := extractACEsFromAttrs(ctx, np, attrs)
+	aces := extractACEsFromAttrs(ctx, np, attrNames)
 
 	uid := ctxpkg.ContextMustGetUser(ctx).GetId()
 	grants = make([]*provider.Grant, 0, len(aces))
@@ -303,7 +309,7 @@ func (fs *Decomposedfs) storeGrant(ctx context.Context, n *node.Node, g *provide
 	// set the grant
 	e := ace.FromGrant(g)
 	principal, value := e.Marshal()
-	if err := n.SetMetadata(xattrs.GrantPrefix+principal, string(value)); err != nil {
+	if err := n.SetXattr(xattrs.GrantPrefix+principal, string(value)); err != nil {
 		appctx.GetLogger(ctx).Error().Err(err).
 			Str("principal", principal).Msg("Could not set grant for principal")
 		return err

diff --git a/pkg/storage/utils/decomposedfs/lookup/lookup.go b/pkg/storage/utils/decomposedfs/lookup/lookup.go
@@ -134,7 +134,7 @@ func (lu *Lookup) WalkPath(ctx context.Context, r *node.Node, p string, followRe
 		}
 
 		if followReferences {
-			if attrBytes, err := r.GetMetadata(xattrs.ReferenceAttr); err == nil {
+			if attrBytes, err := r.Xattr(xattrs.ReferenceAttr); err == nil {
 				realNodeID := attrBytes
 				ref, err := xattrs.ReferenceFromAttr([]byte(realNodeID))
 				if err != nil {
@@ -147,7 +147,7 @@ func (lu *Lookup) WalkPath(ctx context.Context, r *node.Node, p string, followRe
 				}
 			}
 		}
-		if node.IsSpaceRoot(r) {
+		if r.IsSpaceRoot() {
 			r.SpaceRoot = r
 		}
 

diff --git a/pkg/storage/utils/decomposedfs/metadata.go b/pkg/storage/utils/decomposedfs/metadata.go
@@ -33,7 +33,6 @@ import (
 	"github.com/cs3org/reva/v2/pkg/storagespace"
 	"github.com/cs3org/reva/v2/pkg/utils"
 	"github.com/pkg/errors"
-	"github.com/pkg/xattr"
 )
 
 // SetArbitraryMetadata sets the metadata on a resource
@@ -69,8 +68,6 @@ func (fs *Decomposedfs) SetArbitraryMetadata(ctx context.Context, ref *provider.
 		return err
 	}
 
-	nodePath := n.InternalPath()
-
 	errs := []error{}
 	// TODO should we really continue updating when an error occurs?
 	if md.Metadata != nil {
@@ -115,7 +112,7 @@ func (fs *Decomposedfs) SetArbitraryMetadata(ctx context.Context, ref *provider.
 	}
 	for k, v := range md.Metadata {
 		attrName := xattrs.MetadataPrefix + k
-		if err = xattr.Set(nodePath, attrName, []byte(v)); err != nil {
+		if err = n.SetXattr(attrName, v); err != nil {
 			errs = append(errs, errors.Wrap(err, "Decomposedfs: could not set metadata attribute "+attrName+" to "+k))
 		}
 	}
@@ -166,7 +163,6 @@ func (fs *Decomposedfs) UnsetArbitraryMetadata(ctx context.Context, ref *provide
 		return err
 	}
 
-	nodePath := n.InternalPath()
 	errs := []error{}
 	for _, k := range keys {
 		switch k {
@@ -189,7 +185,7 @@ func (fs *Decomposedfs) UnsetArbitraryMetadata(ctx context.Context, ref *provide
 				continue
 			}
 			fa := fmt.Sprintf("%s:%s:%s@%s", xattrs.FavPrefix, utils.UserTypeToString(uid.GetType()), uid.GetOpaqueId(), uid.GetIdp())
-			if err := xattrs.Remove(nodePath, fa); err != nil {
+			if err := n.RemoveXattr(fa); err != nil {
 				if xattrs.IsAttrUnset(err) {
 					continue // already gone, ignore
 				}
@@ -200,7 +196,7 @@ func (fs *Decomposedfs) UnsetArbitraryMetadata(ctx context.Context, ref *provide
 				errs = append(errs, errors.Wrap(err, "could not unset favorite flag"))
 			}
 		default:
-			if err = xattrs.Remove(nodePath, xattrs.MetadataPrefix+k); err != nil {
+			if err = n.RemoveXattr(xattrs.MetadataPrefix + k); err != nil {
 				if xattrs.IsAttrUnset(err) {
 					continue // already gone, ignore
 				}

diff --git a/pkg/storage/utils/decomposedfs/node/locks_test.go b/pkg/storage/utils/decomposedfs/node/locks_test.go
@@ -81,8 +81,12 @@ var _ = Describe("Node locks", func() {
 			AppName: "app2",
 			LockId:  uuid.New().String(),
 		}
-		n = node.New("u-s-e-r-id", "tobelockedid", "", "tobelocked", 10, "", env.Owner.Id, env.Lookup)
-		n2 = node.New("u-s-e-r-id", "neverlockedid", "", "neverlocked", 10, "", env.Owner.Id, env.Lookup)
+		spaceResID, err := env.CreateTestStorageSpace("project", &provider.Quota{QuotaMaxBytes: 2000})
+		Expect(err).ToNot(HaveOccurred())
+		n, err = env.CreateTestFile("tobelockedid", "blob", spaceResID.OpaqueId, spaceResID.OpaqueId, 10)
+		Expect(err).ToNot(HaveOccurred())
+		n2, err = env.CreateTestFile("neverlockedlockedid", "blob", spaceResID.OpaqueId, spaceResID.OpaqueId, 10)
+		Expect(err).ToNot(HaveOccurred())
 	})
 
 	AfterEach(func() {