Skip to content

Commit

Permalink
Change to only store a prefix of the datastore's unique ID in the zed…
Browse files Browse the repository at this point in the history
…token

Results in smaller tokens but given that datastore IDs are generated, should still minimize the chances of a conflict
  • Loading branch information
josephschorr committed Jan 17, 2025
1 parent 0dc015b commit 7782a0e
Show file tree
Hide file tree
Showing 9 changed files with 116 additions and 98 deletions.
2 changes: 1 addition & 1 deletion internal/services/v1/debug_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -906,7 +906,7 @@ func TestBulkCheckPermissionWithDebug(t *testing.T) {
checkResp, err := client.CheckBulkPermissions(ctx, &v1.CheckBulkPermissionsRequest{
Consistency: &v1.Consistency{
Requirement: &v1.Consistency_AtLeastAsFresh{
AtLeastAsFresh: zedtoken.MustNewFromRevision(revision),
AtLeastAsFresh: zedtoken.MustNewFromRevisionForTesting(revision),
},
},
WithTracing: true,
Expand Down
7 changes: 6 additions & 1 deletion internal/services/v1/watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,13 @@ func (ws *watchServer) Watch(req *v1.WatchRequest, stream v1.WatchService_WatchS
return status.Errorf(codes.InvalidArgument, "failed to decode start revision: %s", err)
}

// NOTE: if the token given is for a different datastore, we return an error and immediately fail the watch.
// This is necessary because asking for a resumed watch on a *different* datastore instance makes little semantic
// sense and, if we allowed it here, would likely result in missing or wrong events being emitted. This is the
// safest option and should only cause an issue if a client is connecting between instances of SpiceDBs which are,
// themselves, connecting to different datastore instances, which should itself be an invalid configuration.
if tokenStatus == zedtoken.StatusMismatchedDatastoreID {
return status.Errorf(codes.InvalidArgument, "start revision was generated by a different datastore")
return status.Errorf(codes.InvalidArgument, "start revision was generated by a different datastore instance")
}

afterRevision = decodedRevision
Expand Down
12 changes: 6 additions & 6 deletions pkg/middleware/consistency/consistency_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ func TestAtExactSnapshotWithMismatchedToken(t *testing.T) {
updated := ContextWithHandle(context.Background())
updated = datastoremw.ContextWithDatastore(updated, ds)

// mint a token with a different datastore ID.
// mint a token with a different datastore instance ID.
ds.CurrentUniqueID = "foo"
zedToken, err := zedtoken.NewFromRevision(context.Background(), optimized, ds)
require.NoError(err)
Expand All @@ -234,7 +234,7 @@ func TestAtExactSnapshotWithMismatchedToken(t *testing.T) {
},
}, ds, "somelabel", TreatMismatchingTokensAsError)
require.Error(err)
require.ErrorContains(err, "ZedToken specified references an older datastore but at-exact-snapshot")
require.ErrorContains(err, "ZedToken specified references a different datastore instance but at-exact-snapshot")
}

func TestAtLeastAsFreshWithMismatchedTokenExpectError(t *testing.T) {
Expand All @@ -248,7 +248,7 @@ func TestAtLeastAsFreshWithMismatchedTokenExpectError(t *testing.T) {
updated := ContextWithHandle(context.Background())
updated = datastoremw.ContextWithDatastore(updated, ds)

// mint a token with a different datastore ID.
// mint a token with a different datastore instance ID.
ds.CurrentUniqueID = "foo"
zedToken, err := zedtoken.NewFromRevision(context.Background(), optimized, ds)
require.NoError(err)
Expand All @@ -262,7 +262,7 @@ func TestAtLeastAsFreshWithMismatchedTokenExpectError(t *testing.T) {
},
}, ds, "somelabel", TreatMismatchingTokensAsError)
require.Error(err)
require.ErrorContains(err, "ZedToken specified references an older datastore and SpiceDB is configured to raise an error in this scenario")
require.ErrorContains(err, "ZedToken specified references a different datastore instance and SpiceDB is configured to raise an error in this scenario")
}

func TestAtLeastAsFreshWithMismatchedTokenExpectMinLatency(t *testing.T) {
Expand All @@ -276,7 +276,7 @@ func TestAtLeastAsFreshWithMismatchedTokenExpectMinLatency(t *testing.T) {
updated := ContextWithHandle(context.Background())
updated = datastoremw.ContextWithDatastore(updated, ds)

// mint a token with a different datastore ID.
// mint a token with a different datastore instance ID.
ds.CurrentUniqueID = "foo"
zedToken, err := zedtoken.NewFromRevision(context.Background(), optimized, ds)
require.NoError(err)
Expand Down Expand Up @@ -310,7 +310,7 @@ func TestAtLeastAsFreshWithMismatchedTokenExpectFullConsistency(t *testing.T) {
updated := ContextWithHandle(context.Background())
updated = datastoremw.ContextWithDatastore(updated, ds)

// mint a token with a different datastore ID.
// mint a token with a different datastore instance ID.
ds.CurrentUniqueID = "foo"
zedToken, err := zedtoken.NewFromRevision(context.Background(), optimized, ds)
require.NoError(err)
Expand Down
137 changes: 69 additions & 68 deletions pkg/proto/impl/v1/impl.pb.go

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

2 changes: 1 addition & 1 deletion pkg/proto/impl/v1/impl.pb.validate.go

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

Loading

0 comments on commit 7782a0e

Please sign in to comment.