From 8ee808f836c51ea1a53dde9e7f519c719a1b7293 Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Wed, 29 Sep 2021 17:20:16 +0200 Subject: [PATCH 1/5] Add ocs cache warmup strategy for first request from the user --- changelog/unreleased/ocs-first-req-warmup.md | 3 + cmd/revad/runtime/loader.go | 1 + internal/http/services/owncloud/ocs/cache.go | 52 ++++++++++++ .../handlers/apps/sharing/shares/shares.go | 14 ++-- internal/http/services/owncloud/ocs/ocs.go | 16 +++- pkg/share/cache/cbox/cbox.go | 82 ++++++++++--------- pkg/share/cache/loader/loader.go | 25 ++++++ pkg/storage/utils/eosfs/eosfs.go | 3 + 8 files changed, 152 insertions(+), 44 deletions(-) create mode 100644 changelog/unreleased/ocs-first-req-warmup.md create mode 100644 internal/http/services/owncloud/ocs/cache.go create mode 100644 pkg/share/cache/loader/loader.go diff --git a/changelog/unreleased/ocs-first-req-warmup.md b/changelog/unreleased/ocs-first-req-warmup.md new file mode 100644 index 0000000000..ac3c9a9070 --- /dev/null +++ b/changelog/unreleased/ocs-first-req-warmup.md @@ -0,0 +1,3 @@ +Enhancement: Add ocs cache warmup strategy for first request from the user + +https://github.com/cs3org/reva/pull/2117 diff --git a/cmd/revad/runtime/loader.go b/cmd/revad/runtime/loader.go index 241f625bae..ed50b3913e 100644 --- a/cmd/revad/runtime/loader.go +++ b/cmd/revad/runtime/loader.go @@ -40,6 +40,7 @@ import ( _ "github.com/cs3org/reva/pkg/ocm/share/manager/loader" _ "github.com/cs3org/reva/pkg/publicshare/manager/loader" _ "github.com/cs3org/reva/pkg/rhttp/datatx/manager/loader" + _ "github.com/cs3org/reva/pkg/share/cache/loader" _ "github.com/cs3org/reva/pkg/share/manager/loader" _ "github.com/cs3org/reva/pkg/storage/fs/loader" _ "github.com/cs3org/reva/pkg/storage/registry/loader" diff --git a/internal/http/services/owncloud/ocs/cache.go b/internal/http/services/owncloud/ocs/cache.go new file mode 100644 index 0000000000..fb1539a9f5 --- /dev/null +++ b/internal/http/services/owncloud/ocs/cache.go @@ -0,0 +1,52 @@ +// Copyright 2018-2021 CERN +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// In applying this license, CERN does not waive the privileges and immunities +// granted to it by virtue of its status as an Intergovernmental Organization +// or submit itself to any jurisdiction. + +package ocs + +import ( + "context" + "net/http" + "net/http/httptest" + + ctxpkg "github.com/cs3org/reva/pkg/ctx" + "google.golang.org/grpc/metadata" +) + +func (s *svc) cacheWarmup(w http.ResponseWriter, r *http.Request) { + if s.warmupCache != nil { + u := ctxpkg.ContextMustGetUser(r.Context()) + tkn := ctxpkg.ContextMustGetToken(r.Context()) + + ctx := context.Background() + ctx = ctxpkg.ContextSetUser(ctx, u) + ctx = ctxpkg.ContextSetToken(ctx, tkn) + ctx = metadata.AppendToOutgoingContext(ctx, ctxpkg.TokenHeader, tkn) + req := r.Clone(ctx) + req.Method = http.MethodGet + + id := u.Id.OpaqueId + if _, err := s.warmupCache.Get(id); err != nil { + p := httptest.NewRecorder() + _ = s.warmupCache.Set(id, true) + req.URL.Path = "/apps/files_sharing/api/v1/shares" + go s.router.ServeHTTP(p, req) + req.URL.Path = "/apps/files_sharing/api/v1/shares?shared_with_me=true" + go s.router.ServeHTTP(p, req) + } + } +} 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 afc3252811..0a46ce49a4 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 @@ -111,13 +111,14 @@ func (h *Handler) Init(c *config.Config) { } func (h *Handler) startCacheWarmup(c cache.Warmup) { + time.Sleep(2 * time.Second) infos, err := c.GetResourceInfos() if err != nil { return } for _, r := range infos { key := wrapResourceID(r.Id) - _ = h.resourceInfoCache.SetWithExpire(key, r, time.Second*h.resourceInfoCacheTTL) + _ = h.resourceInfoCache.SetWithExpire(key, r, h.resourceInfoCacheTTL) } } @@ -493,11 +494,11 @@ func (h *Handler) ListShares(w http.ResponseWriter, r *http.Request) { response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error mapping share data", err) } if listSharedWithMe { - h.listSharesWithMe(w, r) + h.ListSharesWithMe(w, r) return } } - h.listSharesWithOthers(w, r) + h.ListSharesWithOthers(w, r) } const ( @@ -507,7 +508,8 @@ const ( ocsStateRejected = 2 ) -func (h *Handler) listSharesWithMe(w http.ResponseWriter, r *http.Request) { +// ListSharesWithMe returns the shares receieved by the user present in the context +func (h *Handler) ListSharesWithMe(w http.ResponseWriter, r *http.Request) { // which pending state to list stateFilter := getStateFilter(r.FormValue("state")) @@ -704,7 +706,9 @@ func findMatch(shareJailInfos []*provider.ResourceInfo, id *provider.ResourceId) return nil } -func (h *Handler) listSharesWithOthers(w http.ResponseWriter, r *http.Request) { +// ListSharesWithOthers returns the public, user, group and federated shares +// created by the user present in the context +func (h *Handler) ListSharesWithOthers(w http.ResponseWriter, r *http.Request) { shares := make([]*conversions.ShareData, 0) filters := []*collaboration.Filter{} diff --git a/internal/http/services/owncloud/ocs/ocs.go b/internal/http/services/owncloud/ocs/ocs.go index cb2b993135..91c2b824de 100644 --- a/internal/http/services/owncloud/ocs/ocs.go +++ b/internal/http/services/owncloud/ocs/ocs.go @@ -20,7 +20,9 @@ package ocs import ( "net/http" + "time" + "github.com/ReneKroon/ttlcache/v2" "github.com/cs3org/reva/internal/http/services/owncloud/ocs/config" "github.com/cs3org/reva/internal/http/services/owncloud/ocs/handlers/apps/sharing/sharees" "github.com/cs3org/reva/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares" @@ -41,8 +43,9 @@ func init() { } type svc struct { - c *config.Config - router *chi.Mux + c *config.Config + router *chi.Mux + warmupCache *ttlcache.Cache } func New(m map[string]interface{}, log *zerolog.Logger) (global.Service, error) { @@ -63,6 +66,11 @@ func New(m map[string]interface{}, log *zerolog.Logger) (global.Service, error) return nil, err } + if conf.CacheWarmupDriver == "first-request" && conf.ResourceInfoCacheTTL > 0 { + s.warmupCache = ttlcache.NewCache() + _ = s.warmupCache.SetTTL(time.Second * time.Duration(conf.ResourceInfoCacheTTL)) + } + return s, nil } @@ -138,6 +146,10 @@ func (s *svc) Handler() http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { log := appctx.GetLogger(r.Context()) log.Debug().Str("path", r.URL.Path).Msg("ocs routing") + + // Warmup the share cache for the user + s.cacheWarmup(w, r) + // unset raw path, otherwise chi uses it to route and then fails to match percent encoded path segments r.URL.RawPath = "" s.router.ServeHTTP(w, r) diff --git a/pkg/share/cache/cbox/cbox.go b/pkg/share/cache/cbox/cbox.go index 5a629b7d48..6f3d42295f 100644 --- a/pkg/share/cache/cbox/cbox.go +++ b/pkg/share/cache/cbox/cbox.go @@ -16,7 +16,7 @@ // granted to it by virtue of its status as an Intergovernmental Organization // or submit itself to any jurisdiction. -package eos +package cbox import ( "context" @@ -24,14 +24,17 @@ import ( "fmt" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" - types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" + "github.com/cs3org/reva/pkg/auth/scope" ctxpkg "github.com/cs3org/reva/pkg/ctx" + "github.com/cs3org/reva/pkg/rgrpc/todo/pool" "github.com/cs3org/reva/pkg/share/cache" "github.com/cs3org/reva/pkg/share/cache/registry" - "github.com/cs3org/reva/pkg/storage/fs/eos" + "github.com/cs3org/reva/pkg/token/manager/jwt" "github.com/mitchellh/mapstructure" "github.com/pkg/errors" + "google.golang.org/grpc/metadata" // Provides mysql drivers _ "github.com/go-sql-driver/mysql" @@ -49,6 +52,7 @@ type config struct { DbName string `mapstructure:"db_name"` EOSNamespace string `mapstructure:"namespace"` GatewaySvc string `mapstructure:"gatewaysvc"` + JWTSecret string `mapstructure:"jwt_secret"` } type manager struct { @@ -90,6 +94,36 @@ func (m *manager) GetResourceInfos() ([]*provider.ResourceInfo, error) { } defer rows.Close() + tokenManager, err := jwt.New(map[string]interface{}{ + "secret": m.conf.JWTSecret, + }) + if err != nil { + return nil, err + } + + u := &userpb.User{ + Id: &userpb.UserId{ + OpaqueId: "root", + }, + UidNumber: 0, + GidNumber: 0, + } + scope, err := scope.AddOwnerScope(nil) + if err != nil { + return nil, err + } + + tkn, err := tokenManager.MintToken(context.Background(), u, scope) + if err != nil { + return nil, err + } + ctx := metadata.AppendToOutgoingContext(context.Background(), ctxpkg.TokenHeader, tkn) + + client, err := pool.GetGatewayServiceClient(m.conf.GatewaySvc) + if err != nil { + return nil, err + } + infos := []*provider.ResourceInfo{} for rows.Next() { var storageID, nodeID string @@ -97,45 +131,19 @@ func (m *manager) GetResourceInfos() ([]*provider.ResourceInfo, error) { continue } - eosOpts := map[string]interface{}{ - "namespace": m.conf.EOSNamespace, - "master_url": fmt.Sprintf("root://%s.cern.ch", storageID), - "version_invariant": true, - "gatewaysvc": m.conf.GatewaySvc, - } - eos, err := eos.New(eosOpts) - if err != nil { - return nil, err - } - - ctx := ctxpkg.ContextSetUser(context.Background(), &userpb.User{ - Id: &userpb.UserId{ - OpaqueId: "root", - }, - Opaque: &types.Opaque{ - Map: map[string]*types.OpaqueEntry{ - "uid": { - Decoder: "plain", - Value: []byte("0"), - }, - "gid": { - Decoder: "plain", - Value: []byte("0"), - }, - }, - }, - }) - - inf, err := eos.GetMD(ctx, &provider.Reference{ + statReq := provider.StatRequest{Ref: &provider.Reference{ ResourceId: &provider.ResourceId{ StorageId: storageID, OpaqueId: nodeID, }, - }, []string{}) - if err != nil { - return nil, err + }} + + statRes, err := client.Stat(ctx, &statReq) + if err != nil || statRes.Status.Code != rpc.Code_CODE_OK { + continue } - infos = append(infos, inf) + + infos = append(infos, statRes.Info) } if err = rows.Err(); err != nil { diff --git a/pkg/share/cache/loader/loader.go b/pkg/share/cache/loader/loader.go new file mode 100644 index 0000000000..a3d8ec7cbb --- /dev/null +++ b/pkg/share/cache/loader/loader.go @@ -0,0 +1,25 @@ +// Copyright 2018-2021 CERN +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// In applying this license, CERN does not waive the privileges and immunities +// granted to it by virtue of its status as an Intergovernmental Organization +// or submit itself to any jurisdiction. + +package loader + +import ( + // Load share cache drivers. + _ "github.com/cs3org/reva/pkg/share/cache/cbox" + // Add your own here +) diff --git a/pkg/storage/utils/eosfs/eosfs.go b/pkg/storage/utils/eosfs/eosfs.go index da5012e22e..702b703ef1 100644 --- a/pkg/storage/utils/eosfs/eosfs.go +++ b/pkg/storage/utils/eosfs/eosfs.go @@ -1771,6 +1771,9 @@ func getResourceType(isDir bool) provider.ResourceType { } func (fs *eosfs) extractUIDAndGID(u *userpb.User) (eosclient.Authorization, error) { + if u.Id.OpaqueId == "root" { + return eosclient.Authorization{Role: eosclient.Role{UID: "0", GID: "0"}}, nil + } if u.UidNumber == 0 { return eosclient.Authorization{}, errors.New("eosfs: uid missing for user") } From 7dc52017937aaad370066dcdf59abbb0af5005f6 Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Thu, 21 Oct 2021 17:02:01 +0200 Subject: [PATCH 2/5] Fixes --- internal/http/services/owncloud/ocs/cache.go | 14 +++++++------- .../ocs/handlers/apps/sharing/shares/shares.go | 11 ++++------- internal/http/services/owncloud/ocs/ocs.go | 12 ++++++------ pkg/storage/utils/eosfs/eosfs.go | 3 --- 4 files changed, 17 insertions(+), 23 deletions(-) diff --git a/internal/http/services/owncloud/ocs/cache.go b/internal/http/services/owncloud/ocs/cache.go index fb1539a9f5..2db8e33702 100644 --- a/internal/http/services/owncloud/ocs/cache.go +++ b/internal/http/services/owncloud/ocs/cache.go @@ -28,7 +28,7 @@ import ( ) func (s *svc) cacheWarmup(w http.ResponseWriter, r *http.Request) { - if s.warmupCache != nil { + if s.warmupCacheTracker != nil { u := ctxpkg.ContextMustGetUser(r.Context()) tkn := ctxpkg.ContextMustGetToken(r.Context()) @@ -40,13 +40,13 @@ func (s *svc) cacheWarmup(w http.ResponseWriter, r *http.Request) { req.Method = http.MethodGet id := u.Id.OpaqueId - if _, err := s.warmupCache.Get(id); err != nil { + if _, err := s.warmupCacheTracker.Get(id); err != nil { p := httptest.NewRecorder() - _ = s.warmupCache.Set(id, true) - req.URL.Path = "/apps/files_sharing/api/v1/shares" - go s.router.ServeHTTP(p, req) - req.URL.Path = "/apps/files_sharing/api/v1/shares?shared_with_me=true" - go s.router.ServeHTTP(p, req) + _ = s.warmupCacheTracker.Set(id, true) + req.URL.Path = "/v1.php/apps/files_sharing/api/v1/shares" + s.router.ServeHTTP(p, req) + req.URL.Path = "/v1.php/apps/files_sharing/api/v1/shares?shared_with_me=true" + s.router.ServeHTTP(p, req) } } } 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 0a46ce49a4..47bc328efb 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 @@ -494,11 +494,11 @@ func (h *Handler) ListShares(w http.ResponseWriter, r *http.Request) { response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error mapping share data", err) } if listSharedWithMe { - h.ListSharesWithMe(w, r) + h.listSharesWithMe(w, r) return } } - h.ListSharesWithOthers(w, r) + h.listSharesWithOthers(w, r) } const ( @@ -508,8 +508,7 @@ const ( ocsStateRejected = 2 ) -// ListSharesWithMe returns the shares receieved by the user present in the context -func (h *Handler) ListSharesWithMe(w http.ResponseWriter, r *http.Request) { +func (h *Handler) listSharesWithMe(w http.ResponseWriter, r *http.Request) { // which pending state to list stateFilter := getStateFilter(r.FormValue("state")) @@ -706,9 +705,7 @@ func findMatch(shareJailInfos []*provider.ResourceInfo, id *provider.ResourceId) return nil } -// ListSharesWithOthers returns the public, user, group and federated shares -// created by the user present in the context -func (h *Handler) ListSharesWithOthers(w http.ResponseWriter, r *http.Request) { +func (h *Handler) listSharesWithOthers(w http.ResponseWriter, r *http.Request) { shares := make([]*conversions.ShareData, 0) filters := []*collaboration.Filter{} diff --git a/internal/http/services/owncloud/ocs/ocs.go b/internal/http/services/owncloud/ocs/ocs.go index 91c2b824de..d411f4dace 100644 --- a/internal/http/services/owncloud/ocs/ocs.go +++ b/internal/http/services/owncloud/ocs/ocs.go @@ -43,9 +43,9 @@ func init() { } type svc struct { - c *config.Config - router *chi.Mux - warmupCache *ttlcache.Cache + c *config.Config + router *chi.Mux + warmupCacheTracker *ttlcache.Cache } func New(m map[string]interface{}, log *zerolog.Logger) (global.Service, error) { @@ -67,8 +67,8 @@ func New(m map[string]interface{}, log *zerolog.Logger) (global.Service, error) } if conf.CacheWarmupDriver == "first-request" && conf.ResourceInfoCacheTTL > 0 { - s.warmupCache = ttlcache.NewCache() - _ = s.warmupCache.SetTTL(time.Second * time.Duration(conf.ResourceInfoCacheTTL)) + s.warmupCacheTracker = ttlcache.NewCache() + _ = s.warmupCacheTracker.SetTTL(time.Second * time.Duration(conf.ResourceInfoCacheTTL)) } return s, nil @@ -148,7 +148,7 @@ func (s *svc) Handler() http.Handler { log.Debug().Str("path", r.URL.Path).Msg("ocs routing") // Warmup the share cache for the user - s.cacheWarmup(w, r) + go s.cacheWarmup(w, r) // unset raw path, otherwise chi uses it to route and then fails to match percent encoded path segments r.URL.RawPath = "" diff --git a/pkg/storage/utils/eosfs/eosfs.go b/pkg/storage/utils/eosfs/eosfs.go index 702b703ef1..da5012e22e 100644 --- a/pkg/storage/utils/eosfs/eosfs.go +++ b/pkg/storage/utils/eosfs/eosfs.go @@ -1771,9 +1771,6 @@ func getResourceType(isDir bool) provider.ResourceType { } func (fs *eosfs) extractUIDAndGID(u *userpb.User) (eosclient.Authorization, error) { - if u.Id.OpaqueId == "root" { - return eosclient.Authorization{Role: eosclient.Role{UID: "0", GID: "0"}}, nil - } if u.UidNumber == 0 { return eosclient.Authorization{}, errors.New("eosfs: uid missing for user") } From d5c8760e209ae257f953ec9777e21331f1b32d64 Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Mon, 25 Oct 2021 14:06:36 +0200 Subject: [PATCH 3/5] More debugging --- .../grpc/services/gateway/storageprovider.go | 5 ++++- internal/http/services/owncloud/ocs/cache.go | 19 ++++++++++++++++--- .../handlers/apps/sharing/shares/shares.go | 4 +++- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/internal/grpc/services/gateway/storageprovider.go b/internal/grpc/services/gateway/storageprovider.go index 3ea0d881f4..da46ffbd8e 100644 --- a/internal/grpc/services/gateway/storageprovider.go +++ b/internal/grpc/services/gateway/storageprovider.go @@ -1945,7 +1945,10 @@ func (s *svc) getPath(ctx context.Context, ref *provider.Reference, keys ...stri if ref.ResourceId != nil { req := &provider.StatRequest{Ref: ref, ArbitraryMetadataKeys: keys} res, err := s.stat(ctx, req) - if (res != nil && res.Status.Code != rpc.Code_CODE_OK) || err != nil { + if err != nil { + return "", status.NewStatusFromErrType(ctx, "getPath ref="+ref.String(), err) + } + if res != nil && res.Status.Code != rpc.Code_CODE_OK { return "", res.Status } diff --git a/internal/http/services/owncloud/ocs/cache.go b/internal/http/services/owncloud/ocs/cache.go index 2db8e33702..ea587ae5fa 100644 --- a/internal/http/services/owncloud/ocs/cache.go +++ b/internal/http/services/owncloud/ocs/cache.go @@ -23,6 +23,7 @@ import ( "net/http" "net/http/httptest" + "github.com/cs3org/reva/pkg/appctx" ctxpkg "github.com/cs3org/reva/pkg/ctx" "google.golang.org/grpc/metadata" ) @@ -31,21 +32,33 @@ func (s *svc) cacheWarmup(w http.ResponseWriter, r *http.Request) { if s.warmupCacheTracker != nil { u := ctxpkg.ContextMustGetUser(r.Context()) tkn := ctxpkg.ContextMustGetToken(r.Context()) + log := appctx.GetLogger(r.Context()) ctx := context.Background() + ctx = appctx.WithLogger(ctx, log) ctx = ctxpkg.ContextSetUser(ctx, u) ctx = ctxpkg.ContextSetToken(ctx, tkn) ctx = metadata.AppendToOutgoingContext(ctx, ctxpkg.TokenHeader, tkn) - req := r.Clone(ctx) - req.Method = http.MethodGet + + req, _ := http.NewRequest("GET", "", nil) + req = req.WithContext(ctx) + req.URL = r.URL id := u.Id.OpaqueId if _, err := s.warmupCacheTracker.Get(id); err != nil { p := httptest.NewRecorder() _ = s.warmupCacheTracker.Set(id, true) + + log.Info().Msgf("cache warmup getting created shares for user %s", id) req.URL.Path = "/v1.php/apps/files_sharing/api/v1/shares" s.router.ServeHTTP(p, req) - req.URL.Path = "/v1.php/apps/files_sharing/api/v1/shares?shared_with_me=true" + + log.Info().Msgf("cache warmup getting received shares for user %s", id) + req.URL.Path = "/v1.php/apps/files_sharing/api/v1/shares" + q := req.URL.Query() + q.Set("shared_with_me", "true") + q.Set("state", "all") + req.URL.RawQuery = q.Encode() s.router.ServeHTTP(p, req) } } 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 47bc328efb..73167f35e8 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 @@ -100,7 +100,7 @@ func (h *Handler) Init(c *config.Config) { h.additionalInfoTemplate, _ = template.New("additionalInfo").Parse(c.AdditionalInfoAttribute) h.userIdentifierCache = ttlcache.NewCache() - _ = h.userIdentifierCache.SetTTL(time.Second * 60) + _ = h.userIdentifierCache.SetTTL(24 * time.Hour) if h.resourceInfoCacheTTL > 0 { cwm, err := getCacheWarmupManager(c) @@ -512,6 +512,7 @@ func (h *Handler) listSharesWithMe(w http.ResponseWriter, r *http.Request) { // which pending state to list stateFilter := getStateFilter(r.FormValue("state")) + log := appctx.GetLogger(r.Context()) client, err := pool.GetGatewayServiceClient(h.gatewayAddr) if err != nil { response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error getting grpc gateway client", err) @@ -1022,6 +1023,7 @@ func (h *Handler) getResourceInfo(ctx context.Context, client gateway.GatewayAPI pinfo = infoIf.(*provider.ResourceInfo) status = &rpc.Status{Code: rpc.Code_CODE_OK} } else { + logger.Debug().Msgf("cache miss for resource %+v, statting", key) statReq := &provider.StatRequest{ Ref: ref, } From 0cfacfd523e4cfae08a4274ef99846a29aaec62b Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Tue, 26 Oct 2021 09:11:02 +0200 Subject: [PATCH 4/5] Make user identifier cache TTL configurable --- internal/http/services/owncloud/ocs/config/config.go | 5 +++++ .../owncloud/ocs/handlers/apps/sharing/shares/shares.go | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/internal/http/services/owncloud/ocs/config/config.go b/internal/http/services/owncloud/ocs/config/config.go index 20d785c011..266c5308f6 100644 --- a/internal/http/services/owncloud/ocs/config/config.go +++ b/internal/http/services/owncloud/ocs/config/config.go @@ -38,6 +38,7 @@ type Config struct { CacheWarmupDrivers map[string]map[string]interface{} `mapstructure:"cache_warmup_drivers"` ResourceInfoCacheSize int `mapstructure:"resource_info_cache_size"` ResourceInfoCacheTTL int `mapstructure:"resource_info_cache_ttl"` + UserIdentifierCacheTTL int `mapstructure:"user_identifier_cache_ttl"` } // Init sets sane defaults @@ -66,5 +67,9 @@ func (c *Config) Init() { c.ResourceInfoCacheSize = 1000000 } + if c.UserIdentifierCacheTTL == 0 { + c.UserIdentifierCacheTTL = 60 + } + c.GatewaySvc = sharedconf.GetGatewaySVC(c.GatewaySvc) } 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 73167f35e8..39d664efea 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 @@ -70,6 +70,7 @@ type Handler struct { homeNamespace string additionalInfoTemplate *template.Template userIdentifierCache *ttlcache.Cache + userIdentifierCacheTTL time.Duration resourceInfoCache gcache.Cache resourceInfoCacheTTL time.Duration } @@ -100,7 +101,7 @@ func (h *Handler) Init(c *config.Config) { h.additionalInfoTemplate, _ = template.New("additionalInfo").Parse(c.AdditionalInfoAttribute) h.userIdentifierCache = ttlcache.NewCache() - _ = h.userIdentifierCache.SetTTL(24 * time.Hour) + _ = h.userIdentifierCache.SetTTL(time.Second * time.Duration(c.UserIdentifierCacheTTL)) if h.resourceInfoCacheTTL > 0 { cwm, err := getCacheWarmupManager(c) From af3db78697fe45a4cbbfb6bc80555ba946d32161 Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Tue, 26 Oct 2021 09:27:56 +0200 Subject: [PATCH 5/5] Add comment about copying context --- internal/http/services/owncloud/ocs/cache.go | 4 ++++ .../owncloud/ocs/handlers/apps/sharing/shares/shares.go | 1 - 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/http/services/owncloud/ocs/cache.go b/internal/http/services/owncloud/ocs/cache.go index ea587ae5fa..8381f6150e 100644 --- a/internal/http/services/owncloud/ocs/cache.go +++ b/internal/http/services/owncloud/ocs/cache.go @@ -34,6 +34,10 @@ func (s *svc) cacheWarmup(w http.ResponseWriter, r *http.Request) { tkn := ctxpkg.ContextMustGetToken(r.Context()) log := appctx.GetLogger(r.Context()) + // We make a copy of the context because the original one comes with its cancel channel, + // so once the initial request is finished, this ctx gets cancelled as well. + // And in most of the cases, the warmup takes a longer amount of time to complete than the original request. + // TODO: Check if we can come up with a better solution, eg, https://stackoverflow.com/a/54132324 ctx := context.Background() ctx = appctx.WithLogger(ctx, log) ctx = ctxpkg.ContextSetUser(ctx, u) 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 39d664efea..475842c242 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 @@ -70,7 +70,6 @@ type Handler struct { homeNamespace string additionalInfoTemplate *template.Template userIdentifierCache *ttlcache.Cache - userIdentifierCacheTTL time.Duration resourceInfoCache gcache.Cache resourceInfoCacheTTL time.Duration }