From aed3d0d3adabfdae096aef0116530e365316e586 Mon Sep 17 00:00:00 2001 From: Carl Bergquist Date: Wed, 15 May 2019 11:24:04 +0200 Subject: [PATCH] Remotecache: Avoid race condition in Set causing error on insert. (#17082) * remotecache: avoid race condition in set since set called the database twice without transactions another operation could insert a value before the first operation completed. which would raise an error on insert since the data have been inserted by the other request. closes #17079 --- pkg/infra/remotecache/database_storage.go | 76 +++++++++++++---------- 1 file changed, 42 insertions(+), 34 deletions(-) diff --git a/pkg/infra/remotecache/database_storage.go b/pkg/infra/remotecache/database_storage.go index 3e15bd9a2cd..03ac0f0c5e8 100644 --- a/pkg/infra/remotecache/database_storage.go +++ b/pkg/infra/remotecache/database_storage.go @@ -39,10 +39,14 @@ func (dc *databaseCache) Run(ctx context.Context) error { } func (dc *databaseCache) internalRunGC() { - now := getTime().Unix() - sql := `DELETE FROM cache_data WHERE (? - created_at) >= expires AND expires <> 0` + err := dc.SQLStore.WithDbSession(context.Background(), func(session *sqlstore.DBSession) error { + now := getTime().Unix() + sql := `DELETE FROM cache_data WHERE (? - created_at) >= expires AND expires <> 0` + + _, err := session.Exec(sql, now) + return err + }) - _, err := dc.SQLStore.NewSession().Exec(sql, now) if err != nil { dc.log.Error("failed to run garbage collect", "error", err) } @@ -80,44 +84,48 @@ func (dc *databaseCache) Get(key string) (interface{}, error) { } func (dc *databaseCache) Set(key string, value interface{}, expire time.Duration) error { - item := &cachedItem{Val: value} - data, err := encodeGob(item) - if err != nil { + return dc.SQLStore.WithTransactionalDbSession(context.Background(), func(session *sqlstore.DBSession) error { + item := &cachedItem{Val: value} + data, err := encodeGob(item) + if err != nil { + return err + } + + var cacheHit CacheData + has, err := session.Where("cache_key = ?", key).Get(&cacheHit) + if err != nil { + return err + } + + var expiresInSeconds int64 + if expire != 0 { + expiresInSeconds = int64(expire) / int64(time.Second) + } + + // insert or update depending on if item already exist + if has { + sql := `UPDATE cache_data SET data=?, created_at=?, expires=? WHERE cache_key=?` + _, err = session.Exec(sql, data, getTime().Unix(), expiresInSeconds, key) + } else { + sql := `INSERT INTO cache_data (cache_key,data,created_at,expires) VALUES(?,?,?,?)` + _, err = session.Exec(sql, key, data, getTime().Unix(), expiresInSeconds) + } + return err - } - - session := dc.SQLStore.NewSession() - - var cacheHit CacheData - has, err := session.Where("cache_key = ?", key).Get(&cacheHit) - if err != nil { - return err - } - - var expiresInSeconds int64 - if expire != 0 { - expiresInSeconds = int64(expire) / int64(time.Second) - } - - // insert or update depending on if item already exist - if has { - sql := `UPDATE cache_data SET data=?, created_at=?, expires=? WHERE cache_key=?` - _, err = session.Exec(sql, data, getTime().Unix(), expiresInSeconds, key) - } else { - sql := `INSERT INTO cache_data (cache_key,data,created_at,expires) VALUES(?,?,?,?)` - _, err = session.Exec(sql, key, data, getTime().Unix(), expiresInSeconds) - } - - return err + }) } func (dc *databaseCache) Delete(key string) error { - sql := "DELETE FROM cache_data WHERE cache_key=?" - _, err := dc.SQLStore.NewSession().Exec(sql, key) + return dc.SQLStore.WithDbSession(context.Background(), func(session *sqlstore.DBSession) error { + sql := "DELETE FROM cache_data WHERE cache_key=?" + _, err := session.Exec(sql, key) + + return err + }) - return err } +// CacheData is the struct representing the table in the database type CacheData struct { CacheKey string Data []byte