Browse Source

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
Carl Bergquist 6 years ago
parent
commit
aed3d0d3ad
1 changed files with 39 additions and 31 deletions
  1. 39 31
      pkg/infra/remotecache/database_storage.go

+ 39 - 31
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 err
-	}
-
-	session := dc.SQLStore.NewSession()
+	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 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)
-	}
+		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)
-	}
+		// 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
+		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