Skip to content

Commit

Permalink
Switch delete relationships API to use the new limit-ed DeleteRelatio…
Browse files Browse the repository at this point in the history
…nships call
  • Loading branch information
josephschorr committed Feb 29, 2024
1 parent 8476881 commit ed90d46
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 26 deletions.
49 changes: 27 additions & 22 deletions internal/services/v1/relationships.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"github.com/authzed/spicedb/pkg/datastore/pagination"
"github.com/authzed/spicedb/pkg/genutil/mapz"
"github.com/authzed/spicedb/pkg/middleware/consistency"
core "github.com/authzed/spicedb/pkg/proto/core/v1"
dispatchv1 "github.com/authzed/spicedb/pkg/proto/dispatch/v1"
"github.com/authzed/spicedb/pkg/tuple"
"github.com/authzed/spicedb/pkg/zedtoken"
Expand Down Expand Up @@ -351,12 +350,19 @@ func (ps *permissionServer) DeleteRelationships(ctx context.Context, req *v1.Del
return err
}

var deleteMutations []*core.RelationTupleUpdate
usagemetrics.SetInContext(ctx, &dispatchv1.ResponseMeta{
// One request per precondition and one request for the actual delete.
DispatchCount: uint32(len(req.OptionalPreconditions)) + 1,
})

if req.OptionalLimit > 0 {
limit := uint64(req.OptionalLimit)
deleteMutations = make([]*core.RelationTupleUpdate, 0, limit)
if err := checkPreconditions(ctx, rwt, req.OptionalPreconditions); err != nil {
return err
}

// If a limit was specified but partial deletion is not allowed, we need to check if the
// number of relationships to be deleted exceeds the limit.
if req.OptionalLimit > 0 && !req.OptionalAllowPartialDeletions {
limit := uint64(req.OptionalLimit)
limitPlusOne := limit + 1
filter := datastore.RelationshipsFilterFromPublicFilter(req.RelationshipFilter)

Expand All @@ -366,38 +372,37 @@ func (ps *permissionServer) DeleteRelationships(ctx context.Context, req *v1.Del
}
defer iter.Close()

counter := 0
for tpl := iter.Next(); tpl != nil; tpl = iter.Next() {
if iter.Err() != nil {
return ps.rewriteError(ctx, err)
}

if len(deleteMutations) == int(limit) {
deletionProgress = v1.DeleteRelationshipsResponse_DELETION_PROGRESS_PARTIAL
if !req.OptionalAllowPartialDeletions {
return ps.rewriteError(ctx, NewCouldNotTransactionallyDeleteErr(req.RelationshipFilter, req.OptionalLimit))
}

break
if counter == int(limit) {
return ps.rewriteError(ctx, NewCouldNotTransactionallyDeleteErr(req.RelationshipFilter, req.OptionalLimit))
}

deleteMutations = append(deleteMutations, tuple.Delete(tpl))
counter++
}
iter.Close()
}

usagemetrics.SetInContext(ctx, &dispatchv1.ResponseMeta{
// One request per precondition and one request for the actual delete.
DispatchCount: uint32(len(req.OptionalPreconditions)) + 1,
})
// Delete with the specified limit.
if req.OptionalLimit > 0 {
deleteLimit := uint64(req.OptionalLimit)
reachedLimit, err := rwt.DeleteRelationships(ctx, req.RelationshipFilter, options.WithDeleteLimit(&deleteLimit))
if err != nil {
return err
}

if err := checkPreconditions(ctx, rwt, req.OptionalPreconditions); err != nil {
return err
}
if reachedLimit {
deletionProgress = v1.DeleteRelationshipsResponse_DELETION_PROGRESS_PARTIAL
}

if len(deleteMutations) > 0 {
return rwt.WriteRelationships(ctx, deleteMutations)
return nil
}

// Otherwise, kick off an unlimited deletion.
_, err := rwt.DeleteRelationships(ctx, req.RelationshipFilter)
return err
})
Expand Down
5 changes: 1 addition & 4 deletions internal/services/v1/relationships_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1122,10 +1122,7 @@ func TestDeleteRelationshipsBeyondLimitPartial(t *testing.T) {
})
require.NoError(err)

headRev, err = ds.HeadRevision(context.Background())
require.NoError(err)

afterDelete := readOfType(require, "document", client, zedtoken.MustNewFromRevision(headRev))
afterDelete := readOfType(require, "document", client, resp.DeletedAt)
require.LessOrEqual(len(beforeDelete)-len(afterDelete), batchSize)

if i == 0 {
Expand Down

0 comments on commit ed90d46

Please sign in to comment.