-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add workers to PostHog #455
Comments
To comment on Tim's update and 1vs2: django-postgres-queue also has 51 stars and while I'm sure it does the job, it doesn't look like a very active project. Compared to Rails's delayed_job for example (4.5k stars). It's better to use proven and widely used technology. No comment on the other open questions at the moment. |
Hi, trying to answer the rest of the questions: Migration for various options:
Cron jobs Running without workers as a fallback option |
@mariusandra I tried deploying this update to Heroku and I did have to manually turn the worker on, though that might have been because I was using professional dyno's. I also think I had to set the $REDIS_URL specifically in the Procfile: I think that means people could upgrade Heroku and not know they need to turn on their worker, losing data in the process. I definitely think it's worth doing this warning message in the frontend before we move any actual work to the workers. |
What about this idea: we make redis and the worker optional for at least another week. It'll be there, but nothing will happen if you start the app without it. I'll also implement a worker heartbeat system (ping every minute, stored in redis), which we'll use to say if the worker is actually running or not. Somewhere in the header of the app we'll show a large red This should provide at least some feedback to the users in a non-destructive way. |
I like that, we can release 1.0.11 tomorrow (ideally with that error message) so if people upgrade they'll know they need to turn on workers but they won't have lost data. Then when we release 1.1.0 next week we can have moved stuff over to workers. Sounds good to me. Created #489 for you |
Great! I'll start with it tomorrow morning. I think I can get some warning up by noon latest. Regarding losing data, if redis is up and just the worker down, as would be the case with heroku deploys where the worker won't get added, but redis will, then there will be no data loss. The events will still get queued in redis, just nothing will process them. A big red warning will hopefully get ops people to fix the queue. If you start a posthog instance without redis attahed to it, the code that creates jobs will just throw an error and we will probably experience data loss indeed. That's with 1.1.0, not for tomorrow's release. |
I think the docs can be better clarified that "turn redis worker on" means to activate the heroku redis addon. |
@Immortalin Hm the Heroku redis addon should have been automatically provisioned. Did that not happen for you? The docs talk about provisioning a worker dyno, but I see I have worded that poorly |
Nope, it did not. Here's the stack trace:
|
It works after I manually added the addon through the Heroku web ui and then triggered another deployment. |
Thanks for that, I think I know what's going on here. In terms of the worker not being activated, that's what's referred to in the docs. You might have to manually provision the worker Dyno. |
I've merged #508 which should prevent this for people in the future! |
* Build healthcheck into index.js * Add "Alternative modes" to README * Fix uncaught reject in Sentry flush
We've had a couple of instances now where we'd like to use workers:
Solution
I think there's two options:
The upside of 1 is it's more proven and is basically the standard for how everyone does queues. The downside is it means we have to get everyone to deploy a Redis database in addition to a Postgres database.
UPDATE
After discussion with @mariusandra, decided that we're going to go for Redis. it's not that hard to add it to the Helm charts for example, and Redis will allow us to scale much further than the Postgres solution would.
Open questions
Solution 1 vs 2The text was updated successfully, but these errors were encountered: