Skip to content

Add setting for users to determine frequency of backup creation #13199

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

Open
wants to merge 41 commits into
base: main
Choose a base branch
from

Conversation

farewelltospring
Copy link

@farewelltospring farewelltospring commented Oct 2, 2023

First time contributor checklist

Contributor checklist

  • Sony Xperia 5iii, Android 13
  • Virtual device Pixel 3, Android 13
  • My contribution is fully baked and ready to be merged as is
  • I ensure that all the open issues my contribution fixes are mentioned in the commit message of my first commit using the Fixes #1234 syntax

Description

This pull request allows users change the frequency that backups are made to a selected interval (weekly, monthly, quarterly, every 6 months, annually).

Currently, when backups are enabled, Signal will create a fresh backup every 24-hours. This causes unnecessary write cycles on the device's internal storage or SD card.

  • Making backups is a power-intensive process which drains battery and uses CPU cycles which can put a strain on daily usage when users are out and about, and slow down other apps on the device while the backups are being created.
  • Different users have different needs from backups. Some users message on Signal all day long, and others message on Signal infrequently. A 24-hour backup schedule is useful for the first group, but is of dubious value for the second group. It will write an identical file over and over.
  • Low-end devices are wearing out prematurely. Many low-end Android phones have low-durability eMMC chips. A 24-hour backup schedule makes their already-short lifespan even shorter. eMMC chips will fail catastrophically when they reach end-of-life. The device bricks, and the user must replace the whole thing. This disproportionately hurts Signal's poor users and produces environmental pollution.

A setting to control the backup frequency would also give people more control over their data, which is an important value to cultivate in today's society.

Verified that this behaviour works by setting backup frequency to 30 days and then waiting that long to see if a backup got created (I restarted the device several times during that time period to verify that it persists).

Screen_recording_20240111_000341.mp4

farewelltospring added 14 commits May 10, 2023 01:07
Yes I know this is bad practice but it's just for testing purposes
Rename OK button and add cancel button
Apparently it's a Unix timestamp in milliseconds and this took me way
too long to figure out myself
When changing backup frequency settings, schedule the next backup using
the new setting, but relative to the timestamp of the latest backup,
instead of immediately.

This ensures that backups are not created too frequently. A backup will
only be immediately created if the latest backup was too long ago,
relative to the new frequency setting, or if there is no latest backup.
Copy link

stale bot commented Dec 1, 2023

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 wontfix label Dec 1, 2023
farewelltospring added 3 commits December 3, 2023 01:24
- Replace debug toast with debug log statement
- Use MaterialAlertDialog instead of boring AlertDialog
@stale stale bot removed the wontfix label Dec 3, 2023
@farewelltospring
Copy link
Author

Fixed the localisation issue. I have only added a string for English but at least there are resources now.

farewelltospring added 2 commits December 4, 2023 01:22
Just because the plural has a single manifestation in English doesn't
mean we don't need to have plurals for it in other languages.
@farewelltospring
Copy link
Author

Is there anything I can do to make this PR better?

@farewelltospring
Copy link
Author

farewelltospring commented Jan 11, 2024

Added a video recording demonstrating the feature. (link)

Copy link

stale bot commented Mar 12, 2024

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 wontfix label Mar 12, 2024
@stale stale bot removed the wontfix label Mar 12, 2024
@stale stale bot removed the wontfix label Jan 5, 2025
@tkrah
Copy link

tkrah commented Jan 6, 2025

Found this PR as this is the feature I really miss - what needs to be done to get this merged? This 49ba83d is not be able to replace this PR because it is meant to be used in the upcoming remote chat backup - if you WANT a local backup, we still need this PR.

@greyson-signal
Copy link
Contributor

I'm interested in the feature in general, but I think the UX would be different. Rather than time and frequency sharing a settings row, we'd leave the time row alone, then make a new frequence row with the options "daily, weekly, monthly, manual".

@farewelltospring
Copy link
Author

By "manual" you mean backups will only be created when the user presses the create button? I can get behind that.

farewelltospring added 2 commits February 17, 2025 09:32
- Split frequency setting into separate setting entry in the UI
- Add 'never' option for backup frequency
- Do not schedule a backup when the user has selected 'never' for backup
  frequency
- Cleanups
@farewelltospring
Copy link
Author

Ok I uploaded some code. Full disclosure I haven't tested it yet because I gotta head out now and it'll take too long to compile, but I'll test it and take a video when I get back.

Copy link

stale bot commented Apr 20, 2025

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 wontfix label Apr 20, 2025
@stale stale bot removed the wontfix label Apr 22, 2025
@farewelltospring
Copy link
Author

Demo recording attached.

Screen_recording_20250428_015236.mp4

@farewelltospring
Copy link
Author

farewelltospring commented Apr 28, 2025

Played around with it a little and I think found an edge case where if you change the frequency from a schedule to NEVER, then the original scheduled backup never gets canceled. So it'll create one more backup according to the previous frequency, and then stop afterwards. Do you think this is acceptable behaviour?

FWIW, I tried to dig into the code to figure out how to avoid this. There's a lot of abstraction and general cruft in the job management system (whose files are also not very well documented) and I can't immediately see a clean way to cancel a scheduled backup. Could you give me a code pointer to cancel a scheduled job? Or I can dig deeper myself at the cost of more time. Having said that, doesn't seem to persist across reboots, so there's that.

Curiously, one thing that does persist across reboots is in-progress backups. This seems to be a built-in feature to the JobRunner/JobDatabase/etc files, because logcat shows that it only tracks when the job was submitted and doesn't seem to respect the backup frequency setting when I switch it to NEVER.

IMO a good UX would be for switching the frequency setting to NEVER to:

  1. Cancel future scheduled backups without relying on the user to reboot to clear it out
  2. Cancel in-progress backups

If that additional change is not needed then we can review the code more or less as-is (which I'm fine with as well). Please let me know how you would like to move this PR forward.

@farewelltospring
Copy link
Author

Come through

@alex-signal
Copy link
Contributor

Cancellation methods are on JobManager, which you can access via AppDependencies.getJobManager()

@gabefair
Copy link

Thank you for all the updates to make this feature possible

@farewelltospring
Copy link
Author

I've been daily driving this for the past month and the new "never" setting has been working as expected.

  • Setting backup frequency to NEVER cancels a scheduled job without a need for reboot, confirmed by having a latest backup from April, setting the frequency to DAILY, and then setting it to NEVER, and confirming that a backup never begins getting created. ☑️
  • Setting backup frequency to NEVERcancels an in-progress job, confirmed by checking logcat for W LocalBackupJobApi29: Backup cancelled. ☑️

If there are no other comments, I believe that this PR is ready to be reviewed for full merge.

@farewelltospring
Copy link
Author

🙂

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.

7 participants