Skip to content

Commit

Permalink
Merge pull request authzed#289 from josephschorr/cached-dispatch-count
Browse files Browse the repository at this point in the history 
Add proper dispatch and cached dispatch tracking
  • Loading branch information
@jakedt
jakedt authored Nov 23, 2021
2 parents a75cd64 + 7251d77 commit 6600338
Show file tree
Hide file tree
Showing 11 changed files with 341 additions and 212 deletions.
40 changes: 23 additions & 17 deletions internal/dispatch/caching/caching.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/dgraph-io/ristretto"
"github.com/prometheus/client_golang/prometheus"
"github.com/rs/zerolog/log"
"google.golang.org/protobuf/proto"

"github.com/authzed/spicedb/internal/dispatch"
v1 "github.com/authzed/spicedb/internal/proto/dispatch/v1"
Expand All @@ -30,13 +31,11 @@ type CachingDispatcher struct {
}

type checkResultEntry struct {
result *v1.DispatchCheckResponse
depthRequired uint32
response *v1.DispatchCheckResponse
}

type lookupResultEntry struct {
result *v1.DispatchLookupResponse
depthRequired uint32
response *v1.DispatchLookupResponse
}

var (
Expand Down Expand Up @@ -154,18 +153,21 @@ func (cd *CachingDispatcher) DispatchCheck(ctx context.Context, req *v1.Dispatch

if cachedResultRaw, found := cd.c.Get(requestKey); found {
cachedResult := cachedResultRaw.(checkResultEntry)
if req.Metadata.DepthRemaining >= cachedResult.depthRequired {
if req.Metadata.DepthRemaining >= cachedResult.response.Metadata.DepthRequired {
cd.checkFromCacheCounter.Inc()
return cachedResult.result, nil
return cachedResult.response, nil
}
}

computed, err := cd.d.DispatchCheck(ctx, req)

// We only want to cache the result if there was no error
if err == nil {
toCache := checkResultEntry{computed, computed.Metadata.DepthRequired}
toCache.result.Metadata.DispatchCount = 0
adjustedComputed := proto.Clone(computed).(*v1.DispatchCheckResponse)
adjustedComputed.Metadata.CachedDispatchCount = adjustedComputed.Metadata.DispatchCount
adjustedComputed.Metadata.DispatchCount = 0

toCache := checkResultEntry{adjustedComputed}
cd.c.Set(requestKey, toCache, checkResultEntryCost)
}

Expand All @@ -176,7 +178,8 @@ func (cd *CachingDispatcher) DispatchCheck(ctx context.Context, req *v1.Dispatch

// DispatchExpand implements dispatch.Expand interface and does not do any caching yet.
func (cd *CachingDispatcher) DispatchExpand(ctx context.Context, req *v1.DispatchExpandRequest) (*v1.DispatchExpandResponse, error) {
return cd.d.DispatchExpand(ctx, req)
resp, err := cd.d.DispatchExpand(ctx, req)
return resp, err
}

// DispatchLookup implements dispatch.Lookup interface and does not do any caching yet.
Expand All @@ -186,10 +189,10 @@ func (cd *CachingDispatcher) DispatchLookup(ctx context.Context, req *v1.Dispatc
requestKey := dispatch.LookupRequestToKey(req)
if cachedResultRaw, found := cd.c.Get(requestKey); found {
cachedResult := cachedResultRaw.(lookupResultEntry)
if req.Metadata.DepthRemaining >= cachedResult.depthRequired {
log.Trace().Object("using cached lookup", req).Int("result count", len(cachedResult.result.ResolvedOnrs)).Send()
if req.Metadata.DepthRemaining >= cachedResult.response.Metadata.DepthRequired {
log.Trace().Object("using cached lookup", req).Int("result count", len(cachedResult.response.ResolvedOnrs)).Send()
cd.lookupFromCacheCounter.Inc()
return cachedResult.result, nil
return cachedResult.response, nil
}
}

Expand All @@ -199,14 +202,17 @@ func (cd *CachingDispatcher) DispatchLookup(ctx context.Context, req *v1.Dispatc
if err == nil && len(computed.Metadata.LookupExcludedDirect) == 0 && len(computed.Metadata.LookupExcludedTtu) == 0 {
log.Trace().Object("caching lookup", req).Int("result count", len(computed.ResolvedOnrs)).Send()

adjustedComputed := proto.Clone(computed).(*v1.DispatchLookupResponse)
adjustedComputed.Metadata.CachedDispatchCount = adjustedComputed.Metadata.DispatchCount
adjustedComputed.Metadata.DispatchCount = 0
adjustedComputed.Metadata.LookupExcludedDirect = nil
adjustedComputed.Metadata.LookupExcludedTtu = nil

requestKey := dispatch.LookupRequestToKey(req)
toCache := lookupResultEntry{computed, computed.Metadata.DepthRequired}
toCache.result.Metadata.DispatchCount = 0
toCache.result.Metadata.LookupExcludedDirect = nil
toCache.result.Metadata.LookupExcludedTtu = nil
toCache := lookupResultEntry{adjustedComputed}

estimatedSize := lookupResultEntryEmptyCost
for _, onr := range toCache.result.ResolvedOnrs {
for _, onr := range toCache.response.ResolvedOnrs {
estimatedSize += int64(len(onr.Namespace) + len(onr.ObjectId) + len(onr.Relation))
}

Expand Down
96 changes: 95 additions & 1 deletion internal/dispatch/graph/check_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func TestSimple(t *testing.T) {
for _, userset := range tc.usersets {
for _, expected := range userset.expected {
name := fmt.Sprintf(
"%s:%s#%s@%s:%s#%s=>%t",
"simple:%s:%s#%s@%s:%s#%s=>%t",
tc.namespace,
tc.objectID,
expected.relation,
Expand Down Expand Up @@ -197,6 +197,100 @@ func TestMaxDepth(t *testing.T) {
require.Equal(v1.DispatchCheckResponse_UNKNOWN, checkResult.Membership)
}

func TestCheckMetadata(t *testing.T) {
type expected struct {
relation string
isMember bool
expectedDispatchCount int
expectedDepthRequired int
}

type userset struct {
userset *v0.ObjectAndRelation
expected []expected
}

testCases := []struct {
namespace string
objectID string
usersets []userset
}{
{"document", "masterplan", []userset{
{
ONR("user", "product_manager", graph.Ellipsis),
[]expected{
{"owner", true, 1, 1},
{"editor", true, 2, 2},
{"viewer", true, 3, 3},
},
},
{
ONR("user", "owner", graph.Ellipsis),
[]expected{
{"owner", false, 1, 1},
{"editor", false, 2, 2},
{"viewer", true, 5, 5},
},
},
}},
{"folder", "strategy", []userset{
{
ONR("user", "vp_product", graph.Ellipsis),
[]expected{
{"owner", true, 1, 1},
{"editor", true, 2, 2},
{"viewer", true, 3, 3},
},
},
}},
{"folder", "company", []userset{
{
ONR("user", "unknown", graph.Ellipsis),
[]expected{
{"viewer", false, 6, 4},
},
},
}},
}

for _, tc := range testCases {
for _, userset := range tc.usersets {
for _, expected := range userset.expected {
name := fmt.Sprintf(
"metadata:%s:%s#%s@%s:%s#%s=>%t",
tc.namespace,
tc.objectID,
expected.relation,
userset.userset.Namespace,
userset.userset.ObjectId,
userset.userset.Relation,
expected.isMember,
)

t.Run(name, func(t *testing.T) {
require := require.New(t)

dispatch, revision := newLocalDispatcher(require)

checkResult, err := dispatch.DispatchCheck(context.Background(), &v1.DispatchCheckRequest{
ObjectAndRelation: ONR(tc.namespace, tc.objectID, expected.relation),
Subject: userset.userset,
Metadata: &v1.ResolverMeta{
AtRevision: revision.String(),
DepthRemaining: 50,
},
})

require.NoError(err)
require.Equal(expected.isMember, checkResult.Membership == v1.DispatchCheckResponse_MEMBER)
require.Equal(expected.expectedDispatchCount, int(checkResult.Metadata.DispatchCount))
require.Equal(expected.expectedDepthRequired, int(checkResult.Metadata.DepthRequired))
})
}
}
}
}

func newLocalDispatcher(require *require.Assertions) (dispatch.Dispatcher, decimal.Decimal) {
rawDS, err := memdb.NewMemdbDatastore(0, 0, memdb.DisableGC, 0)
require.NoError(err)
Expand Down
38 changes: 21 additions & 17 deletions internal/dispatch/graph/expand_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,24 +114,26 @@ var (

func TestExpand(t *testing.T) {
testCases := []struct {
start *v0.ObjectAndRelation
expansionMode v1.DispatchExpandRequest_ExpansionMode
expected *v0.RelationTupleTreeNode
start *v0.ObjectAndRelation
expansionMode v1.DispatchExpandRequest_ExpansionMode
expected *v0.RelationTupleTreeNode
expectedDispatchCount int
expectedDepthRequired int
}{
{start: ONR("folder", "company", "owner"), expansionMode: v1.DispatchExpandRequest_SHALLOW, expected: companyOwner},
{start: ONR("folder", "company", "editor"), expansionMode: v1.DispatchExpandRequest_SHALLOW, expected: companyEditor},
{start: ONR("folder", "company", "viewer"), expansionMode: v1.DispatchExpandRequest_SHALLOW, expected: companyViewer},
{start: ONR("document", "masterplan", "owner"), expansionMode: v1.DispatchExpandRequest_SHALLOW, expected: docOwner},
{start: ONR("document", "masterplan", "editor"), expansionMode: v1.DispatchExpandRequest_SHALLOW, expected: docEditor},
{start: ONR("document", "masterplan", "viewer"), expansionMode: v1.DispatchExpandRequest_SHALLOW, expected: docViewer},

{start: ONR("folder", "auditors", "owner"), expansionMode: v1.DispatchExpandRequest_RECURSIVE, expected: auditorsOwner},
{start: ONR("folder", "auditors", "editor"), expansionMode: v1.DispatchExpandRequest_RECURSIVE, expected: auditorsEditor},
{start: ONR("folder", "auditors", "viewer"), expansionMode: v1.DispatchExpandRequest_RECURSIVE, expected: auditorsViewerRecursive},

{start: ONR("folder", "company", "owner"), expansionMode: v1.DispatchExpandRequest_RECURSIVE, expected: companyOwner},
{start: ONR("folder", "company", "editor"), expansionMode: v1.DispatchExpandRequest_RECURSIVE, expected: companyEditor},
{start: ONR("folder", "company", "viewer"), expansionMode: v1.DispatchExpandRequest_RECURSIVE, expected: companyViewerRecursive},
{start: ONR("folder", "company", "owner"), expansionMode: v1.DispatchExpandRequest_SHALLOW, expected: companyOwner, expectedDispatchCount: 1, expectedDepthRequired: 1},
{start: ONR("folder", "company", "editor"), expansionMode: v1.DispatchExpandRequest_SHALLOW, expected: companyEditor, expectedDispatchCount: 2, expectedDepthRequired: 2},
{start: ONR("folder", "company", "viewer"), expansionMode: v1.DispatchExpandRequest_SHALLOW, expected: companyViewer, expectedDispatchCount: 3, expectedDepthRequired: 3},
{start: ONR("document", "masterplan", "owner"), expansionMode: v1.DispatchExpandRequest_SHALLOW, expected: docOwner, expectedDispatchCount: 1, expectedDepthRequired: 1},
{start: ONR("document", "masterplan", "editor"), expansionMode: v1.DispatchExpandRequest_SHALLOW, expected: docEditor, expectedDispatchCount: 2, expectedDepthRequired: 2},
{start: ONR("document", "masterplan", "viewer"), expansionMode: v1.DispatchExpandRequest_SHALLOW, expected: docViewer, expectedDispatchCount: 12, expectedDepthRequired: 5},

{start: ONR("folder", "auditors", "owner"), expansionMode: v1.DispatchExpandRequest_RECURSIVE, expected: auditorsOwner, expectedDispatchCount: 1, expectedDepthRequired: 1},
{start: ONR("folder", "auditors", "editor"), expansionMode: v1.DispatchExpandRequest_RECURSIVE, expected: auditorsEditor, expectedDispatchCount: 2, expectedDepthRequired: 2},
{start: ONR("folder", "auditors", "viewer"), expansionMode: v1.DispatchExpandRequest_RECURSIVE, expected: auditorsViewerRecursive, expectedDispatchCount: 3, expectedDepthRequired: 3},

{start: ONR("folder", "company", "owner"), expansionMode: v1.DispatchExpandRequest_RECURSIVE, expected: companyOwner, expectedDispatchCount: 1, expectedDepthRequired: 1},
{start: ONR("folder", "company", "editor"), expansionMode: v1.DispatchExpandRequest_RECURSIVE, expected: companyEditor, expectedDispatchCount: 2, expectedDepthRequired: 2},
{start: ONR("folder", "company", "viewer"), expansionMode: v1.DispatchExpandRequest_RECURSIVE, expected: companyViewerRecursive, expectedDispatchCount: 6, expectedDepthRequired: 4},
}

for _, tc := range testCases {
Expand All @@ -152,6 +154,8 @@ func TestExpand(t *testing.T) {
require.NoError(err)
require.NotNil(expandResult.TreeNode)
require.GreaterOrEqual(expandResult.Metadata.DepthRequired, uint32(1))
require.Equal(tc.expectedDispatchCount, int(expandResult.Metadata.DispatchCount))
require.Equal(tc.expectedDepthRequired, int(expandResult.Metadata.DepthRequired))

if diff := cmp.Diff(tc.expected, expandResult.TreeNode, protocmp.Transform()); diff != "" {
fset := token.NewFileSet()
Expand Down
27 changes: 24 additions & 3 deletions internal/dispatch/graph/lookup_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,28 +36,36 @@ func init() {

func TestSimpleLookup(t *testing.T) {
testCases := []struct {
start *v0.RelationReference
target *v0.ObjectAndRelation
resolvedObjects []*v0.ObjectAndRelation
start *v0.RelationReference
target *v0.ObjectAndRelation
resolvedObjects []*v0.ObjectAndRelation
expectedDispatchCount int
expectedDepthRequired int
}{
{
RR("document", "viewer"),
ONR("user", "unknown", "..."),
[]*v0.ObjectAndRelation{},
9,
5,
},
{
RR("document", "viewer"),
ONR("user", "eng_lead", "..."),
[]*v0.ObjectAndRelation{
ONR("document", "masterplan", "viewer"),
},
18,
6,
},
{
RR("document", "owner"),
ONR("user", "product_manager", "..."),
[]*v0.ObjectAndRelation{
ONR("document", "masterplan", "owner"),
},
2,
2,
},
{
RR("document", "viewer"),
Expand All @@ -66,13 +74,17 @@ func TestSimpleLookup(t *testing.T) {
ONR("document", "companyplan", "viewer"),
ONR("document", "masterplan", "viewer"),
},
48,
7,
},
{
RR("document", "viewer_and_editor"),
ONR("user", "multiroleguy", "..."),
[]*v0.ObjectAndRelation{
ONR("document", "specialplan", "viewer_and_editor"),
},
8,
4,
},
{
RR("folder", "viewer"),
Expand All @@ -81,6 +93,8 @@ func TestSimpleLookup(t *testing.T) {
ONR("folder", "strategy", "viewer"),
ONR("folder", "company", "viewer"),
},
33,
6,
},
}

Expand Down Expand Up @@ -111,6 +125,10 @@ func TestSimpleLookup(t *testing.T) {

require.NoError(err)
require.ElementsMatch(tc.resolvedObjects, lookupResult.ResolvedOnrs)
require.GreaterOrEqual(lookupResult.Metadata.DepthRequired, uint32(1))
require.Equal(tc.expectedDispatchCount, int(lookupResult.Metadata.DispatchCount))
require.Equal(0, int(lookupResult.Metadata.CachedDispatchCount))
require.Equal(tc.expectedDepthRequired, int(lookupResult.Metadata.DepthRequired))

// We have to sleep a while to let the cache converge:
// https://github.com/dgraph-io/ristretto/blob/01b9f37dd0fd453225e042d6f3a27cd14f252cd0/cache_test.go#L17
Expand All @@ -132,6 +150,9 @@ func TestSimpleLookup(t *testing.T) {
require.NoError(err)
require.ElementsMatch(tc.resolvedObjects, lookupResult.ResolvedOnrs)
require.GreaterOrEqual(lookupResult.Metadata.DepthRequired, uint32(1))
require.Equal(0, int(lookupResult.Metadata.DispatchCount))
require.Equal(tc.expectedDispatchCount, int(lookupResult.Metadata.CachedDispatchCount))
require.Equal(tc.expectedDepthRequired, int(lookupResult.Metadata.DepthRequired))
})
}
}
Expand Down
Loading

0 comments on commit 6600338

Please sign in to comment.