-
Notifications
You must be signed in to change notification settings - Fork 501
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
Make backups thread safe #213
Make backups thread safe #213
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, @lukechilds! Quick question — should we remove ${UMBREL_ROOT}/statuses/backup-in-progress
via the next OTA update if it exists (just as a cleanup so everything's consistent with repo, even though it wouldn't make a difference)?
BACKUP_FOLDER="backup" | ||
BACKUP_ROOT="${UMBREL_ROOT}/${BACKUP_FOLDER}" | ||
BACKUP_FILE="${UMBREL_ROOT}/backup.tar.gz.pgp" | ||
BACKUP_ROOT="${UMBREL_ROOT}/.backup/$RANDOM" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, didn't know there was an always available $RANDOM
in shell 😍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not very random, in fact it's not even uniformly random, but yeah, it's always available in a shell and fine for stuff like this.
It's also not actually a variable but a function, so you can use it multiple times in the same line
e.g:
$ echo "${RANDOM}:${RANDOM}:${RANDOM}"
16448:13088:7550
Good idea! I'll add a line to the update script. |
So it turns out the start script would've cleared the backup lock file anyway. So I've removed all instances of cleaning up the lock file that were left in since we don't need them anymore, and added a single cleanup in the OTA update script. |
This change removes the need for a lock file to prevent concurrent backups. Multiple backup threads can now run concurrently without issue.
This improves reliability, since int he edge case that a lock file doesn't get cleaned up correctly, no future backups would ever run.