PostHog / posthog

🦔 PostHog provides open-source product analytics, session recording, feature flagging and A/B testing that you can self-host.
https://posthog.com
Other
19.35k stars 1.13k forks source link

fix: prevent runtime error - make timings work with threads #23341

Closed aspicer closed 1 day ago

aspicer commented 4 days ago

Problem

We're getting runtime errors in trends queries for dictionary changed size during iteration: Fixes #23306

We're using the same timings object in multi threaded queries and its getting all jumbled up.

Changes

Modify the timings object to handle cloning in order to pass to a thread while maintaining the current key stack.

Change the trends query to not return redundant timing info and to stack it correctly.

How did you test this code?

Wrote a test for it to make sure it's working as intended. Tested in dev. Can't test multi threaded behavior directly (easily).

sentry-io[bot] commented 4 days ago

🔍 Existing Issues For Review

Your pull request is modifying functions with the following pre-existing issues:

📄 File: posthog/hogql/timings.py

Function Unhandled Issue
to_dict RuntimeError: dictionary changed size during iteration posthog.tasks.tasks.process_quer...
Event Count: 96
to_dict TypeError: unsupported operand type(s) for -: 'float' and 'StringDatabaseField' posthog...
Event Count: 1
📄 File: posthog/hogql_queries/insights/trends/trends_query_runner.py (Click to Expand) | Function | Unhandled Issue | | :------- | :----- | | **`run`** | [**IndexError: list index out of range**](https://posthog.sentry.io/issues/5482508134/?referrer=github-open-pr-bot) posthog.tasks...
`Event Count:` **210** | | **`calculate`** | [**RuntimeError: dictionary changed size during iteration**](https://posthog.sentry.io/issues/5481827655/?referrer=github-open-pr-bot) posthog.tasks.tasks.process_quer...
`Event Count:` **96** | | **`run`** | [**ValidationError: 1 validation error for HogQLQueryResponse**](https://posthog.sentry.io/issues/5485263394/?referrer=github-open-pr-bot) posthog.tasks.tasks.process_...
`Event Count:` **34** | | **`run`** | [**CHQueryErrorCannotParseInputAssertionFailed: DB::Exception: Cannot parse NaN.: while converting 'no-10904' to Float64: while executing 'FUNCTI...**](https://posthog.sentry.io/issues/5543417518/?referrer=github-open-pr-bot) ...
`Event Count:` **28** | | **`run`** | [**CHQueryErrorAuthenticationFailed: DB::Exception: Received from ch12.posthog.net:9000. DB::Exception: default: Authentication failed...**](https://posthog.sentry.io/issues/5543295381/?referrer=github-open-pr-bot) ...
`Event Count:` **24** |

Did you find this useful? React with a 👍 or 👎