From cec978851b6b6d0fe5479271d9c6a48f5448fd80 Mon Sep 17 00:00:00 2001 From: Vinicius Vasconcelos Date: Tue, 26 Mar 2024 11:22:24 -0300 Subject: [PATCH] feat: implement store caching (#441) * chore: fix typo in contribute doc * feat: add support for store caching. * feat: change concat key in store cache key * fix: unit test error --- docs/contributing.md | 2 +- internal/app/server/open_saves.go | 35 ++++++++++++++++--- internal/app/server/open_saves_test.go | 21 +++++++++++ internal/pkg/metadb/store/store.go | 31 +++++++++++++++++ internal/pkg/metadb/store/store_test.go | 46 +++++++++++++++++++++++++ 5 files changed, 130 insertions(+), 5 deletions(-) diff --git a/docs/contributing.md b/docs/contributing.md index 8fce1637..f21be1ae 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -23,7 +23,7 @@ use GitHub pull requests for this purpose. Consult information on using pull requests. When opening a pull request, please enter the title with a matching -[conentional commit](https://www.conventionalcommits.org/en/v1.0.0/) tag. +[conventional commit](https://www.conventionalcommits.org/en/v1.0.0/) tag. For example, a patch to fix a bug should be titles as "fix: server crash with malformed json." ## Community Guidelines diff --git a/internal/app/server/open_saves.go b/internal/app/server/open_saves.go index e39434c6..1374ea56 100644 --- a/internal/app/server/open_saves.go +++ b/internal/app/server/open_saves.go @@ -103,6 +103,7 @@ func (s *openSavesServer) CreateStore(ctx context.Context, req *pb.CreateStoreRe return nil, status.Convert(err).Err() } log.Debugf("Created store: %+v", store) + s.storeCache(ctx, &store) return newStore.ToProto(), nil } @@ -143,12 +144,11 @@ func (s *openSavesServer) DeleteRecord(ctx context.Context, req *pb.DeleteRecord } func (s *openSavesServer) GetStore(ctx context.Context, req *pb.GetStoreRequest) (*pb.Store, error) { - store, err := s.metaDB.GetStore(ctx, req.GetKey()) + str, err := s.getStoreAndCache(ctx, req.GetKey()) if err != nil { - log.Warnf("GetStore failed for store (%s): %v", req.GetKey(), err) - return nil, status.Convert(err).Err() + return nil, err } - return store.ToProto(), nil + return str.ToProto(), nil } func (s *openSavesServer) ListStores(ctx context.Context, req *pb.ListStoresRequest) (*pb.ListStoresResponse, error) { @@ -1012,6 +1012,33 @@ func (s *openSavesServer) cacheRecord(ctx context.Context, r *record.Record, hin return err } +func (s *openSavesServer) getStoreAndCache(ctx context.Context, storeKey string) (*store.Store, error) { + st := new(store.Store) + if err := s.cacheStore.Get(ctx, store.CacheKey(storeKey), st); err == nil { + log.Debug("store cache hit") + return st, nil + } + log.Debug("store cache miss") + + str, err := s.metaDB.GetStore(ctx, storeKey) + if err != nil { + log.Warnf("GetStore failed for store (%s): %v", + storeKey, err) + return nil, status.Convert(err).Err() + } + s.storeCache(ctx, str) + log.Tracef("Got store %+v", s) + return str, nil +} + +func (s *openSavesServer) storeCache(ctx context.Context, st *store.Store) error { + var err error + if err = s.cacheStore.Set(ctx, st); err != nil { + log.Warnf("failed to encode record for cache for store (%s): %v", st.Key, err) + } + return err +} + // shouldCache returns whether or not Open Saves should try to store // the record in the cache store. Default behavior is to cache // if hint is not specified. diff --git a/internal/app/server/open_saves_test.go b/internal/app/server/open_saves_test.go index 77862b0b..c9253f62 100644 --- a/internal/app/server/open_saves_test.go +++ b/internal/app/server/open_saves_test.go @@ -17,6 +17,7 @@ package server import ( "context" "fmt" + "github.com/googleforgames/open-saves/internal/pkg/metadb/store" "io" "net" "reflect" @@ -397,6 +398,26 @@ func TestOpenSaves_CreateGetDeleteStore(t *testing.T) { assert.Equal(t, store.GetUpdatedAt(), store2.GetUpdatedAt()) } +func TestOpenSaves_GetCreateStoreFromCache(t *testing.T) { + ctx := context.Background() + server, listener := getOpenSavesServer(ctx, t, "gcp") + _, client := getTestClient(ctx, t, listener) + storeKey := uuid.NewString() + str := &pb.Store{ + Key: storeKey, + Name: "test-GetCreateStore-store-cache", + Tags: []string{"tag1"}, + OwnerId: "owner", + } + setupTestStore(ctx, t, client, str) + // should be already cached + cacheKey := store.CacheKey(storeKey) + storeFromCache := new(store.Store) + err := server.cacheStore.Get(ctx, cacheKey, storeFromCache) + assert.NoError(t, err, "should have retrieved store from cache after Create") + +} + func TestOpenSaves_CreateGetDeleteRecord(t *testing.T) { ctx := context.Background() _, listener := getOpenSavesServer(ctx, t, "gcp") diff --git a/internal/pkg/metadb/store/store.go b/internal/pkg/metadb/store/store.go index 565f4062..d54e1e9f 100644 --- a/internal/pkg/metadb/store/store.go +++ b/internal/pkg/metadb/store/store.go @@ -16,8 +16,10 @@ package store import ( "cloud.google.com/go/datastore" + "fmt" pb "github.com/googleforgames/open-saves/api" "github.com/googleforgames/open-saves/internal/pkg/metadb/timestamps" + "github.com/vmihailenco/msgpack/v5" ) // Store represents a Open Saves store in the metadata database. @@ -49,6 +51,12 @@ func (s *Store) ToProto() *pb.Store { } } +// CacheKey returns a cache key string to manage cached entries. +// concatenates "store" + store keys separated by a dash. +func CacheKey(storeKey string) string { + return fmt.Sprintf("store:%s", storeKey) +} + // These functions need to be implemented here instead of the datastore package because // go doesn't permit to define additional receivers in another package. // Save and Load replicates the default behaviors, however, they are required @@ -72,6 +80,29 @@ func (s *Store) LoadKey(k *datastore.Key) error { return nil } +// Cacheable implementations + +// CacheKey returns cache formatted key +func (s *Store) CacheKey() string { + return CacheKey(s.Key) +} + +// DecodeBytes deserializes the byte slice given by by. +func (s *Store) DecodeBytes(by []byte) error { + return msgpack.Unmarshal(by, s) +} + +// EncodeBytes returns a serialized byte slice of the object. +func (s *Store) EncodeBytes() ([]byte, error) { + b, err := msgpack.Marshal(s) + if err != nil { + return nil, err + } + return b, nil +} + +// Proto + // FromProto creates a new Store instance from a proto. // Passing nil returns a zero-initialized Store. func FromProto(p *pb.Store) *Store { diff --git a/internal/pkg/metadb/store/store_test.go b/internal/pkg/metadb/store/store_test.go index 08391910..e256d552 100644 --- a/internal/pkg/metadb/store/store_test.go +++ b/internal/pkg/metadb/store/store_test.go @@ -88,3 +88,49 @@ func TestStore_LoadKey(t *testing.T) { assert.NoError(t, store.LoadKey(key)) assert.Equal(t, "testkey", store.Key) } + +func TestStore_CacheKey(t *testing.T) { + s := &Store{ + Key: "abc", + } + key := s.CacheKey() + assert.Equal(t, "store:abc", key) +} + +func TestStore_SerializeRecord(t *testing.T) { + rr := []*Store{ + { + Timestamps: timestamps.Timestamps{ + CreatedAt: time.Unix(100, 0), + UpdatedAt: time.Unix(110, 0), + }, + }, + { + Key: "some-key", + Name: "some-store", + Tags: []string{"tag1", "tag2"}, + OwnerID: "owner", + Timestamps: timestamps.Timestamps{ + CreatedAt: time.Unix(100, 0), + UpdatedAt: time.Unix(110, 0), + }, + }, + { + Key: "some-key", + OwnerID: "new-owner", + Tags: []string{"tag1", "tag2"}, + Timestamps: timestamps.Timestamps{ + CreatedAt: time.Unix(100, 0), + UpdatedAt: time.Unix(110, 0), + }, + }, + } + + for _, s := range rr { + e, err := s.EncodeBytes() + assert.NoError(t, err) + decoded := new(Store) + assert.NoError(t, decoded.DecodeBytes(e)) + assert.Equal(t, s, decoded) + } +}