Skip to content

Commit

Permalink
add a non-caching namespace manager
Browse files Browse the repository at this point in the history
this allows certain operations to deal with non-persisted namespaces
with the same interface as persisted namespaces
  • Loading branch information
ecordell committed Feb 22, 2022
1 parent e8edc61 commit 51297ca
Show file tree
Hide file tree
Showing 16 changed files with 77 additions and 64 deletions.
5 changes: 2 additions & 3 deletions internal/dispatch/graph/check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"fmt"
"os"
"testing"
"time"

v0 "github.com/authzed/authzed-go/proto/authzed/api/v0"
v1_api "github.com/authzed/authzed-go/proto/authzed/api/v1"
Expand Down Expand Up @@ -181,7 +180,7 @@ func TestMaxDepth(t *testing.T) {
require.NoError(err)
require.True(revision.GreaterThan(decimal.Zero))

nsm, err := namespace.NewCachingNamespaceManager(1*time.Second, testCacheConfig)
nsm, err := namespace.NewCachingNamespaceManager(testCacheConfig)
require.NoError(err)

dispatch := NewLocalOnlyDispatcher(nsm)
Expand Down Expand Up @@ -299,7 +298,7 @@ func newLocalDispatcher(require *require.Assertions) (context.Context, dispatch.

ds, revision := testfixtures.StandardDatastoreWithData(rawDS, require)

nsm, err := namespace.NewCachingNamespaceManager(1*time.Second, testCacheConfig)
nsm, err := namespace.NewCachingNamespaceManager(testCacheConfig)
require.NoError(err)

dispatch := NewLocalOnlyDispatcher(nsm)
Expand Down
3 changes: 1 addition & 2 deletions internal/dispatch/graph/expand_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"go/token"
"os"
"testing"
"time"

v0 "github.com/authzed/authzed-go/proto/authzed/api/v0"
v1_api "github.com/authzed/authzed-go/proto/authzed/api/v1"
Expand Down Expand Up @@ -280,7 +279,7 @@ func TestMaxDepthExpand(t *testing.T) {
require.True(revision.GreaterThan(decimal.Zero))
require.NoError(datastoremw.SetInContext(ctx, ds))

nsm, err := namespace.NewCachingNamespaceManager(1*time.Second, testCacheConfig)
nsm, err := namespace.NewCachingNamespaceManager(testCacheConfig)
require.NoError(err)

dispatch := NewLocalOnlyDispatcher(nsm)
Expand Down
2 changes: 1 addition & 1 deletion internal/dispatch/graph/lookup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func TestMaxDepthLookup(t *testing.T) {

ds, revision := testfixtures.StandardDatastoreWithData(rawDS, require)

nsm, err := namespace.NewCachingNamespaceManager(1*time.Second, testCacheConfig)
nsm, err := namespace.NewCachingNamespaceManager(testCacheConfig)
require.NoError(err)

dispatch := NewLocalOnlyDispatcher(nsm)
Expand Down
49 changes: 34 additions & 15 deletions internal/namespace/caching.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"errors"
"fmt"
"time"

v0 "github.com/authzed/authzed-go/proto/authzed/api/v0"
"github.com/dgraph-io/ristretto"
Expand All @@ -21,14 +20,20 @@ const (
errInitialization = "unable to initialize namespace manager: %w"
)

// nsCache holds the subset of the underlying ristretto.Cache interface that the
// manager needs
type nsCache interface {
Get(key interface{}) (interface{}, bool)
Set(key, value interface{}, cost int64) bool
Close()
}

type cachingManager struct {
expiration time.Duration
c *ristretto.Cache
c nsCache
readNsGroup singleflight.Group
}

func NewCachingNamespaceManager(
expiration time.Duration,
cacheConfig *ristretto.Config,
) (Manager, error) {
if cacheConfig == nil {
Expand All @@ -45,11 +50,15 @@ func NewCachingNamespaceManager(
}

return &cachingManager{
expiration: expiration,
c: cache,
c: cache,
}, nil
}

// NewNonCachingNamespaceManager returns a namespace manager that doesn't cache
func NewNonCachingNamespaceManager() Manager {
return &cachingManager{c: noCache{}}
}

func (nsc *cachingManager) ReadNamespaceAndTypes(ctx context.Context, nsName string, revision decimal.Decimal) (*v0.NamespaceDefinition, *NamespaceTypeSystem, error) {
nsDef, err := nsc.ReadNamespace(ctx, nsName, revision)
if err != nil {
Expand All @@ -67,7 +76,7 @@ func (nsc *cachingManager) ReadNamespace(ctx context.Context, nsName string, rev

ds := datastoremw.MustFromContext(ctx)

// Check the cache.
// Check the nsCache.
nsRevisionKey, err := ds.NamespaceCacheKey(nsName, revision)
if err != nil {
return nil, err
Expand All @@ -88,14 +97,9 @@ func (nsc *cachingManager) ReadNamespace(ctx context.Context, nsName string, rev
// Remove user-defined metadata.
loaded = namespace.FilterUserDefinedMetadata(loaded)

cacheKey, err := ds.NamespaceCacheKey(nsName, revision)
if err != nil {
return nil, err
}

// Save it to the cache
nsc.c.Set(cacheKey, loaded, int64(proto.Size(loaded)))
span.AddEvent("Saved to cache")
// Save it to the nsCache
nsc.c.Set(nsRevisionKey, loaded, int64(proto.Size(loaded)))
span.AddEvent("Saved to nsCache")

return loaded, err
})
Expand Down Expand Up @@ -132,3 +136,18 @@ func (nsc *cachingManager) Close() error {
nsc.c.Close()
return nil
}

// noCache is an implementation of nsCache that doesn't cache
type noCache struct{}

var _ nsCache = noCache{}

func (n noCache) Get(key interface{}) (interface{}, bool) {
return nil, false
}

func (n noCache) Set(key, value interface{}, cost int64) bool {
return false
}

func (n noCache) Close() {}
28 changes: 25 additions & 3 deletions internal/namespace/caching_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ package namespace
import (
"context"
"testing"
"time"

v0 "github.com/authzed/authzed-go/proto/authzed/api/v0"
"github.com/dgraph-io/ristretto"
"github.com/stretchr/testify/require"

"github.com/authzed/spicedb/internal/datastore/memdb"
Expand All @@ -14,7 +14,7 @@ import (
)

func TestDisjointCacheKeys(t *testing.T) {
cache, err := NewCachingNamespaceManager(10*time.Second, nil)
cache, err := NewCachingNamespaceManager(nil)
require.NoError(t, err)

ds, err := memdb.NewMemdbDatastore(0, 0, memdb.DisableGC, 0)
Expand All @@ -37,7 +37,7 @@ func TestDisjointCacheKeys(t *testing.T) {
keyA, err := dsA.NamespaceCacheKey("test/user", rev)
require.NoError(t, err)

rCache := cache.(*cachingManager).c
rCache := cache.(*cachingManager).c.(*ristretto.Cache)
rCache.Wait()

nsA, ok := rCache.Get(keyA)
Expand All @@ -62,3 +62,25 @@ func TestDisjointCacheKeys(t *testing.T) {
// namespaces are different
require.NotEmpty(t, nsA, nsB)
}

func TestNoCache(t *testing.T) {
cache := NewNonCachingNamespaceManager()
ds, err := memdb.NewMemdbDatastore(0, 0, memdb.DisableGC, 0)
require.NoError(t, err)

ctx := datastoremw.ContextWithDatastore(context.Background(), ds)

// write a namespace to the "A" store
rev, err := ds.WriteNamespace(ctx, &v0.NamespaceDefinition{Name: "test/user"})
require.NoError(t, err)

def, err := cache.ReadNamespace(ctx, "test/user", rev)
require.NoError(t, err)
require.Equal(t, "test/user", def.Name)

defB, err := cache.ReadNamespace(ctx, "test/user", rev)
require.NoError(t, err)
require.Equal(t, "test/user", defB.Name)

require.Equal(t, def, defB)
}
2 changes: 1 addition & 1 deletion internal/namespace/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (

var tracer = otel.Tracer("spicedb/internal/namespace")

// Manager is a subset of the datastore interface that can read (and possibly cache) namespaces.
// Manager is a subset of the datastore interface that can read (and possibly nsCache) namespaces.
type Manager interface {
// ReadNamespace reads a namespace definition and version and returns it if found.
//
Expand Down
3 changes: 1 addition & 2 deletions internal/namespace/typesystem_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package namespace
import (
"context"
"testing"
"time"

v0 "github.com/authzed/authzed-go/proto/authzed/api/v0"
"github.com/shopspring/decimal"
Expand Down Expand Up @@ -223,7 +222,7 @@ func TestTypeSystem(t *testing.T) {
require.NoError(err)

ctx := datastoremw.ContextWithDatastore(context.Background(), ds)
nsm, err := NewCachingNamespaceManager(0*time.Second, nil)
nsm, err := NewCachingNamespaceManager(nil)
require.NoError(err)

var lastRevision decimal.Decimal
Expand Down
2 changes: 1 addition & 1 deletion internal/services/consistency_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func TestConsistency(t *testing.T) {
})
t.Cleanup(cleanup)

ns, err := namespace.NewCachingNamespaceManager(1*time.Second, nil)
ns, err := namespace.NewCachingNamespaceManager(nil)
lrequire.NoError(err)

dsCtx := datastoremw.ContextWithHandle(context.Background())
Expand Down
6 changes: 1 addition & 5 deletions internal/services/v0/namespace.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package v0
import (
"context"
"errors"
"time"

v0 "github.com/authzed/authzed-go/proto/authzed/api/v0"
v1 "github.com/authzed/authzed-go/proto/authzed/api/v1"
Expand Down Expand Up @@ -43,10 +42,7 @@ func NewNamespaceServer() v0.NamespaceServiceServer {

func (nss *nsServer) WriteConfig(ctx context.Context, req *v0.WriteConfigRequest) (*v0.WriteConfigResponse, error) {
ds := datastoremw.MustFromContext(ctx)
nsm, err := namespace.NewCachingNamespaceManager(0*time.Second, nil)
if err != nil {
return nil, rewriteNamespaceError(ctx, err)
}
nsm := namespace.NewNonCachingNamespaceManager()

readRevision, _ := consistency.MustRevisionFromContext(ctx)

Expand Down
6 changes: 1 addition & 5 deletions internal/services/v1alpha1/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,7 @@ func (ss *schemaServiceServer) ReadSchema(ctx context.Context, in *v1alpha1.Read
func (ss *schemaServiceServer) WriteSchema(ctx context.Context, in *v1alpha1.WriteSchemaRequest) (*v1alpha1.WriteSchemaResponse, error) {
log.Ctx(ctx).Trace().Str("schema", in.GetSchema()).Msg("requested Schema to be written")
ds := datastoremw.MustFromContext(ctx)

nsm, err := namespace.NewCachingNamespaceManager(0, nil) // non-caching manager
if err != nil {
return nil, rewriteError(ctx, err)
}
nsm := namespace.NewNonCachingNamespaceManager()

inputSchema := compiler.InputSchema{
Source: input.Source("schema"),
Expand Down
4 changes: 1 addition & 3 deletions internal/testserver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,17 @@ func NewTestServer(require *require.Assertions,
emptyDS, err := memdb.NewMemdbDatastore(0, revisionFuzzingTimedelta, gcWindow, simulatedLatency)
require.NoError(err)
ds, revision := dsInitFunc(emptyDS, require)
ns, err := namespace.NewCachingNamespaceManager(1*time.Second, nil)
ns, err := namespace.NewCachingNamespaceManager(nil)
require.NoError(err)
srv, err := server.NewConfigWithOptions(
server.WithDatastore(ds),
server.WithNamespaceCacheExpiration(1*time.Second),
server.WithDispatcher(graph.NewLocalOnlyDispatcher(ns)),
server.WithDispatchMaxDepth(50),
server.WithGRPCServer(util.GRPCServerConfig{
Network: util.BufferedNetwork,
Enabled: true,
}),
server.WithSchemaPrefixesRequired(schemaPrefixRequired),
server.WithNamespaceCacheExpiration(1*time.Second),
server.WithGRPCAuthFunc(func(ctx context.Context) (context.Context, error) {
return ctx, nil
}),
Expand Down
5 changes: 4 additions & 1 deletion pkg/cmd/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ func RegisterServeFlags(cmd *cobra.Command, config *server.Config) {
datastore.RegisterDatastoreFlags(cmd, &config.DatastoreConfig)

// Flags for the namespace manager
cmd.Flags().DurationVar(&config.NamespaceCacheExpiration, "ns-cache-expiration", 1*time.Minute, "amount of time a namespace entry should remain cached")
cmd.Flags().Duration("ns-cache-expiration", 1*time.Minute, "amount of time a namespace entry should remain cached")
if err := cmd.Flags().MarkHidden("ns-cache-expiration"); err != nil {
panic("failed to mark flag hidden: " + err.Error())
}

// Flags for parsing and validating schemas.
cmd.Flags().BoolVar(&config.SchemaPrefixesRequired, "schema-prefixes-required", false, "require prefixes on all object definitions in schemas")
Expand Down
5 changes: 1 addition & 4 deletions pkg/cmd/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,6 @@ type Config struct {
DatastoreConfig datastorecfg.Config
Datastore datastore.Datastore

// Namespace cache
NamespaceCacheExpiration time.Duration

// Schema options
SchemaPrefixesRequired bool

Expand Down Expand Up @@ -99,7 +96,7 @@ func (c *Config) Complete() (RunnableServer, error) {
}
}

nsm, err := namespace.NewCachingNamespaceManager(c.NamespaceCacheExpiration, nil)
nsm, err := namespace.NewCachingNamespaceManager(nil)
if err != nil {
return nil, fmt.Errorf("failed to create namespace manager: %w", err)
}
Expand Down
8 changes: 0 additions & 8 deletions pkg/cmd/server/zz_generated.options.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 2 additions & 6 deletions pkg/cmd/testserver/testserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package testserver
import (
"context"
"fmt"
"time"

"github.com/rs/zerolog"
"github.com/rs/zerolog/log"
Expand All @@ -22,10 +21,7 @@ import (
"github.com/authzed/spicedb/pkg/cmd/util"
)

const (
nsCacheExpiration = 0 * time.Minute // No caching
maxDepth = 50
)
const maxDepth = 50

//go:generate go run github.com/ecordell/optgen -output zz_generated.options.go . Config
type Config struct {
Expand All @@ -41,7 +37,7 @@ type RunnableTestServer interface {
}

func (c *Config) Complete() (RunnableTestServer, error) {
nsm, err := namespace.NewCachingNamespaceManager(nsCacheExpiration, nil)
nsm, err := namespace.NewCachingNamespaceManager(nil)
if err != nil {
return nil, fmt.Errorf("failed to initialize namespace manager: %w", err)
}
Expand Down
5 changes: 1 addition & 4 deletions pkg/development/devcontext.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,7 @@ func NewDevContext(ctx context.Context, developerRequestContext *v0.RequestConte
ctx = datastoremw.ContextWithDatastore(ctx, ds)

// Instantiate the namespace manager with *no caching*.
nsm, err := namespace.NewCachingNamespaceManager(0*time.Second, nil)
if err != nil {
return nil, nil, err
}
nsm := namespace.NewNonCachingNamespaceManager()

dctx, devErrs, nerr := newDevContextWithDatastore(ctx, developerRequestContext, ds, nsm)
if nerr != nil || devErrs != nil {
Expand Down

0 comments on commit 51297ca

Please sign in to comment.