-
Notifications
You must be signed in to change notification settings - Fork 77
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
jemalloc 5.1.0 causing problems in heroku deploys release phase #3
Comments
For those who can no longer deploy because of this issue: revert to the previous version Fork this repo and revert to jemallock 5.0.1
Use your modified buildpack
|
You should be able to set From your stack trace, the problem is actually coming from NodeJS and not Ruby. There was a note in #2 about NodeJS printing @concept47 If you're still in a position to, could you try setting JEMALLOC_VERSION to see if that fixes the issue? |
It worked a treat @gaffneyc. Thanks! |
I'm considering reverting the change to make 5.1.0 the default. If you've run into this then 👍 this comment and I'll take that into consideration. I hate to move backwards but I also don't want to break things for people. |
@gaffneyc I get a problem after release (running migrations) with jemalloc enabled, even with 5.0.1, on the heroku-16 stack. I'll experiment with other versions. |
@jvdp Can you give some specifics on the problem you're seeing? For example: are you getting a stack trace, memory usage is going up, or response times are worse, etc... |
@gaffneyc the same exception as OP pretty much. This is with ruby 2.5.3. (It worked well with Heroku-18 but it turns out that breaks wkhtmltopdf.) |
@jvdp The issue itself is with NodeJS (being called through ExecJS). I haven't had a chance to dig into the problem with Node but there are two things worth experimenting with. If you're seeing it on 5.0.1 then that's a new issue we haven't seen before (make sure you're pushing a code change as just changing JEMALLOC_VERSION isn't enough since jemalloc is baked into the slug).
|
@gaffneyc thanks for the pointers! I think I wasn't actually running 5.0.1, I've retried with a code change after setting the version and now it seems to work OK! |
Just heads-up that I ran into this same issue using the nodejs buildpack and Node 9.5.0. Also, I got tripped up that simply setting |
Has any one run into this issue recently? I tried to reproduce it a couple months ago and wasn't able to. Also, several things have changed in the months since the issue was first opened:
Would one of you that has run into the issue be willing to try out Jemalloc 5.1.0 again? Or even kick the tires on jemalloc 5.2.0? |
@gaffneyc I just ran into it with the default version today. Setting 5.0.1 fixed it.
This was the line in our code that threw, if that helps:
|
@ianwremmel Would you be able to give jemalloc 5.2.0 a try? It's looking like it might be an issue with the Heroku 16 stack |
trying it now |
same problem with 5.2.0 |
@ianwremmel Awesome, thank you for testing it! |
I've been able to reproduce the issue and I've created a minimal test repository for it if anyone wants to try to solve it. I've done some initial digging and it appears the issue is fixed on the
I've run a bisect on jemalloc and this appears to be the commit that introduced the issue: I am going to experiment with configure options to see if we can get builds working for cedar-14 and heroku-16. Since the problem will effectively go away as people move to Heroku 18 or newer versions of Node I'm not sure what else could be done. |
@gaffneyc thanks for the update! We're looking to move to Heroku-18 in the coming weeks, so I think simply running on the older version is perfectly fine for us for the time being. |
After more research it looks like it was fixed in 12.4.0 with this commit and is triggered by thread stack size. Annoyingly, NodeJS is printing the message that is breaking Uglifier. This is looking to be an issue in NodeJS (which has been fixed) that happens to be triggered by something in jemalloc. I've tested a number of build configs with jemalloc and none of them have addressed the issue. Unfortunately I think the "fix" is to document the issue in the README and recommend folks use one of: Heroku 18, NodeJS 12.4.0+, or jemalloc 5.0.1 (in that order). |
Heroku should be updating Ruby apps to use Node 12.16.2 by default sometime soon which should close this out once and for all. Initial release yesterday (June 22nd) that was rolled back: https://devcenter.herokuapp.com/changelog-items/1818 |
Heroku finally rolled out the upgrade to Node 12.16.2 for Rails apps for all of their stacks. That should mean this is finally fixed for good (unless of course you're pinned to an old version of NodeJS with the NodeJS buildpack). |
Since the change of version of jemalloc, we've started getting failures in the release phase of our heroku deploys that look like this
The line in question in staging.rb that seems to fail is this one
All our deploys with the jemalloc buildpack removed work just fine. (Figured this because our review apps don't use jemalloc, but it seemed that we had to run
To get it to work.
The text was updated successfully, but these errors were encountered: