upload new photos in background with a service

[fyfrey]

trafficstars

This is very crude attempt at automatically uploading new photos in a background service even if the app is not open. Partly fixes issue #235

Only works on Android!

Disclaimer: I'm new to writing dart, especially with the frameworks hive/riverpod that both might have severe concurrency issues with the draft approach (although in quick manual testing it was ok so far).

As of now, the integration into the existing backup state/logic is also far from optimal.

• All images in the folders on the device are scanned... although the new service explicitly states which files have changed (might be new, moved or deleted etc.)
• Background currently does / needs to do everything the open app also does (regarding logging in, changing state, ...)
• Almost no error handling

[fyfrey]

@alextran1502 I've made some improvements to the service and its handling. However, before continuing further, I'd rather like to plan with you how to integrate the background service nicely. Both code/architecture and user experience/interface aspects.

[alextran1502]

@zoodyy Agree, I haven't had a chance to go through the code fully. I will let you know as soon as I've done that. I've noticed a few things we might be able to make this simpler on the implementation side.

[alextran1502]

What would trigger the background service to start running? What are some points you would like to discuss? What is the recommended testing method for this feature?

[fyfrey]

What would trigger the background service to start running?

Current setup is the following: User activates automatic backup in the app, this enqueues the BackupWorker in the Android WorkManager. Whenever a photo/video is changed on the device, the Android OS starts the BackupWorker (and provides a list of the changed photos/videos). I further added configurable constraints such as battery not low, device is charging (not required by default currently), device has unmetered network connection (WIFI). Only when all constraints are met, the BackupWorker is executed by the system. It has some builtin waiting period (several seconds, I could check if its configurable in the WorkManager API if we wanted to increase it) before starting the worker after a new picture is added so that it is not executed for every single file when taking multiple pictures in a row. Once the service is active, it automatically enqueues itself again with the same constraints. This is necessary because the file-changed constraints are not available to periodic tasks but only to one-time tasks. We could actually configure the constraints for changed (added, moved, deleted) photos/video files, such that only folders that are selected for backup in the app are monitored by system. Have not done that yet because it is more work :-)

An alternative (simpler) approach would be to let the system execute the service every x > 15 minutes. This is the minimum interval for background services on Android. However, this is quite wasteful regarding battery usage when not taking any pictures AND backups are delayed longer after taking a photo.

What are some points you would like to discuss?

1. How to handle app and background service potentially performing backup at the same time (already doing that in the comments you started in the other discussion thread
2. Is communication between background service and foreground app necessary?: Currently not implemented and I am honestly not sure if it is possible (it is somehow possible between native Android app/service and between Dart/Flutter isolates... but might be very challenging in combination)
3. Does the background service need update any persistent state of the app? Is there persisted state e.g. which local files are already backed up to the server, ..) I've read that opening the same hive store concurrently is not supported.
4. How to let the user decide/configure the background service and its constraints (mostly UI)
5. How to make the background service as lean as possible: Not loading riverpod and all services etc. (no login, no checking all assets on device etc.) Simply load auth Bearer token from hive/sharedpreferences etc., make API calls to upload all new photos (as given by files changed info from Android WorkManager), optionally update local persistent state (which files are backed up), exit. This is very important to preserve battery life because currently it is too costly to upload a single new photo.
6. BackgroundService is currently a util with only static methods and two top-level methods. Not sure whether this fits in well. Maybe change the service to a singleton or put it somewhere else (currently modules/backup/services)? That one top-level method (as stated in its documentation comment) needs to stay, however.
7. Error handling in the background service: First, let the WorkManager retry the task after some time. If an unrecoverable errors occurs / number of retries exhausted, stop the service & show a native notification to the user: Clicking on it opens the app to possible show more info and let the app enqueue the service again?
8. Edge case handling: If the service uploads only changed files notified by the system, some files will be missed (e.g. after stopping the app by removing it from recent apps, after errors in background upload, etc.). Thus, I feel it is sometimes necessary to check all assets (as it is currently done) and upload all files that have been missed. This could be done in foreground when the user opens the app - this has issues as discussed in 1. here. Maybe schedule a periodic task or execute it on app start once per day: This task pauses the normal BackupWorker, performs the full backup with finding any missed files, reenabled the normal BackupWorker, exits.

What is the recommended testing method for this feature?

