Skip to content

Commit

Permalink
datastore: remove NamespaceCacheKey method
Browse files Browse the repository at this point in the history
  • Loading branch information
jakedt committed May 4, 2022
1 parent 5495cf2 commit d36f2fd
Show file tree
Hide file tree
Showing 14 changed files with 6 additions and 142 deletions.
4 changes: 0 additions & 4 deletions internal/datastore/crdb/crdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,10 +186,6 @@ func (cds *crdbDatastore) IsReady(ctx context.Context) (bool, error) {
return version == headMigration, nil
}

func (cds *crdbDatastore) NamespaceCacheKey(namespaceName string, revision datastore.Revision) (string, error) {
return fmt.Sprintf("%s@%s", namespaceName, revision), nil
}

func (cds *crdbDatastore) Close() error {
cds.conn.Close()
return nil
Expand Down
3 changes: 0 additions & 3 deletions internal/datastore/datastore.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,6 @@ type Reader interface {

// ListNamespaces lists all namespaces defined.
ListNamespaces(ctx context.Context) ([]*core.NamespaceDefinition, error)

// NamespaceCacheKey returns a string key for use in a NamespaceManager's cache
NamespaceCacheKey(namespaceName string) (string, error)
}

type ReadWriteTransaction interface {
Expand Down
12 changes: 0 additions & 12 deletions internal/datastore/memdb/readonly.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,18 +198,6 @@ func (rot *memdbReader) CheckRevision(ctx context.Context) error {
return nil
}

func (rot *memdbReader) NamespaceCacheKey(namespaceName string) (string, error) {
if rot.initErr != nil {
return "", rot.initErr
}

if rot.revision.IsZero() {
return "", errors.New("unable to call namespace cache key without snapshot")
}

return fmt.Sprintf("%s@%s", namespaceName, rot.revision), nil
}

func iteratorForFilter(txn *memdb.Txn, filter *v1.RelationshipFilter) (memdb.ResultIterator, error) {
switch {
case filter.OptionalResourceId != "":
Expand Down
5 changes: 0 additions & 5 deletions internal/datastore/mysql/namespace.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,6 @@ const (
errUnableToListNamespaces = "unable to list namespaces: %w"
)

func (mds *Datastore) NamespaceCacheKey(namespaceName string, revision datastore.Revision) (string, error) {
// TODO (@vroldanbet) dupe from postgres datastore - need to refactor
return fmt.Sprintf("%s@%s", namespaceName, revision), nil
}

func (mds *Datastore) WriteNamespace(ctx context.Context, newNamespace *core.NamespaceDefinition) (datastore.Revision, error) {
// TODO (@vroldanbet) dupe from postgres datastore - need to refactor
ctx = datastore.SeparateContextWithTracing(ctx)
Expand Down
4 changes: 0 additions & 4 deletions internal/datastore/postgres/postgres.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,6 @@ type pgDatastore struct {
cancelGc context.CancelFunc
}

func (pgd *pgDatastore) NamespaceCacheKey(namespaceName string, revision datastore.Revision) (string, error) {
return fmt.Sprintf("%s@%s", namespaceName, revision), nil
}

func (pgd *pgDatastore) Close() error {
pgd.cancelGc()

Expand Down
40 changes: 0 additions & 40 deletions internal/datastore/proxy/cachekey.go

This file was deleted.

6 changes: 2 additions & 4 deletions internal/datastore/proxy/caching.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,12 @@ func (r *nsCachingReader) ReadNamespace(
nsName string,
) (*core.NamespaceDefinition, datastore.Revision, error) {
// Check the nsCache.
nsRevisionKey, err := r.NamespaceCacheKey(nsName)
if err != nil {
return nil, datastore.NoRevision, err
}
nsRevisionKey := fmt.Sprintf("%s@%s", nsName, r.rev)

loadedRaw, found := r.p.c.Get(nsRevisionKey)
if !found {
// We couldn't use the cached entry, load one
var err error
loadedRaw, err, _ = r.p.readNsGroup.Do(nsRevisionKey, func() (interface{}, error) {
loaded, updatedRev, err := r.Reader.ReadNamespace(ctx, nsName)
if err != nil && !errors.Is(err, &datastore.ErrNamespaceNotFound{}) {
Expand Down
10 changes: 3 additions & 7 deletions internal/datastore/proxy/caching_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ import (
"testing"
"time"

"github.com/authzed/spicedb/internal/datastore"
"github.com/authzed/spicedb/internal/datastore/proxy/proxy_test"
"github.com/shopspring/decimal"
"github.com/stretchr/testify/require"
"golang.org/x/sync/errgroup"

"github.com/authzed/spicedb/internal/datastore"
"github.com/authzed/spicedb/internal/datastore/proxy/proxy_test"
)

var (
Expand All @@ -31,15 +32,11 @@ func TestSnapshotNamespaceCaching(t *testing.T) {
dsMock.On("SnapshotReader", one).Return(oneReader)
oneReader.On("ReadNamespace", nsA).Return(nil, old, nil).Once()
oneReader.On("ReadNamespace", nsB).Return(nil, zero, nil).Once()
oneReader.On("NamespaceCacheKey", nsA).Return("nsA@1", nil)
oneReader.On("NamespaceCacheKey", nsB).Return("nsB@1", nil)

twoReader := &proxy_test.MockReader{}
dsMock.On("SnapshotReader", two).Return(twoReader)
twoReader.On("ReadNamespace", nsA).Return(nil, zero, nil).Once()
twoReader.On("ReadNamespace", nsB).Return(nil, one, nil).Once()
twoReader.On("NamespaceCacheKey", nsA).Return("nsA@2", nil)
twoReader.On("NamespaceCacheKey", nsB).Return("nsB@2", nil)

require := require.New(t)
ctx := context.Background()
Expand Down Expand Up @@ -127,7 +124,6 @@ func TestSingleFlight(t *testing.T) {
WaitUntil(time.After(10*time.Millisecond)).
Return(nil, old, nil).
Once()
oneReader.On("NamespaceCacheKey", nsA).Return("nsA@1", nil)

require := require.New(t)
ctx := context.Background()
Expand Down
10 changes: 0 additions & 10 deletions internal/datastore/proxy/proxy_test/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,6 @@ func (dm *MockReader) ListNamespaces(ctx context.Context) ([]*core.NamespaceDefi
return args.Get(0).([]*core.NamespaceDefinition), args.Error(1)
}

func (dm *MockReader) NamespaceCacheKey(namespaceName string) (string, error) {
args := dm.Called(namespaceName)
return args.String(0), args.Error(1)
}

type MockReadWriteTransaction struct {
mock.Mock
}
Expand Down Expand Up @@ -200,11 +195,6 @@ func (dm *MockReadWriteTransaction) ListNamespaces(ctx context.Context) ([]*core
return args.Get(0).([]*core.NamespaceDefinition), args.Error(1)
}

func (dm *MockReadWriteTransaction) NamespaceCacheKey(namespaceName string) (string, error) {
args := dm.Called(namespaceName)
return args.String(0), args.Error(1)
}

func (dm *MockReadWriteTransaction) WriteRelationships(mutations []*v1.RelationshipUpdate) error {
args := dm.Called(mutations)
return args.Error(0)
Expand Down
4 changes: 0 additions & 4 deletions internal/datastore/spanner/namespace.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,3 @@ func readAllNamespaces(iter *spanner.RowIterator) ([]*core.NamespaceDefinition,

return allNamespaces, nil
}

func (sd spannerDatastore) NamespaceCacheKey(namespaceName string, revision datastore.Revision) (string, error) {
return fmt.Sprintf("%s@%s", namespaceName, revision), nil
}
1 change: 0 additions & 1 deletion internal/datastore/test/datastore.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ func All(t *testing.T, tester DatastoreTester) {
t.Run("TestNamespaceWrite", func(t *testing.T) { NamespaceWriteTest(t, tester) })
t.Run("TestNamespaceDelete", func(t *testing.T) { NamespaceDeleteTest(t, tester) })
t.Run("TestEmptyNamespaceDelete", func(t *testing.T) { EmptyNamespaceDeleteTest(t, tester) })
t.Run("TestNamespaceCacheKey", func(t *testing.T) { NamespaceCacheKeyTest(t, tester) })

t.Run("TestSimple", func(t *testing.T) { SimpleTest(t, tester) })
t.Run("TestDeleteRelationships", func(t *testing.T) { DeleteRelationshipsTest(t, tester) })
Expand Down
38 changes: 0 additions & 38 deletions internal/datastore/test/namespace.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,41 +176,3 @@ func EmptyNamespaceDeleteTest(t *testing.T, tester DatastoreTester) {
_, _, err = ds.SnapshotReader(deletedRev).ReadNamespace(ctx, testfixtures.UserNS.Name)
require.True(errors.As(err, &datastore.ErrNamespaceNotFound{}))
}

// NamespaceCacheKeyTest tests that the datastore returns namespace cache keys that make sense.
func NamespaceCacheKeyTest(t *testing.T, tester DatastoreTester) {
require := require.New(t)

rawDS, err := tester.New(0, veryLargeGCWindow, 1)
require.NoError(err)

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

key, err := ds.SnapshotReader(revision).NamespaceCacheKey(testfixtures.DocumentNS.Name)
require.NoError(err)
require.NotEmpty(key)

sameKey, err := ds.SnapshotReader(revision).NamespaceCacheKey(testfixtures.DocumentNS.Name)
require.NoError(err)
require.Equal(key, sameKey)

diffNamespace, err := ds.SnapshotReader(revision).NamespaceCacheKey(testfixtures.FolderNS.Name)
require.NoError(err)
require.NotEmpty(diffNamespace)
require.NotEqual(key, diffNamespace)

newRevision, err := ds.ReadWriteTx(context.Background(), func(ctx context.Context, rwt datastore.ReadWriteTransaction) error {
_, err = rwt.NamespaceCacheKey(testfixtures.DocumentNS.Name)
require.Error(err, "should not be able to get the cache key inside of a read-write transaction")

err := rwt.WriteNamespace(testfixtures.DocumentNS)
require.NoError(err)
return err
})
require.NoError(err)

diffRevision, err := ds.SnapshotReader(newRevision).NamespaceCacheKey(testfixtures.DocumentNS.Name)
require.NoError(err)
require.NotEmpty(diffRevision)
require.NotEqual(key, diffRevision)
}
4 changes: 1 addition & 3 deletions internal/middleware/pertoken/pertoken.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (

"github.com/authzed/spicedb/internal/datastore"
"github.com/authzed/spicedb/internal/datastore/memdb"
"github.com/authzed/spicedb/internal/datastore/proxy"
datastoremw "github.com/authzed/spicedb/internal/middleware/datastore"
"github.com/authzed/spicedb/pkg/validationfile"
)
Expand Down Expand Up @@ -57,8 +56,7 @@ func (m *MiddlewareForTesting) getOrCreateDatastore(ctx context.Context) (datast
return nil, fmt.Errorf("failed to load config files: %w", err)
}

prefixedDS := proxy.NewCacheKeyPrefixProxy(ds, "tokenStr")
m.datastoreByToken.Store(tokenStr, prefixedDS)
m.datastoreByToken.Store(tokenStr, ds)

return ds, nil
}
Expand Down
7 changes: 0 additions & 7 deletions internal/testfixtures/validating.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,13 +144,6 @@ func (vsr validatingSnapshotReader) ReverseQueryRelationships(ctx context.Contex
return vsr.delegate.ReverseQueryRelationships(ctx, subjectFilter, opts...)
}

func (vsr validatingSnapshotReader) NamespaceCacheKey(namespaceName string) (string, error) {
if namespaceName == "" {
return "", errors.New("NamespaceCacheKey called with empty namespace name")
}
return vsr.delegate.NamespaceCacheKey(namespaceName)
}

type validatingReadWriteTransaction struct {
validatingSnapshotReader
delegate datastore.ReadWriteTransaction
Expand Down

0 comments on commit d36f2fd

Please sign in to comment.