PostHog / plugin-server

Service to process and save PostHog events, supporting JS/TS plugins at that
https://posthog.com
8 stars 5 forks source link

Throw errors on deletePerson transaction #540

Closed yakkomajuri closed 3 years ago

yakkomajuri commented 3 years ago

Changes

It's pretty late for me and I'm looking into CH duplicate IDs again. Apologies for the somewhat messy solution but this is what I came up with that keeps everything else the same (and types satisfied) while isolating errors for that deletePerson transaction particularly so we can quickly roll out a fix.

The TL;DR is that https://github.com/PostHog/plugin-server/pull/202 silenced https://sentry.io/organizations/posthog/issues/2225546229/?project=5592816&query=is%3Aunresolved&statsPeriod=14d which is problematic because we rely on that deletePerson transaction throwing if a distinct ID has lingered. See https://github.com/PostHog/plugin-server/blob/e96b0e408e2d53841e791d9db9bc6a488c6bbd44/src/worker/ingestion/process-event.ts#L385

Given instrumentQuery silences the error, we never try to merge the person again, and thus can end up not deleting IDs we should delete.

Happy to explain more, but now, 😴

Checklist

yakkomajuri commented 3 years ago

😅

yakkomajuri commented 3 years ago

Late night coding...