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

Handle upload fail #215

Merged
merged 2 commits into from
Sep 2, 2020
Merged

Handle upload fail #215

merged 2 commits into from
Sep 2, 2020

Conversation

lukechilds
Copy link
Member

@lukechilds lukechilds commented Sep 2, 2020

Currently if the upload fails the backup script exits uncleanly.

This PR correctly handles a failed upload by allowing the script to clean up and prints the error in the logs.

@mayankchhabra
Copy link
Member

Looks good! Makes me wonder if we should add a while loop and retry uploading the backup for like 5 or 10 times before exiting the script?

@lukechilds
Copy link
Member Author

If we're going to retry we should probably do that in a a wrapper around the entire backup logic to avoid a race condition.

It opens the possibility of getting in a retry loop due to a network failure, a separate backup gets triggered, networking now works again, the new backup uploads, then our retry works and we overwrite the latest backup with an outdated backup.

So we should rebuild the backup file from the latest files on disk.

Maybe we can merge this as is and look into that as a separate PR.

Copy link
Member

@mayankchhabra mayankchhabra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that makes sense!

@lukechilds lukechilds merged commit 9363d62 into getumbrel:master Sep 2, 2020
@lukechilds lukechilds mentioned this pull request Sep 2, 2020
lukechilds added a commit that referenced this pull request Sep 2, 2020
This update improves the reliability of automatic channel backups.

Changes:

- Increase decoy backup max interval to 12 hours (#214) a88541a
- Handle backup upload failure (#215) 9363d62
- Make backups thread safe (#213) 514fad9

Diff: v0.2.8...v0.2.7
Ahmed262111 pushed a commit to Ahmed262111/umbrel that referenced this pull request May 27, 2024
- special case to show textarea for "message" arg on "verifymessage" method
- strip carriage returns before sending to node
- after obtaining result, fix for rendering newlines back into textarea (since the newlines got mashed via JSON.stringify before being sent via RPC interface)
- minor frontend style tweaks on /rpc-browser
- minor logging tweaks for /rpc-browser
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

2 participants