Merge pull request #6354 from kobergj/IDCache

Configurable IDCache

changelog/unreleased/id-cache.md

@@ -0,0 +1,5 @@
+Enhancement: Configurable ID Cache
+
+Makes the integrated idcache (used to reduce reads from disc) configurable with the general cache envvars
+
+https://github.com/owncloud/ocis/pull/6353

go.mod

@@ -13,7 +13,7 @@ require (
 	github.com/coreos/go-oidc v2.2.1+incompatible
 	github.com/coreos/go-oidc/v3 v3.4.0
 	github.com/cs3org/go-cs3apis v0.0.0-20221012090518-ef2996678965
-	github.com/cs3org/reva/v2 v2.13.4-0.20230517171752-b39e78fd7b28
+	github.com/cs3org/reva/v2 v2.13.4-0.20230522074943-fde10c4f3dea
 	github.com/disintegration/imaging v1.6.2
 	github.com/dutchcoders/go-clamd v0.0.0-20170520113014-b970184f4d9e
 	github.com/egirna/icap-client v0.1.1

go.sum

@@ -627,8 +627,8 @@ github.com/crewjam/httperr v0.2.0 h1:b2BfXR8U3AlIHwNeFFvZ+BV1LFvKLlzMjzaTnZMybNo
 github.com/crewjam/httperr v0.2.0/go.mod h1:Jlz+Sg/XqBQhyMjdDiC+GNNRzZTD7x39Gu3pglZ5oH4=
 github.com/crewjam/saml v0.4.13 h1:TYHggH/hwP7eArqiXSJUvtOPNzQDyQ7vwmwEqlFWhMc=
 github.com/crewjam/saml v0.4.13/go.mod h1:igEejV+fihTIlHXYP8zOec3V5A8y3lws5bQBFsTm4gA=
-github.com/cs3org/reva/v2 v2.13.4-0.20230517171752-b39e78fd7b28 h1:y8oUHEWGXrs9/jdU0sG3zNRyoDBONyxCRJJfb9Nsbqw=
-github.com/cs3org/reva/v2 v2.13.4-0.20230517171752-b39e78fd7b28/go.mod h1:MoymB39kU/myG7LFkaCwqtoXQHct+/8uoZAvJEmNi+I=
+github.com/cs3org/reva/v2 v2.13.4-0.20230522074943-fde10c4f3dea h1:UDl1PP9Ydr/0NE1pXPzYSUJrmltFottU9QKcFgf4o40=
+github.com/cs3org/reva/v2 v2.13.4-0.20230522074943-fde10c4f3dea/go.mod h1:MoymB39kU/myG7LFkaCwqtoXQHct+/8uoZAvJEmNi+I=
 github.com/cubewise-code/go-mime v0.0.0-20200519001935-8c5762b177d8 h1:Z9lwXumT5ACSmJ7WGnFl+OMLLjpz5uR2fyz7dC255FI=
 github.com/cubewise-code/go-mime v0.0.0-20200519001935-8c5762b177d8/go.mod h1:4abs/jPXcmJzYoYGF91JF9Uq9s/KL5n1jvFDix8KcqY=
 github.com/cyberdelia/templates v0.0.0-20141128023046-ca7fffd4298c/go.mod h1:GyV+0YP4qX0UQ7r2MoYZ+AvYDp12OF5yg4q8rGnyNh4=

services/storage-users/pkg/config/config.go

@@ -31,6 +31,7 @@ type Config struct {
 	Events            Events            `yaml:"events"`
 	StatCache         StatCache         `yaml:"stat_cache"`
 	FilemetadataCache FilemetadataCache `yaml:"filemetadata_cache"`
+	IDCache           IDCache           `yaml:"id_cache"`
 	MountID           string            `yaml:"mount_id" env:"STORAGE_USERS_MOUNT_ID" desc:"Mount ID of this storage."`
 	ExposeDataServer  bool              `yaml:"expose_data_server" env:"STORAGE_USERS_EXPOSE_DATA_SERVER" desc:"Exposes the data server directly to users and bypasses the data gateway. Ensure that the data server address is reachable by users."`
 	ReadOnly          bool              `yaml:"readonly" env:"STORAGE_USERS_READ_ONLY" desc:"Set this storage to be read-only."`
@@ -188,6 +189,15 @@ type FilemetadataCache struct {
 	Size     int           `yaml:"size" env:"OCIS_CACHE_SIZE;STORAGE_USERS_FILEMETADATA_CACHE_SIZE" desc:"The maximum quantity of items in the user info cache. Only applies when store type 'ocmem' is configured. Defaults to 512."`
 }
 
+// IDCache holds cache config
+type IDCache struct {
+	Store    string        `yaml:"store" env:"OCIS_CACHE_STORE;STORAGE_USERS_ID_CACHE_STORE" desc:"The type of the cache store. Supported values are: 'memory', 'ocmem', 'etcd', 'redis', 'redis-sentinel', 'nats-js', 'noop'. See the text description for details."`
+	Nodes    []string      `yaml:"nodes" env:"OCIS_CACHE_STORE_NODES;STORAGE_USERS_ID_CACHE_STORE_NODES" desc:"A comma separated list of nodes to access the configured store. This has no effect when 'memory' or 'ocmem' stores are configured. Note that the behaviour how nodes are used is dependent on the library of the configured store."`
+	Database string        `yaml:"database" env:"OCIS_CACHE_DATABASE" desc:"The database name the configured store should use."`
+	TTL      time.Duration `yaml:"ttl" env:"OCIS_CACHE_TTL;STORAGE_USERS_ID_CACHE_TTL" desc:"Default time to live for user info in the user info cache. Only applied when access tokens has no expiration. The duration can be set as number followed by a unit identifier like s, m or h. Defaults to '300s' (300 seconds)."`
+	Size     int           `yaml:"size" env:"OCIS_CACHE_SIZE;STORAGE_USERS_ID_CACHE_SIZE" desc:"The maximum quantity of items in the user info cache. Only applies when store type 'ocmem' is configured. Defaults to 512."`
+}
+
 // S3Driver is the storage driver configuration when using 's3' storage driver
 type S3Driver struct {
 	// Root is the absolute path to the location of the data

services/storage-users/pkg/revaconfig/drivers.go

@@ -144,6 +144,13 @@ func Ocis(cfg *config.Config) map[string]interface{} {
 			"cache_ttl":      cfg.FilemetadataCache.TTL / time.Second,
 			"cache_size":     cfg.FilemetadataCache.Size,
 		},
+		"idcache": map[string]interface{}{
+			"cache_store":    cfg.IDCache.Store,
+			"cache_nodes":    cfg.IDCache.Nodes,
+			"cache_database": cfg.IDCache.Database,
+			"cache_ttl":      cfg.IDCache.TTL / time.Second,
+			"cache_size":     cfg.IDCache.Size,
+		},
 		"events": map[string]interface{}{
 			"natsaddress":          cfg.Events.Addr,
 			"natsclusterid":        cfg.Events.ClusterID,
@@ -191,6 +198,13 @@ func OcisNoEvents(cfg *config.Config) map[string]interface{} {
 			"cache_ttl":      cfg.FilemetadataCache.TTL / time.Second,
 			"cache_size":     cfg.FilemetadataCache.Size,
 		},
+		"idcache": map[string]interface{}{
+			"cache_store":    cfg.IDCache.Store,
+			"cache_nodes":    cfg.IDCache.Nodes,
+			"cache_database": cfg.IDCache.Database,
+			"cache_ttl":      cfg.IDCache.TTL / time.Second,
+			"cache_size":     cfg.IDCache.Size,
+		},
 	}
 }
 
@@ -243,6 +257,13 @@ func S3NG(cfg *config.Config) map[string]interface{} {
 			"cache_ttl":      cfg.FilemetadataCache.TTL / time.Second,
 			"cache_size":     cfg.FilemetadataCache.Size,
 		},
+		"idcache": map[string]interface{}{
+			"cache_store":    cfg.IDCache.Store,
+			"cache_nodes":    cfg.IDCache.Nodes,
+			"cache_database": cfg.IDCache.Database,
+			"cache_ttl":      cfg.IDCache.TTL / time.Second,
+			"cache_size":     cfg.IDCache.Size,
+		},
 		"events": map[string]interface{}{
 			"natsaddress":          cfg.Events.Addr,
 			"natsclusterid":        cfg.Events.ClusterID,
@@ -294,5 +315,12 @@ func S3NGNoEvents(cfg *config.Config) map[string]interface{} {
 			"cache_ttl":      cfg.FilemetadataCache.TTL / time.Second,
 			"cache_size":     cfg.FilemetadataCache.Size,
 		},
+		"idcache": map[string]interface{}{
+			"cache_store":    cfg.IDCache.Store,
+			"cache_nodes":    cfg.IDCache.Nodes,
+			"cache_database": cfg.IDCache.Database,
+			"cache_ttl":      cfg.IDCache.TTL / time.Second,
+			"cache_size":     cfg.IDCache.Size,
+		},
 	}
 }

vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/decomposedfs.go

@@ -56,9 +56,11 @@ import (
 	"github.com/cs3org/reva/v2/pkg/storage/utils/filelocks"
 	"github.com/cs3org/reva/v2/pkg/storage/utils/templates"
 	"github.com/cs3org/reva/v2/pkg/storagespace"
+	"github.com/cs3org/reva/v2/pkg/store"
 	"github.com/cs3org/reva/v2/pkg/utils"
 	"github.com/jellydator/ttlcache/v2"
 	"github.com/pkg/errors"
+	microstore "go-micro.dev/v4/store"
 	"golang.org/x/sync/errgroup"
 )
 
@@ -117,8 +119,14 @@ func NewDefault(m map[string]interface{}, bs tree.Blobstore, es events.Stream) (
 		return nil, fmt.Errorf("unknown metadata backend %s, only 'messagepack' or 'xattrs' (default) supported", o.MetadataBackend)
 	}
 
-	tp := tree.New(lu, bs, o)
-
+	tp := tree.New(lu, bs, o, store.Create(
+		store.Store(o.IDCache.Store),
+		store.TTL(time.Duration(o.IDCache.TTL)*time.Second),
+		store.Size(o.IDCache.Size),
+		microstore.Nodes(o.IDCache.Nodes...),
+		microstore.Database(o.IDCache.Database),
+		microstore.Table(o.IDCache.Table),
+	))
 	permissionsClient, err := pool.GetPermissionsClient(o.PermissionsSVC, pool.WithTLSMode(o.PermTLSMode))
 	if err != nil {
 		return nil, err

vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/node/xattrs.go

@@ -88,6 +88,12 @@ func (n *Node) RemoveXattr(key string) error {
 // XattrsWithReader returns the extended attributes of the node. If the attributes have already
 // been cached they are not read from disk again.
 func (n *Node) XattrsWithReader(r io.Reader) (Attributes, error) {
+	if n.ID == "" {
+		// Do not try to read the attribute of an empty node. The InternalPath points to the
+		// base nodes directory in this case.
+		return Attributes{}, &xattr.Error{Op: "node.XattrsWithReader", Path: n.InternalPath(), Err: xattr.ENOATTR}
+	}
+
 	if n.xattrsCache != nil {
 		return n.xattrsCache, nil
 	}
@@ -116,6 +122,12 @@ func (n *Node) Xattrs() (Attributes, error) {
 // Xattr returns an extended attribute of the node. If the attributes have already
 // been cached it is not read from disk again.
 func (n *Node) Xattr(key string) ([]byte, error) {
+	if n.ID == "" {
+		// Do not try to read the attribute of an empty node. The InternalPath points to the
+		// base nodes directory in this case.
+		return []byte{}, &xattr.Error{Op: "node.Xattr", Path: n.InternalPath(), Name: key, Err: xattr.ENOATTR}
+	}
+
 	if n.xattrsCache == nil {
 		attrs, err := n.lu.MetadataBackend().All(n.InternalPath())
 		if err != nil {
@@ -128,7 +140,7 @@ func (n *Node) Xattr(key string) ([]byte, error) {
 		return val, nil
 	}
 	// wrap the error as xattr does
-	return []byte{}, &xattr.Error{Op: "xattr.get", Path: n.InternalPath(), Name: key, Err: xattr.ENOATTR}
+	return []byte{}, &xattr.Error{Op: "node.Xattr", Path: n.InternalPath(), Name: key, Err: xattr.ENOATTR}
 }
 
 // XattrString returns the string representation of an attribute

vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/options/options.go

@@ -69,6 +69,7 @@ type Options struct {
 
 	StatCache         cache.Config `mapstructure:"statcache"`
 	FileMetadataCache cache.Config `mapstructure:"filemetadatacache"`
+	IDCache           cache.Config `mapstructure:"idcache"`
 
 	MaxAcquireLockCycles    int `mapstructure:"max_acquire_lock_cycles"`
 	LockCycleDurationFactor int `mapstructure:"lock_cycle_duration_factor"`

vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/tree/tree.go

@@ -32,7 +32,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/bluele/gcache"
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	"github.com/cs3org/reva/v2/pkg/appctx"
 	"github.com/cs3org/reva/v2/pkg/errtypes"
@@ -47,6 +46,7 @@ import (
 	"github.com/pkg/errors"
 	"github.com/rogpeppe/go-internal/lockedfile"
 	"github.com/rs/zerolog/log"
+	"go-micro.dev/v4/store"
 	"golang.org/x/sync/errgroup"
 )
 
@@ -80,19 +80,19 @@ type Tree struct {
 
 	options *options.Options
 
-	idCache gcache.Cache
+	idCache store.Store
 }
 
 // PermissionCheckFunc defined a function used to check resource permissions
 type PermissionCheckFunc func(rp *provider.ResourcePermissions) bool
 
 // New returns a new instance of Tree
-func New(lu PathLookup, bs Blobstore, o *options.Options) *Tree {
+func New(lu PathLookup, bs Blobstore, o *options.Options, cache store.Store) *Tree {
 	return &Tree{
 		lookup:    lu,
 		blobstore: bs,
 		options:   o,
-		idCache:   gcache.New(1_000_000).LRU().Build(),
+		idCache:   cache,
 	}
 }
 
@@ -234,6 +234,9 @@ func (t *Tree) Move(ctx context.Context, oldNode *node.Node, newNode *node.Node)
 		}
 	}
 
+	// remove cache entry in any case to avoid inconsistencies
+	defer func() { _ = t.idCache.Delete(filepath.Join(oldNode.ParentPath(), oldNode.Name)) }()
+
 	// Always target the old node ID for xattr updates.
 	// The new node id is empty if the target does not exist
 	// and we need to overwrite the new one when overwriting an existing path.
@@ -271,7 +274,6 @@ func (t *Tree) Move(ctx context.Context, oldNode *node.Node, newNode *node.Node)
 	if err != nil {
 		return errors.Wrap(err, "Decomposedfs: could not move child")
 	}
-	t.idCache.Remove(filepath.Join(oldNode.ParentPath(), oldNode.Name))
 
 	// update target parentid and name
 	attribs := node.Attributes{}
@@ -361,16 +363,14 @@ func (t *Tree) ListFolder(ctx context.Context, n *node.Node) ([]*node.Node, erro
 	for i := 0; i < numWorkers; i++ {
 		g.Go(func() error {
 			for name := range work {
-				nodeID := ""
 				path := filepath.Join(dir, name)
-				if val, err := t.idCache.Get(path); err == nil {
-					nodeID = val.(string)
-				} else {
+				nodeID := getNodeIDFromCache(path, t.idCache)
+				if nodeID == "" {
 					nodeID, err = readChildNodeFromLink(path)
 					if err != nil {
 						return err
 					}
-					err = t.idCache.Set(path, nodeID)
+					err = storeNodeIDInCache(path, nodeID, t.idCache)
 					if err != nil {
 						return err
 					}
@@ -416,6 +416,10 @@ func (t *Tree) ListFolder(ctx context.Context, n *node.Node) ([]*node.Node, erro
 
 // Delete deletes a node in the tree by moving it to the trash
 func (t *Tree) Delete(ctx context.Context, n *node.Node) (err error) {
+	path := filepath.Join(n.ParentPath(), n.Name)
+	// remove entry from cache immediately to avoid inconsistencies
+	defer func() { _ = t.idCache.Delete(path) }()
+
 	deletingSharedResource := ctx.Value(appctx.DeletingSharedResource)
 
 	if deletingSharedResource != nil && deletingSharedResource.(bool) {
@@ -492,10 +496,7 @@ func (t *Tree) Delete(ctx context.Context, n *node.Node) (err error) {
 	_ = os.Remove(n.LockFilePath())
 
 	// finally remove the entry from the parent dir
-	path := filepath.Join(n.ParentPath(), n.Name)
-	err = os.Remove(path)
-	t.idCache.Remove(path)
-	if err != nil {
+	if err = os.Remove(path); err != nil {
 		// To roll back changes
 		// TODO revert the rename
 		// TODO remove symlink
@@ -980,3 +981,18 @@ func (t *Tree) readRecycleItem(ctx context.Context, spaceID, key, path string) (
 
 	return
 }
+
+func getNodeIDFromCache(path string, cache store.Store) string {
+	recs, err := cache.Read(path)
+	if err == nil && len(recs) > 0 {
+		return string(recs[0].Value)
+	}
+	return ""
+}
+
+func storeNodeIDInCache(path string, nodeID string, cache store.Store) error {
+	return cache.Write(&store.Record{
+		Key:   path,
+		Value: []byte(nodeID),
+	})
+}

vendor/modules.txt

@@ -349,7 +349,7 @@ github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1
 github.com/cs3org/go-cs3apis/cs3/storage/registry/v1beta1
 github.com/cs3org/go-cs3apis/cs3/tx/v1beta1
 github.com/cs3org/go-cs3apis/cs3/types/v1beta1
-# github.com/cs3org/reva/v2 v2.13.4-0.20230517171752-b39e78fd7b28
+# github.com/cs3org/reva/v2 v2.13.4-0.20230522074943-fde10c4f3dea
 ## explicit; go 1.19
 github.com/cs3org/reva/v2/cmd/revad/internal/grace
 github.com/cs3org/reva/v2/cmd/revad/runtime