From a4bd025cb8553d63da06e2a4a67927712c4a0897 Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Tue, 26 Oct 2021 15:32:01 +0200 Subject: [PATCH 1/2] Reduce redundant stat calls when statting by resource ID --- .../unreleased/reduce-redundant-stat-calls.md | 3 ++ .../grpc/services/gateway/storageprovider.go | 52 +++++++++---------- pkg/storage/utils/eosfs/eosfs.go | 33 +++++++----- 3 files changed, 50 insertions(+), 38 deletions(-) create mode 100644 changelog/unreleased/reduce-redundant-stat-calls.md diff --git a/changelog/unreleased/reduce-redundant-stat-calls.md b/changelog/unreleased/reduce-redundant-stat-calls.md new file mode 100644 index 0000000000..195068ed6f --- /dev/null +++ b/changelog/unreleased/reduce-redundant-stat-calls.md @@ -0,0 +1,3 @@ +Enhancement: Reduce redundant stat calls when statting by resource ID + +https://github.com/cs3org/reva/pull/2208 \ No newline at end of file diff --git a/internal/grpc/services/gateway/storageprovider.go b/internal/grpc/services/gateway/storageprovider.go index da46ffbd8e..f809d69876 100644 --- a/internal/grpc/services/gateway/storageprovider.go +++ b/internal/grpc/services/gateway/storageprovider.go @@ -1367,16 +1367,26 @@ func (s *svc) statOnProvider(ctx context.Context, req *provider.StatRequest, res } func (s *svc) Stat(ctx context.Context, req *provider.StatRequest) (*provider.StatResponse, error) { - if utils.IsRelativeReference(req.Ref) { return s.stat(ctx, req) } - p, st := s.getPath(ctx, req.Ref, req.ArbitraryMetadataKeys...) - if st.Code != rpc.Code_CODE_OK { - return &provider.StatResponse{ - Status: st, - }, nil + p := "" + var res *provider.StatResponse + var err error + if utils.IsAbsolutePathReference(req.Ref) { + p = req.Ref.Path + } else { + res, err = s.stat(ctx, req) + if err != nil { + return &provider.StatResponse{ + Status: status.NewInternal(ctx, err, "gateway: error stating ref:"+req.Ref.String()), + }, nil + } + if res != nil && res.Status.Code != rpc.Code_CODE_OK { + return res, nil + } + p = res.Info.Path } if path.Clean(p) == s.getHome(ctx) { @@ -1388,33 +1398,23 @@ func (s *svc) Stat(ctx context.Context, req *provider.StatRequest) (*provider.St } if !s.inSharedFolder(ctx, p) { + if res != nil { + return res, nil + } return s.stat(ctx, req) } // we need to provide the info of the target, not the reference. if s.isShareName(ctx, p) { - statRes, err := s.stat(ctx, req) + ri, protocol, err := s.checkRef(ctx, res.Info) if err != nil { return &provider.StatResponse{ - Status: status.NewInternal(ctx, err, "gateway: error stating ref:"+req.Ref.String()), - }, nil - } - - if statRes.Status.Code != rpc.Code_CODE_OK { - return &provider.StatResponse{ - Status: statRes.Status, - }, nil - } - - ri, protocol, err := s.checkRef(ctx, statRes.Info) - if err != nil { - return &provider.StatResponse{ - Status: status.NewStatusFromErrType(ctx, "error resolving reference "+statRes.Info.Target, err), + Status: status.NewStatusFromErrType(ctx, "error resolving reference "+res.Info.Target, err), }, nil } if protocol == "webdav" { - ri, err = s.webdavRefStat(ctx, statRes.Info.Target) + ri, err = s.webdavRefStat(ctx, res.Info.Target) if err != nil { return &provider.StatResponse{ Status: status.NewInternal(ctx, err, "gateway: error resolving webdav reference: "+p), @@ -1426,10 +1426,10 @@ func (s *svc) Stat(ctx context.Context, req *provider.StatRequest) (*provider.St // information. For example, if requests comes to: /home/MyShares/photos and photos // is reference to /user/peter/Holidays/photos, we need to still return to the user // /home/MyShares/photos - orgPath := statRes.Info.Path - statRes.Info = ri - statRes.Info.Path = orgPath - return statRes, nil + orgPath := res.Info.Path + res.Info = ri + res.Info.Path = orgPath + return res, nil } diff --git a/pkg/storage/utils/eosfs/eosfs.go b/pkg/storage/utils/eosfs/eosfs.go index 362922dfb7..664c7b0ced 100644 --- a/pkg/storage/utils/eosfs/eosfs.go +++ b/pkg/storage/utils/eosfs/eosfs.go @@ -771,11 +771,30 @@ func (fs *eosfs) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []st log := appctx.GetLogger(ctx) log.Info().Msg("eosfs: get md for ref:" + ref.String()) - p, err := fs.resolve(ctx, ref) + u, err := getUser(ctx) if err != nil { - return nil, errors.Wrap(err, "eosfs: error resolving reference") + return nil, err + } + auth, err := fs.getUserAuth(ctx, u, "") + if err != nil { + return nil, err + } + + if ref.ResourceId != nil { + fid, err := strconv.ParseUint(ref.ResourceId.OpaqueId, 10, 64) + if err != nil { + return nil, fmt.Errorf("error converting string to int for eos fileid: %s", ref.ResourceId.OpaqueId) + } + + eosFileInfo, err := fs.c.GetFileInfoByInode(ctx, auth, fid) + if err != nil { + return nil, err + } + return fs.convertToResourceInfo(ctx, eosFileInfo) } + p := ref.Path + // if path is home we need to add in the response any shadow folder in the shadow homedirectory. if fs.conf.EnableHome { if fs.isShareFolder(ctx, p) { @@ -784,16 +803,6 @@ func (fs *eosfs) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []st } fn := fs.wrap(ctx, p) - - u, err := getUser(ctx) - if err != nil { - return nil, err - } - auth, err := fs.getUserAuth(ctx, u, fn) - if err != nil { - return nil, err - } - eosFileInfo, err := fs.c.GetFileInfoByPath(ctx, auth, fn) if err != nil { return nil, err From ec038fd28c070d0103c71380b8d194e3df25f679 Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Tue, 26 Oct 2021 15:56:35 +0200 Subject: [PATCH 2/2] Stat path references corresponding to share names --- .../grpc/services/gateway/storageprovider.go | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/internal/grpc/services/gateway/storageprovider.go b/internal/grpc/services/gateway/storageprovider.go index f809d69876..b29d983ac1 100644 --- a/internal/grpc/services/gateway/storageprovider.go +++ b/internal/grpc/services/gateway/storageprovider.go @@ -1377,6 +1377,8 @@ func (s *svc) Stat(ctx context.Context, req *provider.StatRequest) (*provider.St if utils.IsAbsolutePathReference(req.Ref) { p = req.Ref.Path } else { + // Reference by just resource ID + // Stat it and store for future use res, err = s.stat(ctx, req) if err != nil { return &provider.StatResponse{ @@ -1406,6 +1408,24 @@ func (s *svc) Stat(ctx context.Context, req *provider.StatRequest) (*provider.St // we need to provide the info of the target, not the reference. if s.isShareName(ctx, p) { + // If we haven't returned an error by now and res is nil, it means that + // req is an absolute path based ref, so we didn't stat it previously. + // So stat it now + if res == nil { + res, err = s.stat(ctx, req) + if err != nil { + return &provider.StatResponse{ + Status: status.NewInternal(ctx, err, "gateway: error stating ref:"+req.Ref.String()), + }, nil + } + + if res.Status.Code != rpc.Code_CODE_OK { + return &provider.StatResponse{ + Status: res.Status, + }, nil + } + } + ri, protocol, err := s.checkRef(ctx, res.Info) if err != nil { return &provider.StatResponse{