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

200 create nested crash #5746

Closed
atiertant opened this issue Feb 16, 2016 · 9 comments
Closed

200 create nested crash #5746

atiertant opened this issue Feb 16, 2016 · 9 comments

Comments

@atiertant
Copy link

i'm trying to write an import it use about 200 create nested with 2 associations, and every time, waterline crash : a create never call the callback... and not at the same time so i think it's a asynchronous problem...

@particlebanana
Copy link
Contributor

@atiertant are you doing 200 nested writes at once? Can you give an example, the default pool size is 10 connections usually and each request is done on it's own connection. It could be an issue with timeouts because the connections are exhausted. If you limit the writes to 10 at a time using something like async.eachLimit does it still behave the same way?

@atiertant
Copy link
Author

@particlebanana code is a bit long to post here ... but i do create on event... so i can't do something like async.eachLimit. when doing flat create the code work and every data are wrote, with nested on one belongsTo association this is working too, with a second code crash fast.
i tryed it on sails-mysql and offshore-sql adapter with the same result.

@atiertant
Copy link
Author

@particlebanana i wrote a function using asynk to replace create nested by multiple flat create of the same data:
using create nested it crash after 89 top objet created
using the asynk function all 242 top objet were created with no problems and faster...
this is a waterline bug in create nested !

@particlebanana
Copy link
Contributor

Yup I can see that, the nested graph stuff needs to be pulled out. It's not transactional and causes more errors than it's worth.

Can you post a sample repo that I can clone that reproduces the issue?

@atiertant
Copy link
Author

i'm working on this to understand what's apend, this is a memory leak problem...

_.cloneDeep is evil !

while database is storing some data, waterline continue enqueuing them with many duplication of objects, the heap growing... and node crach !

first cloneDeep easy to remove

the second is a bit more complex because its duplicate data and pass them to all nested operations in valuesObject.originalValues ...
valuesObject should not contain originalValues, only associations object' references should be saved.

@atiertant
Copy link
Author

fixed in offshore-PR 14
with this patch heap growing realy slowly and all data were inserted correctly in nested create with 2 associations and one deep nested containing ~5 Mb binary.
sails-mysql has memory leak too but don't crash while using offshore-sql use 2x lesser memory.

@mikermcneil
Copy link
Member

@atiertant Thanks for all your time putting this together. As @particlebanana pointed out, we'd need some more information to recreate the original bug you mentioned. While creating 200 records in a single Waterline method call is a relatively rare use case, it absolutely should work and we take these kinds of reports very seriously. I went ahead and verified that this functions as expected when using Waterline with a production-ready database adapter.

I'm having a bit of a hard time following the other topics discussed in this issue, since they focus on changes you have made to Waterline in your fork, including adding a package you wrote as a dependency. Unfortunately, I'm just not familiar enough with that package or your changes to help. That said, if you have performance improvements you'd like to suggest for Waterline, please suggest those in a pull request accompanied by benchmarks (see also CONTRIBUTING.md in this repo for more information).

@mikermcneil
Copy link
Member

Also, @atiertant (or anyone reading this in the future) if you notice this issue again with the latest stable Waterline, please open a new issue with details (esp. your Sails/Waterline version, which adapter you're using, and the version). Thank you!

@balderdashy balderdashy locked and limited conversation to collaborators Mar 11, 2016
@mikermcneil
Copy link
Member

For future reference, there's more information and tips on diagnosing memory leaks in Node apps, and on reproducing+reporting suspected memory leaks in Sails here: #3782 (comment)

@balderdashy balderdashy unlocked this conversation May 8, 2019
@balderdashy balderdashy locked as too heated and limited conversation to collaborators May 8, 2019
@balderdashy balderdashy unlocked this conversation May 20, 2019
@johnabrams7 johnabrams7 transferred this issue from balderdashy/waterline May 20, 2019
@balderdashy balderdashy locked as resolved and limited conversation to collaborators May 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

3 participants