From c49ea328c77e90011250fc7c64b385420385ac4f Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Tue, 24 May 2022 12:25:26 -0400 Subject: [PATCH] Move the cache implementation behind an interface, to allow for different implementations based on platform --- internal/datastore/proxy/caching.go | 10 ++-- internal/dispatch/caching/caching.go | 18 +++---- internal/dispatch/cluster/cluster.go | 7 ++- internal/dispatch/combined/combined.go | 6 +-- pkg/cache/cache.go | 73 ++++++++++++++++++++++++++ pkg/cache/cache_ristretto.go | 32 +++++++++++ pkg/cmd/server/cacheconfig.go | 9 ++-- 7 files changed, 130 insertions(+), 25 deletions(-) create mode 100644 pkg/cache/cache.go create mode 100644 pkg/cache/cache_ristretto.go diff --git a/internal/datastore/proxy/caching.go b/internal/datastore/proxy/caching.go index 478ec9d40e..382fa7f489 100644 --- a/internal/datastore/proxy/caching.go +++ b/internal/datastore/proxy/caching.go @@ -6,13 +6,13 @@ import ( "fmt" "sync" - "github.com/dgraph-io/ristretto" "github.com/dustin/go-humanize" "github.com/rs/zerolog/log" "github.com/shopspring/decimal" "golang.org/x/sync/singleflight" "google.golang.org/protobuf/proto" + "github.com/authzed/spicedb/pkg/cache" "github.com/authzed/spicedb/pkg/datastore" core "github.com/authzed/spicedb/pkg/proto/core/v1" ) @@ -21,10 +21,10 @@ import ( // are loaded at specific datastore revisions. func NewCachingDatastoreProxy( delegate datastore.Datastore, - cacheConfig *ristretto.Config, + cacheConfig *cache.Config, ) (datastore.Datastore, error) { if cacheConfig == nil { - cacheConfig = &ristretto.Config{ + cacheConfig = &cache.Config{ NumCounters: 1e4, // number of keys to track frequency of (10k). MaxCost: 1 << 24, // maximum cost of cache (16MB). BufferItems: 64, // number of keys per Get buffer. @@ -33,7 +33,7 @@ func NewCachingDatastoreProxy( log.Info().Int64("numCounters", cacheConfig.NumCounters).Str("maxCost", humanize.Bytes(uint64(cacheConfig.MaxCost))).Msg("configured caching namespace manager") } - cache, err := ristretto.NewCache(cacheConfig) + cache, err := cache.NewCache(cacheConfig) if err != nil { return nil, fmt.Errorf("unable to create cache: %w", err) } @@ -46,7 +46,7 @@ func NewCachingDatastoreProxy( type nsCachingProxy struct { datastore.Datastore - c *ristretto.Cache + c cache.Cache readNsGroup singleflight.Group } diff --git a/internal/dispatch/caching/caching.go b/internal/dispatch/caching/caching.go index d8a47e9971..7d31a7ad44 100644 --- a/internal/dispatch/caching/caching.go +++ b/internal/dispatch/caching/caching.go @@ -6,7 +6,6 @@ import ( "sync" "unsafe" - "github.com/dgraph-io/ristretto" "github.com/dustin/go-humanize" "github.com/prometheus/client_golang/prometheus" "github.com/rs/zerolog/log" @@ -14,6 +13,7 @@ import ( "github.com/authzed/spicedb/internal/dispatch" "github.com/authzed/spicedb/internal/dispatch/keys" + "github.com/authzed/spicedb/pkg/cache" v1 "github.com/authzed/spicedb/pkg/proto/dispatch/v1" ) @@ -26,7 +26,7 @@ const ( // Dispatcher is a dispatcher with built-in caching. type Dispatcher struct { d dispatch.Dispatcher - c *ristretto.Cache + c cache.Cache keyHandler keys.Handler checkTotalCounter prometheus.Counter @@ -63,12 +63,12 @@ var ( // NewCachingDispatcher creates a new dispatch.Dispatcher which delegates dispatch requests // and caches the responses when possible and desirable. func NewCachingDispatcher( - cacheConfig *ristretto.Config, + cacheConfig *cache.Config, prometheusSubsystem string, keyHandler keys.Handler, ) (*Dispatcher, error) { if cacheConfig == nil { - cacheConfig = &ristretto.Config{ + cacheConfig = &cache.Config{ NumCounters: 1e4, // number of keys to track frequency of (10k). MaxCost: 1 << 24, // maximum cost of cache (16MB). BufferItems: 64, // number of keys per Get buffer. @@ -78,7 +78,7 @@ func NewCachingDispatcher( log.Info().Int64("numCounters", cacheConfig.NumCounters).Str("maxCost", humanize.Bytes(uint64(cacheConfig.MaxCost))).Msg("configured caching dispatcher") } - cache, err := ristretto.NewCache(cacheConfig) + cache, err := cache.NewCache(cacheConfig) if err != nil { return nil, fmt.Errorf(errCachingInitialization, err) } @@ -121,14 +121,14 @@ func NewCachingDispatcher( Subsystem: prometheusSubsystem, Name: "cache_hits_total", }, func() float64 { - return float64(cache.Metrics.Hits()) + return float64(cache.GetMetrics().Hits()) }) cacheMissesTotal := prometheus.NewCounterFunc(prometheus.CounterOpts{ Namespace: prometheusNamespace, Subsystem: prometheusSubsystem, Name: "cache_misses_total", }, func() float64 { - return float64(cache.Metrics.Misses()) + return float64(cache.GetMetrics().Misses()) }) costAddedBytes := prometheus.NewCounterFunc(prometheus.CounterOpts{ @@ -136,7 +136,7 @@ func NewCachingDispatcher( Subsystem: prometheusSubsystem, Name: "cost_added_bytes", }, func() float64 { - return float64(cache.Metrics.CostAdded()) + return float64(cache.GetMetrics().CostAdded()) }) costEvictedBytes := prometheus.NewCounterFunc(prometheus.CounterOpts{ @@ -144,7 +144,7 @@ func NewCachingDispatcher( Subsystem: prometheusSubsystem, Name: "cost_evicted_bytes", }, func() float64 { - return float64(cache.Metrics.CostEvicted()) + return float64(cache.GetMetrics().CostEvicted()) }) if prometheusSubsystem != "" { diff --git a/internal/dispatch/cluster/cluster.go b/internal/dispatch/cluster/cluster.go index 318248120b..dfc9fbc4ab 100644 --- a/internal/dispatch/cluster/cluster.go +++ b/internal/dispatch/cluster/cluster.go @@ -1,12 +1,11 @@ package cluster import ( - "github.com/dgraph-io/ristretto" - "github.com/authzed/spicedb/internal/dispatch" "github.com/authzed/spicedb/internal/dispatch/caching" "github.com/authzed/spicedb/internal/dispatch/graph" "github.com/authzed/spicedb/internal/dispatch/keys" + "github.com/authzed/spicedb/pkg/cache" ) // Option is a function-style option for configuring a combined Dispatcher. @@ -14,7 +13,7 @@ type Option func(*optionState) type optionState struct { prometheusSubsystem string - cacheConfig *ristretto.Config + cacheConfig *cache.Config } // PrometheusSubsystem sets the subsystem name for the prometheus metrics @@ -25,7 +24,7 @@ func PrometheusSubsystem(name string) Option { } // CacheConfig sets the configuration for the local dispatcher's cache. -func CacheConfig(config *ristretto.Config) Option { +func CacheConfig(config *cache.Config) Option { return func(state *optionState) { state.cacheConfig = config } diff --git a/internal/dispatch/combined/combined.go b/internal/dispatch/combined/combined.go index da54538ca8..6f1c7ef8db 100644 --- a/internal/dispatch/combined/combined.go +++ b/internal/dispatch/combined/combined.go @@ -6,7 +6,6 @@ import ( "os" "github.com/authzed/grpcutil" - "github.com/dgraph-io/ristretto" "github.com/rs/zerolog/log" "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" @@ -16,6 +15,7 @@ import ( "github.com/authzed/spicedb/internal/dispatch/graph" "github.com/authzed/spicedb/internal/dispatch/keys" "github.com/authzed/spicedb/internal/dispatch/remote" + "github.com/authzed/spicedb/pkg/cache" v1 "github.com/authzed/spicedb/pkg/proto/dispatch/v1" ) @@ -28,7 +28,7 @@ type optionState struct { upstreamCAPath string grpcPresharedKey string grpcDialOpts []grpc.DialOption - cacheConfig *ristretto.Config + cacheConfig *cache.Config } // PrometheusSubsystem sets the subsystem name for the prometheus metrics @@ -70,7 +70,7 @@ func GrpcDialOpts(opts ...grpc.DialOption) Option { } // CacheConfig sets the configuration for the local dispatcher's cache. -func CacheConfig(config *ristretto.Config) Option { +func CacheConfig(config *cache.Config) Option { return func(state *optionState) { state.cacheConfig = config } diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go new file mode 100644 index 0000000000..0bbc8b0e7e --- /dev/null +++ b/pkg/cache/cache.go @@ -0,0 +1,73 @@ +package cache + +// Config for caching. +// See: https://github.com/dgraph-io/ristretto#Config +type Config struct { + // NumCounters determines the number of counters (keys) to keep that hold + // access frequency information. It's generally a good idea to have more + // counters than the max cache capacity, as this will improve eviction + // accuracy and subsequent hit ratios. + // + // For example, if you expect your cache to hold 1,000,000 items when full, + // NumCounters should be 10,000,000 (10x). Each counter takes up roughly + // 3 bytes (4 bits for each counter * 4 copies plus about a byte per + // counter for the bloom filter). Note that the number of counters is + // internally rounded up to the nearest power of 2, so the space usage + // may be a little larger than 3 bytes * NumCounters. + NumCounters int64 + + // MaxCost can be considered as the cache capacity, in whatever units you + // choose to use. + // + // For example, if you want the cache to have a max capacity of 100MB, you + // would set MaxCost to 100,000,000 and pass an item's number of bytes as + // the `cost` parameter for calls to Set. If new items are accepted, the + // eviction process will take care of making room for the new item and not + // overflowing the MaxCost value. + MaxCost int64 + + // BufferItems determines the size of Get buffers. + // + // Unless you have a rare use case, using `64` as the BufferItems value + // results in good performance. + BufferItems int64 + + // Metrics determines whether cache statistics are kept during the cache's + // lifetime. There *is* some overhead to keeping statistics, so you should + // only set this flag to true when testing or throughput performance isn't a + // major factor. + Metrics bool +} + +// Cache defines an interface for a generic cache. +type Cache interface { + // Get returns the value for the given key in the cache, if it exists. + Get(key interface{}) (interface{}, bool) + + // Set sets a value for the key in the cache, with the given cost. + Set(key interface{}, entry interface{}, cost int64) bool + + // Wait waits for the cache to process and apply updates. + Wait() + + // Close closes the cache's background workers (if any). + Close() + + // GetMetrics returns the metrics block for the cache. + GetMetrics() Metrics +} + +// Metrics defines metrics exported by the cache. +type Metrics interface { + // Hits is the number of cache hits. + Hits() uint64 + + // Misses is the number of cache misses. + Misses() uint64 + + // CostAdded returns the total cost of added items. + CostAdded() uint64 + + // CostEvicted returns the total cost of evicted items. + CostEvicted() uint64 +} diff --git a/pkg/cache/cache_ristretto.go b/pkg/cache/cache_ristretto.go new file mode 100644 index 0000000000..00c43149ab --- /dev/null +++ b/pkg/cache/cache_ristretto.go @@ -0,0 +1,32 @@ +//go:build !wasm +// +build !wasm + +package cache + +import ( + "github.com/dgraph-io/ristretto" +) + +// NewCache creates a new ristretto cache from the given config. +func NewCache(config *Config) (Cache, error) { + cache, err := ristretto.NewCache(&ristretto.Config{ + NumCounters: config.NumCounters, + MaxCost: config.MaxCost, + BufferItems: config.BufferItems, + Metrics: config.Metrics, + }) + if err != nil { + return nil, err + } + return wrapped{cache}, nil +} + +type wrapped struct { + *ristretto.Cache +} + +func (w wrapped) GetMetrics() Metrics { + return w.Cache.Metrics +} + +var _ Cache = &wrapped{} diff --git a/pkg/cmd/server/cacheconfig.go b/pkg/cmd/server/cacheconfig.go index b8ce0e63d6..02b341dccd 100644 --- a/pkg/cmd/server/cacheconfig.go +++ b/pkg/cmd/server/cacheconfig.go @@ -3,10 +3,11 @@ package server import ( "fmt" - "github.com/dgraph-io/ristretto" "github.com/dustin/go-humanize" "github.com/jzelinskie/stringz" "github.com/spf13/pflag" + + "github.com/authzed/spicedb/pkg/cache" ) // CacheConfig defines configuration for a ristretto cache. @@ -23,8 +24,8 @@ const ( defaultBufferItems = 64 ) -// Complete converts the cache config into a ristretto cache config. -func (cc *CacheConfig) Complete() (*ristretto.Config, error) { +// Complete translates the CLI cache config into a cache config. +func (cc *CacheConfig) Complete() (*cache.Config, error) { if cc.MaxCost == "" || cc.NumCounters == 0 { return nil, nil } @@ -34,7 +35,7 @@ func (cc *CacheConfig) Complete() (*ristretto.Config, error) { return nil, fmt.Errorf("error parsing cache max cost `%s`: %w", cc.MaxCost, err) } - return &ristretto.Config{ + return &cache.Config{ MaxCost: int64(maxCost), NumCounters: cc.NumCounters, Metrics: cc.Metrics,