diff --git a/changelog/unreleased/ocs-resource-cache.md b/changelog/unreleased/ocs-resource-cache.md new file mode 100644 index 00000000000..0372fd25992 --- /dev/null +++ b/changelog/unreleased/ocs-resource-cache.md @@ -0,0 +1,8 @@ +Enhancement: Cache resources from share getter methods in OCS + +In OCS, once we retrieve the shares from the shareprovider service, we stat each +of those separately to obtain the required info, which introduces a lot of +latency. This PR introduces a resoource info cache in OCS, which would prevent +this latency. + +https://github.com/cs3org/reva/pull/1643 diff --git a/go.mod b/go.mod index 5c33a7b29b0..1311203bf72 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( github.com/Masterminds/sprig v2.22.0+incompatible github.com/ReneKroon/ttlcache/v2 v2.4.0 github.com/aws/aws-sdk-go v1.38.13 + github.com/bluele/gcache v0.0.2 // indirect github.com/c-bata/go-prompt v0.2.5 github.com/cheggaaa/pb v1.0.29 github.com/coreos/go-oidc v2.2.1+incompatible diff --git a/go.sum b/go.sum index 767f754d0bb..44b17d0ab20 100644 --- a/go.sum +++ b/go.sum @@ -92,6 +92,8 @@ github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+Ce github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/bgentry/speakeasy v0.1.0/go.mod h1:+zsyZBPWlz7T6j88CTgSN5bM796AkVf0kBD4zp0CCIs= +github.com/bluele/gcache v0.0.2 h1:WcbfdXICg7G/DGBh1PFfcirkWOQV+v077yF1pSy3DGw= +github.com/bluele/gcache v0.0.2/go.mod h1:m15KV+ECjptwSPxKhOhQoAFQVtUFjTVkc3H8o0t/fp0= github.com/bmatcuk/doublestar/v2 v2.0.3/go.mod h1:QMmcs3H2AUQICWhfzLXz+IYln8lRQmTZRptLie8RgRw= github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869/go.mod h1:Ekp36dRnpXw/yCqJaO+ZrUyxD+3VXMFFr56k5XYrpB4= github.com/bmizerany/pat v0.0.0-20170815010413-6226ea591a40 h1:y4B3+GPxKlrigF1ha5FFErxK+sr6sWxQovRMzwMhejo= diff --git a/internal/http/services/owncloud/ocs/config/config.go b/internal/http/services/owncloud/ocs/config/config.go index e62ba20c60d..f5361ece7b5 100644 --- a/internal/http/services/owncloud/ocs/config/config.go +++ b/internal/http/services/owncloud/ocs/config/config.go @@ -34,6 +34,8 @@ type Config struct { SharePrefix string `mapstructure:"share_prefix"` HomeNamespace string `mapstructure:"home_namespace"` AdditionalInfoAttribute string `mapstructure:"additional_info_attribute"` + ResourceInfoCacheSize int `mapstructure:"resource_info_cache_size"` + ResourceInfoCacheTTL int `mapstructure:"resource_info_cache_ttl"` } // Init sets sane defaults @@ -58,5 +60,13 @@ func (c *Config) Init() { c.AdditionalInfoAttribute = "{{.Mail}}" } + if c.ResourceInfoCacheSize == 0 { + c.ResourceInfoCacheSize = 1000000 + } + + if c.ResourceInfoCacheTTL == 0 { + c.ResourceInfoCacheTTL = 86400 + } + c.GatewaySvc = sharedconf.GetGatewaySVC(c.GatewaySvc) } diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/public.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/public.go index 88ce48bf4f5..8d1b7f1e0a8 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/public.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/public.go @@ -23,6 +23,7 @@ import ( "fmt" "net/http" "strconv" + "time" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1" @@ -159,33 +160,40 @@ func (h *Handler) listPublicShares(r *http.Request, filters []*link.ListPublicSh return ocsDataPayload, res.Status, nil } + var info *provider.ResourceInfo for _, share := range res.GetShare() { - - statRequest := &provider.StatRequest{ - Ref: &provider.Reference{ - Spec: &provider.Reference_Id{ - Id: share.ResourceId, + key := wrapResourceID(share.ResourceId) + if infoIf, err := h.resourceInfoCache.Get(key); err == nil { + info = infoIf.(*provider.ResourceInfo) + } else { + statRequest := &provider.StatRequest{ + Ref: &provider.Reference{ + Spec: &provider.Reference_Id{ + Id: share.ResourceId, + }, }, - }, - } - - statResponse, err := c.Stat(ctx, statRequest) - if err != nil || res.Status.Code != rpc.Code_CODE_OK { - log.Debug().Interface("share", share).Interface("response", statResponse).Err(err).Msg("could not stat share, skipping") - continue + } + + statResponse, err := c.Stat(ctx, statRequest) + if err != nil || res.Status.Code != rpc.Code_CODE_OK { + log.Debug().Interface("share", share).Interface("response", statResponse).Err(err).Msg("could not stat share, skipping") + continue + } + info = statResponse.Info + _ = h.resourceInfoCache.SetWithExpire(key, info, time.Second*h.resourceInfoCacheTTL) } sData := conversions.PublicShare2ShareData(share, r, h.publicURL) sData.Name = share.DisplayName - if err := h.addFileInfo(ctx, sData, statResponse.Info); err != nil { - log.Debug().Interface("share", share).Interface("info", statResponse.Info).Err(err).Msg("could not add file info, skipping") + if err := h.addFileInfo(ctx, sData, info); err != nil { + log.Debug().Interface("share", share).Interface("info", info).Err(err).Msg("could not add file info, skipping") continue } h.mapUserIds(ctx, c, sData) - log.Debug().Interface("share", share).Interface("info", statResponse.Info).Interface("shareData", share).Msg("mapped") + log.Debug().Interface("share", share).Interface("info", info).Interface("shareData", share).Msg("mapped") ocsDataPayload = append(ocsDataPayload, sData) diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go index c62a3409df7..b553784e02c 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go @@ -42,6 +42,7 @@ import ( "github.com/rs/zerolog/log" "github.com/ReneKroon/ttlcache/v2" + "github.com/bluele/gcache" "github.com/cs3org/reva/internal/http/services/owncloud/ocdav" "github.com/cs3org/reva/internal/http/services/owncloud/ocs/config" "github.com/cs3org/reva/internal/http/services/owncloud/ocs/conversions" @@ -58,8 +59,10 @@ type Handler struct { publicURL string sharePrefix string homeNamespace string + resourceInfoCacheTTL time.Duration additionalInfoTemplate *template.Template userIdentifierCache *ttlcache.Cache + resourceInfoCache gcache.Cache } // we only cache the minimal set of data instead of the full user metadata @@ -75,12 +78,15 @@ func (h *Handler) Init(c *config.Config) error { h.publicURL = c.Config.Host h.sharePrefix = c.SharePrefix h.homeNamespace = c.HomeNamespace + h.resourceInfoCacheTTL = time.Duration(c.ResourceInfoCacheTTL) h.additionalInfoTemplate, _ = template.New("additionalInfo").Parse(c.AdditionalInfoAttribute) h.userIdentifierCache = ttlcache.NewCache() _ = h.userIdentifierCache.SetTTL(60 * time.Second) + h.resourceInfoCache = gcache.New(c.ResourceInfoCacheSize).LFU().Build() + return nil } @@ -391,31 +397,39 @@ func (h *Handler) getShare(w http.ResponseWriter, r *http.Request, shareID strin return } - // prepare the stat request - statReq := &provider.StatRequest{ - // prepare the reference - Ref: &provider.Reference{ - // using ResourceId from the share - Spec: &provider.Reference_Id{Id: resourceID}, - }, - } + var info *provider.ResourceInfo + key := wrapResourceID(resourceID) + if infoIf, err := h.resourceInfoCache.Get(key); err == nil { + info = infoIf.(*provider.ResourceInfo) + } else { + // prepare the stat request + statReq := &provider.StatRequest{ + // prepare the reference + Ref: &provider.Reference{ + // using ResourceId from the share + Spec: &provider.Reference_Id{Id: resourceID}, + }, + } - statResponse, err := client.Stat(ctx, statReq) - if err != nil { - log.Error().Err(err).Msg("error mapping share data") - response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error mapping share data", err) - return - } + statResponse, err := client.Stat(ctx, statReq) + if err != nil { + log.Error().Err(err).Msg("error mapping share data") + response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error mapping share data", err) + return + } - if statResponse.Status.Code != rpc.Code_CODE_OK { - log.Error().Err(err).Str("status", statResponse.Status.Code.String()).Msg("error mapping share data") - response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error mapping share data", err) - return + if statResponse.Status.Code != rpc.Code_CODE_OK { + log.Error().Err(err).Str("status", statResponse.Status.Code.String()).Msg("error mapping share data") + response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error mapping share data", err) + return + } + info = statResponse.Info + _ = h.resourceInfoCache.SetWithExpire(key, info, time.Second*h.resourceInfoCacheTTL) } - err = h.addFileInfo(ctx, share, statResponse.Info) + err = h.addFileInfo(ctx, share, info) if err != nil { - log.Error().Err(err).Str("status", statResponse.Status.Code.String()).Msg("error mapping share data") + log.Error().Err(err).Msg("error mapping share data") response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error mapping share data", err) } h.mapUserIds(ctx, client, share) @@ -593,33 +607,38 @@ func (h *Handler) listSharesWithMe(w http.ResponseWriter, r *http.Request) { // prefix the path with the owners home, because ocs share requests are relative to the home dir target := path.Join(h.homeNamespace, r.FormValue("path")) - statReq := &provider.StatRequest{ - Ref: &provider.Reference{ - Spec: &provider.Reference_Path{ - Path: target, + if infoIf, err := h.resourceInfoCache.Get(target); err == nil { + pinfo = infoIf.(*provider.ResourceInfo) + } else { + statReq := &provider.StatRequest{ + Ref: &provider.Reference{ + Spec: &provider.Reference_Path{ + Path: target, + }, }, - }, - } + } - statRes, err := gwc.Stat(ctx, statReq) - if err != nil { - response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error sending a grpc stat request", err) - return - } + statRes, err := gwc.Stat(ctx, statReq) + if err != nil { + response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error sending a grpc stat request", err) + return + } - if statRes.Status.Code != rpc.Code_CODE_OK { - switch statRes.Status.Code { - case rpc.Code_CODE_NOT_FOUND: - response.WriteOCSError(w, r, response.MetaNotFound.StatusCode, "path not found", nil) - case rpc.Code_CODE_PERMISSION_DENIED: - response.WriteOCSError(w, r, response.MetaUnauthorized.StatusCode, "permission denied", nil) - default: - response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "grpc stat request failed", nil) + if statRes.Status.Code != rpc.Code_CODE_OK { + switch statRes.Status.Code { + case rpc.Code_CODE_NOT_FOUND: + response.WriteOCSError(w, r, response.MetaNotFound.StatusCode, "path not found", nil) + case rpc.Code_CODE_PERMISSION_DENIED: + response.WriteOCSError(w, r, response.MetaUnauthorized.StatusCode, "permission denied", nil) + default: + response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "grpc stat request failed", nil) + } + return } - return - } - pinfo = statRes.GetInfo() + pinfo = statRes.GetInfo() + _ = h.resourceInfoCache.SetWithExpire(target, pinfo, time.Second*h.resourceInfoCacheTTL) + } } lrsReq := collaboration.ListReceivedSharesRequest{} @@ -659,22 +678,28 @@ func (h *Handler) listSharesWithMe(w http.ResponseWriter, r *http.Request) { // we can reuse the stat info info = pinfo } else { - // we need to do a stat call - statRequest := provider.StatRequest{ - Ref: &provider.Reference{ - Spec: &provider.Reference_Id{ - Id: rs.Share.ResourceId, + key := wrapResourceID(rs.Share.ResourceId) + if infoIf, err := h.resourceInfoCache.Get(key); err == nil { + info = infoIf.(*provider.ResourceInfo) + } else { + // we need to do a stat call + statRequest := provider.StatRequest{ + Ref: &provider.Reference{ + Spec: &provider.Reference_Id{ + Id: rs.Share.ResourceId, + }, }, - }, - } + } - statRes, err := gwc.Stat(r.Context(), &statRequest) - if err != nil || statRes.Status.Code != rpc.Code_CODE_OK { - h.logProblems(statRes.GetStatus(), err, "could not stat, skipping") - continue - } + statRes, err := gwc.Stat(r.Context(), &statRequest) + if err != nil || statRes.Status.Code != rpc.Code_CODE_OK { + h.logProblems(statRes.GetStatus(), err, "could not stat, skipping") + continue + } - info = statRes.GetInfo() + info = statRes.GetInfo() + _ = h.resourceInfoCache.SetWithExpire(key, info, time.Second*h.resourceInfoCacheTTL) + } } data, err := conversions.CS3Share2ShareData(r.Context(), rs.Share) @@ -773,32 +798,36 @@ func (h *Handler) addFilters(w http.ResponseWriter, r *http.Request, prefix stri } target := path.Join(prefix, r.FormValue("path")) - - statReq := &provider.StatRequest{ - Ref: &provider.Reference{ - Spec: &provider.Reference_Path{ - Path: target, + if infoIf, err := h.resourceInfoCache.Get(target); err == nil { + info = infoIf.(*provider.ResourceInfo) + } else { + statReq := &provider.StatRequest{ + Ref: &provider.Reference{ + Spec: &provider.Reference_Path{ + Path: target, + }, }, - }, - } + } - res, err := gwClient.Stat(ctx, statReq) - if err != nil { - response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error sending a grpc stat request", err) - return nil, nil, err - } + res, err := gwClient.Stat(ctx, statReq) + if err != nil { + response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error sending a grpc stat request", err) + return nil, nil, err + } - if res.Status.Code != rpc.Code_CODE_OK { - err = errors.New(res.Status.Message) - if res.Status.Code == rpc.Code_CODE_NOT_FOUND { - response.WriteOCSError(w, r, response.MetaNotFound.StatusCode, "not found", err) + if res.Status.Code != rpc.Code_CODE_OK { + err = errors.New(res.Status.Message) + if res.Status.Code == rpc.Code_CODE_NOT_FOUND { + response.WriteOCSError(w, r, response.MetaNotFound.StatusCode, "not found", err) + return nil, nil, err + } + response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "grpc stat request failed", err) return nil, nil, err } - response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "grpc stat request failed", err) - return nil, nil, err - } - info = res.Info + info = res.Info + _ = h.resourceInfoCache.SetWithExpire(target, info, time.Second*h.resourceInfoCacheTTL) + } collaborationFilters = append(collaborationFilters, &collaboration.ListSharesRequest_Filter{ Type: collaboration.ListSharesRequest_Filter_TYPE_RESOURCE_ID, diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/user.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/user.go index 5f615ba92e2..f5796aba899 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/user.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/user.go @@ -20,6 +20,7 @@ package shares import ( "net/http" + "time" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" @@ -146,7 +147,6 @@ func (h *Handler) removeUserShare(w http.ResponseWriter, r *http.Request, shareI } func (h *Handler) listUserShares(r *http.Request, filters []*collaboration.ListSharesRequest_Filter) ([]*conversions.ShareData, *rpc.Status, error) { - var rInfo *provider.ResourceInfo ctx := r.Context() log := appctx.GetLogger(ctx) @@ -179,26 +179,34 @@ func (h *Handler) listUserShares(r *http.Request, filters []*collaboration.ListS continue } - // prepare the stat request - statReq := &provider.StatRequest{ - Ref: &provider.Reference{ - Spec: &provider.Reference_Id{Id: s.ResourceId}, - }, - } - - statResponse, err := c.Stat(ctx, statReq) - if err != nil || statResponse.Status.Code != rpc.Code_CODE_OK { - log.Debug().Interface("share", s).Interface("response", statResponse).Interface("shareData", data).Err(err).Msg("could not stat share, skipping") - continue + var info *provider.ResourceInfo + key := wrapResourceID(s.ResourceId) + if infoIf, err := h.resourceInfoCache.Get(key); err == nil { + info = infoIf.(*provider.ResourceInfo) + } else { + // prepare the stat request + statReq := &provider.StatRequest{ + Ref: &provider.Reference{ + Spec: &provider.Reference_Id{Id: s.ResourceId}, + }, + } + + statResponse, err := c.Stat(ctx, statReq) + if err != nil || statResponse.Status.Code != rpc.Code_CODE_OK { + log.Debug().Interface("share", s).Interface("response", statResponse).Interface("shareData", data).Err(err).Msg("could not stat share, skipping") + continue + } + info = statResponse.Info + _ = h.resourceInfoCache.SetWithExpire(key, info, time.Second*h.resourceInfoCacheTTL) } - if err := h.addFileInfo(ctx, data, statResponse.Info); err != nil { - log.Debug().Interface("share", s).Interface("info", statResponse.Info).Interface("shareData", data).Err(err).Msg("could not add file info, skipping") + if err := h.addFileInfo(ctx, data, info); err != nil { + log.Debug().Interface("share", s).Interface("info", info).Interface("shareData", data).Err(err).Msg("could not add file info, skipping") continue } h.mapUserIds(ctx, c, data) - log.Debug().Interface("share", s).Interface("info", rInfo).Interface("shareData", data).Msg("mapped") + log.Debug().Interface("share", s).Interface("info", info).Interface("shareData", data).Msg("mapped") ocsDataPayload = append(ocsDataPayload, data) } }