Start the app, click "Turn on backup" in the backup settings, switch to another app, e.g. camera, take a photo, wait ~10 seconds (might be device depended), (optionally look at app debug log, currently there should be [resumeBackup] Start back up and Worker result SUCCESS for Work [ id=..., tags={ app.alextran.immich.BackupWorker } ]), check that the newly taken photo is backed up the server by looking at the web app. Maybe the whole process could somehow be tested automatically with an E2E integration test.. I'm not familiar with automated testing on mobile though.

[alextran1502]

1. How to handle app and background service potentially performing backup at the same time We will need to figure out a way to handle communication between the two, ideally through persistent storage (Hive/SharePreference)

2. Is communication between background service and foreground app necessary?: Currently not implemented and I am honestly not sure if it is possible (it is somehow possible between native Android app/service and between Dart/Flutter isolates... but might be very challenging in combination) Yes, we will need this to make the UI as sensible as possible

3. Does the background service need update any persistent state of the app? Is there persisted state e.g. which local files are already backed up to the server, ..) I've read that opening the same hive store concurrently is not supported. I would say no. Before uploading, workers can query which files are in the database and start backup files that are not. This is already built in the backup workflow

4. How to let the user decide/configure the background service and its constraints (mostly UI) I think this can be set in an app's options page, which still needed to be built. Once we have the mechanism down, I can handle this task

5. How to make the background service as lean as possible: Not loading riverpod and all services etc. (no login, no checking all assets on device etc.) Simply load auth Bearer token from hive/sharedpreferences etc., make API calls to upload all new photos (as given by files changed info from Android WorkManager), optionally update local persistent state (which files are backed up), exit. This is very important to preserve battery life because currently it is too costly to upload a single new photo.

With the current ApiService, we can do this very easily.

6. BackgroundService is currently a util with only static methods and two top-level methods. Not sure whether this fits in well. Maybe change the service to a singleton or put it somewhere else (currently modules/backup/services)? That one top-level method (as stated in its documentation comment) needs to stay, however.

I can imagine that we will eventually have some more background services or another background service with different logic for iOS, so we can put them in module/backup/background_service/[android/ios]_background.service.dart

7. Error handling in the background service: First, let the WorkManager retry the task after some time. If an unrecoverable errors occurs / number of retries exhausted, stop the service & show a native notification to the user: Clicking on it opens the app to possible show more info and let the app enqueue the service again? This sounds good to me.

8. Edge case handling: If the service uploads only changed files notified by the system, some files will be missed (e.g. after stopping the app by removing it from recent apps, after errors in background upload, etc.). Thus, I feel it is sometimes necessary to check all assets (as it is currently done) and upload all files that have been missed. This could be done in foreground when the user opens the app - this has issues as discussed in https://github.com/immich-app/immich/pull/382#discussion_r932978404. Maybe schedule a periodic task or execute it on app start once per day: This task pauses the normal BackupWorker, performs the full backup with finding any missed files, reenabled the normal BackupWorker, exits.

Why don't we always check for all assets with the server before uploading in background service as it is being done in the foreground? This ensures that the most recent information is up to date.

[fyfrey]

Thanks for the extensive answers!

1. I'll try out accessing Hive boxes at the same time and see if any issues arise
2. I guess if the background upload is stopped while the app is open, we do not need live process-to-process communication, and communication via persistent state is enough?
3. Okay, so every photo on the device is checked whether it already exists on the server every time the backup runs? Seems less than ideal from an efficiency perspective... however, it keeps local state very small :-)
4. Alright, I can wait for the option page. An alternative could be to put the options on the backup page (you can already select which albums to backup, how/when to backup all albums also makes sense in that context)
5. ApiService looks good! Remains the question if/how to skip the backupProvider... currently startBackupProcess seems to me like a very costly method doing a lot of work (getBackupInfo and building the set of assetsWillBeBackup) to derive which files to upload. I'll measure this on my phone with a few thousand photos to see if there are substantial improvements to be gained. This is not an issue if done once per app opening, but it will negatively impact the battery usage when the service executes frequently (due to taking new photos). The background service retrieves this set of new files almost for free from the system. This could be used to create AssetEntitys from the file URLs and call _backupService.backupAsset (or the ApiService directly with the necessary info). As an alternative I could try to make the current backup code more efficient (that would benefit both foreground and background backup)
6. Yeah, iOS works quite different as far as I know. Putting the background services in their own folder seems a good choice to not confuse them with the existing riverpod services
7. I'll look into building such a notification that opens the app and either store the error information in a Hive box or send it along with the app opening intent
8. We can certainly do that (this is currently implemented in this PR). This edge case handling would only be an issue if the background service does not check all photos for battery efficiency reasons (see 5.) and foreground backup would be disabled. Since you prefer to keep the foreground upload active all the time, these edge cases are also resolved automatically by opening the app.

[fyfrey]

Did some performance testing to check if the current backup preparation (code in method startBackupProcess until _backupService.backupAsset is called) takes too much time/CPU/battery to run often (as a potentially frequently running background service). Experiment environment: release build of immich, ~6000 photos on the device and on the server, triggering a start of the background service by taking a new picture.

total  overhead  getBackupInfo  assetsWillBeBackup
 2561      2473           1469                 837
 2357      2253            882                1248
 2829      2764           1258                1359

All measurements are in milliseconds (3 runs in total). total is the time from the line in Kotlin BackupWorker.startWork until last line in Kotlin BackupWorker.stopEngine. overhead is the entire time spent in _onPhotosChanged (opening Hive boxes, instantiating riverpod services, calling backupProvider.resumeBackup until _backupService.backupAsset is called (where the actual backup is performed). getBackupInfo is the time taken by that function in backupProvider (note: it's actually called twice in parallel, once by instatiating the backupProvider, once by calling startBackupProcess, thus multiply its value by 2 to get CPU time usage). assetsWillBeBackup refers to time taken in startBackupProcess where the set of assets to upload is created (by removing assets already present on the server).

Using the existing backup logic burns (when already optimizing the duplicate call to getBackupInfo) roughly 2.5 seconds worth of CPU time (and battery!). This heavy cost is paid every time the background service is triggered to upload any number of new photos (usually just 1). Thus, I'll definitely look into ways to significantly reduce these 2.5 seconds, otherwise using the background service eats up the users phone battery too quickly. Most likely I'll use the list of changed assets given by Android when calling the worker and directly calling _backupService.backupAsset with a set of new assets (after verifying these are not already backed up to the server).

[alextran1502]

Did some performance testing to check if the current backup preparation (code in method startBackupProcess until _backupService.backupAsset is called) takes too much time/CPU/battery to run often (as a potentially frequently running background service). Experiment environment: release build of immich, ~6000 photos on the device and on the server, triggering a start of the background service by taking a new picture.

total  overhead  getBackupInfo  assetsWillBeBackup
 2561      2473           1469                 837
 2357      2253            882                1248
 2829      2764           1258                1359

All measurements are in milliseconds (3 runs in total). total is the time from the line in Kotlin BackupWorker.startWork until last line in Kotlin BackupWorker.stopEngine. overhead is the entire time spent in _onPhotosChanged (opening Hive boxes, instantiating riverpod services, calling backupProvider.resumeBackup until _backupService.backupAsset is called (where the actual backup is performed). getBackupInfo is the time taken by that function in backupProvider (note: it's actually called twice in parallel, once by instatiating the backupProvider, once by calling startBackupProcess, thus multiply its value by 2 to get CPU time usage). assetsWillBeBackup refers to time taken in startBackupProcess where the set of assets to upload is created (by removing assets already present on the server).

Using the existing backup logic burns (when already optimizing the duplicate call to getBackupInfo) roughly 2.5 seconds worth of CPU time (and battery!). This heavy cost is paid every time the background service is triggered to upload any number of new photos (usually just 1). Thus, I'll definitely look into ways to significantly reduce these 2.5 seconds, otherwise using the background service eats up the users phone battery too quickly. Most likely I'll use the list of changed assets given by Android when calling the worker and directly calling _backupService.backupAsset with a set of new assets (after verifying these are not already backed up to the server).

This is a very good assessment and testing, thank you. Another direction I think we can try to mitigate the overhead of using our traditional backup method is to check if the photos/videos are already on the server by directly asking the server. We currently have that API/Endpoint (/asset/check) or using the SDK

With the minimal assets will be back up each time. I think most cases will be under 10, and I think this won't sweat the server too much even if it has 100,000 records in the database ~since those two parameters are indexes.~

Edited: Those two are not indexes. They are unique columns, we can also make them indexes.

[fyfrey]

This is a very good assessment and testing, thank you. Another direction I think we can try to mitigate the overhead of using our traditional backup method is to check if the photos/videos are already on the server by directly asking the server. We currently have that API/Endpoint (/asset/check) or using the SDK

With the minimal assets will be back up each time. I think most cases will be under 10, and I think this won't sweat the server too much even if it has 100,000 records in the database since those two parameters are indexes

Edited: Those two are not indexes. They are unique columns, we can also make them indexes.

Good idea! I'll absolutely use that API to check for duplicates. Pairs extremely well with the short list of changed files the background service receives. This should reduce the assetsWillBeBackup CPU time considerably, leaving mostly getBackupInfo. Luckily, this is easy to replace in the background service :-)

Might even work for the currently used full check (i.e. uploading 6000 photo IDs to server and let it check if they are already there instead of downloading 6000 IDs and checking on device). Thus, the server does the comparison by looping over all ids.. or one could maybe write a single, optimized SQL query.

[alextran1502]

Might even work for the currently used full check (i.e. uploading 6000 photo IDs to server and let it check if they are already there instead of downloading 6000 IDs and checking on device). Thus, the server does the comparison by looping over all ids.. or one could maybe write a single, optimized SQL query.

I like this idea, any return IDs mean that they are good to go.

[matthinc]

I used this for some hours now and it seems to work very well. There could always be situations where some power saving logic kicks in (my phone is very strict with that) but we have to see how this works on the long term. The code looks super well written and I learned a lot about those Android workers :smile:

My suggestion would be to merge this (it does not seem to break anything existing) and do potential optimizations in a second PR once we received some user feedback about this. This PR in its current state already is a huge improvement IMO.

LGTM!

[zackpollard]

Hey,

Just catching up on this PR and the work you've done is great! I concur with @matthinc that we could merge this and then make performance improvements in the second iteration. I do have some suggestions though that might be worth taking onboard before we do so:

• We should provide a way for users to only allow background uploading when the phone is on WiFi and ideally also a toggle for only allowing it when the phone is charging.
• Background uploading should always upload everything that hasn't yet been uploaded, the new method sounds like it will only upload files that have just changed, so if we implemented toggles like these, files would be missed.
• It might make sense to avoid having both a foreground and background way of processing uploads. Personally I would approach this by having the foreground uploader just invoke the background process with a list of assets that need backing up, then if the user closes the app after clicking backup, the process continues. It would also likely simplify the logic as you don't need handoff between the foreground and background processes.
• On performance improvements, I believe we can improve this by having a better local state. We can store which assets the phone considers to have uploaded so we don't need to check them again. This would also have the benefit that if the asset is manually deleted from the server, the phone doesn't just put it straight back. This would vastly improve the performance of the check with the server to see which assets need to be uploaded as it would be checking a much smaller list of assets each time.

Let me know what you think about all of this and feel free to reply here or ping me on the discord if you want to discuss further.

Cheers for the work on this PR!

[fyfrey]

I used this for some hours now and it seems to work very well. There could always be situations where some power saving logic kicks in (my phone is very strict with that) but we have to see how this works on the long term. The code looks super well written and I learned a lot about those Android workers :smile:

Thanks! And you are right about the power saving logic. Android (in somewhat recent versions) is very strict about background battery usage. Thus, the system decides when to run the backup worker (usually scheduled together with other tasks to only wake up the CPU from sleep once and use battery expensive network in one batch).

My suggestion would be to merge this (it does not seem to break anything existing) and do potential optimizations in a second PR once we received some user feedback about this. This PR in its current state already is a huge improvement IMO.

LGTM!

Great! I've still got some TODOs in my code that I want to resolve beforehand (mainly internationalization)

[fyfrey]

1. We should provide a way for users to only allow background uploading when the phone is on WiFi and ideally also a toggle for only allowing it when the phone is charging.

I agree. Currently, WiFi is always required, but not charging. It's already configurable in Dart code. I could add toggles for these two settings in the backup screen as well, likely below the button to start/stop the background service.

2. Background uploading should always upload everything that hasn't yet been uploaded, the new method sounds like it will only upload files that have just changed, so if we implemented toggles like these, files would be missed.

It uploads almost everything that has not yet been uploaded. There is one exception: The user could move a photo from an excluded album to an included album. This would currently not be detected. I'd rather solve this issue in a follow-up PR.

3. It might make sense to avoid having both a foreground and background way of processing uploads. Personally I would approach this by having the foreground uploader just invoke the background process with a list of assets that need backing up, then if the user closes the app after clicking backup, the process continues. It would also likely simplify the logic as you don't need handoff between the foreground and background processes.

Agreed. I'd do that in another PR as well once the background service has been tested to work well in general.

4. On performance improvements, I believe we can improve this by having a better local state. We can store which assets the phone considers to have uploaded so we don't need to check them again. This would also have the benefit that if the asset is manually deleted from the server, the phone doesn't just put it straight back. This would vastly improve the performance of the check with the server to see which assets need to be uploaded as it would be checking a much smaller list of assets each time.

True, I would also advocate for a local client state to keep track which assets have already been backed up. It also allows to mitigate the issue 2. However, as it would severely change the current foreground backup, I'd rather tackle this in a separate PR.

Let me know what you think about all of this and feel free to reply here or ping me on the discord if you want to discuss further.

Cheers for the work on this PR!

Thanks! I'll reach out to you on discord to discuss some of the details

[alextran1502]

Thanks, @zoodyy, for an amazing PR. I just did a quick test, and there are some bugs that I want to put the steps to reproduce here. Tested on Samsung S9 release version

Case 1 - App's states aren't updating

1. Start the app
2. Enable background backup
3. Switch to the camera app, take a few photos
4. Immediately switch back to Immich
5. Background service is notified to be running, but there is an error in opening the HiveBox, it seems.
6. [ERROR] The state is in deadlock until explicitly closing the app.

Case 2 - Disable background backup not working

1. Start the app
2. Enable background backup
3. Take some photos and make sure the photos are uploaded in the background
4. Go to Immich
5. Disable background backup
6. Take some more photos
7. [ERROR] The workers are still uploading the new photos.

I would like to sort out these issues before merging the PR

Some info about the bug

Here is the log when I toggle the enable background backup button

Turning on

Turning off

The methods in question are disableBatteryOptimizations and stopService. My suggestion is have try-catch block in these methods and handle the error accordingly

[alextran1502]

@zoodyy I've just done some testing. One of the behaviors that are worth reporting is

1. Enable auto backup.
2. Select an album, let's say Screenshot only
3. Take a photo - This supposes to go to the Recent album
4. Now select only Recent.
5. Put the app into the background.
6. Is the app supposed to perform background backup here? The current behavior is that it doesn't upload that new photo.
7. If I take a new photo, then it will perform an upload on the last two.

This is not a major issue, this can be treated as expected behavior if it simplifies the code. The user simply needs to take a new photo to re-enable the background backup, and this is something we can include in the background backup info on the WIKI.

Thanks for the amazing work; I know a lot of Android users will be very happy with this feature.

[fyfrey]

6. Is the app supposed to perform background backup here? The current behavior is that it doesn't upload that new photo.

7. If I take a new photo, then it will perform an upload on the last two.

This is exactly the expected behavior. Everything works as intended: The background backup is only triggered to run whenever assets change (are added/deleted/modified). Since the photo was taken and thus the background service ran before it should backup the photo (only Screenshot album was allowed at that time), the service found that it must not upload that photo. Later, you changed the settings to upload everything (Recent), the next background backup run adds the old photo. However, changing the settings does not directly trigger it, only changing assets (in this case adding a new photo)

In a future PR, we can improve upon this by replacing the foreground upload with the background upload and schedule it to run immediately whenever the user changes which albums to backup etc. In theory, I could easily add this to the current PR, however would rather not force my luck that the foreground and background backup do not interfere :-)

This is not a major issue, this can be treated as expected behavior if it simplifies the code. The user simply needs to take a new photo to re-enable the background backup, and this is something we can include in the background backup info on the WIKI.

Some documentation on the WIKI is certainly handy

Thanks for the amazing work; I know a lot of Android users will be very happy with this feature.

Indeed! Feels a lot better knowing that photos are automatically backed up without needing to regularly start the app to perform the backup

[fyfrey]

This PR should now be ready to be merged :tada:

• Added options to perform background backup only when on WiFi and/or charging
• Now using translatable texts