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

fix: Fix high memory usage in send_usage_reports #17447

Merged
merged 4 commits into from
Sep 15, 2023

Conversation

frankh
Copy link
Contributor

@frankh frankh commented Sep 14, 2023

Problem

As we are sending usage reports we store them all in an all_reports variable. We don't use this variable at all except if it's a dry run, and to print len(reports) at the end.

Keep this behaviour for dry runs only

Changes

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

How did you test this code?

@frankh frankh requested a review from raquelmsmith September 14, 2023 14:13
@frankh frankh changed the title [fix] Fix high memory usage in send_usage_reports fix: Fix high memory usage in send_usage_reports Sep 14, 2023
@raquelmsmith
Copy link
Member

The was it's written currently is used for tests so I'm going to do some reorganization and cleanup here to make it work for tests and more readable in general.

@raquelmsmith
Copy link
Member

Ha I took your 3-line pr and 10x'd it. It's mostly reorganizing code and fixing up tests to cope with not keeping all the reports around in memory. If you can give it a quick once-over then we can call it mutually-approved @frankh

Copy link
Member

@raquelmsmith raquelmsmith left a comment

Choose a reason for hiding this comment

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

👻

@raquelmsmith raquelmsmith self-assigned this Sep 15, 2023
@frankh frankh merged commit 0b62ee2 into master Sep 15, 2023
57 checks passed
@frankh frankh deleted the frank/fix-usage-report-memory-leak branch September 15, 2023 13:51
daibhin pushed a commit that referenced this pull request Sep 15, 2023
* Fix high memory usage in send_usage_reports

* do a lot of reorg

* fix tests

* fix mypy

---------

Co-authored-by: Raquel Smith <[email protected]>
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