Failure to delete relationships when not allowing partial deletions

[jmadureira]

What platforms are affected?

macos

What architectures are affected?

arm64

What SpiceDB version are you using?

v.1.30.0

Steps to Reproduce

The following snippet is Java code but I'll try to explain each step.

Tests are run against postgreSQL and SpiceDB not running with serve-testing.

public void test() {
        var testSchema = """
                definition identity/user {}

                definition test/organisation {
                  relation member: identity/user
                                
                  permission view = member
                }
                """;
        Function<Integer, Core.RelationshipUpdate> toRelationshipRequest = (i) -> {
            return Core.RelationshipUpdate.newBuilder()
                    .setRelationship(Core.Relationship.newBuilder()
                            .setResource(Core.ObjectReference.newBuilder()
                                    .setObjectType("test/organisation")
                                    .setObjectId(String.valueOf(i))
                                    .build())
                            .setRelation("member")
                            .setSubject(Core.SubjectReference.newBuilder()
                                    .setObject(Core.ObjectReference.newBuilder()
                                            .setObjectType("identity/user")
                                            .setObjectId("1")
                                            .build())
                                    .build())
                            .build())
                    .setOperation(Core.RelationshipUpdate.Operation.OPERATION_CREATE)
                    .build();
        };
        Function<Integer, PermissionService.CheckPermissionRequest> toCheckPermissionRequest = (i) -> {
            return PermissionService.CheckPermissionRequest.newBuilder()
                    .setResource(Core.ObjectReference.newBuilder()
                            .setObjectType("test/organisation")
                            .setObjectId(String.valueOf(i))
                            .build())
                    .setPermission("view")
                    .setSubject(Core.SubjectReference.newBuilder()
                            .setObject(Core.ObjectReference.newBuilder()
                                    .setObjectType("identity/user")
                                    .setObjectId("1")
                                    .build())
                            .build())
                    .setConsistency(PermissionService.Consistency.newBuilder().setFullyConsistent(true).build())
                    .build();
        };
        var schemaRequest = WriteSchemaRequest.newBuilder().setSchema(testSchema).build();
        // load the test schema
        assertThat(schemaService.writeSchema(schemaRequest)).succeedsWithin(Duration.ofSeconds(2));
        // create 100 test relationships
        var relationships = IntStream.range(0, 100).boxed().map(toRelationshipRequest).toList();
        writeRelationships(relationships);
        // check permissions that affect all relationships created
        IntStream.range(0, 100).boxed().map(toCheckPermissionRequest).forEach((request) -> {
            assertThat(checkPermission(request).getPermissionship()).isEqualTo(PermissionService.CheckPermissionResponse.Permissionship.PERMISSIONSHIP_HAS_PERMISSION);
        });
        // delete all relationships
        deleteRelationships(PermissionService.DeleteRelationshipsRequest.newBuilder()
                .setRelationshipFilter(PermissionService.RelationshipFilter.newBuilder()
                         .setOptionalSubjectFilter(PermissionService.SubjectFilter.newBuilder()
                                .setSubjectType("identity/user")
                                .setOptionalSubjectId("1")
                                 .build())
                        .setOptionalRelation("member")
                        .setResourceType("test/organisation")
                        .build())
                .setOptionalAllowPartialDeletions(false)
                .setOptionalLimit(101)
                .build());
        // check that all relationships have been deleted
        IntStream.range(0, 100).boxed().map(toCheckPermissionRequest).forEach((request) -> {
            assertThat(checkPermission(request).getPermissionship()).isEqualTo(PermissionService.CheckPermissionResponse.Permissionship.PERMISSIONSHIP_NO_PERMISSION);
        });
        // reload the same permissions again
        relationships = IntStream.range(0, 100).boxed().map(toRelationshipRequest).toList();
        writeRelationships(relationships);
        // check all relationships
        IntStream.range(0, 100).boxed().map(toCheckPermissionRequest).forEach((request) -> {
            assertThat(checkPermission(request).getPermissionship()).isEqualTo(PermissionService.CheckPermissionResponse.Permissionship.PERMISSIONSHIP_HAS_PERMISSION);
        });
        // delete all relationships again
        deleteRelationships(PermissionService.DeleteRelationshipsRequest.newBuilder()
                .setRelationshipFilter(PermissionService.RelationshipFilter.newBuilder()
                        .setOptionalSubjectFilter(PermissionService.SubjectFilter.newBuilder()
                                .setSubjectType("identity/user")
                                .setOptionalSubjectId("1")
                                .build())
                        .setOptionalRelation("member")
                        .setResourceType("test/organisation")
                        .build())
                .setOptionalAllowPartialDeletions(false)
                .setOptionalLimit(101)
                .build());
        // check that all relationships have been deleted
        IntStream.range(0, 100).boxed().map(toCheckPermissionRequest).forEach((request) -> {
            assertThat(checkPermission(request).getPermissionship()).isEqualTo(PermissionService.CheckPermissionResponse.Permissionship.PERMISSIONSHIP_NO_PERMISSION);
        });
    }

Expected Result

The previous test should not throw any error.

Actual Result

The last deleteRelationship operation fails with:

ERROR:  duplicate key value violates unique constraint "uq_relation_tuple_living_xid"
STDERR: 2024-03-28 16:52:28.940 UTC [81] DETAIL:  Key (namespace, object_id, relation, userset_namespace, userset_object_id, userset_relation, deleted_xid)=(test/organisation, 0, member, identity/user, 1, ..., 811) already exists.
STDERR: 2024-03-28 16:52:28.940 UTC [81] STATEMENT:  WITH found_tuples AS (SELECT namespace, object_id, relation, userset_namespace, userset_object_id, userset_relation FROM relation_tuple WHERE deleted_xid = $1 AND namespace = $2 AND relation = $3 AND userset_namespace = $4 AND userset_object_id = $5 LIMIT 101)
STDERR: 	UPDATE relation_tuple SET deleted_xid = $6 WHERE (namespace, object_id, relation, userset_namespace, userset_object_id, userset_relation) IN (select * from found_tuples)
STDERR: {"level":"warn","error":"could not CREATE relationship `test/organisation:0#member@identity/user:1`, as it already existed. If this is persistent, please switch to TOUCH operations or specify a precondition","time":"2024-03-28T16:52:28Z","message":"unable to determine if pgx error is retryable"}

Some additional info:

• The test pass if using SpivreDB 1.29.5;
• The test also pass if we remove the optionalLimit and optionalAllowPartialDeletions from the deleteRelationships request.

[alecmerdler]

@jmadureira Would you mind checking out this branch and see if you can still reproduce the issue?

[vroldanbet]

Fixed in #1843, thanks @jmadureira for reporting! 🙌🏻

[jmadureira]

@jmadureira Would you mind checking out this branch and see if you can still reproduce the issue?

Hello @alecmerdler yes those changes fixed the issue I found. Thanks for the quick reply.