From d4f578966056ea1820186327ca35138df5032fa2 Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 14 Feb 2019 23:13:46 +0100 Subject: [PATCH 01/28] cache: initial version of db cache --- pkg/infra/distcache/database_storage.go | 82 +++++++++++++++++ pkg/infra/distcache/distcache.go | 68 +++++++++++++++ pkg/infra/distcache/distcache_test.go | 87 +++++++++++++++++++ .../sqlstore/migrations/cache_data_mig.go | 17 ++++ .../sqlstore/migrations/migrations.go | 1 + 5 files changed, 255 insertions(+) create mode 100644 pkg/infra/distcache/database_storage.go create mode 100644 pkg/infra/distcache/distcache.go create mode 100644 pkg/infra/distcache/distcache_test.go create mode 100644 pkg/services/sqlstore/migrations/cache_data_mig.go diff --git a/pkg/infra/distcache/database_storage.go b/pkg/infra/distcache/database_storage.go new file mode 100644 index 00000000000..8286f65fea6 --- /dev/null +++ b/pkg/infra/distcache/database_storage.go @@ -0,0 +1,82 @@ +package distcache + +import ( + "time" + + "github.com/grafana/grafana/pkg/services/sqlstore" +) + +type databaseCache struct { + SQLStore *sqlstore.SqlStore +} + +var getTime = time.Now + +func (dc *databaseCache) Get(key string) (interface{}, error) { + //now := getTime().Unix() + + cacheHits := []CacheData{} + err := dc.SQLStore.NewSession().Where(`key = ?`, key).Find(&cacheHits) + if err != nil { + return nil, err + } + + var cacheHit CacheData + if len(cacheHits) == 0 { + return nil, ErrCacheItemNotFound + } + + cacheHit = cacheHits[0] + if cacheHit.Expires > 0 { + if getTime().Unix()-cacheHit.CreatedAt >= cacheHit.Expires { + dc.Delete(key) + return nil, ErrCacheItemNotFound + } + } + + item := &Item{} + if err = DecodeGob(cacheHit.Data, item); err != nil { + return nil, err + } + + return item.Val, nil +} + +type CacheData struct { + Key string + Data []byte + Expires int64 + CreatedAt int64 +} + +func (dc *databaseCache) Put(key string, value interface{}, expire int64) error { + item := &Item{Val: value} + data, err := EncodeGob(item) + if err != nil { + return err + } + + now := getTime().Unix() + + cacheHits := []CacheData{} + err = dc.SQLStore.NewSession().Where(`key = ?`, key).Find(&cacheHits) + if err != nil { + return err + } + + if len(cacheHits) > 0 { + _, err = dc.SQLStore.NewSession().Exec("UPDATE cached_data SET data=?, created=?, expire=? WHERE key=?", data, now, expire, key) + } else { + _, err = dc.SQLStore.NewSession().Exec("INSERT INTO cache_data(key,data,created_at,expires) VALUES(?,?,?,?)", key, data, now, expire) + } + + return err +} + +func (dc *databaseCache) Delete(key string) error { + sql := `DELETE FROM cache_data WHERE key = ?` + + _, err := dc.SQLStore.NewSession().Exec(sql, key) + + return err +} diff --git a/pkg/infra/distcache/distcache.go b/pkg/infra/distcache/distcache.go new file mode 100644 index 00000000000..11efd435de3 --- /dev/null +++ b/pkg/infra/distcache/distcache.go @@ -0,0 +1,68 @@ +package distcache + +import ( + "bytes" + "encoding/gob" + "errors" + + "github.com/grafana/grafana/pkg/log" + "github.com/grafana/grafana/pkg/services/sqlstore" + + "github.com/grafana/grafana/pkg/registry" +) + +var ( + ErrCacheItemNotFound = errors.New("cache item not found") +) + +func init() { + registry.RegisterService(&DistributedCache{}) +} + +// Init initializes the service +func (ds *DistributedCache) Init() error { + ds.log = log.New("distributed.cache") + + // memory + // redis + // memcache + // database. using SQLSTORE + ds.Client = &databaseCache{SQLStore: ds.SQLStore} + + return nil +} + +// DistributedCache allows Grafana to cache data outside its own process +type DistributedCache struct { + log log.Logger + Client cacheStorage + SQLStore *sqlstore.SqlStore `inject:""` +} + +type Item struct { + Val interface{} + Created int64 + Expire int64 +} + +func EncodeGob(item *Item) ([]byte, error) { + buf := bytes.NewBuffer(nil) + err := gob.NewEncoder(buf).Encode(item) + return buf.Bytes(), err +} + +func DecodeGob(data []byte, out *Item) error { + buf := bytes.NewBuffer(data) + return gob.NewDecoder(buf).Decode(&out) +} + +type cacheStorage interface { + // Get reads object from Cache + Get(key string) (interface{}, error) + + // Puts an object into the cache + Put(key string, value interface{}, expire int64) error + + // Delete object from cache + Delete(key string) error +} diff --git a/pkg/infra/distcache/distcache_test.go b/pkg/infra/distcache/distcache_test.go new file mode 100644 index 00000000000..88066daec7e --- /dev/null +++ b/pkg/infra/distcache/distcache_test.go @@ -0,0 +1,87 @@ +package distcache + +import ( + "encoding/gob" + "testing" + "time" + + "github.com/bmizerany/assert" + + "github.com/grafana/grafana/pkg/log" + "github.com/grafana/grafana/pkg/services/sqlstore" +) + +type CacheableStruct struct { + String string + Int64 int64 +} + +func init() { + gob.Register(CacheableStruct{}) +} + +func createClient(t *testing.T) cacheStorage { + t.Helper() + + sqlstore := sqlstore.InitTestDB(t) + dc := DistributedCache{log: log.New("test.logger"), SQLStore: sqlstore} + dc.Init() + return dc.Client +} + +func TestCanPutIntoDatabaseStorage(t *testing.T) { + client := createClient(t) + cacheableStruct := CacheableStruct{String: "hej", Int64: 2000} + + err := client.Put("key", cacheableStruct, 1000) + assert.Equal(t, err, nil) + + data, err := client.Get("key") + s, ok := data.(CacheableStruct) + + assert.Equal(t, ok, true) + assert.Equal(t, s.String, "hej") + assert.Equal(t, s.Int64, int64(2000)) + + err = client.Delete("key") + assert.Equal(t, err, nil) + + _, err = client.Get("key") + assert.Equal(t, err, ErrCacheItemNotFound) +} + +func TestCanNotFetchExpiredItems(t *testing.T) { + client := createClient(t) + + cacheableStruct := CacheableStruct{String: "hej", Int64: 2000} + + // insert cache item one day back + getTime = func() time.Time { return time.Now().AddDate(0, 0, -2) } + err := client.Put("key", cacheableStruct, 10000) + assert.Equal(t, err, nil) + + // should not be able to read that value since its expired + getTime = time.Now + _, err = client.Get("key") + assert.Equal(t, err, ErrCacheItemNotFound) +} + +func TestCanSetInfiniteCacheExpiration(t *testing.T) { + client := createClient(t) + + cacheableStruct := CacheableStruct{String: "hej", Int64: 2000} + + // insert cache item one day back + getTime = func() time.Time { return time.Now().AddDate(0, 0, -2) } + err := client.Put("key", cacheableStruct, 0) + assert.Equal(t, err, nil) + + // should not be able to read that value since its expired + getTime = time.Now + data, err := client.Get("key") + s, ok := data.(CacheableStruct) + + assert.Equal(t, ok, true) + assert.Equal(t, s.String, "hej") + assert.Equal(t, s.Int64, int64(2000)) +} diff --git a/pkg/services/sqlstore/migrations/cache_data_mig.go b/pkg/services/sqlstore/migrations/cache_data_mig.go new file mode 100644 index 00000000000..1201b38e337 --- /dev/null +++ b/pkg/services/sqlstore/migrations/cache_data_mig.go @@ -0,0 +1,17 @@ +package migrations + +import . "github.com/grafana/grafana/pkg/services/sqlstore/migrator" + +func addCacheMigration(mg *Migrator) { + var cacheDataV1 = Table{ + Name: "cache_data", + Columns: []*Column{ + {Name: "key", Type: DB_Char, IsPrimaryKey: true, Length: 16}, + {Name: "data", Type: DB_Blob}, + {Name: "expires", Type: DB_Integer, Length: 255, Nullable: false}, + {Name: "created_at", Type: DB_Integer, Length: 255, Nullable: false}, + }, + } + + mg.AddMigration("create cache_data table", NewAddTableMigration(cacheDataV1)) +} diff --git a/pkg/services/sqlstore/migrations/migrations.go b/pkg/services/sqlstore/migrations/migrations.go index 931259ec3ed..3e40c749f37 100644 --- a/pkg/services/sqlstore/migrations/migrations.go +++ b/pkg/services/sqlstore/migrations/migrations.go @@ -33,6 +33,7 @@ func AddMigrations(mg *Migrator) { addUserAuthMigrations(mg) addServerlockMigrations(mg) addUserAuthTokenMigrations(mg) + addCacheMigration(mg) } func addMigrationLogMigrations(mg *Migrator) { From 996d5059b119a9927059812a5384edda7bf2a9d8 Mon Sep 17 00:00:00 2001 From: bergquist Date: Fri, 15 Feb 2019 09:48:32 +0100 Subject: [PATCH 02/28] test at interface level instead impl --- pkg/infra/distcache/distcache.go | 26 +++++++++++++++++----- pkg/infra/distcache/distcache_test.go | 32 +++++++++++++++------------ 2 files changed, 39 insertions(+), 19 deletions(-) diff --git a/pkg/infra/distcache/distcache.go b/pkg/infra/distcache/distcache.go index 11efd435de3..3a2d553953a 100644 --- a/pkg/infra/distcache/distcache.go +++ b/pkg/infra/distcache/distcache.go @@ -23,15 +23,31 @@ func init() { func (ds *DistributedCache) Init() error { ds.log = log.New("distributed.cache") - // memory - // redis - // memcache - // database. using SQLSTORE - ds.Client = &databaseCache{SQLStore: ds.SQLStore} + ds.Client = createClient(CacheOpts{}, ds.SQLStore) return nil } +type CacheOpts struct { + name string +} + +func createClient(opts CacheOpts, sqlstore *sqlstore.SqlStore) cacheStorage { + if opts.name == "redis" { + return nil + } + + if opts.name == "memcache" { + return nil + } + + if opts.name == "memory" { + return nil + } + + return &databaseCache{SQLStore: sqlstore} +} + // DistributedCache allows Grafana to cache data outside its own process type DistributedCache struct { log log.Logger diff --git a/pkg/infra/distcache/distcache_test.go b/pkg/infra/distcache/distcache_test.go index 88066daec7e..d3009753a14 100644 --- a/pkg/infra/distcache/distcache_test.go +++ b/pkg/infra/distcache/distcache_test.go @@ -7,7 +7,6 @@ import ( "github.com/bmizerany/assert" - "github.com/grafana/grafana/pkg/log" "github.com/grafana/grafana/pkg/services/sqlstore" ) @@ -20,20 +19,29 @@ func init() { gob.Register(CacheableStruct{}) } -func createClient(t *testing.T) cacheStorage { +func createTestClient(t *testing.T, name string) cacheStorage { t.Helper() sqlstore := sqlstore.InitTestDB(t) - dc := DistributedCache{log: log.New("test.logger"), SQLStore: sqlstore} - dc.Init() - return dc.Client + return createClient(CacheOpts{name: name}, sqlstore) } -func TestCanPutIntoDatabaseStorage(t *testing.T) { - client := createClient(t) +func TestAllCacheClients(t *testing.T) { + clients := []string{"database"} // add redis, memcache, memory + + for _, v := range clients { + client := createTestClient(t, v) + + CanPutGetAndDeleteCachedObjects(t, client) + CanNotFetchExpiredItems(t, client) + CanSetInfiniteCacheExpiration(t, client) + } +} + +func CanPutGetAndDeleteCachedObjects(t *testing.T, client cacheStorage) { cacheableStruct := CacheableStruct{String: "hej", Int64: 2000} - err := client.Put("key", cacheableStruct, 1000) + err := client.Put("key", cacheableStruct, 0) assert.Equal(t, err, nil) data, err := client.Get("key") @@ -50,9 +58,7 @@ func TestCanPutIntoDatabaseStorage(t *testing.T) { assert.Equal(t, err, ErrCacheItemNotFound) } -func TestCanNotFetchExpiredItems(t *testing.T) { - client := createClient(t) - +func CanNotFetchExpiredItems(t *testing.T, client cacheStorage) { cacheableStruct := CacheableStruct{String: "hej", Int64: 2000} // insert cache item one day back @@ -66,9 +72,7 @@ func TestCanNotFetchExpiredItems(t *testing.T) { assert.Equal(t, err, ErrCacheItemNotFound) } -func TestCanSetInfiniteCacheExpiration(t *testing.T) { - client := createClient(t) - +func CanSetInfiniteCacheExpiration(t *testing.T, client cacheStorage) { cacheableStruct := CacheableStruct{String: "hej", Int64: 2000} // insert cache item one day back From d99af239462cf015db935d2e34a6fd885f350dc0 Mon Sep 17 00:00:00 2001 From: bergquist Date: Fri, 15 Feb 2019 14:31:52 +0100 Subject: [PATCH 03/28] add garbage collector for database cache --- pkg/infra/distcache/database_storage.go | 36 +++++++++++-- pkg/infra/distcache/database_storage_test.go | 50 +++++++++++++++++++ .../sqlstore/migrations/cache_data_mig.go | 23 +++++---- 3 files changed, 97 insertions(+), 12 deletions(-) create mode 100644 pkg/infra/distcache/database_storage_test.go diff --git a/pkg/infra/distcache/database_storage.go b/pkg/infra/distcache/database_storage.go index 8286f65fea6..ed55208e18d 100644 --- a/pkg/infra/distcache/database_storage.go +++ b/pkg/infra/distcache/database_storage.go @@ -3,18 +3,48 @@ package distcache import ( "time" + "github.com/grafana/grafana/pkg/log" "github.com/grafana/grafana/pkg/services/sqlstore" ) type databaseCache struct { SQLStore *sqlstore.SqlStore + log log.Logger +} + +func newDatabaseCache(sqlstore *sqlstore.SqlStore) *databaseCache { + dc := &databaseCache{ + SQLStore: sqlstore, + log: log.New("distcache.database"), + } + + go dc.StartGC() + return dc } var getTime = time.Now -func (dc *databaseCache) Get(key string) (interface{}, error) { - //now := getTime().Unix() +func (dc *databaseCache) internalRunGC() { + now := getTime().Unix() + sql := `DELETE FROM cache_data WHERE (? - created) >= expire` + //EXTRACT(EPOCH FROM NOW()) - created >= expire + //UNIX_TIMESTAMP(NOW()) - created >= expire + _, err := dc.SQLStore.NewSession().Exec(sql, now) + if err != nil { + dc.log.Error("failed to run garbage collect", "error", err) + } +} + +func (dc *databaseCache) StartGC() { + dc.internalRunGC() + + time.AfterFunc(time.Second*10, func() { + dc.StartGC() + }) +} + +func (dc *databaseCache) Get(key string) (interface{}, error) { cacheHits := []CacheData{} err := dc.SQLStore.NewSession().Where(`key = ?`, key).Find(&cacheHits) if err != nil { @@ -65,7 +95,7 @@ func (dc *databaseCache) Put(key string, value interface{}, expire int64) error } if len(cacheHits) > 0 { - _, err = dc.SQLStore.NewSession().Exec("UPDATE cached_data SET data=?, created=?, expire=? WHERE key=?", data, now, expire, key) + _, err = dc.SQLStore.NewSession().Exec("UPDATE cache_data SET data=?, created=?, expire=? WHERE key=?", data, now, expire, key) } else { _, err = dc.SQLStore.NewSession().Exec("INSERT INTO cache_data(key,data,created_at,expires) VALUES(?,?,?,?)", key, data, now, expire) } diff --git a/pkg/infra/distcache/database_storage_test.go b/pkg/infra/distcache/database_storage_test.go new file mode 100644 index 00000000000..2e6339c7c32 --- /dev/null +++ b/pkg/infra/distcache/database_storage_test.go @@ -0,0 +1,50 @@ +package distcache + +import ( + "testing" + "time" + + "github.com/bmizerany/assert" + + "github.com/grafana/grafana/pkg/log" + "github.com/grafana/grafana/pkg/services/sqlstore" +) + +func TestDatabaseStorageGarbageCollection(t *testing.T) { + sqlstore := sqlstore.InitTestDB(t) + + db := &databaseCache{ + SQLStore: sqlstore, + log: log.New("distcache.database"), + } + + obj := &CacheableStruct{String: "foolbar"} + + //set time.now to 2 weeks ago + getTime = func() time.Time { return time.Now().AddDate(0, 0, -2) } + db.Put("key1", obj, 1000) + db.Put("key2", obj, 1000) + db.Put("key3", obj, 1000) + + // insert object that should never expire + db.Put("key4", obj, 0) + + getTime = time.Now + db.Put("key5", obj, 1000) + + //run GC + db.internalRunGC() + + //try to read values + _, err := db.Get("key1") + assert.Equal(t, err, ErrCacheItemNotFound) + _, err = db.Get("key2") + assert.Equal(t, err, ErrCacheItemNotFound) + _, err = db.Get("key3") + assert.Equal(t, err, ErrCacheItemNotFound) + + _, err = db.Get("key4") + assert.Equal(t, err, nil) + _, err = db.Get("key5") + assert.Equal(t, err, nil) +} diff --git a/pkg/services/sqlstore/migrations/cache_data_mig.go b/pkg/services/sqlstore/migrations/cache_data_mig.go index 1201b38e337..f12f7f797c8 100644 --- a/pkg/services/sqlstore/migrations/cache_data_mig.go +++ b/pkg/services/sqlstore/migrations/cache_data_mig.go @@ -1,17 +1,22 @@ package migrations -import . "github.com/grafana/grafana/pkg/services/sqlstore/migrator" +import "github.com/grafana/grafana/pkg/services/sqlstore/migrator" -func addCacheMigration(mg *Migrator) { - var cacheDataV1 = Table{ +func addCacheMigration(mg *migrator.Migrator) { + var cacheDataV1 = migrator.Table{ Name: "cache_data", - Columns: []*Column{ - {Name: "key", Type: DB_Char, IsPrimaryKey: true, Length: 16}, - {Name: "data", Type: DB_Blob}, - {Name: "expires", Type: DB_Integer, Length: 255, Nullable: false}, - {Name: "created_at", Type: DB_Integer, Length: 255, Nullable: false}, + Columns: []*migrator.Column{ + {Name: "key", Type: migrator.DB_NVarchar, IsPrimaryKey: true, Length: 168}, + {Name: "data", Type: migrator.DB_Blob}, + {Name: "expires", Type: migrator.DB_Integer, Length: 255, Nullable: false}, + {Name: "created_at", Type: migrator.DB_Integer, Length: 255, Nullable: false}, + }, + Indices: []*migrator.Index{ + {Cols: []string{"key"}, Type: migrator.UniqueIndex}, }, } - mg.AddMigration("create cache_data table", NewAddTableMigration(cacheDataV1)) + mg.AddMigration("create cache_data table", migrator.NewAddTableMigration(cacheDataV1)) + + mg.AddMigration("add unique index cache_data.key", migrator.NewAddIndexMigration(cacheDataV1, cacheDataV1.Indices[0])) } From 5ced863f7527a1eb366ff8d63df7ff78e6e4b51f Mon Sep 17 00:00:00 2001 From: bergquist Date: Sat, 23 Feb 2019 16:12:37 +0100 Subject: [PATCH 04/28] add support for redis storage --- package.json | 5 -- pkg/infra/distcache/database_storage.go | 13 +++- pkg/infra/distcache/database_storage_test.go | 8 +- pkg/infra/distcache/distcache.go | 7 +- pkg/infra/distcache/distcache_test.go | 20 +++-- pkg/infra/distcache/redis_storage.go | 80 ++++++++++++++++++++ pkg/infra/distcache/redis_storage_test.go | 1 + 7 files changed, 110 insertions(+), 24 deletions(-) create mode 100644 pkg/infra/distcache/redis_storage.go create mode 100644 pkg/infra/distcache/redis_storage_test.go diff --git a/package.json b/package.json index a937ba6f717..af270d47ad0 100644 --- a/package.json +++ b/package.json @@ -142,11 +142,6 @@ "gui:release": "ts-node --project ./scripts/cli/tsconfig.json ./scripts/cli/index.ts gui:release -p", "cli": "ts-node --project ./scripts/cli/tsconfig.json ./scripts/cli/index.ts" }, - "husky": { - "hooks": { - "pre-commit": "lint-staged && grunt precommit" - } - }, "lint-staged": { "*.{ts,tsx,json,scss}": [ "prettier --write", diff --git a/pkg/infra/distcache/database_storage.go b/pkg/infra/distcache/database_storage.go index ed55208e18d..cff5e0fc499 100644 --- a/pkg/infra/distcache/database_storage.go +++ b/pkg/infra/distcache/database_storage.go @@ -18,7 +18,7 @@ func newDatabaseCache(sqlstore *sqlstore.SqlStore) *databaseCache { log: log.New("distcache.database"), } - go dc.StartGC() + //go dc.StartGC() //TODO: start the GC somehow return dc } @@ -79,7 +79,7 @@ type CacheData struct { CreatedAt int64 } -func (dc *databaseCache) Put(key string, value interface{}, expire int64) error { +func (dc *databaseCache) Put(key string, value interface{}, expire time.Duration) error { item := &Item{Val: value} data, err := EncodeGob(item) if err != nil { @@ -94,10 +94,15 @@ func (dc *databaseCache) Put(key string, value interface{}, expire int64) error return err } + var expiresInEpoch int64 + if expire != 0 { + expiresInEpoch = int64(expire) / int64(time.Second) + } + if len(cacheHits) > 0 { - _, err = dc.SQLStore.NewSession().Exec("UPDATE cache_data SET data=?, created=?, expire=? WHERE key=?", data, now, expire, key) + _, err = dc.SQLStore.NewSession().Exec("UPDATE cache_data SET data=?, created=?, expire=? WHERE key=?", data, now, expiresInEpoch, key) } else { - _, err = dc.SQLStore.NewSession().Exec("INSERT INTO cache_data(key,data,created_at,expires) VALUES(?,?,?,?)", key, data, now, expire) + _, err = dc.SQLStore.NewSession().Exec("INSERT INTO cache_data(key,data,created_at,expires) VALUES(?,?,?,?)", key, data, now, expiresInEpoch) } return err diff --git a/pkg/infra/distcache/database_storage_test.go b/pkg/infra/distcache/database_storage_test.go index 2e6339c7c32..931fbc81c7f 100644 --- a/pkg/infra/distcache/database_storage_test.go +++ b/pkg/infra/distcache/database_storage_test.go @@ -22,15 +22,15 @@ func TestDatabaseStorageGarbageCollection(t *testing.T) { //set time.now to 2 weeks ago getTime = func() time.Time { return time.Now().AddDate(0, 0, -2) } - db.Put("key1", obj, 1000) - db.Put("key2", obj, 1000) - db.Put("key3", obj, 1000) + db.Put("key1", obj, 1000*time.Second) + db.Put("key2", obj, 1000*time.Second) + db.Put("key3", obj, 1000*time.Second) // insert object that should never expire db.Put("key4", obj, 0) getTime = time.Now - db.Put("key5", obj, 1000) + db.Put("key5", obj, 1000*time.Second) //run GC db.internalRunGC() diff --git a/pkg/infra/distcache/distcache.go b/pkg/infra/distcache/distcache.go index 3a2d553953a..a60b3d309c2 100644 --- a/pkg/infra/distcache/distcache.go +++ b/pkg/infra/distcache/distcache.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/gob" "errors" + "time" "github.com/grafana/grafana/pkg/log" "github.com/grafana/grafana/pkg/services/sqlstore" @@ -34,7 +35,7 @@ type CacheOpts struct { func createClient(opts CacheOpts, sqlstore *sqlstore.SqlStore) cacheStorage { if opts.name == "redis" { - return nil + return newRedisStorage(nil) } if opts.name == "memcache" { @@ -45,7 +46,7 @@ func createClient(opts CacheOpts, sqlstore *sqlstore.SqlStore) cacheStorage { return nil } - return &databaseCache{SQLStore: sqlstore} + return newDatabaseCache(sqlstore) //&databaseCache{SQLStore: sqlstore} } // DistributedCache allows Grafana to cache data outside its own process @@ -77,7 +78,7 @@ type cacheStorage interface { Get(key string) (interface{}, error) // Puts an object into the cache - Put(key string, value interface{}, expire int64) error + Put(key string, value interface{}, expire time.Duration) error // Delete object from cache Delete(key string) error diff --git a/pkg/infra/distcache/distcache_test.go b/pkg/infra/distcache/distcache_test.go index d3009753a14..dd35744506c 100644 --- a/pkg/infra/distcache/distcache_test.go +++ b/pkg/infra/distcache/distcache_test.go @@ -27,18 +27,18 @@ func createTestClient(t *testing.T, name string) cacheStorage { } func TestAllCacheClients(t *testing.T) { - clients := []string{"database"} // add redis, memcache, memory + clients := []string{"database", "redis"} // add redis, memcache, memory for _, v := range clients { client := createTestClient(t, v) - CanPutGetAndDeleteCachedObjects(t, client) - CanNotFetchExpiredItems(t, client) - CanSetInfiniteCacheExpiration(t, client) + CanPutGetAndDeleteCachedObjects(t, v, client) + CanNotFetchExpiredItems(t, v, client) + CanSetInfiniteCacheExpiration(t, v, client) } } -func CanPutGetAndDeleteCachedObjects(t *testing.T, client cacheStorage) { +func CanPutGetAndDeleteCachedObjects(t *testing.T, name string, client cacheStorage) { cacheableStruct := CacheableStruct{String: "hej", Int64: 2000} err := client.Put("key", cacheableStruct, 0) @@ -58,12 +58,16 @@ func CanPutGetAndDeleteCachedObjects(t *testing.T, client cacheStorage) { assert.Equal(t, err, ErrCacheItemNotFound) } -func CanNotFetchExpiredItems(t *testing.T, client cacheStorage) { +func CanNotFetchExpiredItems(t *testing.T, name string, client cacheStorage) { + if name == "redis" { + t.Skip() //this test does not work with redis since it uses its own getTime fn + } + cacheableStruct := CacheableStruct{String: "hej", Int64: 2000} // insert cache item one day back getTime = func() time.Time { return time.Now().AddDate(0, 0, -2) } - err := client.Put("key", cacheableStruct, 10000) + err := client.Put("key", cacheableStruct, 10000*time.Second) assert.Equal(t, err, nil) // should not be able to read that value since its expired @@ -72,7 +76,7 @@ func CanNotFetchExpiredItems(t *testing.T, client cacheStorage) { assert.Equal(t, err, ErrCacheItemNotFound) } -func CanSetInfiniteCacheExpiration(t *testing.T, client cacheStorage) { +func CanSetInfiniteCacheExpiration(t *testing.T, name string, client cacheStorage) { cacheableStruct := CacheableStruct{String: "hej", Int64: 2000} // insert cache item one day back diff --git a/pkg/infra/distcache/redis_storage.go b/pkg/infra/distcache/redis_storage.go new file mode 100644 index 00000000000..06fc6931758 --- /dev/null +++ b/pkg/infra/distcache/redis_storage.go @@ -0,0 +1,80 @@ +package distcache + +import ( + "time" + + redis "gopkg.in/redis.v2" +) + +type redisStorage struct { + c *redis.Client +} + +func newRedisStorage(c *redis.Client) *redisStorage { + opt := &redis.Options{ + Network: "tcp", + Addr: "localhost:6379", + } + return &redisStorage{ + c: redis.NewClient(opt), + } +} + +// Set sets value to given key in session. +func (s *redisStorage) Put(key string, val interface{}, expires time.Duration) error { + item := &Item{Created: getTime().Unix(), Val: val} + value, err := EncodeGob(item) + if err != nil { + return err + } + + var status *redis.StatusCmd + if expires == 0 { + status = s.c.Set(key, string(value)) + } else { + status = s.c.SetEx(key, expires, string(value)) + } + + return status.Err() +} + +// Get gets value by given key in session. +func (s *redisStorage) Get(key string) (interface{}, error) { + v := s.c.Get(key) + + item := &Item{} + err := DecodeGob([]byte(v.Val()), item) + + if err == nil { + return item.Val, nil + } + + if err.Error() == "EOF" { + return nil, ErrCacheItemNotFound + } + + if err != nil { + return nil, err + } + + return item.Val, nil +} + +// Delete delete a key from session. +func (s *redisStorage) Delete(key string) error { + cmd := s.c.Del(key) + return cmd.Err() +} + +// RedisProvider represents a redis session provider implementation. +type RedisProvider struct { + c *redis.Client + duration time.Duration + prefix string +} + +// Exist returns true if session with given ID exists. +func (p *RedisProvider) Exist(sid string) bool { + has, err := p.c.Exists(p.prefix + sid).Result() + return err == nil && has +} diff --git a/pkg/infra/distcache/redis_storage_test.go b/pkg/infra/distcache/redis_storage_test.go new file mode 100644 index 00000000000..e793fbec4c4 --- /dev/null +++ b/pkg/infra/distcache/redis_storage_test.go @@ -0,0 +1 @@ +package distcache From 11d671c637fea63b74d9082a907b0a97f424e6e0 Mon Sep 17 00:00:00 2001 From: bergquist Date: Sat, 23 Feb 2019 18:28:33 +0100 Subject: [PATCH 05/28] add support for memcached --- pkg/infra/distcache/distcache.go | 8 +-- pkg/infra/distcache/distcache_test.go | 2 +- pkg/infra/distcache/memcached_storage.go | 62 ++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 5 deletions(-) create mode 100644 pkg/infra/distcache/memcached_storage.go diff --git a/pkg/infra/distcache/distcache.go b/pkg/infra/distcache/distcache.go index a60b3d309c2..8a6f7daf90c 100644 --- a/pkg/infra/distcache/distcache.go +++ b/pkg/infra/distcache/distcache.go @@ -39,12 +39,12 @@ func createClient(opts CacheOpts, sqlstore *sqlstore.SqlStore) cacheStorage { } if opts.name == "memcache" { - return nil + return newMemcacheStorage("localhost:9090") } - if opts.name == "memory" { - return nil - } + // if opts.name == "memory" { + // return nil + // } return newDatabaseCache(sqlstore) //&databaseCache{SQLStore: sqlstore} } diff --git a/pkg/infra/distcache/distcache_test.go b/pkg/infra/distcache/distcache_test.go index dd35744506c..a04b5d0228f 100644 --- a/pkg/infra/distcache/distcache_test.go +++ b/pkg/infra/distcache/distcache_test.go @@ -27,7 +27,7 @@ func createTestClient(t *testing.T, name string) cacheStorage { } func TestAllCacheClients(t *testing.T) { - clients := []string{"database", "redis"} // add redis, memcache, memory + clients := []string{"database", "redis", "memcached"} // add redis, memcache, memory for _, v := range clients { client := createTestClient(t, v) diff --git a/pkg/infra/distcache/memcached_storage.go b/pkg/infra/distcache/memcached_storage.go new file mode 100644 index 00000000000..44fbbcc33c6 --- /dev/null +++ b/pkg/infra/distcache/memcached_storage.go @@ -0,0 +1,62 @@ +package distcache + +import ( + "time" + + "github.com/bradfitz/gomemcache/memcache" +) + +type memcacheStorage struct { + c *memcache.Client +} + +func newMemcacheStorage(connStr string) *memcacheStorage { + return &memcacheStorage{ + c: memcache.New(connStr), + } +} + +func NewItem(sid string, data []byte, expire int32) *memcache.Item { + return &memcache.Item{ + Key: sid, + Value: data, + Expiration: expire, + } +} + +// Set sets value to given key in the cache. +func (s *memcacheStorage) Put(key string, val interface{}, expires time.Duration) error { + item := &Item{Val: val} + + bytes, err := EncodeGob(item) + if err != nil { + return err + } + + memcacheItem := NewItem(key, bytes, int32(expires)) + + s.c.Add(memcacheItem) + return nil +} + +// Get gets value by given key in the cache. +func (s *memcacheStorage) Get(key string) (interface{}, error) { + i, err := s.c.Get(key) + if err != nil { + return nil, err + } + + item := &Item{} + + err = DecodeGob(i.Value, item) + if err != nil { + return nil, err + } + + return item.Val, nil +} + +// Delete delete a key from the cache +func (s *memcacheStorage) Delete(key string) error { + return s.c.Delete(key) +} From 3890bd14ebebe9518c33d99727b7267f33e3765b Mon Sep 17 00:00:00 2001 From: bergquist Date: Sat, 23 Feb 2019 22:59:12 +0100 Subject: [PATCH 06/28] fixes typo in redis devenv --- devenv/docker/blocks/redis/docker-compose.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devenv/docker/blocks/redis/docker-compose.yaml b/devenv/docker/blocks/redis/docker-compose.yaml index 65071d4966b..fb56afaac1c 100644 --- a/devenv/docker/blocks/redis/docker-compose.yaml +++ b/devenv/docker/blocks/redis/docker-compose.yaml @@ -1,4 +1,4 @@ - memcached: + redis: image: redis:latest ports: - "6379:6379" From c8ff698d9094dc43192f825874ccb5ea8f27bd83 Mon Sep 17 00:00:00 2001 From: bergquist Date: Sat, 23 Feb 2019 22:59:34 +0100 Subject: [PATCH 07/28] avoid exposing internal structs and functions --- pkg/infra/distcache/database_storage.go | 16 ++++++++-------- pkg/infra/distcache/distcache.go | 12 +++++------- pkg/infra/distcache/distcache_test.go | 14 +++++--------- pkg/infra/distcache/memcached_storage.go | 12 ++++++------ pkg/infra/distcache/redis_storage.go | 21 ++++----------------- 5 files changed, 28 insertions(+), 47 deletions(-) diff --git a/pkg/infra/distcache/database_storage.go b/pkg/infra/distcache/database_storage.go index cff5e0fc499..f4365383b82 100644 --- a/pkg/infra/distcache/database_storage.go +++ b/pkg/infra/distcache/database_storage.go @@ -45,13 +45,13 @@ func (dc *databaseCache) StartGC() { } func (dc *databaseCache) Get(key string) (interface{}, error) { - cacheHits := []CacheData{} + cacheHits := []cacheData{} err := dc.SQLStore.NewSession().Where(`key = ?`, key).Find(&cacheHits) if err != nil { return nil, err } - var cacheHit CacheData + var cacheHit cacheData if len(cacheHits) == 0 { return nil, ErrCacheItemNotFound } @@ -64,15 +64,15 @@ func (dc *databaseCache) Get(key string) (interface{}, error) { } } - item := &Item{} - if err = DecodeGob(cacheHit.Data, item); err != nil { + item := &cachedItem{} + if err = decodeGob(cacheHit.Data, item); err != nil { return nil, err } return item.Val, nil } -type CacheData struct { +type cacheData struct { Key string Data []byte Expires int64 @@ -80,15 +80,15 @@ type CacheData struct { } func (dc *databaseCache) Put(key string, value interface{}, expire time.Duration) error { - item := &Item{Val: value} - data, err := EncodeGob(item) + item := &cachedItem{Val: value} + data, err := encodeGob(item) if err != nil { return err } now := getTime().Unix() - cacheHits := []CacheData{} + cacheHits := []cacheData{} err = dc.SQLStore.NewSession().Where(`key = ?`, key).Find(&cacheHits) if err != nil { return err diff --git a/pkg/infra/distcache/distcache.go b/pkg/infra/distcache/distcache.go index 8a6f7daf90c..8ba1a306a3f 100644 --- a/pkg/infra/distcache/distcache.go +++ b/pkg/infra/distcache/distcache.go @@ -46,7 +46,7 @@ func createClient(opts CacheOpts, sqlstore *sqlstore.SqlStore) cacheStorage { // return nil // } - return newDatabaseCache(sqlstore) //&databaseCache{SQLStore: sqlstore} + return newDatabaseCache(sqlstore) } // DistributedCache allows Grafana to cache data outside its own process @@ -56,19 +56,17 @@ type DistributedCache struct { SQLStore *sqlstore.SqlStore `inject:""` } -type Item struct { - Val interface{} - Created int64 - Expire int64 +type cachedItem struct { + Val interface{} } -func EncodeGob(item *Item) ([]byte, error) { +func encodeGob(item *cachedItem) ([]byte, error) { buf := bytes.NewBuffer(nil) err := gob.NewEncoder(buf).Encode(item) return buf.Bytes(), err } -func DecodeGob(data []byte, out *Item) error { +func decodeGob(data []byte, out *cachedItem) error { buf := bytes.NewBuffer(data) return gob.NewDecoder(buf).Decode(&out) } diff --git a/pkg/infra/distcache/distcache_test.go b/pkg/infra/distcache/distcache_test.go index a04b5d0228f..6f59c40f0e9 100644 --- a/pkg/infra/distcache/distcache_test.go +++ b/pkg/infra/distcache/distcache_test.go @@ -27,7 +27,7 @@ func createTestClient(t *testing.T, name string) cacheStorage { } func TestAllCacheClients(t *testing.T) { - clients := []string{"database", "redis", "memcached"} // add redis, memcache, memory + clients := []string{"database", "redis"} // add redis, memcache, memory for _, v := range clients { client := createTestClient(t, v) @@ -59,19 +59,15 @@ func CanPutGetAndDeleteCachedObjects(t *testing.T, name string, client cacheStor } func CanNotFetchExpiredItems(t *testing.T, name string, client cacheStorage) { - if name == "redis" { - t.Skip() //this test does not work with redis since it uses its own getTime fn - } - cacheableStruct := CacheableStruct{String: "hej", Int64: 2000} - // insert cache item one day back - getTime = func() time.Time { return time.Now().AddDate(0, 0, -2) } - err := client.Put("key", cacheableStruct, 10000*time.Second) + err := client.Put("key", cacheableStruct, time.Second) assert.Equal(t, err, nil) + //not sure how this can be avoided when testing redis/memcached :/ + <-time.After(time.Second + time.Millisecond) + // should not be able to read that value since its expired - getTime = time.Now _, err = client.Get("key") assert.Equal(t, err, ErrCacheItemNotFound) } diff --git a/pkg/infra/distcache/memcached_storage.go b/pkg/infra/distcache/memcached_storage.go index 44fbbcc33c6..71e037cf196 100644 --- a/pkg/infra/distcache/memcached_storage.go +++ b/pkg/infra/distcache/memcached_storage.go @@ -16,7 +16,7 @@ func newMemcacheStorage(connStr string) *memcacheStorage { } } -func NewItem(sid string, data []byte, expire int32) *memcache.Item { +func newItem(sid string, data []byte, expire int32) *memcache.Item { return &memcache.Item{ Key: sid, Value: data, @@ -26,14 +26,14 @@ func NewItem(sid string, data []byte, expire int32) *memcache.Item { // Set sets value to given key in the cache. func (s *memcacheStorage) Put(key string, val interface{}, expires time.Duration) error { - item := &Item{Val: val} + item := &cachedItem{Val: val} - bytes, err := EncodeGob(item) + bytes, err := encodeGob(item) if err != nil { return err } - memcacheItem := NewItem(key, bytes, int32(expires)) + memcacheItem := newItem(key, bytes, int32(expires)) s.c.Add(memcacheItem) return nil @@ -46,9 +46,9 @@ func (s *memcacheStorage) Get(key string) (interface{}, error) { return nil, err } - item := &Item{} + item := &cachedItem{} - err = DecodeGob(i.Value, item) + err = decodeGob(i.Value, item) if err != nil { return nil, err } diff --git a/pkg/infra/distcache/redis_storage.go b/pkg/infra/distcache/redis_storage.go index 06fc6931758..49055fd8356 100644 --- a/pkg/infra/distcache/redis_storage.go +++ b/pkg/infra/distcache/redis_storage.go @@ -22,8 +22,8 @@ func newRedisStorage(c *redis.Client) *redisStorage { // Set sets value to given key in session. func (s *redisStorage) Put(key string, val interface{}, expires time.Duration) error { - item := &Item{Created: getTime().Unix(), Val: val} - value, err := EncodeGob(item) + item := &cachedItem{Val: val} + value, err := encodeGob(item) if err != nil { return err } @@ -42,8 +42,8 @@ func (s *redisStorage) Put(key string, val interface{}, expires time.Duration) e func (s *redisStorage) Get(key string) (interface{}, error) { v := s.c.Get(key) - item := &Item{} - err := DecodeGob([]byte(v.Val()), item) + item := &cachedItem{} + err := decodeGob([]byte(v.Val()), item) if err == nil { return item.Val, nil @@ -65,16 +65,3 @@ func (s *redisStorage) Delete(key string) error { cmd := s.c.Del(key) return cmd.Err() } - -// RedisProvider represents a redis session provider implementation. -type RedisProvider struct { - c *redis.Client - duration time.Duration - prefix string -} - -// Exist returns true if session with given ID exists. -func (p *RedisProvider) Exist(sid string) bool { - has, err := p.c.Exists(p.prefix + sid).Result() - return err == nil && has -} From a60bb83a70376639ac3460ba5b0d51b2e3fdc6dd Mon Sep 17 00:00:00 2001 From: bergquist Date: Sun, 3 Mar 2019 04:42:11 +0100 Subject: [PATCH 08/28] extract tests into seperate files --- pkg/infra/distcache/distcache.go | 10 ++++++++-- pkg/infra/distcache/distcache_test.go | 15 ++++++++------- pkg/infra/distcache/memcached_storage.go | 10 +++++++--- pkg/infra/distcache/redis_storage.go | 8 +------- pkg/infra/distcache/redis_storage_test.go | 11 +++++++++++ 5 files changed, 35 insertions(+), 19 deletions(-) diff --git a/pkg/infra/distcache/distcache.go b/pkg/infra/distcache/distcache.go index 8ba1a306a3f..87a6da45029 100644 --- a/pkg/infra/distcache/distcache.go +++ b/pkg/infra/distcache/distcache.go @@ -8,6 +8,7 @@ import ( "github.com/grafana/grafana/pkg/log" "github.com/grafana/grafana/pkg/services/sqlstore" + redis "gopkg.in/redis.v2" "github.com/grafana/grafana/pkg/registry" ) @@ -35,11 +36,16 @@ type CacheOpts struct { func createClient(opts CacheOpts, sqlstore *sqlstore.SqlStore) cacheStorage { if opts.name == "redis" { - return newRedisStorage(nil) + opt := &redis.Options{ + Network: "tcp", + Addr: "localhost:6379", + } + + return newRedisStorage(redis.NewClient(opt)) } if opts.name == "memcache" { - return newMemcacheStorage("localhost:9090") + return newMemcacheStorage("localhost:11211") } // if opts.name == "memory" { diff --git a/pkg/infra/distcache/distcache_test.go b/pkg/infra/distcache/distcache_test.go index 6f59c40f0e9..af6f426e1c0 100644 --- a/pkg/infra/distcache/distcache_test.go +++ b/pkg/infra/distcache/distcache_test.go @@ -27,18 +27,19 @@ func createTestClient(t *testing.T, name string) cacheStorage { } func TestAllCacheClients(t *testing.T) { - clients := []string{"database", "redis"} // add redis, memcache, memory + //clients := []string{"database", "redis", "memcache"} // add redis, memcache, memory + clients := []string{} // add redis, memcache, memory for _, v := range clients { client := createTestClient(t, v) - CanPutGetAndDeleteCachedObjects(t, v, client) - CanNotFetchExpiredItems(t, v, client) - CanSetInfiniteCacheExpiration(t, v, client) + CanPutGetAndDeleteCachedObjects(t, client) + CanNotFetchExpiredItems(t, client) + CanSetInfiniteCacheExpiration(t, client) } } -func CanPutGetAndDeleteCachedObjects(t *testing.T, name string, client cacheStorage) { +func CanPutGetAndDeleteCachedObjects(t *testing.T, client cacheStorage) { cacheableStruct := CacheableStruct{String: "hej", Int64: 2000} err := client.Put("key", cacheableStruct, 0) @@ -58,7 +59,7 @@ func CanPutGetAndDeleteCachedObjects(t *testing.T, name string, client cacheStor assert.Equal(t, err, ErrCacheItemNotFound) } -func CanNotFetchExpiredItems(t *testing.T, name string, client cacheStorage) { +func CanNotFetchExpiredItems(t *testing.T, client cacheStorage) { cacheableStruct := CacheableStruct{String: "hej", Int64: 2000} err := client.Put("key", cacheableStruct, time.Second) @@ -72,7 +73,7 @@ func CanNotFetchExpiredItems(t *testing.T, name string, client cacheStorage) { assert.Equal(t, err, ErrCacheItemNotFound) } -func CanSetInfiniteCacheExpiration(t *testing.T, name string, client cacheStorage) { +func CanSetInfiniteCacheExpiration(t *testing.T, client cacheStorage) { cacheableStruct := CacheableStruct{String: "hej", Int64: 2000} // insert cache item one day back diff --git a/pkg/infra/distcache/memcached_storage.go b/pkg/infra/distcache/memcached_storage.go index 71e037cf196..1186bef626b 100644 --- a/pkg/infra/distcache/memcached_storage.go +++ b/pkg/infra/distcache/memcached_storage.go @@ -24,7 +24,7 @@ func newItem(sid string, data []byte, expire int32) *memcache.Item { } } -// Set sets value to given key in the cache. +// Put sets value to given key in the cache. func (s *memcacheStorage) Put(key string, val interface{}, expires time.Duration) error { item := &cachedItem{Val: val} @@ -35,13 +35,17 @@ func (s *memcacheStorage) Put(key string, val interface{}, expires time.Duration memcacheItem := newItem(key, bytes, int32(expires)) - s.c.Add(memcacheItem) - return nil + return s.c.Add(memcacheItem) } // Get gets value by given key in the cache. func (s *memcacheStorage) Get(key string) (interface{}, error) { i, err := s.c.Get(key) + + if err != nil && err.Error() == "memcache: cache miss" { + return nil, ErrCacheItemNotFound + } + if err != nil { return nil, err } diff --git a/pkg/infra/distcache/redis_storage.go b/pkg/infra/distcache/redis_storage.go index 49055fd8356..bb21b26473e 100644 --- a/pkg/infra/distcache/redis_storage.go +++ b/pkg/infra/distcache/redis_storage.go @@ -11,13 +11,7 @@ type redisStorage struct { } func newRedisStorage(c *redis.Client) *redisStorage { - opt := &redis.Options{ - Network: "tcp", - Addr: "localhost:6379", - } - return &redisStorage{ - c: redis.NewClient(opt), - } + return &redisStorage{c: c} } // Set sets value to given key in session. diff --git a/pkg/infra/distcache/redis_storage_test.go b/pkg/infra/distcache/redis_storage_test.go index e793fbec4c4..39d39d41b12 100644 --- a/pkg/infra/distcache/redis_storage_test.go +++ b/pkg/infra/distcache/redis_storage_test.go @@ -1 +1,12 @@ package distcache + +import "testing" + +func TestRedisCacheStorage(t *testing.T) { + + client := createTestClient(t, "redis") + + CanPutGetAndDeleteCachedObjects(t, client) + CanNotFetchExpiredItems(t, client) + CanSetInfiniteCacheExpiration(t, client) +} From 8db2864feef388f9ee1894c84783e5d05ff61a60 Mon Sep 17 00:00:00 2001 From: bergquist Date: Sun, 3 Mar 2019 05:25:17 +0100 Subject: [PATCH 09/28] adds memory as dist storage alt --- .../database_storage_integration_test.go | 12 +++++++ pkg/infra/distcache/distcache.go | 6 ++-- pkg/infra/distcache/distcache_test.go | 3 +- pkg/infra/distcache/memcached_storage_test.go | 12 +++++++ pkg/infra/distcache/memory_storage.go | 35 +++++++++++++++++++ 5 files changed, 63 insertions(+), 5 deletions(-) create mode 100644 pkg/infra/distcache/database_storage_integration_test.go create mode 100644 pkg/infra/distcache/memcached_storage_test.go create mode 100644 pkg/infra/distcache/memory_storage.go diff --git a/pkg/infra/distcache/database_storage_integration_test.go b/pkg/infra/distcache/database_storage_integration_test.go new file mode 100644 index 00000000000..e305759983d --- /dev/null +++ b/pkg/infra/distcache/database_storage_integration_test.go @@ -0,0 +1,12 @@ +package distcache + +import "testing" + +func TestIntegrationDatabaseCacheStorage(t *testing.T) { + + client := createTestClient(t, "database") + + CanPutGetAndDeleteCachedObjects(t, client) + CanNotFetchExpiredItems(t, client) + CanSetInfiniteCacheExpiration(t, client) +} diff --git a/pkg/infra/distcache/distcache.go b/pkg/infra/distcache/distcache.go index 87a6da45029..d21ada1e6a3 100644 --- a/pkg/infra/distcache/distcache.go +++ b/pkg/infra/distcache/distcache.go @@ -48,9 +48,9 @@ func createClient(opts CacheOpts, sqlstore *sqlstore.SqlStore) cacheStorage { return newMemcacheStorage("localhost:11211") } - // if opts.name == "memory" { - // return nil - // } + if opts.name == "memory" { + return newMemoryStorage() + } return newDatabaseCache(sqlstore) } diff --git a/pkg/infra/distcache/distcache_test.go b/pkg/infra/distcache/distcache_test.go index af6f426e1c0..ec778b0c335 100644 --- a/pkg/infra/distcache/distcache_test.go +++ b/pkg/infra/distcache/distcache_test.go @@ -27,8 +27,7 @@ func createTestClient(t *testing.T, name string) cacheStorage { } func TestAllCacheClients(t *testing.T) { - //clients := []string{"database", "redis", "memcache"} // add redis, memcache, memory - clients := []string{} // add redis, memcache, memory + clients := []string{"memory"} // add redis, memcache, memory for _, v := range clients { client := createTestClient(t, v) diff --git a/pkg/infra/distcache/memcached_storage_test.go b/pkg/infra/distcache/memcached_storage_test.go new file mode 100644 index 00000000000..b02f67f062f --- /dev/null +++ b/pkg/infra/distcache/memcached_storage_test.go @@ -0,0 +1,12 @@ +package distcache + +import "testing" + +func TestMemcachedCacheStorage(t *testing.T) { + + client := createTestClient(t, "memcache") + + CanPutGetAndDeleteCachedObjects(t, client) + CanNotFetchExpiredItems(t, client) + CanSetInfiniteCacheExpiration(t, client) +} diff --git a/pkg/infra/distcache/memory_storage.go b/pkg/infra/distcache/memory_storage.go new file mode 100644 index 00000000000..a1203cabe75 --- /dev/null +++ b/pkg/infra/distcache/memory_storage.go @@ -0,0 +1,35 @@ +package distcache + +import ( + "time" + + gocache "github.com/patrickmn/go-cache" +) + +type memoryStorage struct { + c *gocache.Cache +} + +func newMemoryStorage() *memoryStorage { + return &memoryStorage{ + c: gocache.New(time.Minute*30, time.Minute*30), + } +} + +func (s *memoryStorage) Put(key string, val interface{}, expires time.Duration) error { + return s.c.Add(key, val, expires) +} + +func (s *memoryStorage) Get(key string) (interface{}, error) { + val, exist := s.c.Get(key) + if !exist { + return nil, ErrCacheItemNotFound + } + + return val, nil +} + +func (s *memoryStorage) Delete(key string) error { + s.c.Delete(key) + return nil +} From 33935b09f0e543d0fc9583fabd2810685ca2c0cb Mon Sep 17 00:00:00 2001 From: bergquist Date: Sun, 3 Mar 2019 12:34:41 +0100 Subject: [PATCH 10/28] uses set instead of add for memcache set always sets the value regardless. --- .../database_storage_integration_test.go | 6 +----- pkg/infra/distcache/distcache_test.go | 16 ++++++++-------- pkg/infra/distcache/memcached_storage.go | 2 +- pkg/infra/distcache/memcached_storage_test.go | 7 +------ pkg/infra/distcache/redis_storage_test.go | 7 +------ 5 files changed, 12 insertions(+), 26 deletions(-) diff --git a/pkg/infra/distcache/database_storage_integration_test.go b/pkg/infra/distcache/database_storage_integration_test.go index e305759983d..b8f564f9710 100644 --- a/pkg/infra/distcache/database_storage_integration_test.go +++ b/pkg/infra/distcache/database_storage_integration_test.go @@ -4,9 +4,5 @@ import "testing" func TestIntegrationDatabaseCacheStorage(t *testing.T) { - client := createTestClient(t, "database") - - CanPutGetAndDeleteCachedObjects(t, client) - CanNotFetchExpiredItems(t, client) - CanSetInfiniteCacheExpiration(t, client) + RunTestsForClient(t, createTestClient(t, "database")) } diff --git a/pkg/infra/distcache/distcache_test.go b/pkg/infra/distcache/distcache_test.go index ec778b0c335..a40066b788f 100644 --- a/pkg/infra/distcache/distcache_test.go +++ b/pkg/infra/distcache/distcache_test.go @@ -26,16 +26,16 @@ func createTestClient(t *testing.T, name string) cacheStorage { return createClient(CacheOpts{name: name}, sqlstore) } -func TestAllCacheClients(t *testing.T) { - clients := []string{"memory"} // add redis, memcache, memory +func TestMemoryStorageClient(t *testing.T) { - for _, v := range clients { - client := createTestClient(t, v) + client := createTestClient(t, "memory") + RunTestsForClient(t, client) +} - CanPutGetAndDeleteCachedObjects(t, client) - CanNotFetchExpiredItems(t, client) - CanSetInfiniteCacheExpiration(t, client) - } +func RunTestsForClient(t *testing.T, client cacheStorage) { + CanPutGetAndDeleteCachedObjects(t, client) + CanNotFetchExpiredItems(t, client) + CanSetInfiniteCacheExpiration(t, client) } func CanPutGetAndDeleteCachedObjects(t *testing.T, client cacheStorage) { diff --git a/pkg/infra/distcache/memcached_storage.go b/pkg/infra/distcache/memcached_storage.go index 1186bef626b..7f97a043628 100644 --- a/pkg/infra/distcache/memcached_storage.go +++ b/pkg/infra/distcache/memcached_storage.go @@ -35,7 +35,7 @@ func (s *memcacheStorage) Put(key string, val interface{}, expires time.Duration memcacheItem := newItem(key, bytes, int32(expires)) - return s.c.Add(memcacheItem) + return s.c.Set(memcacheItem) } // Get gets value by given key in the cache. diff --git a/pkg/infra/distcache/memcached_storage_test.go b/pkg/infra/distcache/memcached_storage_test.go index b02f67f062f..524a4fcea10 100644 --- a/pkg/infra/distcache/memcached_storage_test.go +++ b/pkg/infra/distcache/memcached_storage_test.go @@ -3,10 +3,5 @@ package distcache import "testing" func TestMemcachedCacheStorage(t *testing.T) { - - client := createTestClient(t, "memcache") - - CanPutGetAndDeleteCachedObjects(t, client) - CanNotFetchExpiredItems(t, client) - CanSetInfiniteCacheExpiration(t, client) + RunTestsForClient(t, createTestClient(t, "memcache")) } diff --git a/pkg/infra/distcache/redis_storage_test.go b/pkg/infra/distcache/redis_storage_test.go index 39d39d41b12..6ba093a205c 100644 --- a/pkg/infra/distcache/redis_storage_test.go +++ b/pkg/infra/distcache/redis_storage_test.go @@ -3,10 +3,5 @@ package distcache import "testing" func TestRedisCacheStorage(t *testing.T) { - - client := createTestClient(t, "redis") - - CanPutGetAndDeleteCachedObjects(t, client) - CanNotFetchExpiredItems(t, client) - CanSetInfiniteCacheExpiration(t, client) + RunTestsForClient(t, createTestClient(t, "redis")) } From f9f2d9fcf3074123d96750ff1b428d2cf3c09911 Mon Sep 17 00:00:00 2001 From: bergquist Date: Sun, 3 Mar 2019 12:41:38 +0100 Subject: [PATCH 11/28] avoid exporting test helpers --- .../database_storage_integration_test.go | 3 +-- pkg/infra/distcache/distcache_test.go | 20 +++++++------------ pkg/infra/distcache/memcached_storage_test.go | 2 +- pkg/infra/distcache/memory_storage_test.go | 7 +++++++ pkg/infra/distcache/redis_storage_test.go | 2 +- 5 files changed, 17 insertions(+), 17 deletions(-) create mode 100644 pkg/infra/distcache/memory_storage_test.go diff --git a/pkg/infra/distcache/database_storage_integration_test.go b/pkg/infra/distcache/database_storage_integration_test.go index b8f564f9710..fac430e7e8d 100644 --- a/pkg/infra/distcache/database_storage_integration_test.go +++ b/pkg/infra/distcache/database_storage_integration_test.go @@ -3,6 +3,5 @@ package distcache import "testing" func TestIntegrationDatabaseCacheStorage(t *testing.T) { - - RunTestsForClient(t, createTestClient(t, "database")) + runTestsForClient(t, createTestClient(t, "database")) } diff --git a/pkg/infra/distcache/distcache_test.go b/pkg/infra/distcache/distcache_test.go index a40066b788f..33a6d2c9c7b 100644 --- a/pkg/infra/distcache/distcache_test.go +++ b/pkg/infra/distcache/distcache_test.go @@ -26,19 +26,13 @@ func createTestClient(t *testing.T, name string) cacheStorage { return createClient(CacheOpts{name: name}, sqlstore) } -func TestMemoryStorageClient(t *testing.T) { - - client := createTestClient(t, "memory") - RunTestsForClient(t, client) +func runTestsForClient(t *testing.T, client cacheStorage) { + canPutGetAndDeleteCachedObjects(t, client) + canNotFetchExpiredItems(t, client) + canSetInfiniteCacheExpiration(t, client) } -func RunTestsForClient(t *testing.T, client cacheStorage) { - CanPutGetAndDeleteCachedObjects(t, client) - CanNotFetchExpiredItems(t, client) - CanSetInfiniteCacheExpiration(t, client) -} - -func CanPutGetAndDeleteCachedObjects(t *testing.T, client cacheStorage) { +func canPutGetAndDeleteCachedObjects(t *testing.T, client cacheStorage) { cacheableStruct := CacheableStruct{String: "hej", Int64: 2000} err := client.Put("key", cacheableStruct, 0) @@ -58,7 +52,7 @@ func CanPutGetAndDeleteCachedObjects(t *testing.T, client cacheStorage) { assert.Equal(t, err, ErrCacheItemNotFound) } -func CanNotFetchExpiredItems(t *testing.T, client cacheStorage) { +func canNotFetchExpiredItems(t *testing.T, client cacheStorage) { cacheableStruct := CacheableStruct{String: "hej", Int64: 2000} err := client.Put("key", cacheableStruct, time.Second) @@ -72,7 +66,7 @@ func CanNotFetchExpiredItems(t *testing.T, client cacheStorage) { assert.Equal(t, err, ErrCacheItemNotFound) } -func CanSetInfiniteCacheExpiration(t *testing.T, client cacheStorage) { +func canSetInfiniteCacheExpiration(t *testing.T, client cacheStorage) { cacheableStruct := CacheableStruct{String: "hej", Int64: 2000} // insert cache item one day back diff --git a/pkg/infra/distcache/memcached_storage_test.go b/pkg/infra/distcache/memcached_storage_test.go index 524a4fcea10..de784730e4a 100644 --- a/pkg/infra/distcache/memcached_storage_test.go +++ b/pkg/infra/distcache/memcached_storage_test.go @@ -3,5 +3,5 @@ package distcache import "testing" func TestMemcachedCacheStorage(t *testing.T) { - RunTestsForClient(t, createTestClient(t, "memcache")) + runTestsForClient(t, createTestClient(t, "memcache")) } diff --git a/pkg/infra/distcache/memory_storage_test.go b/pkg/infra/distcache/memory_storage_test.go new file mode 100644 index 00000000000..cbf4c3790af --- /dev/null +++ b/pkg/infra/distcache/memory_storage_test.go @@ -0,0 +1,7 @@ +package distcache + +import "testing" + +func TestMemoryCacheStorage(t *testing.T) { + runTestsForClient(t, createTestClient(t, "memory")) +} diff --git a/pkg/infra/distcache/redis_storage_test.go b/pkg/infra/distcache/redis_storage_test.go index 6ba093a205c..b33d2b22e53 100644 --- a/pkg/infra/distcache/redis_storage_test.go +++ b/pkg/infra/distcache/redis_storage_test.go @@ -3,5 +3,5 @@ package distcache import "testing" func TestRedisCacheStorage(t *testing.T) { - RunTestsForClient(t, createTestClient(t, "redis")) + runTestsForClient(t, createTestClient(t, "redis")) } From 196cdf97106f1ed8c3d20d11eca17e2286a6a70a Mon Sep 17 00:00:00 2001 From: bergquist Date: Sun, 3 Mar 2019 21:48:00 +0100 Subject: [PATCH 12/28] adds config to default settings --- conf/defaults.ini | 12 ++++++++ .../database_storage_integration_test.go | 7 ----- pkg/infra/distcache/distcache.go | 27 +++++++---------- pkg/infra/distcache/distcache_test.go | 30 +++++++++++++++++-- pkg/infra/distcache/memcached_storage.go | 5 ++-- pkg/infra/distcache/memcached_storage_test.go | 9 ++++-- pkg/infra/distcache/memory_storage_test.go | 9 ++++-- pkg/infra/distcache/redis_storage.go | 9 ++++-- pkg/infra/distcache/redis_storage_test.go | 10 +++++-- pkg/setting/setting.go | 16 ++++++++++ 10 files changed, 97 insertions(+), 37 deletions(-) delete mode 100644 pkg/infra/distcache/database_storage_integration_test.go diff --git a/conf/defaults.ini b/conf/defaults.ini index df02e01235b..d77f980f806 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -106,6 +106,18 @@ path = grafana.db # For "sqlite3" only. cache mode setting used for connecting to the database cache_mode = private +#################################### Cache server ############################# +[cache_server] +# Either "memory", "redis", "memcache" or "database" default is "database" +type = database + +# cache connectionstring options +# memory: no config required. Should only be used on single install grafana. +# database: will use Grafana primary database. +# redis: config like redis server e.g. `addr=127.0.0.1:6379,pool_size=100,db=grafana` +# memcache: 127.0.0.1:11211 +connstr = + #################################### Session ############################# [session] # Either "memory", "file", "redis", "mysql", "postgres", "memcache", default is "file" diff --git a/pkg/infra/distcache/database_storage_integration_test.go b/pkg/infra/distcache/database_storage_integration_test.go deleted file mode 100644 index fac430e7e8d..00000000000 --- a/pkg/infra/distcache/database_storage_integration_test.go +++ /dev/null @@ -1,7 +0,0 @@ -package distcache - -import "testing" - -func TestIntegrationDatabaseCacheStorage(t *testing.T) { - runTestsForClient(t, createTestClient(t, "database")) -} diff --git a/pkg/infra/distcache/distcache.go b/pkg/infra/distcache/distcache.go index d21ada1e6a3..ee824ae4c52 100644 --- a/pkg/infra/distcache/distcache.go +++ b/pkg/infra/distcache/distcache.go @@ -6,9 +6,10 @@ import ( "errors" "time" + "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/log" "github.com/grafana/grafana/pkg/services/sqlstore" - redis "gopkg.in/redis.v2" "github.com/grafana/grafana/pkg/registry" ) @@ -25,30 +26,21 @@ func init() { func (ds *DistributedCache) Init() error { ds.log = log.New("distributed.cache") - ds.Client = createClient(CacheOpts{}, ds.SQLStore) + ds.Client = createClient(ds.Cfg.CacheOptions, ds.SQLStore) return nil } -type CacheOpts struct { - name string -} - -func createClient(opts CacheOpts, sqlstore *sqlstore.SqlStore) cacheStorage { - if opts.name == "redis" { - opt := &redis.Options{ - Network: "tcp", - Addr: "localhost:6379", - } - - return newRedisStorage(redis.NewClient(opt)) +func createClient(opts *setting.CacheOpts, sqlstore *sqlstore.SqlStore) cacheStorage { + if opts.Name == "redis" { + return newRedisStorage(opts) } - if opts.name == "memcache" { - return newMemcacheStorage("localhost:11211") + if opts.Name == "memcache" { + return newMemcacheStorage(opts) } - if opts.name == "memory" { + if opts.Name == "memory" { return newMemoryStorage() } @@ -60,6 +52,7 @@ type DistributedCache struct { log log.Logger Client cacheStorage SQLStore *sqlstore.SqlStore `inject:""` + Cfg *setting.Cfg `inject:""` } type cachedItem struct { diff --git a/pkg/infra/distcache/distcache_test.go b/pkg/infra/distcache/distcache_test.go index 33a6d2c9c7b..f6ed13d4f06 100644 --- a/pkg/infra/distcache/distcache_test.go +++ b/pkg/infra/distcache/distcache_test.go @@ -8,6 +8,7 @@ import ( "github.com/bmizerany/assert" "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/setting" ) type CacheableStruct struct { @@ -19,11 +20,34 @@ func init() { gob.Register(CacheableStruct{}) } -func createTestClient(t *testing.T, name string) cacheStorage { +func createTestClient(t *testing.T, opts *setting.CacheOpts, sqlstore *sqlstore.SqlStore) cacheStorage { t.Helper() - sqlstore := sqlstore.InitTestDB(t) - return createClient(CacheOpts{name: name}, sqlstore) + dc := &DistributedCache{ + SQLStore: sqlstore, + Cfg: &setting.Cfg{ + CacheOptions: opts, + }, + } + + err := dc.Init() + if err != nil { + t.Fatalf("failed to init client for test. error: %v", err) + } + + return dc.Client +} + +func TestCachedBasedOnConfig(t *testing.T) { + + cfg := setting.NewCfg() + cfg.Load(&setting.CommandLineArgs{ + HomePath: "../../../", + }) + + client := createTestClient(t, cfg.CacheOptions, sqlstore.InitTestDB(t)) + + runTestsForClient(t, client) } func runTestsForClient(t *testing.T, client cacheStorage) { diff --git a/pkg/infra/distcache/memcached_storage.go b/pkg/infra/distcache/memcached_storage.go index 7f97a043628..df1346bf350 100644 --- a/pkg/infra/distcache/memcached_storage.go +++ b/pkg/infra/distcache/memcached_storage.go @@ -4,15 +4,16 @@ import ( "time" "github.com/bradfitz/gomemcache/memcache" + "github.com/grafana/grafana/pkg/setting" ) type memcacheStorage struct { c *memcache.Client } -func newMemcacheStorage(connStr string) *memcacheStorage { +func newMemcacheStorage(opts *setting.CacheOpts) *memcacheStorage { return &memcacheStorage{ - c: memcache.New(connStr), + c: memcache.New(opts.ConnStr), } } diff --git a/pkg/infra/distcache/memcached_storage_test.go b/pkg/infra/distcache/memcached_storage_test.go index de784730e4a..3f885700cb4 100644 --- a/pkg/infra/distcache/memcached_storage_test.go +++ b/pkg/infra/distcache/memcached_storage_test.go @@ -1,7 +1,12 @@ package distcache -import "testing" +import ( + "testing" + + "github.com/grafana/grafana/pkg/setting" +) func TestMemcachedCacheStorage(t *testing.T) { - runTestsForClient(t, createTestClient(t, "memcache")) + opts := &setting.CacheOpts{Name: "memcache", ConnStr: "localhost:11211"} + runTestsForClient(t, createTestClient(t, opts, nil)) } diff --git a/pkg/infra/distcache/memory_storage_test.go b/pkg/infra/distcache/memory_storage_test.go index cbf4c3790af..5318b7c19b8 100644 --- a/pkg/infra/distcache/memory_storage_test.go +++ b/pkg/infra/distcache/memory_storage_test.go @@ -1,7 +1,12 @@ package distcache -import "testing" +import ( + "testing" + + "github.com/grafana/grafana/pkg/setting" +) func TestMemoryCacheStorage(t *testing.T) { - runTestsForClient(t, createTestClient(t, "memory")) + opts := &setting.CacheOpts{Name: "memory"} + runTestsForClient(t, createTestClient(t, opts, nil)) } diff --git a/pkg/infra/distcache/redis_storage.go b/pkg/infra/distcache/redis_storage.go index bb21b26473e..4e6a8b6d325 100644 --- a/pkg/infra/distcache/redis_storage.go +++ b/pkg/infra/distcache/redis_storage.go @@ -3,6 +3,7 @@ package distcache import ( "time" + "github.com/grafana/grafana/pkg/setting" redis "gopkg.in/redis.v2" ) @@ -10,8 +11,12 @@ type redisStorage struct { c *redis.Client } -func newRedisStorage(c *redis.Client) *redisStorage { - return &redisStorage{c: c} +func newRedisStorage(opts *setting.CacheOpts) *redisStorage { + opt := &redis.Options{ + Network: "tcp", + Addr: opts.ConnStr, + } + return &redisStorage{c: redis.NewClient(opt)} } // Set sets value to given key in session. diff --git a/pkg/infra/distcache/redis_storage_test.go b/pkg/infra/distcache/redis_storage_test.go index b33d2b22e53..7c63ce46b38 100644 --- a/pkg/infra/distcache/redis_storage_test.go +++ b/pkg/infra/distcache/redis_storage_test.go @@ -1,7 +1,13 @@ package distcache -import "testing" +import ( + "testing" + + "github.com/grafana/grafana/pkg/setting" +) func TestRedisCacheStorage(t *testing.T) { - runTestsForClient(t, createTestClient(t, "redis")) + + opts := &setting.CacheOpts{Name: "redis", ConnStr: "localhost:6379"} + runTestsForClient(t, createTestClient(t, opts, nil)) } diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index 5d44a3585dc..f25f2211b40 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -240,6 +240,9 @@ type Cfg struct { // User EditorsCanOwn bool + + // DistributedCache + CacheOptions *CacheOpts } type CommandLineArgs struct { @@ -779,9 +782,22 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error { enterprise := iniFile.Section("enterprise") cfg.EnterpriseLicensePath = enterprise.Key("license_path").MustString(filepath.Join(cfg.DataPath, "license.jwt")) + cacheServer := iniFile.Section("cache_server") + //cfg.DistCacheType = cacheServer.Key("type").MustString("database") + //cfg.DistCacheConnStr = cacheServer.Key("connstr").MustString("") + cfg.CacheOptions = &CacheOpts{ + Name: cacheServer.Key("type").MustString("database"), + ConnStr: cacheServer.Key("connstr").MustString(""), + } + return nil } +type CacheOpts struct { + Name string + ConnStr string +} + func (cfg *Cfg) readSessionConfig() { sec := cfg.Raw.Section("session") SessionOptions = session.Options{} From b933b4efc8a9dcd9f73e00d063b806d3d429a640 Mon Sep 17 00:00:00 2001 From: bergquist Date: Sun, 3 Mar 2019 22:04:11 +0100 Subject: [PATCH 13/28] test redis and memcached during integration tests --- ...ed_storage_test.go => memcached_storage_integration_test.go} | 2 ++ ...{redis_storage_test.go => redis_storage_integration_test.go} | 2 ++ 2 files changed, 4 insertions(+) rename pkg/infra/distcache/{memcached_storage_test.go => memcached_storage_integration_test.go} (92%) rename pkg/infra/distcache/{redis_storage_test.go => redis_storage_integration_test.go} (93%) diff --git a/pkg/infra/distcache/memcached_storage_test.go b/pkg/infra/distcache/memcached_storage_integration_test.go similarity index 92% rename from pkg/infra/distcache/memcached_storage_test.go rename to pkg/infra/distcache/memcached_storage_integration_test.go index 3f885700cb4..128abb6923f 100644 --- a/pkg/infra/distcache/memcached_storage_test.go +++ b/pkg/infra/distcache/memcached_storage_integration_test.go @@ -1,3 +1,5 @@ +// +build memcached + package distcache import ( diff --git a/pkg/infra/distcache/redis_storage_test.go b/pkg/infra/distcache/redis_storage_integration_test.go similarity index 93% rename from pkg/infra/distcache/redis_storage_test.go rename to pkg/infra/distcache/redis_storage_integration_test.go index 7c63ce46b38..289a3ff4e2d 100644 --- a/pkg/infra/distcache/redis_storage_test.go +++ b/pkg/infra/distcache/redis_storage_integration_test.go @@ -1,3 +1,5 @@ +// +build redis + package distcache import ( From 995647be2c99224ffa60cb5f572e649b11ad0530 Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 5 Mar 2019 14:22:22 +0100 Subject: [PATCH 14/28] removes memory as distcache option if database caching is to expensive if should not use distcache in the first place. --- pkg/infra/distcache/distcache.go | 4 --- pkg/infra/distcache/memory_storage.go | 35 ---------------------- pkg/infra/distcache/memory_storage_test.go | 12 -------- 3 files changed, 51 deletions(-) delete mode 100644 pkg/infra/distcache/memory_storage.go delete mode 100644 pkg/infra/distcache/memory_storage_test.go diff --git a/pkg/infra/distcache/distcache.go b/pkg/infra/distcache/distcache.go index ee824ae4c52..44ab2e08583 100644 --- a/pkg/infra/distcache/distcache.go +++ b/pkg/infra/distcache/distcache.go @@ -40,10 +40,6 @@ func createClient(opts *setting.CacheOpts, sqlstore *sqlstore.SqlStore) cacheSto return newMemcacheStorage(opts) } - if opts.Name == "memory" { - return newMemoryStorage() - } - return newDatabaseCache(sqlstore) } diff --git a/pkg/infra/distcache/memory_storage.go b/pkg/infra/distcache/memory_storage.go deleted file mode 100644 index a1203cabe75..00000000000 --- a/pkg/infra/distcache/memory_storage.go +++ /dev/null @@ -1,35 +0,0 @@ -package distcache - -import ( - "time" - - gocache "github.com/patrickmn/go-cache" -) - -type memoryStorage struct { - c *gocache.Cache -} - -func newMemoryStorage() *memoryStorage { - return &memoryStorage{ - c: gocache.New(time.Minute*30, time.Minute*30), - } -} - -func (s *memoryStorage) Put(key string, val interface{}, expires time.Duration) error { - return s.c.Add(key, val, expires) -} - -func (s *memoryStorage) Get(key string) (interface{}, error) { - val, exist := s.c.Get(key) - if !exist { - return nil, ErrCacheItemNotFound - } - - return val, nil -} - -func (s *memoryStorage) Delete(key string) error { - s.c.Delete(key) - return nil -} diff --git a/pkg/infra/distcache/memory_storage_test.go b/pkg/infra/distcache/memory_storage_test.go deleted file mode 100644 index 5318b7c19b8..00000000000 --- a/pkg/infra/distcache/memory_storage_test.go +++ /dev/null @@ -1,12 +0,0 @@ -package distcache - -import ( - "testing" - - "github.com/grafana/grafana/pkg/setting" -) - -func TestMemoryCacheStorage(t *testing.T) { - opts := &setting.CacheOpts{Name: "memory"} - runTestsForClient(t, createTestClient(t, opts, nil)) -} From 98f54326595f861867aca27f44c7af997f653b72 Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 5 Mar 2019 14:35:36 +0100 Subject: [PATCH 15/28] `memcache` -> `memcached` https://github.com/memcached/memcached --- conf/defaults.ini | 2 +- pkg/infra/distcache/distcache.go | 4 ++-- pkg/infra/distcache/memcached_storage.go | 12 ++++++------ .../distcache/memcached_storage_integration_test.go | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/conf/defaults.ini b/conf/defaults.ini index d77f980f806..3386e552b8a 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -108,7 +108,7 @@ cache_mode = private #################################### Cache server ############################# [cache_server] -# Either "memory", "redis", "memcache" or "database" default is "database" +# Either "memory", "redis", "memcached" or "database" default is "database" type = database # cache connectionstring options diff --git a/pkg/infra/distcache/distcache.go b/pkg/infra/distcache/distcache.go index 44ab2e08583..c293b62f608 100644 --- a/pkg/infra/distcache/distcache.go +++ b/pkg/infra/distcache/distcache.go @@ -36,8 +36,8 @@ func createClient(opts *setting.CacheOpts, sqlstore *sqlstore.SqlStore) cacheSto return newRedisStorage(opts) } - if opts.Name == "memcache" { - return newMemcacheStorage(opts) + if opts.Name == "memcached" { + return newMemcachedStorage(opts) } return newDatabaseCache(sqlstore) diff --git a/pkg/infra/distcache/memcached_storage.go b/pkg/infra/distcache/memcached_storage.go index df1346bf350..ea326d759b7 100644 --- a/pkg/infra/distcache/memcached_storage.go +++ b/pkg/infra/distcache/memcached_storage.go @@ -7,12 +7,12 @@ import ( "github.com/grafana/grafana/pkg/setting" ) -type memcacheStorage struct { +type memcachedStorage struct { c *memcache.Client } -func newMemcacheStorage(opts *setting.CacheOpts) *memcacheStorage { - return &memcacheStorage{ +func newMemcachedStorage(opts *setting.CacheOpts) *memcachedStorage { + return &memcachedStorage{ c: memcache.New(opts.ConnStr), } } @@ -26,7 +26,7 @@ func newItem(sid string, data []byte, expire int32) *memcache.Item { } // Put sets value to given key in the cache. -func (s *memcacheStorage) Put(key string, val interface{}, expires time.Duration) error { +func (s *memcachedStorage) Put(key string, val interface{}, expires time.Duration) error { item := &cachedItem{Val: val} bytes, err := encodeGob(item) @@ -40,7 +40,7 @@ func (s *memcacheStorage) Put(key string, val interface{}, expires time.Duration } // Get gets value by given key in the cache. -func (s *memcacheStorage) Get(key string) (interface{}, error) { +func (s *memcachedStorage) Get(key string) (interface{}, error) { i, err := s.c.Get(key) if err != nil && err.Error() == "memcache: cache miss" { @@ -62,6 +62,6 @@ func (s *memcacheStorage) Get(key string) (interface{}, error) { } // Delete delete a key from the cache -func (s *memcacheStorage) Delete(key string) error { +func (s *memcachedStorage) Delete(key string) error { return s.c.Delete(key) } diff --git a/pkg/infra/distcache/memcached_storage_integration_test.go b/pkg/infra/distcache/memcached_storage_integration_test.go index 128abb6923f..125bf8d2bf1 100644 --- a/pkg/infra/distcache/memcached_storage_integration_test.go +++ b/pkg/infra/distcache/memcached_storage_integration_test.go @@ -9,6 +9,6 @@ import ( ) func TestMemcachedCacheStorage(t *testing.T) { - opts := &setting.CacheOpts{Name: "memcache", ConnStr: "localhost:11211"} + opts := &setting.CacheOpts{Name: "memcached", ConnStr: "localhost:11211"} runTestsForClient(t, createTestClient(t, opts, nil)) } From 6231095f72b0305a50b8d7e926b17db0df7a69eb Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 5 Mar 2019 14:57:45 +0100 Subject: [PATCH 16/28] reverts package.json I made during the flight >.> --- package.json | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/package.json b/package.json index af270d47ad0..a937ba6f717 100644 --- a/package.json +++ b/package.json @@ -142,6 +142,11 @@ "gui:release": "ts-node --project ./scripts/cli/tsconfig.json ./scripts/cli/index.ts gui:release -p", "cli": "ts-node --project ./scripts/cli/tsconfig.json ./scripts/cli/index.ts" }, + "husky": { + "hooks": { + "pre-commit": "lint-staged && grunt precommit" + } + }, "lint-staged": { "*.{ts,tsx,json,scss}": [ "prettier --write", From 9a78c231653bd3b4fb6b412ffa8d9dc6de06778a Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 5 Mar 2019 15:15:05 +0100 Subject: [PATCH 17/28] rename put -> set --- pkg/infra/distcache/database_storage.go | 2 +- pkg/infra/distcache/database_storage_test.go | 10 +++++----- pkg/infra/distcache/distcache.go | 14 +++++++++----- pkg/infra/distcache/distcache_test.go | 16 ++++++++-------- pkg/infra/distcache/memcached_storage.go | 4 ++-- pkg/infra/distcache/redis_storage.go | 2 +- 6 files changed, 26 insertions(+), 22 deletions(-) diff --git a/pkg/infra/distcache/database_storage.go b/pkg/infra/distcache/database_storage.go index f4365383b82..0cf613471db 100644 --- a/pkg/infra/distcache/database_storage.go +++ b/pkg/infra/distcache/database_storage.go @@ -79,7 +79,7 @@ type cacheData struct { CreatedAt int64 } -func (dc *databaseCache) Put(key string, value interface{}, expire time.Duration) error { +func (dc *databaseCache) Set(key string, value interface{}, expire time.Duration) error { item := &cachedItem{Val: value} data, err := encodeGob(item) if err != nil { diff --git a/pkg/infra/distcache/database_storage_test.go b/pkg/infra/distcache/database_storage_test.go index 931fbc81c7f..24d8cea16bb 100644 --- a/pkg/infra/distcache/database_storage_test.go +++ b/pkg/infra/distcache/database_storage_test.go @@ -22,15 +22,15 @@ func TestDatabaseStorageGarbageCollection(t *testing.T) { //set time.now to 2 weeks ago getTime = func() time.Time { return time.Now().AddDate(0, 0, -2) } - db.Put("key1", obj, 1000*time.Second) - db.Put("key2", obj, 1000*time.Second) - db.Put("key3", obj, 1000*time.Second) + db.Set("key1", obj, 1000*time.Second) + db.Set("key2", obj, 1000*time.Second) + db.Set("key3", obj, 1000*time.Second) // insert object that should never expire - db.Put("key4", obj, 0) + db.Set("key4", obj, 0) getTime = time.Now - db.Put("key5", obj, 1000*time.Second) + db.Set("key5", obj, 1000*time.Second) //run GC db.internalRunGC() diff --git a/pkg/infra/distcache/distcache.go b/pkg/infra/distcache/distcache.go index c293b62f608..549774b848b 100644 --- a/pkg/infra/distcache/distcache.go +++ b/pkg/infra/distcache/distcache.go @@ -31,7 +31,7 @@ func (ds *DistributedCache) Init() error { return nil } -func createClient(opts *setting.CacheOpts, sqlstore *sqlstore.SqlStore) cacheStorage { +func createClient(opts *setting.CacheOpts, sqlstore *sqlstore.SqlStore) CacheStorage { if opts.Name == "redis" { return newRedisStorage(opts) } @@ -46,7 +46,7 @@ func createClient(opts *setting.CacheOpts, sqlstore *sqlstore.SqlStore) cacheSto // DistributedCache allows Grafana to cache data outside its own process type DistributedCache struct { log log.Logger - Client cacheStorage + Client CacheStorage SQLStore *sqlstore.SqlStore `inject:""` Cfg *setting.Cfg `inject:""` } @@ -66,12 +66,16 @@ func decodeGob(data []byte, out *cachedItem) error { return gob.NewDecoder(buf).Decode(&out) } -type cacheStorage interface { +// CacheStorage allows the caller to set, get and delete items in the cache. +// Cached items are stored as byte arrays and marshalled using "encoding/gob" +// so any struct added to the cache needs to be registred with `gob.Register` +// ex `gob.Register(CacheableStruct{})`` +type CacheStorage interface { // Get reads object from Cache Get(key string) (interface{}, error) - // Puts an object into the cache - Put(key string, value interface{}, expire time.Duration) error + // Set sets an object into the cache + Set(key string, value interface{}, expire time.Duration) error // Delete object from cache Delete(key string) error diff --git a/pkg/infra/distcache/distcache_test.go b/pkg/infra/distcache/distcache_test.go index f6ed13d4f06..a4a596fd930 100644 --- a/pkg/infra/distcache/distcache_test.go +++ b/pkg/infra/distcache/distcache_test.go @@ -20,7 +20,7 @@ func init() { gob.Register(CacheableStruct{}) } -func createTestClient(t *testing.T, opts *setting.CacheOpts, sqlstore *sqlstore.SqlStore) cacheStorage { +func createTestClient(t *testing.T, opts *setting.CacheOpts, sqlstore *sqlstore.SqlStore) CacheStorage { t.Helper() dc := &DistributedCache{ @@ -50,16 +50,16 @@ func TestCachedBasedOnConfig(t *testing.T) { runTestsForClient(t, client) } -func runTestsForClient(t *testing.T, client cacheStorage) { +func runTestsForClient(t *testing.T, client CacheStorage) { canPutGetAndDeleteCachedObjects(t, client) canNotFetchExpiredItems(t, client) canSetInfiniteCacheExpiration(t, client) } -func canPutGetAndDeleteCachedObjects(t *testing.T, client cacheStorage) { +func canPutGetAndDeleteCachedObjects(t *testing.T, client CacheStorage) { cacheableStruct := CacheableStruct{String: "hej", Int64: 2000} - err := client.Put("key", cacheableStruct, 0) + err := client.Set("key", cacheableStruct, 0) assert.Equal(t, err, nil) data, err := client.Get("key") @@ -76,10 +76,10 @@ func canPutGetAndDeleteCachedObjects(t *testing.T, client cacheStorage) { assert.Equal(t, err, ErrCacheItemNotFound) } -func canNotFetchExpiredItems(t *testing.T, client cacheStorage) { +func canNotFetchExpiredItems(t *testing.T, client CacheStorage) { cacheableStruct := CacheableStruct{String: "hej", Int64: 2000} - err := client.Put("key", cacheableStruct, time.Second) + err := client.Set("key", cacheableStruct, time.Second) assert.Equal(t, err, nil) //not sure how this can be avoided when testing redis/memcached :/ @@ -90,12 +90,12 @@ func canNotFetchExpiredItems(t *testing.T, client cacheStorage) { assert.Equal(t, err, ErrCacheItemNotFound) } -func canSetInfiniteCacheExpiration(t *testing.T, client cacheStorage) { +func canSetInfiniteCacheExpiration(t *testing.T, client CacheStorage) { cacheableStruct := CacheableStruct{String: "hej", Int64: 2000} // insert cache item one day back getTime = func() time.Time { return time.Now().AddDate(0, 0, -2) } - err := client.Put("key", cacheableStruct, 0) + err := client.Set("key", cacheableStruct, 0) assert.Equal(t, err, nil) // should not be able to read that value since its expired diff --git a/pkg/infra/distcache/memcached_storage.go b/pkg/infra/distcache/memcached_storage.go index ea326d759b7..7a29eec0e5d 100644 --- a/pkg/infra/distcache/memcached_storage.go +++ b/pkg/infra/distcache/memcached_storage.go @@ -25,8 +25,8 @@ func newItem(sid string, data []byte, expire int32) *memcache.Item { } } -// Put sets value to given key in the cache. -func (s *memcachedStorage) Put(key string, val interface{}, expires time.Duration) error { +// Set sets value to given key in the cache. +func (s *memcachedStorage) Set(key string, val interface{}, expires time.Duration) error { item := &cachedItem{Val: val} bytes, err := encodeGob(item) diff --git a/pkg/infra/distcache/redis_storage.go b/pkg/infra/distcache/redis_storage.go index 4e6a8b6d325..1414671f05b 100644 --- a/pkg/infra/distcache/redis_storage.go +++ b/pkg/infra/distcache/redis_storage.go @@ -20,7 +20,7 @@ func newRedisStorage(opts *setting.CacheOpts) *redisStorage { } // Set sets value to given key in session. -func (s *redisStorage) Put(key string, val interface{}, expires time.Duration) error { +func (s *redisStorage) Set(key string, val interface{}, expires time.Duration) error { item := &cachedItem{Val: val} value, err := encodeGob(item) if err != nil { From daa3b17951f3c149ecb8434a61a86b4422749589 Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 5 Mar 2019 15:34:51 +0100 Subject: [PATCH 18/28] code layouts and comments --- conf/defaults.ini | 3 +- pkg/cmd/grafana-server/server.go | 1 + pkg/infra/distcache/database_storage.go | 56 +++++++++++---------- pkg/infra/distcache/distcache.go | 63 ++++++++++++++++-------- pkg/infra/distcache/distcache_test.go | 3 +- pkg/infra/distcache/memcached_storage.go | 11 ++--- pkg/setting/setting.go | 2 - 7 files changed, 79 insertions(+), 60 deletions(-) diff --git a/conf/defaults.ini b/conf/defaults.ini index 3386e552b8a..91a58243c04 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -108,11 +108,10 @@ cache_mode = private #################################### Cache server ############################# [cache_server] -# Either "memory", "redis", "memcached" or "database" default is "database" +# Either "redis", "memcached" or "database" default is "database" type = database # cache connectionstring options -# memory: no config required. Should only be used on single install grafana. # database: will use Grafana primary database. # redis: config like redis server e.g. `addr=127.0.0.1:6379,pool_size=100,db=grafana` # memcache: 127.0.0.1:11211 diff --git a/pkg/cmd/grafana-server/server.go b/pkg/cmd/grafana-server/server.go index 53218147ae0..d2852e0b8ca 100644 --- a/pkg/cmd/grafana-server/server.go +++ b/pkg/cmd/grafana-server/server.go @@ -28,6 +28,7 @@ import ( // self registering services _ "github.com/grafana/grafana/pkg/extensions" + _ "github.com/grafana/grafana/pkg/infra/distcache" _ "github.com/grafana/grafana/pkg/infra/metrics" _ "github.com/grafana/grafana/pkg/infra/serverlock" _ "github.com/grafana/grafana/pkg/infra/tracing" diff --git a/pkg/infra/distcache/database_storage.go b/pkg/infra/distcache/database_storage.go index 0cf613471db..6a357005a21 100644 --- a/pkg/infra/distcache/database_storage.go +++ b/pkg/infra/distcache/database_storage.go @@ -1,6 +1,7 @@ package distcache import ( + "context" "time" "github.com/grafana/grafana/pkg/log" @@ -18,32 +19,33 @@ func newDatabaseCache(sqlstore *sqlstore.SqlStore) *databaseCache { log: log.New("distcache.database"), } - //go dc.StartGC() //TODO: start the GC somehow return dc } +func (dc *databaseCache) Run(ctx context.Context) error { + ticker := time.NewTicker(time.Minute * 10) + for { + select { + case <-ctx.Done(): + return ctx.Err() + case <-ticker.C: + dc.internalRunGC() + } + } +} + var getTime = time.Now func (dc *databaseCache) internalRunGC() { now := getTime().Unix() - sql := `DELETE FROM cache_data WHERE (? - created) >= expire` + sql := `DELETE FROM cache_data WHERE (? - created_at) >= expires AND expires <> 0` - //EXTRACT(EPOCH FROM NOW()) - created >= expire - //UNIX_TIMESTAMP(NOW()) - created >= expire _, err := dc.SQLStore.NewSession().Exec(sql, now) if err != nil { dc.log.Error("failed to run garbage collect", "error", err) } } -func (dc *databaseCache) StartGC() { - dc.internalRunGC() - - time.AfterFunc(time.Second*10, func() { - dc.StartGC() - }) -} - func (dc *databaseCache) Get(key string) (interface{}, error) { cacheHits := []cacheData{} err := dc.SQLStore.NewSession().Where(`key = ?`, key).Find(&cacheHits) @@ -57,8 +59,10 @@ func (dc *databaseCache) Get(key string) (interface{}, error) { } cacheHit = cacheHits[0] + // if Expires is set. Make sure its still valid. if cacheHit.Expires > 0 { - if getTime().Unix()-cacheHit.CreatedAt >= cacheHit.Expires { + existedButExpired := getTime().Unix()-cacheHit.CreatedAt >= cacheHit.Expires + if existedButExpired { dc.Delete(key) return nil, ErrCacheItemNotFound } @@ -72,13 +76,6 @@ func (dc *databaseCache) Get(key string) (interface{}, error) { return item.Val, nil } -type cacheData struct { - Key string - Data []byte - Expires int64 - CreatedAt int64 -} - func (dc *databaseCache) Set(key string, value interface{}, expire time.Duration) error { item := &cachedItem{Val: value} data, err := encodeGob(item) @@ -87,22 +84,23 @@ func (dc *databaseCache) Set(key string, value interface{}, expire time.Duration } now := getTime().Unix() - cacheHits := []cacheData{} err = dc.SQLStore.NewSession().Where(`key = ?`, key).Find(&cacheHits) if err != nil { return err } - var expiresInEpoch int64 + var expiresAtEpoch int64 if expire != 0 { - expiresInEpoch = int64(expire) / int64(time.Second) + expiresAtEpoch = int64(expire) / int64(time.Second) } + session := dc.SQLStore.NewSession() + // insert or update depending on if item already exist if len(cacheHits) > 0 { - _, err = dc.SQLStore.NewSession().Exec("UPDATE cache_data SET data=?, created=?, expire=? WHERE key=?", data, now, expiresInEpoch, key) + _, err = session.Exec("UPDATE cache_data SET data=?, created=?, expire=? WHERE key=?", data, now, expiresAtEpoch, key) } else { - _, err = dc.SQLStore.NewSession().Exec("INSERT INTO cache_data(key,data,created_at,expires) VALUES(?,?,?,?)", key, data, now, expiresInEpoch) + _, err = session.Exec("INSERT INTO cache_data(key,data,created_at,expires) VALUES(?,?,?,?)", key, data, now, expiresAtEpoch) } return err @@ -110,8 +108,14 @@ func (dc *databaseCache) Set(key string, value interface{}, expire time.Duration func (dc *databaseCache) Delete(key string) error { sql := `DELETE FROM cache_data WHERE key = ?` - _, err := dc.SQLStore.NewSession().Exec(sql, key) return err } + +type cacheData struct { + Key string + Data []byte + Expires int64 + CreatedAt int64 +} diff --git a/pkg/infra/distcache/distcache.go b/pkg/infra/distcache/distcache.go index 549774b848b..a8f12adaa27 100644 --- a/pkg/infra/distcache/distcache.go +++ b/pkg/infra/distcache/distcache.go @@ -2,6 +2,7 @@ package distcache import ( "bytes" + "context" "encoding/gob" "errors" "time" @@ -22,6 +23,29 @@ func init() { registry.RegisterService(&DistributedCache{}) } +// CacheStorage allows the caller to set, get and delete items in the cache. +// Cached items are stored as byte arrays and marshalled using "encoding/gob" +// so any struct added to the cache needs to be registred with `distcache.Register` +// ex `distcache.Register(CacheableStruct{})`` +type CacheStorage interface { + // Get reads object from Cache + Get(key string) (interface{}, error) + + // Set sets an object into the cache + Set(key string, value interface{}, expire time.Duration) error + + // Delete object from cache + Delete(key string) error +} + +// DistributedCache allows Grafana to cache data outside its own process +type DistributedCache struct { + log log.Logger + Client CacheStorage + SQLStore *sqlstore.SqlStore `inject:""` + Cfg *setting.Cfg `inject:""` +} + // Init initializes the service func (ds *DistributedCache) Init() error { ds.log = log.New("distributed.cache") @@ -31,6 +55,16 @@ func (ds *DistributedCache) Init() error { return nil } +func (ds *DistributedCache) Run(ctx context.Context) error { + backgroundjob, ok := ds.Client.(registry.BackgroundService) + if ok { + return backgroundjob.Run(ctx) + } + + <-ctx.Done() + return ctx.Err() +} + func createClient(opts *setting.CacheOpts, sqlstore *sqlstore.SqlStore) CacheStorage { if opts.Name == "redis" { return newRedisStorage(opts) @@ -43,12 +77,14 @@ func createClient(opts *setting.CacheOpts, sqlstore *sqlstore.SqlStore) CacheSto return newDatabaseCache(sqlstore) } -// DistributedCache allows Grafana to cache data outside its own process -type DistributedCache struct { - log log.Logger - Client CacheStorage - SQLStore *sqlstore.SqlStore `inject:""` - Cfg *setting.Cfg `inject:""` +// Register records a type, identified by a value for that type, under its +// internal type name. That name will identify the concrete type of a value +// sent or received as an interface variable. Only types that will be +// transferred as implementations of interface values need to be registered. +// Expecting to be used only during initialization, it panics if the mapping +// between types and names is not a bijection. +func Register(value interface{}) { + gob.Register(value) } type cachedItem struct { @@ -65,18 +101,3 @@ func decodeGob(data []byte, out *cachedItem) error { buf := bytes.NewBuffer(data) return gob.NewDecoder(buf).Decode(&out) } - -// CacheStorage allows the caller to set, get and delete items in the cache. -// Cached items are stored as byte arrays and marshalled using "encoding/gob" -// so any struct added to the cache needs to be registred with `gob.Register` -// ex `gob.Register(CacheableStruct{})`` -type CacheStorage interface { - // Get reads object from Cache - Get(key string) (interface{}, error) - - // Set sets an object into the cache - Set(key string, value interface{}, expire time.Duration) error - - // Delete object from cache - Delete(key string) error -} diff --git a/pkg/infra/distcache/distcache_test.go b/pkg/infra/distcache/distcache_test.go index a4a596fd930..b631a6283ac 100644 --- a/pkg/infra/distcache/distcache_test.go +++ b/pkg/infra/distcache/distcache_test.go @@ -1,7 +1,6 @@ package distcache import ( - "encoding/gob" "testing" "time" @@ -17,7 +16,7 @@ type CacheableStruct struct { } func init() { - gob.Register(CacheableStruct{}) + Register(CacheableStruct{}) } func createTestClient(t *testing.T, opts *setting.CacheOpts, sqlstore *sqlstore.SqlStore) CacheStorage { diff --git a/pkg/infra/distcache/memcached_storage.go b/pkg/infra/distcache/memcached_storage.go index 7a29eec0e5d..998d05621c9 100644 --- a/pkg/infra/distcache/memcached_storage.go +++ b/pkg/infra/distcache/memcached_storage.go @@ -28,21 +28,18 @@ func newItem(sid string, data []byte, expire int32) *memcache.Item { // Set sets value to given key in the cache. func (s *memcachedStorage) Set(key string, val interface{}, expires time.Duration) error { item := &cachedItem{Val: val} - bytes, err := encodeGob(item) if err != nil { return err } - memcacheItem := newItem(key, bytes, int32(expires)) - - return s.c.Set(memcacheItem) + memcachedItem := newItem(key, bytes, int32(expires)) + return s.c.Set(memcachedItem) } // Get gets value by given key in the cache. func (s *memcachedStorage) Get(key string) (interface{}, error) { - i, err := s.c.Get(key) - + memcachedItem, err := s.c.Get(key) if err != nil && err.Error() == "memcache: cache miss" { return nil, ErrCacheItemNotFound } @@ -53,7 +50,7 @@ func (s *memcachedStorage) Get(key string) (interface{}, error) { item := &cachedItem{} - err = decodeGob(i.Value, item) + err = decodeGob(memcachedItem.Value, item) if err != nil { return nil, err } diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index f25f2211b40..864c29fb382 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -783,8 +783,6 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error { cfg.EnterpriseLicensePath = enterprise.Key("license_path").MustString(filepath.Join(cfg.DataPath, "license.jwt")) cacheServer := iniFile.Section("cache_server") - //cfg.DistCacheType = cacheServer.Key("type").MustString("database") - //cfg.DistCacheConnStr = cacheServer.Key("connstr").MustString("") cfg.CacheOptions = &CacheOpts{ Name: cacheServer.Key("type").MustString("database"), ConnStr: cacheServer.Key("connstr").MustString(""), From dbc1315d6f69bb6ce154e5b907d1077d5301c7f7 Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 7 Mar 2019 17:16:08 +0100 Subject: [PATCH 19/28] build steps for cache servers --- .circleci/config.yml | 18 ++++++++++++++++++ scripts/circle-test-cache-servers.sh | 17 +++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100755 scripts/circle-test-cache-servers.sh diff --git a/.circleci/config.yml b/.circleci/config.yml index 69cea87dccd..9ec8b9dc05d 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -56,6 +56,23 @@ jobs: name: postgres integration tests command: './scripts/circle-test-postgres.sh' + cache-server-test: + docker: + - image: circleci/golang:1.11.5 + - image: circleci/redis:4-alpine + - image: memcached + working_directory: /go/src/github.com/grafana/grafana + steps: + - checkout + #- run: sudo apt update + #- run: sudo apt install -y postgresql-client + - run: dockerize -wait tcp://127.0.0.1:11211 -timeout 120s + - run: dockerize -wait tcp://127.0.0.1:6739 -timeout 120s + #- run: 'PGPASSWORD=grafanatest psql -p 5432 -h 127.0.0.1 -U grafanatest -d grafanatest -f devenv/docker/blocks/postgres_tests/setup.sql' + - run: + name: cache server tests + command: './scripts/circle-test-cache-servers.sh' + codespell: docker: - image: circleci/python @@ -554,4 +571,5 @@ workflows: - gometalinter - mysql-integration-test - postgres-integration-test + - cache-server-test filters: *filter-not-release-or-master diff --git a/scripts/circle-test-cache-servers.sh b/scripts/circle-test-cache-servers.sh new file mode 100755 index 00000000000..6b29be15f42 --- /dev/null +++ b/scripts/circle-test-cache-servers.sh @@ -0,0 +1,17 @@ +#!/bin/bash +function exit_if_fail { + command=$@ + echo "Executing '$command'" + eval $command + rc=$? + if [ $rc -ne 0 ]; then + echo "'$command' returned $rc." + exit $rc + fi +} + +echo "running redis and memcache tests" +#set -e +#time for d in $(go list ./pkg/...); do +time exit_if_fail go test -tags="redis memcached" ./pkg/infra/distcache/... +#done From 66e71b66dd94d6a6ccafae16f8c0cb8fc1da8603 Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 7 Mar 2019 19:07:11 +0100 Subject: [PATCH 20/28] renames key to cache_key apparently key is a reserved keyword in mysql. and the error messages doesnt mention that. can I please have 6h back? --- .circleci/config.yml | 7 ++-- pkg/infra/distcache/database_storage.go | 37 ++++++++++--------- pkg/infra/distcache/database_storage_test.go | 16 +++++--- pkg/infra/distcache/distcache_test.go | 18 ++++----- .../sqlstore/migrations/cache_data_mig.go | 6 +-- scripts/circle-test-cache-servers.sh | 3 +- 6 files changed, 48 insertions(+), 39 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 9ec8b9dc05d..da0e0665285 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -64,11 +64,8 @@ jobs: working_directory: /go/src/github.com/grafana/grafana steps: - checkout - #- run: sudo apt update - #- run: sudo apt install -y postgresql-client - run: dockerize -wait tcp://127.0.0.1:11211 -timeout 120s - - run: dockerize -wait tcp://127.0.0.1:6739 -timeout 120s - #- run: 'PGPASSWORD=grafanatest psql -p 5432 -h 127.0.0.1 -U grafanatest -d grafanatest -f devenv/docker/blocks/postgres_tests/setup.sql' + - run: dockerize -wait tcp://127.0.0.1:6379 -timeout 120s - run: name: cache server tests command: './scripts/circle-test-cache-servers.sh' @@ -562,6 +559,8 @@ workflows: filters: *filter-not-release-or-master - postgres-integration-test: filters: *filter-not-release-or-master + - cache-server-test: + filters: *filter-not-release-or-master - grafana-docker-pr: requires: - build diff --git a/pkg/infra/distcache/database_storage.go b/pkg/infra/distcache/database_storage.go index 6a357005a21..9883751569f 100644 --- a/pkg/infra/distcache/database_storage.go +++ b/pkg/infra/distcache/database_storage.go @@ -8,6 +8,8 @@ import ( "github.com/grafana/grafana/pkg/services/sqlstore" ) +var getTime = time.Now + type databaseCache struct { SQLStore *sqlstore.SqlStore log log.Logger @@ -34,8 +36,6 @@ func (dc *databaseCache) Run(ctx context.Context) error { } } -var getTime = time.Now - func (dc *databaseCache) internalRunGC() { now := getTime().Unix() sql := `DELETE FROM cache_data WHERE (? - created_at) >= expires AND expires <> 0` @@ -47,19 +47,20 @@ func (dc *databaseCache) internalRunGC() { } func (dc *databaseCache) Get(key string) (interface{}, error) { - cacheHits := []cacheData{} - err := dc.SQLStore.NewSession().Where(`key = ?`, key).Find(&cacheHits) + cacheHits := []CacheData{} + sess := dc.SQLStore.NewSession() + defer sess.Close() + err := sess.Where("cache_key= ?", key).Find(&cacheHits) + if err != nil { return nil, err } - var cacheHit cacheData if len(cacheHits) == 0 { return nil, ErrCacheItemNotFound } - cacheHit = cacheHits[0] - // if Expires is set. Make sure its still valid. + cacheHit := cacheHits[0] if cacheHit.Expires > 0 { existedButExpired := getTime().Unix()-cacheHit.CreatedAt >= cacheHit.Expires if existedButExpired { @@ -83,9 +84,10 @@ func (dc *databaseCache) Set(key string, value interface{}, expire time.Duration return err } - now := getTime().Unix() - cacheHits := []cacheData{} - err = dc.SQLStore.NewSession().Where(`key = ?`, key).Find(&cacheHits) + session := dc.SQLStore.NewSession() + + var cacheHit CacheData + has, err := session.Where("cache_key = ?", key).Get(&cacheHit) if err != nil { return err } @@ -95,27 +97,28 @@ func (dc *databaseCache) Set(key string, value interface{}, expire time.Duration expiresAtEpoch = int64(expire) / int64(time.Second) } - session := dc.SQLStore.NewSession() // insert or update depending on if item already exist - if len(cacheHits) > 0 { - _, err = session.Exec("UPDATE cache_data SET data=?, created=?, expire=? WHERE key=?", data, now, expiresAtEpoch, key) + if has { + _, err = session.Exec(`UPDATE cache_data SET data=?, created=?, expire=? WHERE cache_key='?'`, data, getTime().Unix(), expiresAtEpoch, key) } else { - _, err = session.Exec("INSERT INTO cache_data(key,data,created_at,expires) VALUES(?,?,?,?)", key, data, now, expiresAtEpoch) + _, err = session.Exec(`INSERT INTO cache_data (cache_key,data,created_at,expires) VALUES(?,?,?,?)`, key, data, getTime().Unix(), expiresAtEpoch) } return err } func (dc *databaseCache) Delete(key string) error { - sql := `DELETE FROM cache_data WHERE key = ?` + sql := "DELETE FROM cache_data WHERE cache_key=?" _, err := dc.SQLStore.NewSession().Exec(sql, key) return err } -type cacheData struct { - Key string +type CacheData struct { + CacheKey string Data []byte Expires int64 CreatedAt int64 } + +// func (cd CacheData) TableName() string { return "cache_data" } diff --git a/pkg/infra/distcache/database_storage_test.go b/pkg/infra/distcache/database_storage_test.go index 24d8cea16bb..fc526996c89 100644 --- a/pkg/infra/distcache/database_storage_test.go +++ b/pkg/infra/distcache/database_storage_test.go @@ -21,10 +21,16 @@ func TestDatabaseStorageGarbageCollection(t *testing.T) { obj := &CacheableStruct{String: "foolbar"} //set time.now to 2 weeks ago + var err error getTime = func() time.Time { return time.Now().AddDate(0, 0, -2) } - db.Set("key1", obj, 1000*time.Second) - db.Set("key2", obj, 1000*time.Second) - db.Set("key3", obj, 1000*time.Second) + err = db.Set("key1", obj, 1000*time.Second) + assert.Equal(t, err, nil) + + err = db.Set("key2", obj, 1000*time.Second) + assert.Equal(t, err, nil) + + err = db.Set("key3", obj, 1000*time.Second) + assert.Equal(t, err, nil) // insert object that should never expire db.Set("key4", obj, 0) @@ -36,8 +42,8 @@ func TestDatabaseStorageGarbageCollection(t *testing.T) { db.internalRunGC() //try to read values - _, err := db.Get("key1") - assert.Equal(t, err, ErrCacheItemNotFound) + _, err = db.Get("key1") + assert.Equal(t, err, ErrCacheItemNotFound, "expected cache item not found. got: ", err) _, err = db.Get("key2") assert.Equal(t, err, ErrCacheItemNotFound) _, err = db.Get("key3") diff --git a/pkg/infra/distcache/distcache_test.go b/pkg/infra/distcache/distcache_test.go index b631a6283ac..62b07027a05 100644 --- a/pkg/infra/distcache/distcache_test.go +++ b/pkg/infra/distcache/distcache_test.go @@ -58,34 +58,34 @@ func runTestsForClient(t *testing.T, client CacheStorage) { func canPutGetAndDeleteCachedObjects(t *testing.T, client CacheStorage) { cacheableStruct := CacheableStruct{String: "hej", Int64: 2000} - err := client.Set("key", cacheableStruct, 0) - assert.Equal(t, err, nil) + err := client.Set("key1", cacheableStruct, 0) + assert.Equal(t, err, nil, "expected nil. got: ", err) - data, err := client.Get("key") + data, err := client.Get("key1") s, ok := data.(CacheableStruct) assert.Equal(t, ok, true) assert.Equal(t, s.String, "hej") assert.Equal(t, s.Int64, int64(2000)) - err = client.Delete("key") + err = client.Delete("key1") assert.Equal(t, err, nil) - _, err = client.Get("key") + _, err = client.Get("key1") assert.Equal(t, err, ErrCacheItemNotFound) } func canNotFetchExpiredItems(t *testing.T, client CacheStorage) { cacheableStruct := CacheableStruct{String: "hej", Int64: 2000} - err := client.Set("key", cacheableStruct, time.Second) + err := client.Set("key1", cacheableStruct, time.Second) assert.Equal(t, err, nil) //not sure how this can be avoided when testing redis/memcached :/ <-time.After(time.Second + time.Millisecond) // should not be able to read that value since its expired - _, err = client.Get("key") + _, err = client.Get("key1") assert.Equal(t, err, ErrCacheItemNotFound) } @@ -94,12 +94,12 @@ func canSetInfiniteCacheExpiration(t *testing.T, client CacheStorage) { // insert cache item one day back getTime = func() time.Time { return time.Now().AddDate(0, 0, -2) } - err := client.Set("key", cacheableStruct, 0) + err := client.Set("key1", cacheableStruct, 0) assert.Equal(t, err, nil) // should not be able to read that value since its expired getTime = time.Now - data, err := client.Get("key") + data, err := client.Get("key1") s, ok := data.(CacheableStruct) assert.Equal(t, ok, true) diff --git a/pkg/services/sqlstore/migrations/cache_data_mig.go b/pkg/services/sqlstore/migrations/cache_data_mig.go index f12f7f797c8..3467b88962b 100644 --- a/pkg/services/sqlstore/migrations/cache_data_mig.go +++ b/pkg/services/sqlstore/migrations/cache_data_mig.go @@ -6,17 +6,17 @@ func addCacheMigration(mg *migrator.Migrator) { var cacheDataV1 = migrator.Table{ Name: "cache_data", Columns: []*migrator.Column{ - {Name: "key", Type: migrator.DB_NVarchar, IsPrimaryKey: true, Length: 168}, + {Name: "cache_key", Type: migrator.DB_NVarchar, IsPrimaryKey: true, Length: 168}, {Name: "data", Type: migrator.DB_Blob}, {Name: "expires", Type: migrator.DB_Integer, Length: 255, Nullable: false}, {Name: "created_at", Type: migrator.DB_Integer, Length: 255, Nullable: false}, }, Indices: []*migrator.Index{ - {Cols: []string{"key"}, Type: migrator.UniqueIndex}, + {Cols: []string{"cache_key"}, Type: migrator.UniqueIndex}, }, } mg.AddMigration("create cache_data table", migrator.NewAddTableMigration(cacheDataV1)) - mg.AddMigration("add unique index cache_data.key", migrator.NewAddIndexMigration(cacheDataV1, cacheDataV1.Indices[0])) + mg.AddMigration("add unique index cache_data.cache_key", migrator.NewAddIndexMigration(cacheDataV1, cacheDataV1.Indices[0])) } diff --git a/scripts/circle-test-cache-servers.sh b/scripts/circle-test-cache-servers.sh index 6b29be15f42..a75b7235763 100755 --- a/scripts/circle-test-cache-servers.sh +++ b/scripts/circle-test-cache-servers.sh @@ -13,5 +13,6 @@ function exit_if_fail { echo "running redis and memcache tests" #set -e #time for d in $(go list ./pkg/...); do -time exit_if_fail go test -tags="redis memcached" ./pkg/infra/distcache/... +time exit_if_fail go test -tags=redis ./pkg/infra/distcache/... +time exit_if_fail go test -tags=memcached ./pkg/infra/distcache/... #done From 7e7427637cf67e385934a3cc11f04aa641179139 Mon Sep 17 00:00:00 2001 From: bergquist Date: Fri, 8 Mar 2019 20:49:16 +0100 Subject: [PATCH 21/28] renames distcache -> remotecache --- conf/defaults.ini | 2 +- conf/sample.ini | 11 ++++++++++ pkg/cmd/grafana-server/server.go | 2 +- .../database_storage.go | 2 +- .../database_storage_test.go | 2 +- .../memcached_storage.go | 4 ++-- .../memcached_storage_integration_test.go | 4 ++-- .../redis_storage.go | 4 ++-- .../redis_storage_integration_test.go | 4 ++-- .../remotecache.go} | 20 ++++++++++--------- .../remotecache_test.go} | 10 +++++----- pkg/setting/setting.go | 8 ++++---- scripts/circle-test-cache-servers.sh | 4 ++-- 13 files changed, 45 insertions(+), 32 deletions(-) rename pkg/infra/{distcache => remotecache}/database_storage.go (99%) rename pkg/infra/{distcache => remotecache}/database_storage_test.go (98%) rename pkg/infra/{distcache => remotecache}/memcached_storage.go (92%) rename pkg/infra/{distcache => remotecache}/memcached_storage_integration_test.go (64%) rename pkg/infra/{distcache => remotecache}/redis_storage.go (92%) rename pkg/infra/{distcache => remotecache}/redis_storage_integration_test.go (65%) rename pkg/infra/{distcache/distcache.go => remotecache/remotecache.go} (79%) rename pkg/infra/{distcache/distcache_test.go => remotecache/remotecache_test.go} (90%) diff --git a/conf/defaults.ini b/conf/defaults.ini index 91a58243c04..74bb8b057ad 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -107,7 +107,7 @@ path = grafana.db cache_mode = private #################################### Cache server ############################# -[cache_server] +[remote_cache] # Either "redis", "memcached" or "database" default is "database" type = database diff --git a/conf/sample.ini b/conf/sample.ini index 57ff82181de..860efab0140 100644 --- a/conf/sample.ini +++ b/conf/sample.ini @@ -102,6 +102,17 @@ log_queries = # For "sqlite3" only. cache mode setting used for connecting to the database. (private, shared) ;cache_mode = private +#################################### Cache server ############################# +[remote_cache] +# Either "redis", "memcached" or "database" default is "database" +;type = database + +# cache connectionstring options +# database: will use Grafana primary database. +# redis: config like redis server e.g. `addr=127.0.0.1:6379,pool_size=100,db=grafana` +# memcache: 127.0.0.1:11211 +;connstr = + #################################### Session #################################### [session] # Either "memory", "file", "redis", "mysql", "postgres", default is "file" diff --git a/pkg/cmd/grafana-server/server.go b/pkg/cmd/grafana-server/server.go index d2852e0b8ca..c10212329cf 100644 --- a/pkg/cmd/grafana-server/server.go +++ b/pkg/cmd/grafana-server/server.go @@ -28,8 +28,8 @@ import ( // self registering services _ "github.com/grafana/grafana/pkg/extensions" - _ "github.com/grafana/grafana/pkg/infra/distcache" _ "github.com/grafana/grafana/pkg/infra/metrics" + _ "github.com/grafana/grafana/pkg/infra/remotecache" _ "github.com/grafana/grafana/pkg/infra/serverlock" _ "github.com/grafana/grafana/pkg/infra/tracing" _ "github.com/grafana/grafana/pkg/infra/usagestats" diff --git a/pkg/infra/distcache/database_storage.go b/pkg/infra/remotecache/database_storage.go similarity index 99% rename from pkg/infra/distcache/database_storage.go rename to pkg/infra/remotecache/database_storage.go index 9883751569f..cb6c95ce157 100644 --- a/pkg/infra/distcache/database_storage.go +++ b/pkg/infra/remotecache/database_storage.go @@ -1,4 +1,4 @@ -package distcache +package remotecache import ( "context" diff --git a/pkg/infra/distcache/database_storage_test.go b/pkg/infra/remotecache/database_storage_test.go similarity index 98% rename from pkg/infra/distcache/database_storage_test.go rename to pkg/infra/remotecache/database_storage_test.go index fc526996c89..7fde3d325e5 100644 --- a/pkg/infra/distcache/database_storage_test.go +++ b/pkg/infra/remotecache/database_storage_test.go @@ -1,4 +1,4 @@ -package distcache +package remotecache import ( "testing" diff --git a/pkg/infra/distcache/memcached_storage.go b/pkg/infra/remotecache/memcached_storage.go similarity index 92% rename from pkg/infra/distcache/memcached_storage.go rename to pkg/infra/remotecache/memcached_storage.go index 998d05621c9..7356849c1ef 100644 --- a/pkg/infra/distcache/memcached_storage.go +++ b/pkg/infra/remotecache/memcached_storage.go @@ -1,4 +1,4 @@ -package distcache +package remotecache import ( "time" @@ -11,7 +11,7 @@ type memcachedStorage struct { c *memcache.Client } -func newMemcachedStorage(opts *setting.CacheOpts) *memcachedStorage { +func newMemcachedStorage(opts *setting.RemoteCacheOptions) *memcachedStorage { return &memcachedStorage{ c: memcache.New(opts.ConnStr), } diff --git a/pkg/infra/distcache/memcached_storage_integration_test.go b/pkg/infra/remotecache/memcached_storage_integration_test.go similarity index 64% rename from pkg/infra/distcache/memcached_storage_integration_test.go rename to pkg/infra/remotecache/memcached_storage_integration_test.go index 125bf8d2bf1..d55d78ff482 100644 --- a/pkg/infra/distcache/memcached_storage_integration_test.go +++ b/pkg/infra/remotecache/memcached_storage_integration_test.go @@ -1,6 +1,6 @@ // +build memcached -package distcache +package remotecache import ( "testing" @@ -9,6 +9,6 @@ import ( ) func TestMemcachedCacheStorage(t *testing.T) { - opts := &setting.CacheOpts{Name: "memcached", ConnStr: "localhost:11211"} + opts := &setting.RemoteCacheOptions{Name: "memcached", ConnStr: "localhost:11211"} runTestsForClient(t, createTestClient(t, opts, nil)) } diff --git a/pkg/infra/distcache/redis_storage.go b/pkg/infra/remotecache/redis_storage.go similarity index 92% rename from pkg/infra/distcache/redis_storage.go rename to pkg/infra/remotecache/redis_storage.go index 1414671f05b..9d54020fe79 100644 --- a/pkg/infra/distcache/redis_storage.go +++ b/pkg/infra/remotecache/redis_storage.go @@ -1,4 +1,4 @@ -package distcache +package remotecache import ( "time" @@ -11,7 +11,7 @@ type redisStorage struct { c *redis.Client } -func newRedisStorage(opts *setting.CacheOpts) *redisStorage { +func newRedisStorage(opts *setting.RemoteCacheOptions) *redisStorage { opt := &redis.Options{ Network: "tcp", Addr: opts.ConnStr, diff --git a/pkg/infra/distcache/redis_storage_integration_test.go b/pkg/infra/remotecache/redis_storage_integration_test.go similarity index 65% rename from pkg/infra/distcache/redis_storage_integration_test.go rename to pkg/infra/remotecache/redis_storage_integration_test.go index 289a3ff4e2d..bd834fb89ff 100644 --- a/pkg/infra/distcache/redis_storage_integration_test.go +++ b/pkg/infra/remotecache/redis_storage_integration_test.go @@ -1,6 +1,6 @@ // +build redis -package distcache +package remotecache import ( "testing" @@ -10,6 +10,6 @@ import ( func TestRedisCacheStorage(t *testing.T) { - opts := &setting.CacheOpts{Name: "redis", ConnStr: "localhost:6379"} + opts := &setting.RemoteCacheOptions{Name: "redis", ConnStr: "localhost:6379"} runTestsForClient(t, createTestClient(t, opts, nil)) } diff --git a/pkg/infra/distcache/distcache.go b/pkg/infra/remotecache/remotecache.go similarity index 79% rename from pkg/infra/distcache/distcache.go rename to pkg/infra/remotecache/remotecache.go index a8f12adaa27..761a2b3d337 100644 --- a/pkg/infra/distcache/distcache.go +++ b/pkg/infra/remotecache/remotecache.go @@ -1,4 +1,4 @@ -package distcache +package remotecache import ( "bytes" @@ -20,7 +20,7 @@ var ( ) func init() { - registry.RegisterService(&DistributedCache{}) + registry.RegisterService(&RemoteCache{}) } // CacheStorage allows the caller to set, get and delete items in the cache. @@ -38,8 +38,8 @@ type CacheStorage interface { Delete(key string) error } -// DistributedCache allows Grafana to cache data outside its own process -type DistributedCache struct { +// RemoteCache allows Grafana to cache data outside its own process +type RemoteCache struct { log log.Logger Client CacheStorage SQLStore *sqlstore.SqlStore `inject:""` @@ -47,15 +47,17 @@ type DistributedCache struct { } // Init initializes the service -func (ds *DistributedCache) Init() error { - ds.log = log.New("distributed.cache") +func (ds *RemoteCache) Init() error { + ds.log = log.New("cache.remote") - ds.Client = createClient(ds.Cfg.CacheOptions, ds.SQLStore) + ds.Client = createClient(ds.Cfg.RemoteCacheOptions, ds.SQLStore) return nil } -func (ds *DistributedCache) Run(ctx context.Context) error { +// Run start the backend processes for cache clients +func (ds *RemoteCache) Run(ctx context.Context) error { + //create new interface if more clients need GC jobs backgroundjob, ok := ds.Client.(registry.BackgroundService) if ok { return backgroundjob.Run(ctx) @@ -65,7 +67,7 @@ func (ds *DistributedCache) Run(ctx context.Context) error { return ctx.Err() } -func createClient(opts *setting.CacheOpts, sqlstore *sqlstore.SqlStore) CacheStorage { +func createClient(opts *setting.RemoteCacheOptions, sqlstore *sqlstore.SqlStore) CacheStorage { if opts.Name == "redis" { return newRedisStorage(opts) } diff --git a/pkg/infra/distcache/distcache_test.go b/pkg/infra/remotecache/remotecache_test.go similarity index 90% rename from pkg/infra/distcache/distcache_test.go rename to pkg/infra/remotecache/remotecache_test.go index 62b07027a05..8887686c3a1 100644 --- a/pkg/infra/distcache/distcache_test.go +++ b/pkg/infra/remotecache/remotecache_test.go @@ -1,4 +1,4 @@ -package distcache +package remotecache import ( "testing" @@ -19,13 +19,13 @@ func init() { Register(CacheableStruct{}) } -func createTestClient(t *testing.T, opts *setting.CacheOpts, sqlstore *sqlstore.SqlStore) CacheStorage { +func createTestClient(t *testing.T, opts *setting.RemoteCacheOptions, sqlstore *sqlstore.SqlStore) CacheStorage { t.Helper() - dc := &DistributedCache{ + dc := &RemoteCache{ SQLStore: sqlstore, Cfg: &setting.Cfg{ - CacheOptions: opts, + RemoteCacheOptions: opts, }, } @@ -44,7 +44,7 @@ func TestCachedBasedOnConfig(t *testing.T) { HomePath: "../../../", }) - client := createTestClient(t, cfg.CacheOptions, sqlstore.InitTestDB(t)) + client := createTestClient(t, cfg.RemoteCacheOptions, sqlstore.InitTestDB(t)) runTestsForClient(t, client) } diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index 864c29fb382..9d135ca3aae 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -242,7 +242,7 @@ type Cfg struct { EditorsCanOwn bool // DistributedCache - CacheOptions *CacheOpts + RemoteCacheOptions *RemoteCacheOptions } type CommandLineArgs struct { @@ -782,8 +782,8 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error { enterprise := iniFile.Section("enterprise") cfg.EnterpriseLicensePath = enterprise.Key("license_path").MustString(filepath.Join(cfg.DataPath, "license.jwt")) - cacheServer := iniFile.Section("cache_server") - cfg.CacheOptions = &CacheOpts{ + cacheServer := iniFile.Section("remote_cache") + cfg.RemoteCacheOptions = &RemoteCacheOptions{ Name: cacheServer.Key("type").MustString("database"), ConnStr: cacheServer.Key("connstr").MustString(""), } @@ -791,7 +791,7 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error { return nil } -type CacheOpts struct { +type RemoteCacheOptions struct { Name string ConnStr string } diff --git a/scripts/circle-test-cache-servers.sh b/scripts/circle-test-cache-servers.sh index a75b7235763..3ec5dbf1069 100755 --- a/scripts/circle-test-cache-servers.sh +++ b/scripts/circle-test-cache-servers.sh @@ -13,6 +13,6 @@ function exit_if_fail { echo "running redis and memcache tests" #set -e #time for d in $(go list ./pkg/...); do -time exit_if_fail go test -tags=redis ./pkg/infra/distcache/... -time exit_if_fail go test -tags=memcached ./pkg/infra/distcache/... +time exit_if_fail go test -tags=redis ./pkg/infra/remotecache/... +time exit_if_fail go test -tags=memcached ./pkg/infra/remotecache/... #done From 085b63109945b5ae43d71f9ee194e1c3f4285f99 Mon Sep 17 00:00:00 2001 From: bergquist Date: Mon, 11 Mar 2019 09:20:30 +0100 Subject: [PATCH 22/28] add docs about remote cache settings --- docs/sources/installation/configuration.md | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/docs/sources/installation/configuration.md b/docs/sources/installation/configuration.md index f0418ad31a6..9705dd2001c 100644 --- a/docs/sources/installation/configuration.md +++ b/docs/sources/installation/configuration.md @@ -162,9 +162,9 @@ executed with working directory set to the installation path. ### enable_gzip -Set this option to `true` to enable HTTP compression, this can improve -transfer speed and bandwidth utilization. It is recommended that most -users set it to `true`. By default it is set to `false` for compatibility +Set this option to `true` to enable HTTP compression, this can improve +transfer speed and bandwidth utilization. It is recommended that most +users set it to `true`. By default it is set to `false` for compatibility reasons. ### cert_file @@ -179,7 +179,6 @@ Path to the certificate key file (if `protocol` is set to `https`). Set to true for Grafana to log all HTTP requests (not just errors). These are logged as Info level events to grafana log. -
@@ -262,6 +261,19 @@ Set to `true` to log the sql calls and execution times. For "sqlite3" only. [Shared cache](https://www.sqlite.org/sharedcache.html) setting used for connecting to the database. (private, shared) Defaults to private. +
+ +## [remote_cache] + +### type + +Either `redis`, `memcached` or `database` default is `database` + +### connstr + +The remote cache connection string. Leave empty when using `database` since it will use the primary database. +Redis example config: `addr=127.0.0.1:6379,pool_size=100,db=grafana` +Memcache example: `127.0.0.1:11211`
From b2967fbb3747a32a4548eebc4a6fba580f2fa7d3 Mon Sep 17 00:00:00 2001 From: bergquist Date: Mon, 11 Mar 2019 10:44:16 +0100 Subject: [PATCH 23/28] avoid exposing cache client directly --- pkg/infra/remotecache/remotecache.go | 20 ++++++++++++++++---- pkg/infra/remotecache/remotecache_test.go | 2 +- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/pkg/infra/remotecache/remotecache.go b/pkg/infra/remotecache/remotecache.go index 761a2b3d337..1b9d67b9358 100644 --- a/pkg/infra/remotecache/remotecache.go +++ b/pkg/infra/remotecache/remotecache.go @@ -31,7 +31,7 @@ type CacheStorage interface { // Get reads object from Cache Get(key string) (interface{}, error) - // Set sets an object into the cache + // Set sets an object into the cache. if `expire` is set to zero it never expires. Set(key string, value interface{}, expire time.Duration) error // Delete object from cache @@ -41,16 +41,28 @@ type CacheStorage interface { // RemoteCache allows Grafana to cache data outside its own process type RemoteCache struct { log log.Logger - Client CacheStorage + client CacheStorage SQLStore *sqlstore.SqlStore `inject:""` Cfg *setting.Cfg `inject:""` } +func (ds *RemoteCache) Get(key string) (interface{}, error) { + return ds.client.Get(key) +} + +func (ds *RemoteCache) Set(key string, value interface{}, expire time.Duration) error { + return ds.client.Set(key, value, expire) +} + +func (ds *RemoteCache) Delete(key string) error { + return ds.client.Delete(key) +} + // Init initializes the service func (ds *RemoteCache) Init() error { ds.log = log.New("cache.remote") - ds.Client = createClient(ds.Cfg.RemoteCacheOptions, ds.SQLStore) + ds.client = createClient(ds.Cfg.RemoteCacheOptions, ds.SQLStore) return nil } @@ -58,7 +70,7 @@ func (ds *RemoteCache) Init() error { // Run start the backend processes for cache clients func (ds *RemoteCache) Run(ctx context.Context) error { //create new interface if more clients need GC jobs - backgroundjob, ok := ds.Client.(registry.BackgroundService) + backgroundjob, ok := ds.client.(registry.BackgroundService) if ok { return backgroundjob.Run(ctx) } diff --git a/pkg/infra/remotecache/remotecache_test.go b/pkg/infra/remotecache/remotecache_test.go index 8887686c3a1..ac22607ee70 100644 --- a/pkg/infra/remotecache/remotecache_test.go +++ b/pkg/infra/remotecache/remotecache_test.go @@ -34,7 +34,7 @@ func createTestClient(t *testing.T, opts *setting.RemoteCacheOptions, sqlstore * t.Fatalf("failed to init client for test. error: %v", err) } - return dc.Client + return dc.client } func TestCachedBasedOnConfig(t *testing.T) { From 7aeab0a235a515accca2cb7eaae6061cba97a51c Mon Sep 17 00:00:00 2001 From: bergquist Date: Mon, 11 Mar 2019 10:59:55 +0100 Subject: [PATCH 24/28] use `Get` instead of `Find` --- pkg/infra/remotecache/database_storage.go | 22 +++++++++++----------- scripts/circle-test-cache-servers.sh | 4 +--- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/pkg/infra/remotecache/database_storage.go b/pkg/infra/remotecache/database_storage.go index cb6c95ce157..2e34ecd1c73 100644 --- a/pkg/infra/remotecache/database_storage.go +++ b/pkg/infra/remotecache/database_storage.go @@ -47,24 +47,24 @@ func (dc *databaseCache) internalRunGC() { } func (dc *databaseCache) Get(key string) (interface{}, error) { - cacheHits := []CacheData{} - sess := dc.SQLStore.NewSession() - defer sess.Close() - err := sess.Where("cache_key= ?", key).Find(&cacheHits) + cacheHit := CacheData{} + session := dc.SQLStore.NewSession() + defer session.Close() + + exist, err := session.Where("cache_key= ?", key).Get(&cacheHit) if err != nil { return nil, err } - if len(cacheHits) == 0 { + if !exist { return nil, ErrCacheItemNotFound } - cacheHit := cacheHits[0] if cacheHit.Expires > 0 { existedButExpired := getTime().Unix()-cacheHit.CreatedAt >= cacheHit.Expires if existedButExpired { - dc.Delete(key) + _ = dc.Delete(key) //ignore this error since we will return `ErrCacheItemNotFound` anyway return nil, ErrCacheItemNotFound } } @@ -99,9 +99,11 @@ func (dc *databaseCache) Set(key string, value interface{}, expire time.Duration // insert or update depending on if item already exist if has { - _, err = session.Exec(`UPDATE cache_data SET data=?, created=?, expire=? WHERE cache_key='?'`, data, getTime().Unix(), expiresAtEpoch, key) + sql := `UPDATE cache_data SET data=?, created=?, expire=? WHERE cache_key='?'` + _, err = session.Exec(sql, data, getTime().Unix(), expiresAtEpoch, key) } else { - _, err = session.Exec(`INSERT INTO cache_data (cache_key,data,created_at,expires) VALUES(?,?,?,?)`, key, data, getTime().Unix(), expiresAtEpoch) + sql := `INSERT INTO cache_data (cache_key,data,created_at,expires) VALUES(?,?,?,?)` + _, err = session.Exec(sql, key, data, getTime().Unix(), expiresAtEpoch) } return err @@ -120,5 +122,3 @@ type CacheData struct { Expires int64 CreatedAt int64 } - -// func (cd CacheData) TableName() string { return "cache_data" } diff --git a/scripts/circle-test-cache-servers.sh b/scripts/circle-test-cache-servers.sh index 3ec5dbf1069..bacd9928362 100755 --- a/scripts/circle-test-cache-servers.sh +++ b/scripts/circle-test-cache-servers.sh @@ -11,8 +11,6 @@ function exit_if_fail { } echo "running redis and memcache tests" -#set -e -#time for d in $(go list ./pkg/...); do + time exit_if_fail go test -tags=redis ./pkg/infra/remotecache/... time exit_if_fail go test -tags=memcached ./pkg/infra/remotecache/... -#done From 5186273731bd32a05da6844785b3af7b44a13889 Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 14 Mar 2019 08:57:38 +0100 Subject: [PATCH 25/28] return error if cache type is invalid --- .../memcached_storage_integration_test.go | 3 ++- .../redis_storage_integration_test.go | 3 ++- pkg/infra/remotecache/remotecache.go | 23 ++++++++++++------- pkg/infra/remotecache/remotecache_test.go | 6 ++++- 4 files changed, 24 insertions(+), 11 deletions(-) diff --git a/pkg/infra/remotecache/memcached_storage_integration_test.go b/pkg/infra/remotecache/memcached_storage_integration_test.go index d55d78ff482..de7692e25d5 100644 --- a/pkg/infra/remotecache/memcached_storage_integration_test.go +++ b/pkg/infra/remotecache/memcached_storage_integration_test.go @@ -10,5 +10,6 @@ import ( func TestMemcachedCacheStorage(t *testing.T) { opts := &setting.RemoteCacheOptions{Name: "memcached", ConnStr: "localhost:11211"} - runTestsForClient(t, createTestClient(t, opts, nil)) + client := createTestClient(t, opts, nil) + runTestsForClient(t, client) } diff --git a/pkg/infra/remotecache/redis_storage_integration_test.go b/pkg/infra/remotecache/redis_storage_integration_test.go index bd834fb89ff..0a63fbe31ec 100644 --- a/pkg/infra/remotecache/redis_storage_integration_test.go +++ b/pkg/infra/remotecache/redis_storage_integration_test.go @@ -11,5 +11,6 @@ import ( func TestRedisCacheStorage(t *testing.T) { opts := &setting.RemoteCacheOptions{Name: "redis", ConnStr: "localhost:6379"} - runTestsForClient(t, createTestClient(t, opts, nil)) + client := createTestClient(t, opts, nil) + runTestsForClient(t, client) } diff --git a/pkg/infra/remotecache/remotecache.go b/pkg/infra/remotecache/remotecache.go index 1b9d67b9358..bd85529df0a 100644 --- a/pkg/infra/remotecache/remotecache.go +++ b/pkg/infra/remotecache/remotecache.go @@ -16,7 +16,11 @@ import ( ) var ( + // ErrCacheItemNotFound is returned if cache does not exist ErrCacheItemNotFound = errors.New("cache item not found") + + // ErrInvalidCacheType is returned if the type is invalid + ErrInvalidCacheType = errors.New("invalid remote cache name") ) func init() { @@ -61,10 +65,9 @@ func (ds *RemoteCache) Delete(key string) error { // Init initializes the service func (ds *RemoteCache) Init() error { ds.log = log.New("cache.remote") - - ds.client = createClient(ds.Cfg.RemoteCacheOptions, ds.SQLStore) - - return nil + var err error + ds.client, err = createClient(ds.Cfg.RemoteCacheOptions, ds.SQLStore) + return err } // Run start the backend processes for cache clients @@ -79,16 +82,20 @@ func (ds *RemoteCache) Run(ctx context.Context) error { return ctx.Err() } -func createClient(opts *setting.RemoteCacheOptions, sqlstore *sqlstore.SqlStore) CacheStorage { +func createClient(opts *setting.RemoteCacheOptions, sqlstore *sqlstore.SqlStore) (CacheStorage, error) { if opts.Name == "redis" { - return newRedisStorage(opts) + return newRedisStorage(opts), nil } if opts.Name == "memcached" { - return newMemcachedStorage(opts) + return newMemcachedStorage(opts), nil } - return newDatabaseCache(sqlstore) + if opts.Name == "database" { + return newDatabaseCache(sqlstore), nil + } + + return nil, ErrInvalidCacheType } // Register records a type, identified by a value for that type, under its diff --git a/pkg/infra/remotecache/remotecache_test.go b/pkg/infra/remotecache/remotecache_test.go index ac22607ee70..efeb75620ee 100644 --- a/pkg/infra/remotecache/remotecache_test.go +++ b/pkg/infra/remotecache/remotecache_test.go @@ -45,10 +45,14 @@ func TestCachedBasedOnConfig(t *testing.T) { }) client := createTestClient(t, cfg.RemoteCacheOptions, sqlstore.InitTestDB(t)) - runTestsForClient(t, client) } +func TestInvalidCacheTypeReturnsError(t *testing.T) { + _, err := createClient(&setting.RemoteCacheOptions{Name: "invalid"}, nil) + assert.Equal(t, err, ErrInvalidCacheType) +} + func runTestsForClient(t *testing.T, client CacheStorage) { canPutGetAndDeleteCachedObjects(t, client) canNotFetchExpiredItems(t, client) From c001cfe1d931c381e155de5994a886a9cb3a7d25 Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 14 Mar 2019 09:22:03 +0100 Subject: [PATCH 26/28] dont allow inifinite expiration --- pkg/infra/remotecache/database_storage.go | 8 ++++---- pkg/infra/remotecache/memcached_storage.go | 7 ++++++- pkg/infra/remotecache/redis_storage.go | 8 +------- pkg/infra/remotecache/remotecache.go | 11 ++++++++++- pkg/infra/remotecache/remotecache_test.go | 21 +-------------------- 5 files changed, 22 insertions(+), 33 deletions(-) diff --git a/pkg/infra/remotecache/database_storage.go b/pkg/infra/remotecache/database_storage.go index 2e34ecd1c73..51db43474b4 100644 --- a/pkg/infra/remotecache/database_storage.go +++ b/pkg/infra/remotecache/database_storage.go @@ -92,18 +92,18 @@ func (dc *databaseCache) Set(key string, value interface{}, expire time.Duration return err } - var expiresAtEpoch int64 + var expiresInSeconds int64 if expire != 0 { - expiresAtEpoch = int64(expire) / int64(time.Second) + expiresInSeconds = int64(expire) / int64(time.Second) } // insert or update depending on if item already exist if has { sql := `UPDATE cache_data SET data=?, created=?, expire=? WHERE cache_key='?'` - _, err = session.Exec(sql, data, getTime().Unix(), expiresAtEpoch, 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(), expiresAtEpoch) + _, err = session.Exec(sql, key, data, getTime().Unix(), expiresInSeconds) } return err diff --git a/pkg/infra/remotecache/memcached_storage.go b/pkg/infra/remotecache/memcached_storage.go index 7356849c1ef..1947d4ce56d 100644 --- a/pkg/infra/remotecache/memcached_storage.go +++ b/pkg/infra/remotecache/memcached_storage.go @@ -33,7 +33,12 @@ func (s *memcachedStorage) Set(key string, val interface{}, expires time.Duratio return err } - memcachedItem := newItem(key, bytes, int32(expires)) + var expiresInSeconds int64 + if expires != 0 { + expiresInSeconds = int64(expires) / int64(time.Second) + } + + memcachedItem := newItem(key, bytes, int32(expiresInSeconds)) return s.c.Set(memcachedItem) } diff --git a/pkg/infra/remotecache/redis_storage.go b/pkg/infra/remotecache/redis_storage.go index 9d54020fe79..c3ea2354d73 100644 --- a/pkg/infra/remotecache/redis_storage.go +++ b/pkg/infra/remotecache/redis_storage.go @@ -27,13 +27,7 @@ func (s *redisStorage) Set(key string, val interface{}, expires time.Duration) e return err } - var status *redis.StatusCmd - if expires == 0 { - status = s.c.Set(key, string(value)) - } else { - status = s.c.SetEx(key, expires, string(value)) - } - + status := s.c.SetEx(key, expires, string(value)) return status.Err() } diff --git a/pkg/infra/remotecache/remotecache.go b/pkg/infra/remotecache/remotecache.go index bd85529df0a..3e66c0dfb5d 100644 --- a/pkg/infra/remotecache/remotecache.go +++ b/pkg/infra/remotecache/remotecache.go @@ -21,6 +21,8 @@ var ( // ErrInvalidCacheType is returned if the type is invalid ErrInvalidCacheType = errors.New("invalid remote cache name") + + defaultMaxCacheExpiration = time.Hour * 24 ) func init() { @@ -35,7 +37,7 @@ type CacheStorage interface { // Get reads object from Cache Get(key string) (interface{}, error) - // Set sets an object into the cache. if `expire` is set to zero it never expires. + // Set sets an object into the cache. if `expire` is set to zero it will default to 24h Set(key string, value interface{}, expire time.Duration) error // Delete object from cache @@ -50,14 +52,21 @@ type RemoteCache struct { Cfg *setting.Cfg `inject:""` } +// Get reads object from Cache func (ds *RemoteCache) Get(key string) (interface{}, error) { return ds.client.Get(key) } +// Set sets an object into the cache. if `expire` is set to zero it will default to 24h func (ds *RemoteCache) Set(key string, value interface{}, expire time.Duration) error { + if expire == 0 { + expire = defaultMaxCacheExpiration + } + return ds.client.Set(key, value, expire) } +// Delete object from cache func (ds *RemoteCache) Delete(key string) error { return ds.client.Delete(key) } diff --git a/pkg/infra/remotecache/remotecache_test.go b/pkg/infra/remotecache/remotecache_test.go index efeb75620ee..bf1675ec87c 100644 --- a/pkg/infra/remotecache/remotecache_test.go +++ b/pkg/infra/remotecache/remotecache_test.go @@ -34,7 +34,7 @@ func createTestClient(t *testing.T, opts *setting.RemoteCacheOptions, sqlstore * t.Fatalf("failed to init client for test. error: %v", err) } - return dc.client + return dc } func TestCachedBasedOnConfig(t *testing.T) { @@ -56,7 +56,6 @@ func TestInvalidCacheTypeReturnsError(t *testing.T) { func runTestsForClient(t *testing.T, client CacheStorage) { canPutGetAndDeleteCachedObjects(t, client) canNotFetchExpiredItems(t, client) - canSetInfiniteCacheExpiration(t, client) } func canPutGetAndDeleteCachedObjects(t *testing.T, client CacheStorage) { @@ -92,21 +91,3 @@ func canNotFetchExpiredItems(t *testing.T, client CacheStorage) { _, err = client.Get("key1") assert.Equal(t, err, ErrCacheItemNotFound) } - -func canSetInfiniteCacheExpiration(t *testing.T, client CacheStorage) { - cacheableStruct := CacheableStruct{String: "hej", Int64: 2000} - - // insert cache item one day back - getTime = func() time.Time { return time.Now().AddDate(0, 0, -2) } - err := client.Set("key1", cacheableStruct, 0) - assert.Equal(t, err, nil) - - // should not be able to read that value since its expired - getTime = time.Now - data, err := client.Get("key1") - s, ok := data.(CacheableStruct) - - assert.Equal(t, ok, true) - assert.Equal(t, s.String, "hej") - assert.Equal(t, s.Int64, int64(2000)) -} From 0a86a1d7b648395be029e61220ab1a97db419349 Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 14 Mar 2019 09:23:35 +0100 Subject: [PATCH 27/28] updates old distcache names --- pkg/infra/remotecache/database_storage.go | 2 +- pkg/infra/remotecache/database_storage_test.go | 2 +- pkg/infra/remotecache/remotecache.go | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/infra/remotecache/database_storage.go b/pkg/infra/remotecache/database_storage.go index 51db43474b4..e188f25f76b 100644 --- a/pkg/infra/remotecache/database_storage.go +++ b/pkg/infra/remotecache/database_storage.go @@ -18,7 +18,7 @@ type databaseCache struct { func newDatabaseCache(sqlstore *sqlstore.SqlStore) *databaseCache { dc := &databaseCache{ SQLStore: sqlstore, - log: log.New("distcache.database"), + log: log.New("remotecache.database"), } return dc diff --git a/pkg/infra/remotecache/database_storage_test.go b/pkg/infra/remotecache/database_storage_test.go index 7fde3d325e5..d15e26fd07f 100644 --- a/pkg/infra/remotecache/database_storage_test.go +++ b/pkg/infra/remotecache/database_storage_test.go @@ -15,7 +15,7 @@ func TestDatabaseStorageGarbageCollection(t *testing.T) { db := &databaseCache{ SQLStore: sqlstore, - log: log.New("distcache.database"), + log: log.New("remotecache.database"), } obj := &CacheableStruct{String: "foolbar"} diff --git a/pkg/infra/remotecache/remotecache.go b/pkg/infra/remotecache/remotecache.go index 3e66c0dfb5d..25dbedcaff3 100644 --- a/pkg/infra/remotecache/remotecache.go +++ b/pkg/infra/remotecache/remotecache.go @@ -31,8 +31,8 @@ func init() { // CacheStorage allows the caller to set, get and delete items in the cache. // Cached items are stored as byte arrays and marshalled using "encoding/gob" -// so any struct added to the cache needs to be registred with `distcache.Register` -// ex `distcache.Register(CacheableStruct{})`` +// so any struct added to the cache needs to be registred with `remotecache.Register` +// ex `remotecache.Register(CacheableStruct{})`` type CacheStorage interface { // Get reads object from Cache Get(key string) (interface{}, error) From 6d42d43b2251fb2147ce285e924450daef4ae2d4 Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 14 Mar 2019 09:27:41 +0100 Subject: [PATCH 28/28] use constants for cache type --- pkg/infra/remotecache/database_storage.go | 2 ++ pkg/infra/remotecache/memcached_storage.go | 2 ++ pkg/infra/remotecache/memcached_storage_integration_test.go | 2 +- pkg/infra/remotecache/redis_storage.go | 2 ++ pkg/infra/remotecache/redis_storage_integration_test.go | 2 +- pkg/infra/remotecache/remotecache.go | 6 +++--- 6 files changed, 11 insertions(+), 5 deletions(-) diff --git a/pkg/infra/remotecache/database_storage.go b/pkg/infra/remotecache/database_storage.go index e188f25f76b..1c39d74d800 100644 --- a/pkg/infra/remotecache/database_storage.go +++ b/pkg/infra/remotecache/database_storage.go @@ -10,6 +10,8 @@ import ( var getTime = time.Now +const databaseCacheType = "database" + type databaseCache struct { SQLStore *sqlstore.SqlStore log log.Logger diff --git a/pkg/infra/remotecache/memcached_storage.go b/pkg/infra/remotecache/memcached_storage.go index 1947d4ce56d..5424a05ad02 100644 --- a/pkg/infra/remotecache/memcached_storage.go +++ b/pkg/infra/remotecache/memcached_storage.go @@ -7,6 +7,8 @@ import ( "github.com/grafana/grafana/pkg/setting" ) +const memcachedCacheType = "memcached" + type memcachedStorage struct { c *memcache.Client } diff --git a/pkg/infra/remotecache/memcached_storage_integration_test.go b/pkg/infra/remotecache/memcached_storage_integration_test.go index de7692e25d5..d1d82468644 100644 --- a/pkg/infra/remotecache/memcached_storage_integration_test.go +++ b/pkg/infra/remotecache/memcached_storage_integration_test.go @@ -9,7 +9,7 @@ import ( ) func TestMemcachedCacheStorage(t *testing.T) { - opts := &setting.RemoteCacheOptions{Name: "memcached", ConnStr: "localhost:11211"} + opts := &setting.RemoteCacheOptions{Name: memcachedCacheType, ConnStr: "localhost:11211"} client := createTestClient(t, opts, nil) runTestsForClient(t, client) } diff --git a/pkg/infra/remotecache/redis_storage.go b/pkg/infra/remotecache/redis_storage.go index c3ea2354d73..bd54b843119 100644 --- a/pkg/infra/remotecache/redis_storage.go +++ b/pkg/infra/remotecache/redis_storage.go @@ -7,6 +7,8 @@ import ( redis "gopkg.in/redis.v2" ) +const redisCacheType = "redis" + type redisStorage struct { c *redis.Client } diff --git a/pkg/infra/remotecache/redis_storage_integration_test.go b/pkg/infra/remotecache/redis_storage_integration_test.go index 0a63fbe31ec..8d54fc9ff14 100644 --- a/pkg/infra/remotecache/redis_storage_integration_test.go +++ b/pkg/infra/remotecache/redis_storage_integration_test.go @@ -10,7 +10,7 @@ import ( func TestRedisCacheStorage(t *testing.T) { - opts := &setting.RemoteCacheOptions{Name: "redis", ConnStr: "localhost:6379"} + opts := &setting.RemoteCacheOptions{Name: redisCacheType, ConnStr: "localhost:6379"} client := createTestClient(t, opts, nil) runTestsForClient(t, client) } diff --git a/pkg/infra/remotecache/remotecache.go b/pkg/infra/remotecache/remotecache.go index 25dbedcaff3..9219fa33a08 100644 --- a/pkg/infra/remotecache/remotecache.go +++ b/pkg/infra/remotecache/remotecache.go @@ -92,15 +92,15 @@ func (ds *RemoteCache) Run(ctx context.Context) error { } func createClient(opts *setting.RemoteCacheOptions, sqlstore *sqlstore.SqlStore) (CacheStorage, error) { - if opts.Name == "redis" { + if opts.Name == redisCacheType { return newRedisStorage(opts), nil } - if opts.Name == "memcached" { + if opts.Name == memcachedCacheType { return newMemcachedStorage(opts), nil } - if opts.Name == "database" { + if opts.Name == databaseCacheType { return newDatabaseCache(sqlstore), nil }