Add a non-caching namespace manager

internal/dispatch/graph/check_test.go

@@ -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"
@@ -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)
@@ -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)

internal/dispatch/graph/expand_test.go

@@ -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"
@@ -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)

internal/dispatch/graph/lookup_test.go

@@ -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)

internal/namespace/caching.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"time"
 
 	v0 "github.com/authzed/authzed-go/proto/authzed/api/v0"
 	"github.com/dgraph-io/ristretto"
@@ -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 {
@@ -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 {
@@ -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
@@ -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
 	})
@@ -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() {}

internal/namespace/caching_test.go

@@ -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"
@@ -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)
@@ -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)
@@ -62,3 +62,31 @@ 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)
+
+	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)
+
+	rev, err = ds.WriteNamespace(ctx, &v0.NamespaceDefinition{Name: "test/user", Relation: []*v0.Relation{{Name: "test"}}})
+	require.NoError(t, err)
+	defC, err := cache.ReadNamespace(ctx, "test/user", rev)
+	require.NoError(t, err)
+	require.Equal(t, "test/user", defC.Name)
+	require.EqualValues(t, "test", defC.Relation[0].Name)
+
+	require.Equal(t, def, defB)
+}

internal/namespace/manager.go

@@ -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.
 	//

internal/namespace/typesystem_test.go

@@ -3,7 +3,6 @@ package namespace
 import (
 	"context"
 	"testing"
-	"time"
 
 	v0 "github.com/authzed/authzed-go/proto/authzed/api/v0"
 	"github.com/shopspring/decimal"
@@ -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

internal/services/consistency_test.go

@@ -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())

internal/services/v0/namespace.go

@@ -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"
@@ -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)
 

internal/services/v1alpha1/schema.go

@@ -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"),

internal/testserver/server.go

@@ -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
 		}),

pkg/cmd/serve.go

@@ -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")

pkg/cmd/server/server.go

@@ -48,9 +48,6 @@ type Config struct {
 	DatastoreConfig datastorecfg.Config
 	Datastore       datastore.Datastore
 
-	// Namespace cache
-	NamespaceCacheExpiration time.Duration
-
 	// Schema options
 	SchemaPrefixesRequired bool
 
@@ -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)
 	}

pkg/cmd/testserver/testserver.go

@@ -3,7 +3,6 @@ package testserver
 import (
 	"context"
 	"fmt"
-	"time"
 
 	"github.com/rs/zerolog"
 	"github.com/rs/zerolog/log"
@@ -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 {
@@ -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)
 	}

pkg/development/devcontext.go

@@ -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 {