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 Jul 30, 2024
1 parent dfb450a commit 35fbc6f
Show file tree
Hide file tree
Showing 10 changed files with 133 additions and 105 deletions.
22 changes: 16 additions & 6 deletions internal/middleware/consistency/consistency.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,24 @@ func RevisionFromContext(ctx context.Context) (datastore.Revision, *v1.ZedToken,
return nil, nil, fmt.Errorf("consistency middleware did not inject revision")
}

// MismatchingTokenOption is the option specifying the behavior of the consistency middleware
// when a ZedToken provided references a different datastore instance than the current
// datastore instance.
type MismatchingTokenOption int

const (
// TreatMismatchingTokensAsFullConsistency specifies that the middleware should treat
// a ZedToken that references a different datastore instance as a request for full
// consistency.
TreatMismatchingTokensAsFullConsistency MismatchingTokenOption = iota

// TreatMismatchingTokensAsMinLatency specifies that the middleware should treat
// a ZedToken that references a different datastore instance as a request for min
// latency.
TreatMismatchingTokensAsMinLatency

// TreatMismatchingTokensAsError specifies that the middleware should raise an error
// when a ZedToken that references a different datastore instance is provided.
TreatMismatchingTokensAsError
)

Expand Down Expand Up @@ -174,8 +185,7 @@ func addRevisionToContextFromConsistency(ctx context.Context, req hasConsistency
}

if status == zedtoken.StatusMismatchedDatastoreID {
log.Error().Str("zedtoken", consistency.GetAtExactSnapshot().Token).Msg("ZedToken specified references an older datastore but at-exact-snapshot was requested")
return fmt.Errorf("ZedToken specified references an older datastore but at-exact-snapshot was requested")
return fmt.Errorf("ZedToken specified references a different datastore instance but at-exact-snapshot was requested")
}

err = ds.CheckRevision(ctx, requestedRev)
Expand Down Expand Up @@ -267,7 +277,7 @@ func pickBestRevision(ctx context.Context, requested *v1.ZedToken, ds datastore.
if status == zedtoken.StatusMismatchedDatastoreID {
switch option {
case TreatMismatchingTokensAsFullConsistency:
log.Warn().Str("zedtoken", requested.Token).Msg("ZedToken specified references an older datastore and SpiceDB is configured to treat this as a full consistency request")
log.Warn().Str("zedtoken", requested.Token).Msg("ZedToken specified references a different datastore instance and SpiceDB is configured to treat this as a full consistency request")
headRev, err := ds.HeadRevision(ctx)
if err != nil {
return datastore.NoRevision, false, err
Expand All @@ -276,12 +286,12 @@ func pickBestRevision(ctx context.Context, requested *v1.ZedToken, ds datastore.
return headRev, false, nil

case TreatMismatchingTokensAsMinLatency:
log.Warn().Str("zedtoken", requested.Token).Msg("ZedToken specified references an older datastore and SpiceDB is configured to treat this as a min latency request")
log.Warn().Str("zedtoken", requested.Token).Msg("ZedToken specified references a different datastore instance and SpiceDB is configured to treat this as a min latency request")
return databaseRev, false, nil

case TreatMismatchingTokensAsError:
log.Error().Str("zedtoken", requested.Token).Msg("ZedToken specified references an older datastore and SpiceDB is configured to raise an error in this scenario")
return datastore.NoRevision, false, fmt.Errorf("ZedToken specified references an older datastore and SpiceDB is configured to raise an error in this scenario")
log.Warn().Str("zedtoken", requested.Token).Msg("ZedToken specified references a different datastore instance and SpiceDB is configured to raise an error in this scenario")
return datastore.NoRevision, false, fmt.Errorf("ZedToken specified references a different datastore instance and SpiceDB is configured to raise an error in this scenario")

default:
return datastore.NoRevision, false, spiceerrors.MustBugf("unknown mismatching token option: %v", option)
Expand Down
12 changes: 6 additions & 6 deletions internal/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, 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, 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
7 changes: 6 additions & 1 deletion internal/services/v1/watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,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
18 changes: 9 additions & 9 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.

Loading

0 comments on commit 35fbc6f

Please sign in to comment.