From 8e9f66ca7037d956d6a978ad9016900d8447e646 Mon Sep 17 00:00:00 2001 From: nickbest-ph Date: Thu, 1 Oct 2026 11:45:51 -0700 Subject: [PATCH] feat(personhog): tombstone by default in DeletePersons --- .../generated/personhog/personhog/types/v1/person_pb.ts | 2 +- posthog/personhog_client/fake_client.py | 2 +- posthog/personhog_client/test_fake_client.py | 6 +++++- posthog/test/persons.py | 8 +++++++- proto/personhog/types/v1/person.proto | 2 +- rust/personhog-replica/README.md | 2 +- rust/personhog-replica/src/service/mod.rs | 2 +- rust/personhog-replica/tests/service_tests.rs | 7 +++++-- 8 files changed, 22 insertions(+), 9 deletions(-) diff --git a/nodejs/src/common/generated/personhog/personhog/types/v1/person_pb.ts b/nodejs/src/common/generated/personhog/personhog/types/v1/person_pb.ts index 086286f0810e..7b8867247755 100644 --- a/nodejs/src/common/generated/personhog/personhog/types/v1/person_pb.ts +++ b/nodejs/src/common/generated/personhog/personhog/types/v1/person_pb.ts @@ -1873,7 +1873,7 @@ export const FoldPersonDocumentResponseSchema: GenMessage person_pb2.DeletePersonsResponse: self.calls.append(_Call("delete_persons", request)) - tombstone = request.mode == person_pb2.DELETE_PERSONS_MODE_TOMBSTONE + tombstone = request.mode != person_pb2.DELETE_PERSONS_MODE_HARD response = person_pb2.DeletePersonsResponse(tombstoned=tombstone) for uuid in request.person_uuids: person = self._persons_by_uuid.get((request.team_id, uuid)) diff --git a/posthog/personhog_client/test_fake_client.py b/posthog/personhog_client/test_fake_client.py index 3bc998918941..b4a6e7b5aba6 100644 --- a/posthog/personhog_client/test_fake_client.py +++ b/posthog/personhog_client/test_fake_client.py @@ -613,7 +613,11 @@ def test_wrong_team_touches_nothing(self): def test_delete_persons_still_removes_tombstoned_and_live_alike(self): resp = self.client.delete_persons( - person_pb2.DeletePersonsRequest(team_id=self.TEAM_ID, person_uuids=["tombstoned", "live", "blocked"]) + person_pb2.DeletePersonsRequest( + team_id=self.TEAM_ID, + person_uuids=["tombstoned", "live", "blocked"], + mode=person_pb2.DELETE_PERSONS_MODE_HARD, + ) ) assert resp.deleted_count == 3 diff --git a/posthog/test/persons.py b/posthog/test/persons.py index 7e868814e608..68a53b63f70e 100644 --- a/posthog/test/persons.py +++ b/posthog/test/persons.py @@ -431,7 +431,13 @@ def delete_person(person: Person) -> None: person.team_id, did.distinct_id, str(person.uuid), version=(did.version or 0) + 100, is_deleted=True ) - fake.delete_persons(person_pb2.DeletePersonsRequest(team_id=person.team_id, person_uuids=[str(person.uuid)])) + fake.delete_persons( + person_pb2.DeletePersonsRequest( + team_id=person.team_id, + person_uuids=[str(person.uuid)], + mode=person_pb2.DELETE_PERSONS_MODE_HARD, + ) + ) def update_person(person: Person) -> None: diff --git a/proto/personhog/types/v1/person.proto b/proto/personhog/types/v1/person.proto index 9f50457c2d65..138352c64947 100644 --- a/proto/personhog/types/v1/person.proto +++ b/proto/personhog/types/v1/person.proto @@ -188,7 +188,7 @@ message UpdatePersonPropertiesResponse { // How DeletePersons removes the rows. enum DeletePersonsMode { - // Same as HARD. + // Same as TOMBSTONE. DELETE_PERSONS_MODE_UNSPECIFIED = 0; // Remove the rows, tombstoned ones included. For callers that publish no // ClickHouse tombstones and want nothing left behind, such as a purge. diff --git a/rust/personhog-replica/README.md b/rust/personhog-replica/README.md index 997e331265c5..436c2f4deb71 100644 --- a/rust/personhog-replica/README.md +++ b/rust/personhog-replica/README.md @@ -8,7 +8,7 @@ ### Person deletes -`DeletePersons` hard-deletes person and distinct-id rows unless the request asks for `DELETE_PERSONS_MODE_TOMBSTONE`. +`DeletePersons` tombstones person and distinct-id rows unless the request asks for `DELETE_PERSONS_MODE_HARD`. A tombstone keeps the rows with `is_deleted = true`, the version bumped by one, and person properties scrubbed, and the response reports the versions written so the caller can publish ClickHouse tombstones at exactly those versions. The row's version counter survives, so a later create on the same key revives it above its own ClickHouse tombstone instead of restarting at version 0. The tombstone cleanup drain (`DeleteTombstonedPersons`) removes the rows later, once their ClickHouse history is gone. diff --git a/rust/personhog-replica/src/service/mod.rs b/rust/personhog-replica/src/service/mod.rs index acfe40dc3940..5b8b67834ea7 100644 --- a/rust/personhog-replica/src/service/mod.rs +++ b/rust/personhog-replica/src/service/mod.rs @@ -422,7 +422,7 @@ impl PersonHogReplica for PersonHogReplicaService { .map_err(|e| Status::invalid_argument(format!("Invalid UUID: {e}")))?; let mode = match ProtoDeletePersonsMode::try_from(req.mode) { - Ok(ProtoDeletePersonsMode::Unspecified) => DeletePersonsMode::Hard, + Ok(ProtoDeletePersonsMode::Unspecified) => DeletePersonsMode::Tombstone, Ok(ProtoDeletePersonsMode::Hard) => DeletePersonsMode::Hard, Ok(ProtoDeletePersonsMode::Tombstone) => DeletePersonsMode::Tombstone, Err(_) => { diff --git a/rust/personhog-replica/tests/service_tests.rs b/rust/personhog-replica/tests/service_tests.rs index fd9c40c8a41d..e551cba1ebff 100644 --- a/rust/personhog-replica/tests/service_tests.rs +++ b/rust/personhog-replica/tests/service_tests.rs @@ -1427,8 +1427,11 @@ async fn test_delete_tombstoned_persons_reports_each_outcome( ctx.cleanup().await.ok(); } +#[rstest] +#[case::unspecified(DeletePersonsMode::Unspecified as i32)] +#[case::tombstone(DeletePersonsMode::Tombstone as i32)] #[tokio::test] -async fn test_delete_persons_tombstone_mode_reports_versions() { +async fn test_delete_persons_tombstone_mode_reports_versions(#[case] mode: i32) { let ctx = ServiceTestContext::new().await; let person = ctx.insert_person("svc_tomb_mode", None).await.unwrap(); @@ -1437,7 +1440,7 @@ async fn test_delete_persons_tombstone_mode_reports_versions() { .delete_persons(Request::new(DeletePersonsRequest { team_id: ctx.team_id, person_uuids: vec![person.uuid.to_string()], - mode: DeletePersonsMode::Tombstone as i32, + mode, })) .await .expect("RPC failed")