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

put off refreshing env until later #197

Closed
wants to merge 1 commit into from

Conversation

calvinmetcalf
Copy link

because currently newrelic delays the server starting by 1.8 seconds, 1.6 of them are the refresh function, this just avoids environment being initialized until the first time it's used

@wraithan
Copy link
Contributor

Do you have an extraordinary number of dependencies? I've never seen the agent delay startup that bad.

Would love to know the root cause of the delay, this seems reasonable though. I'll add it to our queue of stuff to test and release.

@calvinmetcalf
Copy link
Author

The app has a good amount of deps nothing too extreme. The total start up
time is ~4 secs with the next worst module taking about ~100 ms.

The one thing is that it uses the cluster module, during testing there was
only one child (so 2 copies of new relic)

The root issue for new relic is the large amount of sync use of the fs
module as it looks like it's synchronously recursing a directory, more long
term using async fs here would probably be a better compromise between
getting it set up quickly and not getting in the way of start up.

On Tue, Jun 16, 2015, 5:36 PM Wraithan (Chris McDonald) <
notifications@github.com> wrote:

Do you have an extraordinary number of dependencies? I've never seen the
agent delay startup that bad.

Would love to know the root cause of the delay, this seems reasonable
though. I'll add it to our queue of stuff to test and release.


Reply to this email directly or view it on GitHub
#197 (comment)
.

@wraithan
Copy link
Contributor

Sorry, I assumed the very root cause would be the file io. We avoided async calls there in order to make sure we can get enough data to connect to newrelic in the first turn of the event loop. Unfortunately other code will be changing making that first turn connect not possible in many cases anymore so it might make sense to rewrite this to be async.

What I meant was looking into if it was just shear number of deps or if there was something else going on. I commonly run the agent in a folder with >500 nested deps and it takes less than 100ms. Just wondering what appropriate load test would be. Would you mind telling me the output of npm ls | wc -l is in your project with this really look start time so I can make a test that reflects that.

@calvinmetcalf
Copy link
Author

$ npm ls | wc -l
    1101

@amasad
Copy link

amasad commented Nov 5, 2015

Any real node application will have much more node modules than that. I'm seeing major delays in start up time because of this. @wraithan any plans to fix this?

@wraithan
Copy link
Contributor

wraithan commented Nov 5, 2015

@amasad Yes there are plans to correct it, this is opened as an internal ticket. We have had other higher priority changes going into the agent though. This is definitely on my radar but I have to respect the priority that the team and our product management decides until we have a bug bash week to fix things like this.

We've had very few reports of longer start up times causing trouble for people. The majority of accounts that I've looked at while triaging issues have had <200 dependencies. In the meantime, you can mitigate this by using npm dedupe or moving up to npm 3 which is FAR better at preemptive deduping, lowering the number of package.jsons that need to be traversed.

@amasad
Copy link

amasad commented Nov 5, 2015

Will try that, thanks

@NatalieWolfe
Copy link
Contributor

Hello, we refactored the environment scanner to work asynchronously back in version 2.6.0, released in January of this year. That change should have made the agent's blocking time a lot smaller. If you're still having a problem with the startup time, feel free to re-open this issue or create a new one. Thanks!

bizob2828 added a commit to bizob2828/node-newrelic that referenced this pull request Apr 19, 2024
bizob2828 added a commit to bizob2828/node-newrelic that referenced this pull request Apr 23, 2024
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.

None yet

4 participants