From 91b15eed1acd0b3894b6edcdc14332ddb40333aa Mon Sep 17 00:00:00 2001 From: Gabriel MABILLE Date: Wed, 7 Dec 2022 08:58:15 +0100 Subject: [PATCH] FeatureToggle: for storing signed in user object in a Remote Cache (#59883) * FeatureToggle: for storing sessions in a Remote Cache Co-authored-by: ievaVasiljeva * Fix conflicting modifications :D Co-authored-by: ievaVasiljeva * rename the flag to userRemoteCache * undo unintended change * Users: Add option to use remote cache for SignedInUsers (#59892) * Add remote cache to GetSignedInUserWithCacheCtx Co-authored-by: ievaVasiljeva * Populate SignedInUser remote cache Co-authored-by: ievaVasiljeva * Line * minor fixes to make this work Co-authored-by: ievaVasiljeva * Fix tests * change flag to updated name Co-authored-by: ievaVasiljeva Co-authored-by: ievaVasiljeva --- .../src/types/featureToggles.gen.ts | 1 + pkg/api/org_users_test.go | 6 ++++ pkg/services/featuremgmt/registry.go | 5 ++++ pkg/services/featuremgmt/toggles_gen.go | 4 +++ pkg/services/sqlstore/user.go | 6 ++-- pkg/services/sqlstore/user_test.go | 2 +- pkg/services/user/userimpl/user.go | 28 +++++++++++++++++++ 7 files changed, 48 insertions(+), 4 deletions(-) diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 12996f0cc30..40e7df87ef4 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -67,5 +67,6 @@ export interface FeatureToggles { redshiftAsyncQueryDataSupport?: boolean; athenaAsyncQueryDataSupport?: boolean; increaseInMemDatabaseQueryCache?: boolean; + userRemoteCache?: boolean; datasourceLogger?: boolean; } diff --git a/pkg/api/org_users_test.go b/pkg/api/org_users_test.go index 3711d2ea3ea..0fd1c963084 100644 --- a/pkg/api/org_users_test.go +++ b/pkg/api/org_users_test.go @@ -362,6 +362,7 @@ func TestGetOrgUsersAPIEndpoint_AccessControlMetadata(t *testing.T) { hs.userService = userimpl.ProvideService( hs.SQLStore, nil, nil, nil, nil, nil, nil, nil, nil, nil, hs.SQLStore.(*sqlstore.SQLStore), + nil, featuremgmt.WithFeatures(), ) hs.orgService = orgimpl.ProvideService(hs.SQLStore, cfg) }) @@ -467,6 +468,7 @@ func TestGetOrgUsersAPIEndpoint_AccessControl(t *testing.T) { hs.userService = userimpl.ProvideService( hs.SQLStore, nil, nil, nil, nil, nil, nil, nil, nil, nil, hs.SQLStore.(*sqlstore.SQLStore), + nil, featuremgmt.WithFeatures(), ) hs.orgService = orgimpl.ProvideService(hs.SQLStore, cfg) }) @@ -573,6 +575,7 @@ func TestPostOrgUsersAPIEndpoint_AccessControl(t *testing.T) { hs.userService = userimpl.ProvideService( hs.SQLStore, nil, nil, nil, nil, nil, nil, nil, nil, nil, hs.SQLStore.(*sqlstore.SQLStore), + nil, featuremgmt.WithFeatures(), ) }) setupOrgUsersDBForAccessControlTests(t, sc.db) @@ -699,6 +702,7 @@ func TestOrgUsersAPIEndpointWithSetPerms_AccessControl(t *testing.T) { hs.userService = userimpl.ProvideService( hs.SQLStore, nil, nil, nil, nil, nil, nil, nil, nil, nil, hs.SQLStore.(*sqlstore.SQLStore), + nil, featuremgmt.WithFeatures(), ) }) setInitCtxSignedInViewer(sc.initCtx) @@ -818,6 +822,7 @@ func TestPatchOrgUsersAPIEndpoint_AccessControl(t *testing.T) { hs.userService = userimpl.ProvideService( hs.SQLStore, nil, nil, nil, nil, nil, nil, nil, nil, nil, hs.SQLStore.(*sqlstore.SQLStore), + nil, featuremgmt.WithFeatures(), ) hs.orgService = orgimpl.ProvideService(hs.SQLStore, cfg) }) @@ -946,6 +951,7 @@ func TestDeleteOrgUsersAPIEndpoint_AccessControl(t *testing.T) { hs.userService = userimpl.ProvideService( hs.SQLStore, nil, nil, nil, nil, nil, nil, nil, nil, nil, hs.SQLStore.(*sqlstore.SQLStore), + nil, featuremgmt.WithFeatures(), ) }) setupOrgUsersDBForAccessControlTests(t, sc.db) diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index c32d6d46966..76916f0b31b 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -283,6 +283,11 @@ var ( Name: "increaseInMemDatabaseQueryCache", Description: "Enable more in memory caching for database queries", }, + { + Name: "userRemoteCache", + Description: "Enable using remote cache for users", + State: FeatureStateAlpha, + }, { Name: "datasourceLogger", Description: "Logs all datasource requests", diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index ae5cf6db579..428f3e0030e 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -211,6 +211,10 @@ const ( // Enable more in memory caching for database queries FlagIncreaseInMemDatabaseQueryCache = "increaseInMemDatabaseQueryCache" + // FlagUserRemoteCache + // Enable using remote cache for users + FlagUserRemoteCache = "userRemoteCache" + // FlagDatasourceLogger // Logs all datasource requests FlagDatasourceLogger = "datasourceLogger" diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index b467a6f3454..d6ba76fe8b9 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -475,12 +475,12 @@ func (ss *SQLStore) GetUserOrgList(ctx context.Context, query *models.GetUserOrg }) } -func newSignedInUserCacheKey(orgID, userID int64) string { +func NewSignedInUserCacheKey(orgID, userID int64) string { return fmt.Sprintf("signed-in-user-%d-%d", userID, orgID) } func (ss *SQLStore) GetSignedInUserWithCacheCtx(ctx context.Context, query *models.GetSignedInUserQuery) error { - cacheKey := newSignedInUserCacheKey(query.OrgId, query.UserId) + cacheKey := NewSignedInUserCacheKey(query.OrgId, query.UserId) if cached, found := ss.CacheService.Get(cacheKey); found { cachedUser := cached.(user.SignedInUser) query.Result = &cachedUser @@ -492,7 +492,7 @@ func (ss *SQLStore) GetSignedInUserWithCacheCtx(ctx context.Context, query *mode return err } - cacheKey = newSignedInUserCacheKey(query.Result.OrgID, query.UserId) + cacheKey = NewSignedInUserCacheKey(query.Result.OrgID, query.UserId) ss.CacheService.Set(cacheKey, *query.Result, time.Second*5) return nil } diff --git a/pkg/services/sqlstore/user_test.go b/pkg/services/sqlstore/user_test.go index 07d2e53d940..6b880ae3957 100644 --- a/pkg/services/sqlstore/user_test.go +++ b/pkg/services/sqlstore/user_test.go @@ -457,7 +457,7 @@ func TestIntegrationUserDataAccess(t *testing.T) { require.NotNil(t, query4.Result) require.Equal(t, query4.Result.OrgID, users[0].OrgID) - cacheKey := newSignedInUserCacheKey(query4.Result.OrgID, query4.UserId) + cacheKey := NewSignedInUserCacheKey(query4.Result.OrgID, query4.UserId) _, found := ss.CacheService.Get(cacheKey) require.True(t, found) diff --git a/pkg/services/user/userimpl/user.go b/pkg/services/user/userimpl/user.go index 90eb27714b8..dbc0ca2315a 100644 --- a/pkg/services/user/userimpl/user.go +++ b/pkg/services/user/userimpl/user.go @@ -5,9 +5,11 @@ import ( "errors" "time" + "github.com/grafana/grafana/pkg/infra/remotecache" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/dashboards" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/org" pref "github.com/grafana/grafana/pkg/services/preference" "github.com/grafana/grafana/pkg/services/quota" @@ -33,6 +35,8 @@ type Service struct { userAuthService userauth.Service quotaService quota.Service accessControlStore accesscontrol.Service + remoteCache *remotecache.RemoteCache + features *featuremgmt.FeatureManager // TODO remove sqlstore sqlStore *sqlstore.SQLStore @@ -51,7 +55,13 @@ func ProvideService( accessControlStore accesscontrol.Service, cfg *setting.Cfg, ss *sqlstore.SQLStore, + remoteCache *remotecache.RemoteCache, + features *featuremgmt.FeatureManager, ) user.Service { + if features.IsEnabled(featuremgmt.FlagUserRemoteCache) { + remotecache.Register(user.SignedInUser{}) + } + return &Service{ store: &sqlStore{ db: db, @@ -67,6 +77,8 @@ func ProvideService( accessControlStore: accessControlStore, cfg: cfg, sqlStore: ss, + remoteCache: remoteCache, + features: features, } } @@ -309,6 +321,17 @@ func (s *Service) SetUsingOrg(ctx context.Context, cmd *user.SetUsingOrgCommand) // TODO: remove wrapper around sqlstore func (s *Service) GetSignedInUserWithCacheCtx(ctx context.Context, query *user.GetSignedInUserQuery) (*user.SignedInUser, error) { + // Fetching from remote cache first otherwise fallback to in memory cache + cacheKey := sqlstore.NewSignedInUserCacheKey(query.OrgID, query.UserID) + if s.features.IsEnabled(featuremgmt.FlagUserRemoteCache) { + res, errCache := s.remoteCache.Get(ctx, cacheKey) + if errCache == nil { + if signedInUser, ok := res.(user.SignedInUser); ok { + return &signedInUser, nil + } + } + } + q := &models.GetSignedInUserQuery{ UserId: query.UserID, Login: query.Login, @@ -319,6 +342,11 @@ func (s *Service) GetSignedInUserWithCacheCtx(ctx context.Context, query *user.G if err != nil { return nil, err } + + // Remember user in remote cache + if s.features.IsEnabled(featuremgmt.FlagUserRemoteCache) { + _ = s.remoteCache.Set(ctx, cacheKey, *(q.Result), time.Second*5) + } return q.Result, nil }