From ff786a7de5740828f357d34ebe734e150e7eefd0 Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Tue, 8 Jun 2021 17:15:33 +0200 Subject: [PATCH 1/4] Add user ID cache warmup to EOS storage driver --- changelog/unreleased/eos-cache-warmup.md | 3 ++ pkg/storage/utils/eosfs/config.go | 8 ++++ pkg/storage/utils/eosfs/eosfs.go | 49 +++++++++++++++++++++--- 3 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 changelog/unreleased/eos-cache-warmup.md diff --git a/changelog/unreleased/eos-cache-warmup.md b/changelog/unreleased/eos-cache-warmup.md new file mode 100644 index 0000000000..6a32eba224 --- /dev/null +++ b/changelog/unreleased/eos-cache-warmup.md @@ -0,0 +1,3 @@ +Enhancement: Add user ID cache warmup to EOS storage driver + +https://github.com/cs3org/reva/pull/1774 diff --git a/pkg/storage/utils/eosfs/config.go b/pkg/storage/utils/eosfs/config.go index 515d7c0452..f5942025cb 100644 --- a/pkg/storage/utils/eosfs/config.go +++ b/pkg/storage/utils/eosfs/config.go @@ -111,4 +111,12 @@ type Config struct { // URI of the EOS MGM grpc server // Default is empty GrpcURI string `mapstructure:"master_grpc_uri"` + + // Size of the cache used to store user ID and UID resolution. + // Default value is 1000000. + UserIDCacheSize int `mapstructure:"user_id_cache_size"` + + // The depth to which list resources to warm up the user ID cache. + // Default value is 2. + UserIDCacheWarmupDepth int `mapstructure:"user_id_cache_warmup_depth"` } diff --git a/pkg/storage/utils/eosfs/eosfs.go b/pkg/storage/utils/eosfs/eosfs.go index 70cee23571..a1529a6c6c 100644 --- a/pkg/storage/utils/eosfs/eosfs.go +++ b/pkg/storage/utils/eosfs/eosfs.go @@ -30,8 +30,8 @@ import ( "regexp" "strconv" "strings" - "sync" + "github.com/bluele/gcache" grouppb "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" @@ -42,6 +42,7 @@ import ( "github.com/cs3org/reva/pkg/eosclient/eosbinary" "github.com/cs3org/reva/pkg/eosclient/eosgrpc" "github.com/cs3org/reva/pkg/errtypes" + "github.com/cs3org/reva/pkg/logger" "github.com/cs3org/reva/pkg/mime" "github.com/cs3org/reva/pkg/rgrpc/todo/pool" "github.com/cs3org/reva/pkg/sharedconf" @@ -119,6 +120,14 @@ func (c *Config) init() { c.UserLayout = "{{.Username}}" // TODO set better layout } + if c.UserIDCacheSize == 0 { + c.UserIDCacheSize = 1000000 + } + + if c.UserIDCacheWarmupDepth == 0 { + c.UserIDCacheWarmupDepth = 2 + } + c.GatewaySvc = sharedconf.GetGatewaySVC(c.GatewaySvc) } @@ -128,7 +137,7 @@ type eosfs struct { chunkHandler *chunking.ChunkHandler singleUserUID string singleUserGID string - userIDCache sync.Map + userIDCache gcache.Cache } // NewEOSFS returns a storage.FS interface implementation that connects to an EOS instance @@ -179,12 +188,42 @@ func NewEOSFS(c *Config) (storage.FS, error) { c: eosClient, conf: c, chunkHandler: chunking.NewChunkHandler(c.CacheDirectory), - userIDCache: sync.Map{}, + userIDCache: gcache.New(c.UserIDCacheSize).LFU().Build(), } + go eosfs.userIDcacheWarmup() + return eosfs, nil } +func (fs *eosfs) userIDcacheWarmup() { + if !fs.conf.EnableHome { + log := logger.New().With().Int("pid", os.Getpid()).Logger() + log.Debug().Msg("Starting userIDcacheWarmup") + + count := 0 + ctx := context.Background() + paths := []string{fs.wrap(ctx, "/")} + uid, gid, _ := fs.getRootUIDAndGID(ctx) + + for i := 0; i < fs.conf.UserIDCacheWarmupDepth; i++ { + var newPaths []string + for _, fn := range paths { + if eosFileInfos, err := fs.c.List(ctx, uid, gid, fn); err != nil { + for _, f := range eosFileInfos { + if _, err := fs.getUserIDGateway(ctx, strconv.FormatUint(f.UID, 10)); err != nil { + count++ + } + newPaths = append(newPaths, f.File) + } + } + } + paths = newPaths + } + log.Debug().Msgf("userIDcacheWarmup complete, added %d users", count) + } +} + func (fs *eosfs) Shutdown(ctx context.Context) error { // TODO(labkode): in a grpc implementation we can close connections. return nil @@ -1542,7 +1581,7 @@ func (fs *eosfs) getUIDGateway(ctx context.Context, u *userpb.UserId) (string, s } func (fs *eosfs) getUserIDGateway(ctx context.Context, uid string) (*userpb.UserId, error) { - if userIDInterface, ok := fs.userIDCache.Load(uid); ok { + if userIDInterface, err := fs.userIDCache.Get(uid); err != nil { return userIDInterface.(*userpb.UserId), nil } client, err := pool.GetGatewayServiceClient(fs.conf.GatewaySvc) @@ -1560,7 +1599,7 @@ func (fs *eosfs) getUserIDGateway(ctx context.Context, uid string) (*userpb.User return nil, errors.Wrap(err, "eos: grpc get user failed") } - fs.userIDCache.Store(uid, getUserResp.User.Id) + _ = fs.userIDCache.Set(uid, getUserResp.User.Id) return getUserResp.User.Id, nil } From 9f349e1d8ef2484ff189e000096bb68768a1590a Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Wed, 9 Jun 2021 11:09:54 +0200 Subject: [PATCH 2/4] Remove virtual view filtering --- pkg/storage/utils/eosfs/eosfs.go | 51 ++++++++++++-------------------- 1 file changed, 19 insertions(+), 32 deletions(-) diff --git a/pkg/storage/utils/eosfs/eosfs.go b/pkg/storage/utils/eosfs/eosfs.go index a1529a6c6c..993a999f4f 100644 --- a/pkg/storage/utils/eosfs/eosfs.go +++ b/pkg/storage/utils/eosfs/eosfs.go @@ -26,7 +26,6 @@ import ( "net/url" "os" "path" - "path/filepath" "regexp" "strconv" "strings" @@ -42,7 +41,6 @@ import ( "github.com/cs3org/reva/pkg/eosclient/eosbinary" "github.com/cs3org/reva/pkg/eosclient/eosgrpc" "github.com/cs3org/reva/pkg/errtypes" - "github.com/cs3org/reva/pkg/logger" "github.com/cs3org/reva/pkg/mime" "github.com/cs3org/reva/pkg/rgrpc/todo/pool" "github.com/cs3org/reva/pkg/sharedconf" @@ -198,10 +196,6 @@ func NewEOSFS(c *Config) (storage.FS, error) { func (fs *eosfs) userIDcacheWarmup() { if !fs.conf.EnableHome { - log := logger.New().With().Int("pid", os.Getpid()).Logger() - log.Debug().Msg("Starting userIDcacheWarmup") - - count := 0 ctx := context.Background() paths := []string{fs.wrap(ctx, "/")} uid, gid, _ := fs.getRootUIDAndGID(ctx) @@ -209,18 +203,15 @@ func (fs *eosfs) userIDcacheWarmup() { for i := 0; i < fs.conf.UserIDCacheWarmupDepth; i++ { var newPaths []string for _, fn := range paths { - if eosFileInfos, err := fs.c.List(ctx, uid, gid, fn); err != nil { + if eosFileInfos, err := fs.c.List(ctx, uid, gid, fn); err == nil { for _, f := range eosFileInfos { - if _, err := fs.getUserIDGateway(ctx, strconv.FormatUint(f.UID, 10)); err != nil { - count++ - } + _, _ = fs.getUserIDGateway(ctx, strconv.FormatUint(f.UID, 10)) newPaths = append(newPaths, f.File) } } } paths = newPaths } - log.Debug().Msgf("userIDcacheWarmup complete, added %d users", count) } } @@ -618,7 +609,7 @@ func (fs *eosfs) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []st return nil, err } - return fs.convertToResourceInfo(ctx, eosFileInfo, false) + return fs.convertToResourceInfo(ctx, eosFileInfo) } func (fs *eosfs) getMDShareFolder(ctx context.Context, p string, mdKeys []string) (*provider.ResourceInfo, error) { @@ -641,7 +632,7 @@ func (fs *eosfs) getMDShareFolder(ctx context.Context, p string, mdKeys []string // TODO(labkode): diff between root (dir) and children (ref) if fs.isShareFolderRoot(ctx, p) { - return fs.convertToResourceInfo(ctx, eosFileInfo, false) + return fs.convertToResourceInfo(ctx, eosFileInfo) } return fs.convertToFileReference(ctx, eosFileInfo) } @@ -683,10 +674,6 @@ func (fs *eosfs) listWithNominalHome(ctx context.Context, p string) (finfos []*p } fn := fs.wrap(ctx, p) - virtualView := false - if !fs.conf.EnableHome && filepath.Dir(fn) == filepath.Clean(fs.conf.Namespace) { - virtualView = true - } eosFileInfos, err := fs.c.List(ctx, uid, gid, fn) if err != nil { @@ -704,7 +691,7 @@ func (fs *eosfs) listWithNominalHome(ctx context.Context, p string) (finfos []*p } // Remove the hidden folders in the topmost directory - if finfo, err := fs.convertToResourceInfo(ctx, eosFileInfo, virtualView); err == nil && finfo.Path != "/" && !strings.HasPrefix(finfo.Path, "/.") { + if finfo, err := fs.convertToResourceInfo(ctx, eosFileInfo); err == nil && finfo.Path != "/" && !strings.HasPrefix(finfo.Path, "/.") { finfos = append(finfos, finfo) } } @@ -758,7 +745,7 @@ func (fs *eosfs) listHome(ctx context.Context, home string) ([]*provider.Resourc } } - if finfo, err := fs.convertToResourceInfo(ctx, eosFileInfo, false); err == nil && finfo.Path != "/" && !strings.HasPrefix(finfo.Path, "/.") { + if finfo, err := fs.convertToResourceInfo(ctx, eosFileInfo); err == nil && finfo.Path != "/" && !strings.HasPrefix(finfo.Path, "/.") { finfos = append(finfos, finfo) } } @@ -1363,7 +1350,7 @@ func (fs *eosfs) convertToRecycleItem(ctx context.Context, eosDeletedItem *eoscl } func (fs *eosfs) convertToRevision(ctx context.Context, eosFileInfo *eosclient.FileInfo) (*provider.FileVersion, error) { - md, err := fs.convertToResourceInfo(ctx, eosFileInfo, false) + md, err := fs.convertToResourceInfo(ctx, eosFileInfo) if err != nil { return nil, err } @@ -1376,12 +1363,12 @@ func (fs *eosfs) convertToRevision(ctx context.Context, eosFileInfo *eosclient.F return revision, nil } -func (fs *eosfs) convertToResourceInfo(ctx context.Context, eosFileInfo *eosclient.FileInfo, virtualView bool) (*provider.ResourceInfo, error) { - return fs.convert(ctx, eosFileInfo, virtualView) +func (fs *eosfs) convertToResourceInfo(ctx context.Context, eosFileInfo *eosclient.FileInfo) (*provider.ResourceInfo, error) { + return fs.convert(ctx, eosFileInfo) } func (fs *eosfs) convertToFileReference(ctx context.Context, eosFileInfo *eosclient.FileInfo) (*provider.ResourceInfo, error) { - info, err := fs.convert(ctx, eosFileInfo, false) + info, err := fs.convert(ctx, eosFileInfo) if err != nil { return nil, err } @@ -1471,7 +1458,7 @@ func mergePermissions(l *provider.ResourcePermissions, r *provider.ResourcePermi l.UpdateGrant = l.UpdateGrant || r.UpdateGrant } -func (fs *eosfs) convert(ctx context.Context, eosFileInfo *eosclient.FileInfo, virtualView bool) (*provider.ResourceInfo, error) { +func (fs *eosfs) convert(ctx context.Context, eosFileInfo *eosclient.FileInfo) (*provider.ResourceInfo, error) { path, err := fs.unwrap(ctx, eosFileInfo.File) if err != nil { return nil, err @@ -1482,13 +1469,10 @@ func (fs *eosfs) convert(ctx context.Context, eosFileInfo *eosclient.FileInfo, v size = eosFileInfo.TreeSize } - owner := &userpb.UserId{} - if !virtualView { - owner, err = fs.getUserIDGateway(ctx, strconv.FormatUint(eosFileInfo.UID, 10)) - if err != nil { - sublog := appctx.GetLogger(ctx).With().Logger() - sublog.Warn().Uint64("uid", eosFileInfo.UID).Msg("could not lookup userid, leaving empty") - } + owner, err := fs.getUserIDGateway(ctx, strconv.FormatUint(eosFileInfo.UID, 10)) + if err != nil { + sublog := appctx.GetLogger(ctx).With().Logger() + sublog.Warn().Uint64("uid", eosFileInfo.UID).Msg("could not lookup userid, leaving empty") } var xs provider.ResourceChecksum @@ -1581,9 +1565,12 @@ func (fs *eosfs) getUIDGateway(ctx context.Context, u *userpb.UserId) (string, s } func (fs *eosfs) getUserIDGateway(ctx context.Context, uid string) (*userpb.UserId, error) { - if userIDInterface, err := fs.userIDCache.Get(uid); err != nil { + log := appctx.GetLogger(ctx) + if userIDInterface, err := fs.userIDCache.Get(uid); err == nil { + log.Debug().Msg("eosfs: found cached uid " + uid) return userIDInterface.(*userpb.UserId), nil } + log.Debug().Msg("eosfs: retrieving user from gateway for uid " + uid) client, err := pool.GetGatewayServiceClient(fs.conf.GatewaySvc) if err != nil { return nil, errors.Wrap(err, "eos: error getting gateway grpc client") From c6d6c95828b8b9f1ca05eb729c58505797280217 Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Wed, 9 Jun 2021 11:56:04 +0200 Subject: [PATCH 3/4] Handle the case of root uid --- pkg/storage/utils/eosfs/eosfs.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/storage/utils/eosfs/eosfs.go b/pkg/storage/utils/eosfs/eosfs.go index 993a999f4f..85d1830b22 100644 --- a/pkg/storage/utils/eosfs/eosfs.go +++ b/pkg/storage/utils/eosfs/eosfs.go @@ -1566,10 +1566,16 @@ func (fs *eosfs) getUIDGateway(ctx context.Context, u *userpb.UserId) (string, s func (fs *eosfs) getUserIDGateway(ctx context.Context, uid string) (*userpb.UserId, error) { log := appctx.GetLogger(ctx) + // Handle the case of root + if uid == "0" { + return nil, errtypes.BadRequest("eosfs: cannot return root user") + } + if userIDInterface, err := fs.userIDCache.Get(uid); err == nil { log.Debug().Msg("eosfs: found cached uid " + uid) return userIDInterface.(*userpb.UserId), nil } + log.Debug().Msg("eosfs: retrieving user from gateway for uid " + uid) client, err := pool.GetGatewayServiceClient(fs.conf.GatewaySvc) if err != nil { From 3d04fcb89ed9b486d986c3c878ff8fbfdbd14222 Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Thu, 10 Jun 2021 16:08:14 +0200 Subject: [PATCH 4/4] Elaborate comment --- pkg/storage/utils/eosfs/config.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/storage/utils/eosfs/config.go b/pkg/storage/utils/eosfs/config.go index f5942025cb..69351fb1e8 100644 --- a/pkg/storage/utils/eosfs/config.go +++ b/pkg/storage/utils/eosfs/config.go @@ -116,7 +116,9 @@ type Config struct { // Default value is 1000000. UserIDCacheSize int `mapstructure:"user_id_cache_size"` - // The depth to which list resources to warm up the user ID cache. + // The depth, starting from root, that we'll parse directories to lookup the + // owner and warm up the cache. For example, for a layout of {{substr 0 1 .Username}}/{{.Username}} + // and a depth of 2, we'll lookup each user's home directory. // Default value is 2. UserIDCacheWarmupDepth int `mapstructure:"user_id_cache_warmup_depth"` }