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

Account for value size when restoring key/values from a backup. #1278

Closed

Conversation

gonzojive
Copy link

@gonzojive gonzojive commented Mar 27, 2020

The previous behavior only accounted for key size. For databases where keys are
small (e.g., URLs) and values are much larger (megabytes), OOM errors were easy
to trigger during restore operations.

This change does not set the threshold used for flushing elegantly - a const is
used instead of a configurable option.

Related to #1268, but not a full fix.


This change is Reviewable

The previous behavior only accouned for key size. For databases where keys are
small (e.g., URLs) and values are much larger (megabytes), OOM errors were easy
to trigger during restor operations.

This change does not set the threshold use for flushing elegantly - a const is
used instead of a configurable option.

Related to dgraph-io#1268, but not a full fix.
@CLAassistant
Copy link

CLAassistant commented Mar 27, 2020

CLA assistant check
All committers have signed the CLA.

@jarifibrahim
Copy link
Contributor

Thank you for the PR @gonzojive. Please let me know once it is ready for review.

@gonzojive
Copy link
Author

I have a request in with my company for signing the CLA, but if it possible to skip signing the CLA, then we can proceed right away.

@jarifibrahim
Copy link
Contributor

@gonzojive Unfortunately, due to repository settings, I cannot merge any PRs without the CLA signed.

@gonzojive
Copy link
Author

Signed. I will let you know when cleaned up and ready for review.

@jarifibrahim
Copy link
Contributor

@gonzojive ping! Are you still working on this PR? I can take it from here if you aren't working on it :)

@gonzojive
Copy link
Author

gonzojive commented Apr 29, 2020 via email

@stale
Copy link

stale bot commented May 29, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the status/stale The issue hasn't had activity for a while and it's marked for closing. label May 29, 2020
@lgalatin lgalatin removed the status/stale The issue hasn't had activity for a while and it's marked for closing. label May 29, 2020
@jarifibrahim
Copy link
Contributor

Closing this PR in favor of #1358 . Thank you for the PR @gonzojive

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants