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

Add option to not return modified entries for bulk updates #173

Merged
merged 4 commits into from
Nov 6, 2017

Conversation

MichaelErmer
Copy link
Contributor

Summary

This is a reopening of #117

This pull request is for a new way to omit the result of mass change operations like patch and remove.

This change requires discussion and adaption by all other adapters, the issue this tries to solve does affect people writing large apis with massive amounts of data.

In our case node crashed when updates affected several hundred thousand documents.
While the patch went through on the database, node reached its heap limit when receiving/retrieving the changed docs and crashed before answering the request.

@daffl
Copy link
Member

daffl commented Nov 2, 2017

I'm wondering if we should just adhere to the $limit parameter when making a multi-update. Thoughts?

@MichaelErmer
Copy link
Contributor Author

The response from DBMS to node will kill the node process before $limit and whatsoever

@daffl
Copy link
Member

daffl commented Nov 2, 2017

Ah true. The condition could still apply to params.query.$limit === 0 then (instead of adding a new flag) though right?

@MichaelErmer
Copy link
Contributor Author

@daffl i think there are quite a few people using $limit=-1 for disabling paging, so im not sure this is a good idea. What do you think?

@daffl daffl changed the title Feature/returning Add option to not return modified entries for bulk updates Nov 6, 2017
@daffl
Copy link
Member

daffl commented Nov 6, 2017

I'll merge this one as well. Can you add it to the docs?

@daffl daffl merged commit 8bbdbb9 into feathersjs-ecosystem:master Nov 6, 2017
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.

2 participants