search

cloudfoundry / nodejs-buildpack

Cloud Foundry buildpack for Node.js
http://docs.cloudfoundry.org/buildpacks/
Apache License 2.0
170 stars 384 forks source link

Performance issue with v1.5.35 #91

Closed rportugal closed 7 years ago

rportugal commented 7 years ago

What version of Cloud Foundry and CF CLI are you using? (i.e. What is the output of running cf curl /v2/info && cf version? n.a.

What version of the buildpack you are using? 1.5.35

If you were attempting to accomplish a task, what was it you were attempting to do? Deploying the same version of a Node.js GraphQL server to a two-legged app, one running 1.5.35 and the other running 1.5.34 .

What did you expect to happen? There should be no performance difference between the two legs.

What was the actual behavior? The leg running 1.5.35 is considerably slower than the leg running 1.5.34 . This is a screenshot of our New Relic monitoring:

screen shot 2017-06-05 at 13 27 30

Apologies as I can't attach too many details, but overall it is slower. We use external HTTP web services, Mongo and Redis and they all have much slower response times.

Setting the version manually to v1.5.34 fixed the issue.

Unfortunately I don't have logs for the run with v1.5.35, but this is the one with v1.5.34:

13:00:53                   engines.node (package.json):  6.x
13:00:53                   engines.npm (package.json):   3.10.7
13:00:53                   Downloading and installing node 6.10.3...

Please confirm where necessary:

cf-gitbot commented 7 years ago

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/146612361

The labels on this github issue will be updated when the story is started.

sclevine commented 7 years ago

Thanks for the detailed report!

A number of changes in v1.5.35 could have resulted in this performance change:

  1. A bug in v1.5.34 unset the value of NODE_ENV. It is correctly set to production in v1.5.35+.
  2. Starting in v1.5.35, we set MEMORY_AVAILABLE correctly, based on the container limits.
  3. Starting in v1.5.35, we set WEB_MEMORY=512 and WEB_CONCURRENCY=1, as those were set in pre-v1.5.33 versions for better compatibility with Heroku apps.

Can you try adjusting those parameters? Alternatively, can you supply an example app that demonstrates the performance drop?

sclevine commented 7 years ago

Hi @rportugal,

Are you still experiencing this performance degradation with the latest buildpack version? If so, have you tried setting those env vars to their v1.5.34 equivalents (using cf set-env, or via manifest.yml) with the latest buildpack version?

rportugal commented 7 years ago

Apologies for not replying to this issue earlier. I will try to reproduce the issue in one of our dev environments, and see which env variable caused the problem. Thank you.

paulbeattie commented 7 years ago

Hello @sclevine

@rportugal and I have been doing some investigation on this issue. What we've observed is that if we set WEB_CONCURRENCY=4 in the manifest or via cf set-env this is being overwritten by the buildpack. When enabling LOG_CONCURRENCY=true in this scenario we see the recommended concurrency in line with our setting is 4. When the app launches this is set to 1 in the environment.

paulbeattie commented 7 years ago

@sclevine 🚨 I've done some further investigation on this and I think I've found the bug. From what I can tell in my forked buildpack with minor changes the runtime environment variables WEB_MEMORY and WEB_CONCURRENCY are being overwritten here https://github.com/cloudfoundry/nodejs-buildpack/blob/master/src/nodejs/supply/supply.go#L255

sclevine commented 7 years ago

Thanks @paulbeattie and @rportugal, nice catch!

Closing this in favor of #94, which is prioritized for this week.