From 3e2e8e0fe8a9c0210288c030df7aebe7e8dbf261 Mon Sep 17 00:00:00 2001 From: Santo Shakil Date: Wed, 23 Sep 2026 17:27:45 +0600 Subject: [PATCH] fix(mobile): retry the background backup when connectivity is restored (#30909) * fix(mobile): retry the background backup when connectivity is restored * return the backup result from the zone and drop the unrelated upload test * flip the background worker bool to needsRetry * decide the background retry from the upload callbacks * run the background worker offline again and retry on reconnect with a separate connected gated request * retry the background backup when any upload fails and keep one worker at a time * always hand the retry to the connected worker and release the worker lock if startWork throws * respect the charging only setting on the connected retry worker --- .../immich/background/BackgroundWorker.kt | 96 ++++++++++++------- .../background/BackgroundWorkerApiImpl.kt | 23 +++++ .../services/background_worker.service.dart | 66 +++++++------ mobile/pigeon/background_worker_api.dart | 3 +- 4 files changed, 120 insertions(+), 68 deletions(-) diff --git a/mobile/android/app/src/main/kotlin/app/alextran/immich/background/BackgroundWorker.kt b/mobile/android/app/src/main/kotlin/app/alextran/immich/background/BackgroundWorker.kt index 60387f434..522183c66 100644 --- a/mobile/android/app/src/main/kotlin/app/alextran/immich/background/BackgroundWorker.kt +++ b/mobile/android/app/src/main/kotlin/app/alextran/immich/background/BackgroundWorker.kt @@ -24,6 +24,7 @@ import io.flutter.embedding.engine.FlutterEngineCache import io.flutter.embedding.engine.dart.DartExecutor import io.flutter.embedding.engine.loader.FlutterLoader import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicReference private const val TAG = "BackgroundWorker" @@ -59,56 +60,68 @@ class BackgroundWorker(context: Context, params: WorkerParameters) : companion object { private const val NOTIFICATION_CHANNEL_ID = "immich::background_worker::notif" private const val NOTIFICATION_ID = 100 + private val activeWorker = AtomicReference(null) } override fun startWork(): ListenableFuture { - if (BackgroundWorkerPreferences(ctx).isLocked() && BackgroundEngineLock.connectEngines > 0) { - Log.i(TAG, "Foreground engine active, skipping background worker") + if (!activeWorker.compareAndSet(null, this)) { + Log.i(TAG, "Another background worker is running, skipping") return Futures.immediateFuture(Result.success()) } - Log.i(TAG, "Starting background upload worker") - - if (!loader.initialized()) { - loader.startInitialization(ctx) + if (BackgroundWorkerPreferences(ctx).isLocked() && BackgroundEngineLock.connectEngines > 0) { + Log.i(TAG, "Foreground engine active, skipping background worker") + activeWorker.compareAndSet(this, null) + return Futures.immediateFuture(Result.success()) } - val notificationChannel = NotificationChannel( - NOTIFICATION_CHANNEL_ID, - ctx.getString(R.string.background_worker_notification_channel_name), - NotificationManager.IMPORTANCE_LOW - ) - notificationManager.createNotificationChannel(notificationChannel) - val notificationConfig = BackgroundWorkerPreferences(ctx).getNotificationConfig() - showNotification(notificationConfig.first, notificationConfig.second) + try { + Log.i(TAG, "Starting background upload worker") - loader.ensureInitializationCompleteAsync(ctx, null, Handler(Looper.getMainLooper())) { - if (isStopped || isComplete) { - return@ensureInitializationCompleteAsync + if (!loader.initialized()) { + loader.startInitialization(ctx) } - engine = FlutterEngine(ctx) - FlutterEngineCache.getInstance().put(BackgroundWorkerApiImpl.ENGINE_CACHE_KEY, engine!!) - - // Register custom plugins - MainActivity.registerPlugins(ctx, engine!!) - flutterApi = - BackgroundWorkerFlutterApi(binaryMessenger = engine!!.dartExecutor.binaryMessenger) - BackgroundWorkerBgHostApi.setUp( - binaryMessenger = engine!!.dartExecutor.binaryMessenger, - api = this + val notificationChannel = NotificationChannel( + NOTIFICATION_CHANNEL_ID, + ctx.getString(R.string.background_worker_notification_channel_name), + NotificationManager.IMPORTANCE_LOW ) + notificationManager.createNotificationChannel(notificationChannel) + val notificationConfig = BackgroundWorkerPreferences(ctx).getNotificationConfig() + showNotification(notificationConfig.first, notificationConfig.second) - engine!!.dartExecutor.executeDartEntrypoint( - DartExecutor.DartEntrypoint( - loader.findAppBundlePath(), - "package:immich_mobile/domain/services/background_worker.service.dart", - "backgroundSyncNativeEntrypoint" + loader.ensureInitializationCompleteAsync(ctx, null, Handler(Looper.getMainLooper())) { + if (isStopped || isComplete) { + return@ensureInitializationCompleteAsync + } + + engine = FlutterEngine(ctx) + FlutterEngineCache.getInstance().put(BackgroundWorkerApiImpl.ENGINE_CACHE_KEY, engine!!) + + // Register custom plugins + MainActivity.registerPlugins(ctx, engine!!) + flutterApi = + BackgroundWorkerFlutterApi(binaryMessenger = engine!!.dartExecutor.binaryMessenger) + BackgroundWorkerBgHostApi.setUp( + binaryMessenger = engine!!.dartExecutor.binaryMessenger, + api = this ) - ) - } - return completionHandler + engine!!.dartExecutor.executeDartEntrypoint( + DartExecutor.DartEntrypoint( + loader.findAppBundlePath(), + "package:immich_mobile/domain/services/background_worker.service.dart", + "backgroundSyncNativeEntrypoint" + ) + ) + } + + return completionHandler + } catch (error: Throwable) { + activeWorker.compareAndSet(this, null) + throw error + } } /** @@ -184,13 +197,21 @@ class BackgroundWorker(context: Context, params: WorkerParameters) : close() } - private fun handleHostResult(result: kotlin.Result) { + private fun handleHostResult(result: kotlin.Result) { if (isComplete) { return } result.fold( - onSuccess = { _ -> complete(Result.success()) }, + onSuccess = { needsRetry -> + // The connected worker cannot enqueue itself while it is running, so it retries in place + if (needsRetry && !tags.contains(BackgroundWorkerApiImpl.RETRY_TAG)) { + BackgroundWorkerApiImpl.enqueueBackgroundWorkerWhenConnected(ctx) + complete(Result.success()) + } else { + complete(if (needsRetry) Result.retry() else Result.success()) + } + }, onFailure = { _ -> onStopped() } ) } @@ -214,6 +235,7 @@ class BackgroundWorker(context: Context, params: WorkerParameters) : flutterApi = null notificationManager.cancel(NOTIFICATION_ID) FlutterEngineCache.getInstance().remove(BackgroundWorkerApiImpl.ENGINE_CACHE_KEY) + activeWorker.compareAndSet(this, null) waitForForegroundPromotion() completionHandler.set(success) } diff --git a/mobile/android/app/src/main/kotlin/app/alextran/immich/background/BackgroundWorkerApiImpl.kt b/mobile/android/app/src/main/kotlin/app/alextran/immich/background/BackgroundWorkerApiImpl.kt index bc0766bee..b971da45c 100644 --- a/mobile/android/app/src/main/kotlin/app/alextran/immich/background/BackgroundWorkerApiImpl.kt +++ b/mobile/android/app/src/main/kotlin/app/alextran/immich/background/BackgroundWorkerApiImpl.kt @@ -7,6 +7,7 @@ import androidx.work.BackoffPolicy import androidx.work.Constraints import androidx.work.ExistingPeriodicWorkPolicy import androidx.work.ExistingWorkPolicy +import androidx.work.NetworkType import androidx.work.OneTimeWorkRequestBuilder import androidx.work.PeriodicWorkRequestBuilder import androidx.work.WorkManager @@ -37,6 +38,7 @@ class BackgroundWorkerApiImpl(context: Context) : BackgroundWorkerFgHostApi { WorkManager.getInstance(ctx).apply { cancelUniqueWork(OBSERVER_WORKER_NAME) cancelUniqueWork(BACKGROUND_WORKER_NAME) + cancelUniqueWork(RETRY_WORKER_NAME) cancelUniqueWork(PERIODIC_WORKER_NAME) } Log.i(TAG, "Cancelled background upload tasks") @@ -44,9 +46,11 @@ class BackgroundWorkerApiImpl(context: Context) : BackgroundWorkerFgHostApi { companion object { private const val BACKGROUND_WORKER_NAME = "immich/BackgroundWorkerV1" + private const val RETRY_WORKER_NAME = "immich/BackgroundWorkerRetryV1" private const val OBSERVER_WORKER_NAME = "immich/MediaObserverV1" private const val PERIODIC_WORKER_NAME = "immich/PeriodicBackgroundWorkerV1" const val ENGINE_CACHE_KEY = "immich::background_worker::engine" + const val RETRY_TAG = "immich/retryWhenConnected" fun enqueueMediaObserver(ctx: Context) { @@ -106,6 +110,24 @@ class BackgroundWorkerApiImpl(context: Context) : BackgroundWorkerFgHostApi { Log.i(TAG, "Enqueued background worker with name: $BACKGROUND_WORKER_NAME") } + fun enqueueBackgroundWorkerWhenConnected(ctx: Context) { + val settings = BackgroundWorkerPreferences(ctx).getSettings() + val constraints = Constraints.Builder().apply { + setRequiredNetworkType(NetworkType.CONNECTED) + setRequiresBatteryNotLow(true) + setRequiresCharging(settings.requiresCharging) + }.build() + val work = OneTimeWorkRequestBuilder() + .setConstraints(constraints) + .setBackoffCriteria(BackoffPolicy.EXPONENTIAL, 1, TimeUnit.MINUTES) + .addTag(RETRY_TAG) + .build() + WorkManager.getInstance(ctx) + .enqueueUniqueWork(RETRY_WORKER_NAME, ExistingWorkPolicy.KEEP, work) + + Log.i(TAG, "Enqueued background worker to run once connected") + } + fun isBackgroundWorkerRunning(): Boolean { // Easier to check if the engine is cached as we always cache the engine when starting the worker // and remove it when the worker is finished @@ -114,6 +136,7 @@ class BackgroundWorkerApiImpl(context: Context) : BackgroundWorkerFgHostApi { fun cancelBackgroundWorker(ctx: Context) { WorkManager.getInstance(ctx).cancelUniqueWork(BACKGROUND_WORKER_NAME) + WorkManager.getInstance(ctx).cancelUniqueWork(RETRY_WORKER_NAME) FlutterEngineCache.getInstance().remove(ENGINE_CACHE_KEY) Log.i(TAG, "Cancelled background upload task") diff --git a/mobile/lib/domain/services/background_worker.service.dart b/mobile/lib/domain/services/background_worker.service.dart index 6e04c4646..6286d3748 100644 --- a/mobile/lib/domain/services/background_worker.service.dart +++ b/mobile/lib/domain/services/background_worker.service.dart @@ -141,7 +141,7 @@ class BackgroundWorkerBgService extends BackgroundWorkerFlutterApi { } @override - Future onAndroidUpload(int? maxMinutes) async { + Future onAndroidUpload(int? maxMinutes) async { final hashTimeout = Duration(minutes: _isBackupEnabled ? 3 : 6); final backupTimeout = maxMinutes != null ? Duration(minutes: maxMinutes - 1) : null; await _optimizeDB(); @@ -199,7 +199,7 @@ class BackgroundWorkerBgService extends BackgroundWorkerFlutterApi { } } - Future _backgroundLoop({ + Future _backgroundLoop({ required Duration hashTimeout, required Duration? backupTimeout, required String debugLabel, @@ -211,7 +211,7 @@ class BackgroundWorkerBgService extends BackgroundWorkerFlutterApi { try { if (!await _syncAssets(hashTimeout: hashTimeout)) { _logger.warning("Remote sync did not complete successfully, skipping backup"); - return; + return true; } final backupFuture = _handleBackup(); @@ -225,12 +225,13 @@ class BackgroundWorkerBgService extends BackgroundWorkerFlutterApi { }); } try { - await backupFuture; + return await backupFuture; } finally { cancelTimer?.cancel(); } } catch (error, stack) { _logger.severe("Failed to complete $debugLabel", error, stack); + return true; } finally { sw.stop(); _logger.info("$debugLabel completed in ${sw.elapsed.inSeconds}s"); @@ -290,36 +291,41 @@ class BackgroundWorkerBgService extends BackgroundWorkerFlutterApi { } } - Future _handleBackup() async { - await runZonedGuarded( - () async { - if (_isCleanedUp) { - return; - } + Future _handleBackup() async { + final needsRetry = await runZonedGuarded(() async { + if (_isCleanedUp) { + return false; + } - if (!_isBackupEnabled) { - _logger.info("Backup is disabled. Skipping backup routine"); - return; - } + if (!_isBackupEnabled) { + _logger.info("Backup is disabled. Skipping backup routine"); + return false; + } - final currentUser = _ref?.read(currentUserProvider); - if (currentUser == null) { - _logger.warning("No current user found. Skipping backup from background"); - return; - } + final currentUser = _ref?.read(currentUserProvider); + if (currentUser == null) { + _logger.warning("No current user found. Skipping backup from background"); + return false; + } - if (Platform.isIOS) { - return _ref?.read(backupProvider.notifier).startBackupWithURLSession(currentUser.id); - } + if (Platform.isIOS) { + await _ref?.read(backupProvider.notifier).startBackupWithURLSession(currentUser.id); + return false; + } - return _ref - ?.read(foregroundUploadServiceProvider) - .uploadCandidates(currentUser.id, _cancellationToken, useSequentialUpload: true); - }, - (error, stack) { - dPrint(() => "Error in backup zone $error, $stack"); - }, - ); + final uploadService = _ref?.read(foregroundUploadServiceProvider); + var failed = 0; + await uploadService?.uploadCandidates( + currentUser.id, + _cancellationToken, + callbacks: UploadCallbacks(onError: (_, _) => failed++), + useSequentialUpload: true, + ); + _logger.info("Background backup finished, $failed uploads failed"); + // Retry when at least one upload failed + return failed > 0; + }, (error, stack) => dPrint(() => "Error in backup zone $error, $stack")); + return needsRetry ?? true; } Future _syncAssets({Duration? hashTimeout}) async { diff --git a/mobile/pigeon/background_worker_api.dart b/mobile/pigeon/background_worker_api.dart index 06395fae7..dd62dd069 100644 --- a/mobile/pigeon/background_worker_api.dart +++ b/mobile/pigeon/background_worker_api.dart @@ -46,8 +46,9 @@ abstract class BackgroundWorkerFlutterApi { void onIosUpload(bool isRefresh, int? maxSeconds); // Android Only: Called when the Android background upload is triggered + // Returns whether the native side needs to retry the run @async - void onAndroidUpload(int? maxMinutes); + bool onAndroidUpload(int? maxMinutes); @async void cancel();