Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: rust funnel udfs #25326

Merged
merged 9 commits into from
Oct 2, 2024
Merged

feat: rust funnel udfs #25326

merged 9 commits into from
Oct 2, 2024

Conversation

aspicer
Copy link
Contributor

@aspicer aspicer commented Oct 1, 2024

Based off of #25013

For some reason, this makes CI not run backend tests. The full diff is here

Problem

Memory usage too high on python UDFs in Clickhouse.

Changes

Rewrite in rust for a 10x speed improvement. Set a lifetime value in clickhouse that reloads the UDFs every 10 minutes as a safety valve against memory leaking.

Does this work well for both Cloud and self-hosted?

Yes

How did you test this code?

Passed funnels tests.

@aspicer aspicer marked this pull request as ready for review October 1, 2024 22:23
@aspicer aspicer requested review from skoob13 and a team October 1, 2024 22:24
@aspicer aspicer force-pushed the aspicer/funnel_actors branch from 3abc7a5 to ff7d4e7 Compare October 2, 2024 16:41
@@ -79,7 +79,7 @@ services:
# Note: please keep the default version in sync across
# `posthog` and the `charts-clickhouse` repos
#
image: ${CLICKHOUSE_SERVER_IMAGE:-clickhouse/clickhouse-server:23.12.6.19-alpine}
image: ${CLICKHOUSE_SERVER_IMAGE:-clickhouse/clickhouse-server:23.12.6.19}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably a good idea moving off of alpine since this will reflect more what production is

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah for sure - in this case, a glibc binary is more than 2x faster than a MUSL libc binary - apparently the musl libc binary has a slow memory allocator (going for simplicity and portability over speed), but that's huge lol

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually started wondering if clickhouse would be faster locally running on a normal container vs an alpine one

@aspicer aspicer merged commit cfb2c8c into aspicer/funnel_actors Oct 2, 2024
97 of 99 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants