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

Reduce Heroku worker thread count #2092

Merged
merged 2 commits into from
Oct 30, 2020
Merged

Reduce Heroku worker thread count #2092

merged 2 commits into from
Oct 30, 2020

Conversation

mariusandra
Copy link
Collaborator

Changes

This reduces the number of concurrent threads used in a worker on a "hobby" dyno from 8 to 2, greatly reducing the memory/swap usage:
image

It also reduces the number of redis connections needed from ~19 when idle to ~15.

Checklist

  • All querysets/queries filter by Organization, Team, and User (if this PR affects ANY querysets/queries).
  • Django backend tests (if this PR affects the backend).
  • Cypress end-to-end tests (if this PR affects the frontend).

@mariusandra mariusandra requested a review from timgl October 29, 2020 08:22
@timgl timgl temporarily deployed to posthog-reduce-heroku-w-mpocsb October 29, 2020 08:23 Inactive
@timgl
Copy link
Collaborator

timgl commented Oct 29, 2020

I'm not sure why, but when this was deployed the worker wasn't turned on and I had to manually turn it on so no events where coming in. I don't know if that was a direct result of this PR or not,but haven't seen this on other PRs.

@mariusandra mariusandra mentioned this pull request Oct 29, 2020
3 tasks
@timgl timgl temporarily deployed to posthog-reduce-heroku-w-1bkxcs October 29, 2020 10:22 Inactive
@mariusandra mariusandra mentioned this pull request Oct 29, 2020
3 tasks
@mariusandra
Copy link
Collaborator Author

Indeed the worker didn't come up in my test. #2097

image

Testing if that's also the case with a fresh branch from master: #2099

@mariusandra
Copy link
Collaborator Author

The same happened with a fresh app deployed from master!

image

Not sure why though :/

@mariusandra
Copy link
Collaborator Author

Every branch now deploys without workers started. I don't know why, but will debug. This PR should be safe to merge though.

Screenshot_20201029-134452

@mariusandra
Copy link
Collaborator Author

@timgl the worker is deployed when just deploying a new heroku app (followed the docs to get here)

image

I'll investigate and try to fix this in a new PR and/or make a new issue. Any reason this one should not be merged?

Good news is that redis is quite stable now on an empty instance:

image

@timgl timgl merged commit a5d9f98 into master Oct 30, 2020
@timgl timgl deleted the reduce-heroku-worker-count branch October 30, 2020 09:17
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