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

Return reduce overflow errors to the client #797

Merged
merged 1 commit into from Oct 10, 2017

Conversation

davisp
Copy link
Member

@davisp davisp commented Sep 7, 2017

This changes the reduce overflow error to return an error to the client
rather than blowing up the view build. This allows views that have a
single bad reduce to build while not crushing the server's RAM usage.

Testing recommendations

make javascript suites=view_errors

Checklist

  • Code is written and works correctly;
  • Changes are covered by tests;
  • Documentation reflects the changes;

@tonysun83
Copy link
Contributor

I noticed in fabric_util we have an error condition for this error, and also in view_errors we're testing for this situation. Was this error not propagated correctly before and thus the view builds blew up?

@davisp
Copy link
Member Author

davisp commented Sep 8, 2017

@tonysun83 My bad! Forgot to push after I found the view_errors.js test failure. I hadn't noticed the fabric_util clause but I've removed it since it'd be dead code now.

@tonysun83
Copy link
Contributor

+1

This changes the reduce overflow error to return an error to the client
rather than blowing up the view build. This allows views that have a
single bad reduce to build while not crushing the server's RAM usage.
@janl janl force-pushed the improve-reduce-limit-errors branch from 8d33cd7 to b030a86 Compare October 9, 2017 19:42
@janl
Copy link
Member

janl commented Oct 9, 2017

rebased on latest master, can be merged after travis

@janl janl merged commit a0e0885 into master Oct 10, 2017
@janl janl deleted the improve-reduce-limit-errors branch October 10, 2017 06:38
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

3 participants