Fix dashboard ignored system setting cache (#21621) (#21759)

backport #21621

This is a performance regression from #18058

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Andrew Thornton <art27@cantab.net>
This commit is contained in:
Lunny Xiao 2022-11-10 19:41:44 +08:00 committed by GitHub
parent a2a42cd5de
commit b9dcf991b9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 172 additions and 151 deletions

View file

@ -150,10 +150,11 @@ func generateEmailAvatarLink(email string, size int, final bool) string {
return DefaultAvatarLink() return DefaultAvatarLink()
} }
enableFederatedAvatar, _ := system_model.GetSetting(system_model.KeyPictureEnableFederatedAvatar) enableFederatedAvatarSetting, _ := system_model.GetSetting(system_model.KeyPictureEnableFederatedAvatar)
enableFederatedAvatar := enableFederatedAvatarSetting.GetValueBool()
var err error var err error
if enableFederatedAvatar != nil && enableFederatedAvatar.GetValueBool() && system_model.LibravatarService != nil { if enableFederatedAvatar && system_model.LibravatarService != nil {
emailHash := saveEmailHash(email) emailHash := saveEmailHash(email)
if final { if final {
// for final link, we can spend more time on slow external query // for final link, we can spend more time on slow external query
@ -171,8 +172,10 @@ func generateEmailAvatarLink(email string, size int, final bool) string {
return urlStr return urlStr
} }
disableGravatar, _ := system_model.GetSetting(system_model.KeyPictureDisableGravatar) disableGravatarSetting, _ := system_model.GetSetting(system_model.KeyPictureDisableGravatar)
if disableGravatar != nil && !disableGravatar.GetValueBool() {
disableGravatar := disableGravatarSetting.GetValueBool()
if !disableGravatar {
// copy GravatarSourceURL, because we will modify its Path. // copy GravatarSourceURL, because we will modify its Path.
avatarURLCopy := *system_model.GravatarSourceURL avatarURLCopy := *system_model.GravatarSourceURL
avatarURLCopy.Path = path.Join(avatarURLCopy.Path, HashEmail(email)) avatarURLCopy.Path = path.Join(avatarURLCopy.Path, HashEmail(email))

View file

@ -12,6 +12,7 @@ import (
"strings" "strings"
"code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/modules/cache"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/timeutil"
@ -35,6 +36,10 @@ func (s *Setting) TableName() string {
} }
func (s *Setting) GetValueBool() bool { func (s *Setting) GetValueBool() bool {
if s == nil {
return false
}
b, _ := strconv.ParseBool(s.SettingValue) b, _ := strconv.ParseBool(s.SettingValue)
return b return b
} }
@ -75,8 +80,8 @@ func IsErrDataExpired(err error) bool {
return ok return ok
} }
// GetSetting returns specific setting // GetSettingNoCache returns specific setting without using the cache
func GetSetting(key string) (*Setting, error) { func GetSettingNoCache(key string) (*Setting, error) {
v, err := GetSettings([]string{key}) v, err := GetSettings([]string{key})
if err != nil { if err != nil {
return nil, err return nil, err
@ -87,6 +92,24 @@ func GetSetting(key string) (*Setting, error) {
return v[key], nil return v[key], nil
} }
// GetSetting returns the setting value via the key
func GetSetting(key string) (*Setting, error) {
return cache.Get(genSettingCacheKey(key), func() (*Setting, error) {
res, err := GetSettingNoCache(key)
if err != nil {
return nil, err
}
return res, nil
})
}
// GetSettingBool return bool value of setting,
// none existing keys and errors are ignored and result in false
func GetSettingBool(key string) bool {
s, _ := GetSetting(key)
return s.GetValueBool()
}
// GetSettings returns specific settings // GetSettings returns specific settings
func GetSettings(keys []string) (map[string]*Setting, error) { func GetSettings(keys []string) (map[string]*Setting, error) {
for i := 0; i < len(keys); i++ { for i := 0; i < len(keys); i++ {
@ -139,12 +162,13 @@ func GetAllSettings() (AllSettings, error) {
// DeleteSetting deletes a specific setting for a user // DeleteSetting deletes a specific setting for a user
func DeleteSetting(setting *Setting) error { func DeleteSetting(setting *Setting) error {
cache.Remove(genSettingCacheKey(setting.SettingKey))
_, err := db.GetEngine(db.DefaultContext).Delete(setting) _, err := db.GetEngine(db.DefaultContext).Delete(setting)
return err return err
} }
func SetSettingNoVersion(key, value string) error { func SetSettingNoVersion(key, value string) error {
s, err := GetSetting(key) s, err := GetSettingNoCache(key)
if IsErrSettingIsNotExist(err) { if IsErrSettingIsNotExist(err) {
return SetSetting(&Setting{ return SetSetting(&Setting{
SettingKey: key, SettingKey: key,
@ -160,9 +184,13 @@ func SetSettingNoVersion(key, value string) error {
// SetSetting updates a users' setting for a specific key // SetSetting updates a users' setting for a specific key
func SetSetting(setting *Setting) error { func SetSetting(setting *Setting) error {
if err := upsertSettingValue(strings.ToLower(setting.SettingKey), setting.SettingValue, setting.Version); err != nil { _, err := cache.Set(genSettingCacheKey(setting.SettingKey), func() (*Setting, error) {
return setting, upsertSettingValue(strings.ToLower(setting.SettingKey), setting.SettingValue, setting.Version)
})
if err != nil {
return err return err
} }
setting.Version++ setting.Version++
return nil return nil
} }
@ -213,7 +241,7 @@ var (
func Init() error { func Init() error {
var disableGravatar bool var disableGravatar bool
disableGravatarSetting, err := GetSetting(KeyPictureDisableGravatar) disableGravatarSetting, err := GetSettingNoCache(KeyPictureDisableGravatar)
if IsErrSettingIsNotExist(err) { if IsErrSettingIsNotExist(err) {
disableGravatar = setting.GetDefaultDisableGravatar() disableGravatar = setting.GetDefaultDisableGravatar()
disableGravatarSetting = &Setting{SettingValue: strconv.FormatBool(disableGravatar)} disableGravatarSetting = &Setting{SettingValue: strconv.FormatBool(disableGravatar)}
@ -224,7 +252,7 @@ func Init() error {
} }
var enableFederatedAvatar bool var enableFederatedAvatar bool
enableFederatedAvatarSetting, err := GetSetting(KeyPictureEnableFederatedAvatar) enableFederatedAvatarSetting, err := GetSettingNoCache(KeyPictureEnableFederatedAvatar)
if IsErrSettingIsNotExist(err) { if IsErrSettingIsNotExist(err) {
enableFederatedAvatar = setting.GetDefaultEnableFederatedAvatar(disableGravatar) enableFederatedAvatar = setting.GetDefaultEnableFederatedAvatar(disableGravatar)
enableFederatedAvatarSetting = &Setting{SettingValue: strconv.FormatBool(enableFederatedAvatar)} enableFederatedAvatarSetting = &Setting{SettingValue: strconv.FormatBool(enableFederatedAvatar)}

View file

@ -9,3 +9,8 @@ const (
KeyPictureDisableGravatar = "picture.disable_gravatar" KeyPictureDisableGravatar = "picture.disable_gravatar"
KeyPictureEnableFederatedAvatar = "picture.enable_federated_avatar" KeyPictureEnableFederatedAvatar = "picture.enable_federated_avatar"
) )
// genSettingCacheKey returns the cache key for some configuration
func genSettingCacheKey(key string) string {
return "system.setting." + key
}

View file

@ -68,11 +68,9 @@ func (u *User) AvatarLinkWithSize(size int) string {
useLocalAvatar := false useLocalAvatar := false
autoGenerateAvatar := false autoGenerateAvatar := false
var disableGravatar bool
disableGravatarSetting, _ := system_model.GetSetting(system_model.KeyPictureDisableGravatar) disableGravatarSetting, _ := system_model.GetSetting(system_model.KeyPictureDisableGravatar)
if disableGravatarSetting != nil {
disableGravatar = disableGravatarSetting.GetValueBool() disableGravatar := disableGravatarSetting.GetValueBool()
}
switch { switch {
case u.UseCustomAvatar: case u.UseCustomAvatar:

View file

@ -10,6 +10,7 @@ import (
"strings" "strings"
"code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/modules/cache"
"xorm.io/builder" "xorm.io/builder"
) )
@ -47,9 +48,25 @@ func IsErrUserSettingIsNotExist(err error) bool {
return ok return ok
} }
// GetSetting returns specific setting // genSettingCacheKey returns the cache key for some configuration
func genSettingCacheKey(userID int64, key string) string {
return fmt.Sprintf("user_%d.setting.%s", userID, key)
}
// GetSetting returns the setting value via the key
func GetSetting(uid int64, key string) (*Setting, error) { func GetSetting(uid int64, key string) (*Setting, error) {
v, err := GetUserSettings(uid, []string{key}) return cache.Get(genSettingCacheKey(uid, key), func() (*Setting, error) {
res, err := GetSettingNoCache(uid, key)
if err != nil {
return nil, err
}
return res, nil
})
}
// GetSettingNoCache returns specific setting without using the cache
func GetSettingNoCache(uid int64, key string) (*Setting, error) {
v, err := GetSettings(uid, []string{key})
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -59,8 +76,8 @@ func GetSetting(uid int64, key string) (*Setting, error) {
return v[key], nil return v[key], nil
} }
// GetUserSettings returns specific settings from user // GetSettings returns specific settings from user
func GetUserSettings(uid int64, keys []string) (map[string]*Setting, error) { func GetSettings(uid int64, keys []string) (map[string]*Setting, error) {
settings := make([]*Setting, 0, len(keys)) settings := make([]*Setting, 0, len(keys))
if err := db.GetEngine(db.DefaultContext). if err := db.GetEngine(db.DefaultContext).
Where("user_id=?", uid). Where("user_id=?", uid).
@ -105,6 +122,7 @@ func GetUserSetting(userID int64, key string, def ...string) (string, error) {
if err := validateUserSettingKey(key); err != nil { if err := validateUserSettingKey(key); err != nil {
return "", err return "", err
} }
setting := &Setting{UserID: userID, SettingKey: key} setting := &Setting{UserID: userID, SettingKey: key}
has, err := db.GetEngine(db.DefaultContext).Get(setting) has, err := db.GetEngine(db.DefaultContext).Get(setting)
if err != nil { if err != nil {
@ -124,7 +142,10 @@ func DeleteUserSetting(userID int64, key string) error {
if err := validateUserSettingKey(key); err != nil { if err := validateUserSettingKey(key); err != nil {
return err return err
} }
cache.Remove(genSettingCacheKey(userID, key))
_, err := db.GetEngine(db.DefaultContext).Delete(&Setting{UserID: userID, SettingKey: key}) _, err := db.GetEngine(db.DefaultContext).Delete(&Setting{UserID: userID, SettingKey: key})
return err return err
} }
@ -133,7 +154,12 @@ func SetUserSetting(userID int64, key, value string) error {
if err := validateUserSettingKey(key); err != nil { if err := validateUserSettingKey(key); err != nil {
return err return err
} }
return upsertUserSettingValue(userID, key, value)
_, err := cache.Set(genSettingCacheKey(userID, key), func() (string, error) {
return value, upsertUserSettingValue(userID, key, value)
})
return err
} }
func upsertUserSettingValue(userID int64, key, value string) error { func upsertUserSettingValue(userID int64, key, value string) error {

View file

@ -27,7 +27,7 @@ func TestSettings(t *testing.T) {
assert.NoError(t, err) assert.NoError(t, err)
// get specific setting // get specific setting
settings, err := user_model.GetUserSettings(99, []string{keyName}) settings, err := user_model.GetSettings(99, []string{keyName})
assert.NoError(t, err) assert.NoError(t, err)
assert.Len(t, settings, 1) assert.Len(t, settings, 1)
assert.EqualValues(t, newSetting.SettingValue, settings[keyName].SettingValue) assert.EqualValues(t, newSetting.SettingValue, settings[keyName].SettingValue)

View file

@ -11,7 +11,7 @@ import (
// GetKeyPair function returns a user's private and public keys // GetKeyPair function returns a user's private and public keys
func GetKeyPair(user *user_model.User) (pub, priv string, err error) { func GetKeyPair(user *user_model.User) (pub, priv string, err error) {
var settings map[string]*user_model.Setting var settings map[string]*user_model.Setting
settings, err = user_model.GetUserSettings(user.ID, []string{user_model.UserActivityPubPrivPem, user_model.UserActivityPubPubPem}) settings, err = user_model.GetSettings(user.ID, []string{user_model.UserActivityPubPrivPem, user_model.UserActivityPubPubPem})
if err != nil { if err != nil {
return return
} else if len(settings) == 0 { } else if len(settings) == 0 {

131
modules/cache/cache.go vendored
View file

@ -46,32 +46,64 @@ func GetCache() mc.Cache {
return conn return conn
} }
// Get returns the key value from cache with callback when no key exists in cache
func Get[V interface{}](key string, getFunc func() (V, error)) (V, error) {
if conn == nil || setting.CacheService.TTL == 0 {
return getFunc()
}
cached := conn.Get(key)
if value, ok := cached.(V); ok {
return value, nil
}
value, err := getFunc()
if err != nil {
return value, err
}
return value, conn.Put(key, value, setting.CacheService.TTLSeconds())
}
// Set updates and returns the key value in the cache with callback. The old value is only removed if the updateFunc() is successful
func Set[V interface{}](key string, valueFunc func() (V, error)) (V, error) {
if conn == nil || setting.CacheService.TTL == 0 {
return valueFunc()
}
value, err := valueFunc()
if err != nil {
return value, err
}
return value, conn.Put(key, value, setting.CacheService.TTLSeconds())
}
// GetString returns the key value from cache with callback when no key exists in cache // GetString returns the key value from cache with callback when no key exists in cache
func GetString(key string, getFunc func() (string, error)) (string, error) { func GetString(key string, getFunc func() (string, error)) (string, error) {
if conn == nil || setting.CacheService.TTL == 0 { if conn == nil || setting.CacheService.TTL == 0 {
return getFunc() return getFunc()
} }
if !conn.IsExist(key) {
var ( cached := conn.Get(key)
value string
err error if cached == nil {
) value, err := getFunc()
if value, err = getFunc(); err != nil { if err != nil {
return value, err return value, err
} }
err = conn.Put(key, value, setting.CacheService.TTLSeconds()) return value, conn.Put(key, value, setting.CacheService.TTLSeconds())
if err != nil {
return "", err
} }
if value, ok := cached.(string); ok {
return value, nil
} }
value := conn.Get(key)
if v, ok := value.(string); ok { if stringer, ok := cached.(fmt.Stringer); ok {
return v, nil return stringer.String(), nil
} }
if v, ok := value.(fmt.Stringer); ok {
return v.String(), nil return fmt.Sprintf("%s", cached), nil
}
return fmt.Sprintf("%s", conn.Get(key)), nil
} }
// GetInt returns key value from cache with callback when no key exists in cache // GetInt returns key value from cache with callback when no key exists in cache
@ -79,30 +111,33 @@ func GetInt(key string, getFunc func() (int, error)) (int, error) {
if conn == nil || setting.CacheService.TTL == 0 { if conn == nil || setting.CacheService.TTL == 0 {
return getFunc() return getFunc()
} }
if !conn.IsExist(key) {
var ( cached := conn.Get(key)
value int
err error if cached == nil {
) value, err := getFunc()
if value, err = getFunc(); err != nil { if err != nil {
return value, err return value, err
} }
err = conn.Put(key, value, setting.CacheService.TTLSeconds())
if err != nil { return value, conn.Put(key, value, setting.CacheService.TTLSeconds())
return 0, err
} }
}
switch value := conn.Get(key).(type) { switch v := cached.(type) {
case int: case int:
return value, nil return v, nil
case string: case string:
v, err := strconv.Atoi(value) value, err := strconv.Atoi(v)
if err != nil { if err != nil {
return 0, err return 0, err
} }
return v, nil return value, nil
default: default:
return 0, fmt.Errorf("Unsupported cached value type: %v", value) value, err := getFunc()
if err != nil {
return value, err
}
return value, conn.Put(key, value, setting.CacheService.TTLSeconds())
} }
} }
@ -111,30 +146,34 @@ func GetInt64(key string, getFunc func() (int64, error)) (int64, error) {
if conn == nil || setting.CacheService.TTL == 0 { if conn == nil || setting.CacheService.TTL == 0 {
return getFunc() return getFunc()
} }
if !conn.IsExist(key) {
var ( cached := conn.Get(key)
value int64
err error if cached == nil {
) value, err := getFunc()
if value, err = getFunc(); err != nil { if err != nil {
return value, err return value, err
} }
err = conn.Put(key, value, setting.CacheService.TTLSeconds())
if err != nil { return value, conn.Put(key, value, setting.CacheService.TTLSeconds())
return 0, err
} }
}
switch value := conn.Get(key).(type) { switch v := conn.Get(key).(type) {
case int64: case int64:
return value, nil return v, nil
case string: case string:
v, err := strconv.ParseInt(value, 10, 64) value, err := strconv.ParseInt(v, 10, 64)
if err != nil { if err != nil {
return 0, err return 0, err
} }
return v, nil return value, nil
default: default:
return 0, fmt.Errorf("Unsupported cached value type: %v", value) value, err := getFunc()
if err != nil {
return value, err
}
return value, conn.Put(key, value, setting.CacheService.TTLSeconds())
} }
} }

View file

@ -1,46 +0,0 @@
// Copyright 2021 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.
package system
import (
"strconv"
"code.gitea.io/gitea/models/system"
"code.gitea.io/gitea/modules/cache"
)
func genKey(key string) string {
return "system.setting." + key
}
// GetSetting returns the setting value via the key
func GetSetting(key string) (string, error) {
return cache.GetString(genKey(key), func() (string, error) {
res, err := system.GetSetting(key)
if err != nil {
return "", err
}
return res.SettingValue, nil
})
}
// GetSettingBool return bool value of setting,
// none existing keys and errors are ignored and result in false
func GetSettingBool(key string) bool {
s, _ := GetSetting(key)
b, _ := strconv.ParseBool(s)
return b
}
// SetSetting sets the setting value
func SetSetting(key, value string, version int) error {
cache.Remove(genKey(key))
return system.SetSetting(&system.Setting{
SettingKey: key,
SettingValue: value,
Version: version,
})
}

View file

@ -1,34 +0,0 @@
// Copyright 2021 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.
package system
import (
"fmt"
"code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/cache"
)
func genUserKey(userID int64, key string) string {
return fmt.Sprintf("user_%d.setting.%s", userID, key)
}
// GetUserSetting returns the user setting value via the key
func GetUserSetting(userID int64, key string) (string, error) {
return cache.GetString(genUserKey(userID, key), func() (string, error) {
res, err := user.GetSetting(userID, key)
if err != nil {
return "", err
}
return res.SettingValue, nil
})
}
// SetUserSetting sets the user setting value
func SetUserSetting(userID int64, key, value string) error {
cache.Remove(genUserKey(userID, key))
return user.SetUserSetting(userID, key, value)
}

View file

@ -42,7 +42,6 @@ import (
"code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/repository"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/svg" "code.gitea.io/gitea/modules/svg"
system_module "code.gitea.io/gitea/modules/system"
"code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/timeutil"
"code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/services/gitdiff" "code.gitea.io/gitea/services/gitdiff"
@ -87,7 +86,7 @@ func NewFuncMap() []template.FuncMap {
return setting.AssetVersion return setting.AssetVersion
}, },
"DisableGravatar": func() bool { "DisableGravatar": func() bool {
return system_module.GetSettingBool(system_model.KeyPictureDisableGravatar) return system_model.GetSettingBool(system_model.KeyPictureDisableGravatar)
}, },
"DefaultShowFullName": func() bool { "DefaultShowFullName": func() bool {
return setting.UI.DefaultShowFullName return setting.UI.DefaultShowFullName

View file

@ -18,7 +18,6 @@ import (
"code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/json"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
system_module "code.gitea.io/gitea/modules/system"
"code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/services/mailer" "code.gitea.io/gitea/services/mailer"
@ -203,7 +202,11 @@ func ChangeConfig(ctx *context.Context) {
value := ctx.FormString("value") value := ctx.FormString("value")
version := ctx.FormInt("version") version := ctx.FormInt("version")
if err := system_module.SetSetting(key, value, version); err != nil { if err := system_model.SetSetting(&system_model.Setting{
SettingKey: key,
SettingValue: value,
Version: version,
}); err != nil {
log.Error("set setting failed: %v", err) log.Error("set setting failed: %v", err)
ctx.JSON(http.StatusOK, map[string]string{ ctx.JSON(http.StatusOK, map[string]string{
"err": ctx.Tr("admin.config.set_setting_failed", key), "err": ctx.Tr("admin.config.set_setting_failed", key),