-
Notifications
You must be signed in to change notification settings - Fork 5
fix(android): preserve queued upload scheduling #439
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
veryCrunchy
wants to merge
40
commits into
fix/account-background-isolation
from
fix/durable-upload-scheduling-recovery-stack
+4,229
−344
Open
Changes from all commits
Commits
Show all changes
40 commits
Select commit
Hold shift + click to select a range
85e98cf
fix(android): preserve queued upload scheduling
veryCrunchy c6b4b26
chore(changelog): link upload scheduling fix
veryCrunchy 44a1ca6
fix(android): restore queued uploads at startup
veryCrunchy b26934e
fix(android): retain startup upload retries
veryCrunchy 333c501
fix(android): retain upload reconciliation after journal errors
veryCrunchy 62e31b6
fix(android): defer uploads during credential recovery
veryCrunchy 98ec6ad
fix(uploads): skip WorkManager-owned recovery jobs
veryCrunchy 8daae96
fix(uploads): bound credential recovery retries
veryCrunchy dd2e84f
fix(uploads): recover registry before worker rejection
veryCrunchy cf16876
fix(uploads): keep credential recovery deferred
veryCrunchy 5044081
fix(uploads): bound startup recovery diagnostics
veryCrunchy 8975641
fix(uploads): wake failed scheduling recovery
veryCrunchy a283a90
fix(uploads): wake recovery after worker failure
veryCrunchy 02c60a8
fix(uploads): close recovery wakeup races
veryCrunchy 5eaac61
fix(uploads): centralize queued status recovery
veryCrunchy d4593bb
fix(uploads): back off worker recovery
veryCrunchy 0d4f423
fix(uploads): defer transient source failures
veryCrunchy 311530d
fix(uploads): fail permanently unavailable sources
veryCrunchy 4884c73
fix(uploads): release cancelled unowned selections
veryCrunchy dd4c598
fix(uploads): retry terminal capability cleanup
veryCrunchy ec05da5
fix(uploads): retain pending capability cleanup
veryCrunchy dd65812
test(uploads): cover legacy cleanup marker
veryCrunchy 034daca
fix(uploads): recover pending capability cleanup
veryCrunchy cc81b4e
test(uploads): keep cleanup cancellation test void
veryCrunchy 592fde8
fix(uploads): decouple terminal cleanup recovery
veryCrunchy f9c5bad
fix(uploads): validate persisted cleanup marker
veryCrunchy de87415
fix(uploads): run terminal cleanup offline
veryCrunchy 7c2c332
fix(uploads): preserve cleanup with corrupt registry
veryCrunchy c681afb
fix(uploads): retain unreadable capability metadata
veryCrunchy 23ac995
fix(uploads): clean cancelled picker grants
veryCrunchy 1a01aa9
fix(uploads): release undelivered picker selections
veryCrunchy 08c38bc
fix(uploads): recover orphaned picker grants
veryCrunchy fbeea7e
fix(uploads): preserve immediate recovery intent
veryCrunchy 4bfb482
fix(uploads): enforce picker capability limit
veryCrunchy 6582a09
fix(uploads): consume scheduling wakeups atomically
veryCrunchy 990e648
fix(uploads): defer capability metadata read failures
veryCrunchy 41f7a3e
fix(uploads): reject malformed capability metadata
veryCrunchy 3642e5e
fix(uploads): isolate malformed picker capabilities
veryCrunchy fe146f1
fix(uploads): protect owned malformed capabilities
veryCrunchy a712498
test(uploads): split account resolution coverage
veryCrunchy File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
321 changes: 160 additions & 161 deletions
321
androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidDurableMultipartUploads.kt
Large diffs are not rendered by default.
Oops, something went wrong.
76 changes: 76 additions & 0 deletions
76
...dApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidDurableUploadAccountResolution.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| package dev.obiente.nextcloudnative | ||
|
|
||
| import dev.obiente.nextcloudnative.app.DurableUploadState | ||
| import dev.obiente.nextcloudnative.app.NextcloudAccountId | ||
| import dev.obiente.nextcloudnative.app.NextcloudAccountRecord | ||
| import dev.obiente.nextcloudnative.app.NextcloudSession | ||
|
|
||
| internal sealed interface DurableUploadAccountResolution { | ||
| data class Available(val session: NextcloudSession) : DurableUploadAccountResolution | ||
| data object RegistryUnavailable : DurableUploadAccountResolution | ||
| data object CredentialUnavailable : DurableUploadAccountResolution | ||
| data object DeferAccountActivation : DurableUploadAccountResolution | ||
| data object AccountUnavailable : DurableUploadAccountResolution | ||
| } | ||
|
|
||
| internal sealed interface DurableUploadAccountRegistry { | ||
| data class Available( | ||
| val accounts: List<NextcloudAccountRecord>, | ||
| val activeAccountId: NextcloudAccountId? = null, | ||
| ) : DurableUploadAccountRegistry | ||
|
|
||
| data object Unavailable : DurableUploadAccountRegistry | ||
| } | ||
|
|
||
| internal fun queuedDurableUploadsForAccount( | ||
| jobs: List<AndroidDurableMultipartUploadJob>, | ||
| accountId: String, | ||
| ): List<AndroidDurableMultipartUploadJob> = jobs.filter { job -> | ||
| job.accountId == accountId && job.state == DurableUploadState.Queued | ||
| } | ||
|
|
||
| internal fun resolveDurableUploadSession( | ||
| expectedAccountId: String, | ||
| registry: DurableUploadAccountRegistry, | ||
| loadSession: (NextcloudAccountId) -> NextcloudSession?, | ||
| ): DurableUploadAccountResolution { | ||
| val availableRegistry = when (registry) { | ||
| is DurableUploadAccountRegistry.Available -> registry | ||
| DurableUploadAccountRegistry.Unavailable -> return DurableUploadAccountResolution.RegistryUnavailable | ||
| } | ||
| val account = availableRegistry.accounts.singleOrNull { record -> | ||
| NextcloudDocumentIds.accountKey(record.serverUrl, record.loginName) == expectedAccountId | ||
| } ?: return DurableUploadAccountResolution.AccountUnavailable | ||
| val session = loadSession(account.id) | ||
| ?.takeIf { loaded -> NextcloudDocumentIds.accountKey(loaded) == expectedAccountId } | ||
| ?: return if (account.id == availableRegistry.activeAccountId) { | ||
| DurableUploadAccountResolution.CredentialUnavailable | ||
| } else { | ||
| DurableUploadAccountResolution.DeferAccountActivation | ||
| } | ||
| return DurableUploadAccountResolution.Available(session) | ||
| } | ||
|
|
||
| internal fun resolveDurableUploadSessionWithRegistryRecovery( | ||
| expectedAccountId: String, | ||
| readRegistry: () -> DurableUploadAccountRegistry, | ||
| recoverRegistry: () -> NextcloudSession?, | ||
| loadSession: (NextcloudAccountId) -> NextcloudSession?, | ||
| ): DurableUploadAccountResolution { | ||
| val initial = readRegistry() | ||
| val recoveryRequired = when (initial) { | ||
| DurableUploadAccountRegistry.Unavailable -> true | ||
| is DurableUploadAccountRegistry.Available -> initial.accounts.none { account -> | ||
| NextcloudDocumentIds.accountKey(account.serverUrl, account.loginName) == expectedAccountId | ||
| } | ||
| } | ||
| if (!recoveryRequired) return resolveDurableUploadSession(expectedAccountId, initial, loadSession) | ||
| val recoveredSession = recoverRegistry() | ||
| if ( | ||
| recoveredSession != null && | ||
| NextcloudDocumentIds.accountKey(recoveredSession) == expectedAccountId | ||
| ) { | ||
| return DurableUploadAccountResolution.Available(recoveredSession) | ||
| } | ||
| return resolveDurableUploadSession(expectedAccountId, readRegistry(), loadSession) | ||
| } | ||
322 changes: 322 additions & 0 deletions
322
androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidDurableUploadScheduling.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,322 @@ | ||
| package dev.obiente.nextcloudnative | ||
|
|
||
| import dev.obiente.nextcloudnative.app.DurableUploadEnqueueResult | ||
| import dev.obiente.nextcloudnative.app.DurableUploadState | ||
| import java.util.UUID | ||
| import kotlinx.coroutines.CancellationException | ||
| import kotlinx.coroutines.CoroutineStart | ||
| import kotlinx.coroutines.async | ||
| import kotlinx.coroutines.channels.Channel | ||
| import kotlinx.coroutines.coroutineScope | ||
| import kotlinx.coroutines.selects.select | ||
| import kotlinx.coroutines.sync.Mutex | ||
| import kotlinx.coroutines.sync.withLock | ||
|
|
||
| internal class AndroidDurableUploadStartCoordinator { | ||
| private val monitor = Any() | ||
| private val jobLeases = mutableMapOf<String, JobLease>() | ||
|
|
||
| suspend fun <Result> withJob(jobId: String, action: suspend () -> Result): Result { | ||
| require(jobId.isNotBlank()) | ||
| val lease = synchronized(monitor) { | ||
| jobLeases.getOrPut(jobId) { JobLease() }.also { it.references += 1 } | ||
| } | ||
| return try { | ||
| lease.mutex.withLock { action() } | ||
| } finally { | ||
| synchronized(monitor) { | ||
| lease.references -= 1 | ||
| if (lease.references == 0) jobLeases.remove(jobId, lease) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private class JobLease( | ||
| val mutex: Mutex = Mutex(), | ||
| var references: Int = 0, | ||
| ) | ||
| } | ||
|
|
||
| private val ANDROID_DURABLE_UPLOAD_START_COORDINATOR = AndroidDurableUploadStartCoordinator() | ||
|
|
||
| internal const val ANDROID_DURABLE_UPLOAD_SCHEDULING_FOLLOW_UP_DELAY_MILLIS = 60_000L | ||
|
|
||
| internal data class AndroidDurableUploadSchedulingRecoveryBatch( | ||
| val immediate: Boolean, | ||
| val workIdsToAwait: List<UUID>, | ||
| ) | ||
|
|
||
| internal sealed interface AndroidDurableUploadSchedulingRecoveryStep { | ||
| data object Completed : AndroidDurableUploadSchedulingRecoveryStep | ||
|
|
||
| data class Interrupted( | ||
| val batch: AndroidDurableUploadSchedulingRecoveryBatch, | ||
| ) : AndroidDurableUploadSchedulingRecoveryStep | ||
| } | ||
|
|
||
| internal class AndroidDurableUploadSchedulingRecoverySignal( | ||
| private val beforeBatchClaim: suspend () -> Unit = {}, | ||
| ) { | ||
| private val monitor = Any() | ||
| private val wakeups = Channel<Unit>(Channel.CONFLATED) | ||
| private var immediatePending = false | ||
| private val workIdsToAwait = linkedSetOf<UUID>() | ||
|
|
||
| fun request() { | ||
| synchronized(monitor) { | ||
| immediatePending = true | ||
| wakeups.trySend(Unit) | ||
| } | ||
| } | ||
|
|
||
| fun requestAfterWorkStopsRunning(workId: UUID) { | ||
| synchronized(monitor) { | ||
| workIdsToAwait += workId | ||
| wakeups.trySend(Unit) | ||
| } | ||
| } | ||
|
|
||
| suspend fun await(): AndroidDurableUploadSchedulingRecoveryBatch { | ||
| wakeups.receive() | ||
| beforeBatchClaim() | ||
| return takeBatch() | ||
|
veryCrunchy marked this conversation as resolved.
|
||
| } | ||
|
|
||
| suspend fun runUntilRequested( | ||
| action: suspend () -> Unit, | ||
| ): AndroidDurableUploadSchedulingRecoveryStep = coroutineScope { | ||
| val running = async(start = CoroutineStart.UNDISPATCHED) { action() } | ||
| try { | ||
| select { | ||
| running.onAwait { AndroidDurableUploadSchedulingRecoveryStep.Completed } | ||
| wakeups.onReceive { | ||
| beforeBatchClaim() | ||
| AndroidDurableUploadSchedulingRecoveryStep.Interrupted(takeBatch()) | ||
| } | ||
| } | ||
| } finally { | ||
| running.cancel() | ||
| } | ||
| } | ||
|
|
||
| private fun takeBatch(): AndroidDurableUploadSchedulingRecoveryBatch = synchronized(monitor) { | ||
| while (wakeups.tryReceive().isSuccess) { | ||
| // Every request represented by a drained token is included in the pending state below. | ||
| } | ||
| AndroidDurableUploadSchedulingRecoveryBatch( | ||
| immediate = immediatePending, | ||
| workIdsToAwait = workIdsToAwait.toList(), | ||
| ).also { | ||
| immediatePending = false | ||
| workIdsToAwait.clear() | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private val ANDROID_DURABLE_UPLOAD_SCHEDULING_RECOVERY_SIGNAL = | ||
| AndroidDurableUploadSchedulingRecoverySignal() | ||
|
|
||
| internal fun requestQueuedDurableUploadSchedulingRecovery() { | ||
| ANDROID_DURABLE_UPLOAD_SCHEDULING_RECOVERY_SIGNAL.request() | ||
| } | ||
|
|
||
| internal fun requestQueuedDurableUploadSchedulingRecoveryAfterWorkStopsRunning(workId: UUID) { | ||
| ANDROID_DURABLE_UPLOAD_SCHEDULING_RECOVERY_SIGNAL.requestAfterWorkStopsRunning(workId) | ||
| } | ||
|
|
||
| internal suspend fun monitorQueuedDurableUploadScheduling( | ||
| recover: suspend () -> Unit, | ||
| awaitWorkStopsRunning: suspend (UUID) -> Unit = {}, | ||
| wait: suspend (Long) -> Unit, | ||
| workerFailureFollowUpDelayMillis: Long = | ||
| ANDROID_DURABLE_UPLOAD_SCHEDULING_FOLLOW_UP_DELAY_MILLIS, | ||
| recoverySignal: AndroidDurableUploadSchedulingRecoverySignal = | ||
| ANDROID_DURABLE_UPLOAD_SCHEDULING_RECOVERY_SIGNAL, | ||
| ) { | ||
| require(workerFailureFollowUpDelayMillis > 0L) | ||
| recover() | ||
| var immediatePending = false | ||
| val workIdsToAwait = linkedSetOf<UUID>() | ||
|
|
||
| fun addRequests(batch: AndroidDurableUploadSchedulingRecoveryBatch) { | ||
| immediatePending = immediatePending || batch.immediate | ||
| workIdsToAwait += batch.workIdsToAwait | ||
| } | ||
|
|
||
| while (true) { | ||
| if (!immediatePending && workIdsToAwait.isEmpty()) addRequests(recoverySignal.await()) | ||
| if (!immediatePending && workIdsToAwait.isEmpty()) continue | ||
| if (immediatePending) { | ||
| immediatePending = false | ||
| recover() | ||
| continue | ||
| } | ||
|
|
||
| val workId = workIdsToAwait.first() | ||
| when (val step = recoverySignal.runUntilRequested { awaitWorkStopsRunning(workId) }) { | ||
| AndroidDurableUploadSchedulingRecoveryStep.Completed -> Unit | ||
| is AndroidDurableUploadSchedulingRecoveryStep.Interrupted -> { | ||
| addRequests(step.batch) | ||
| continue | ||
| } | ||
| } | ||
| when ( | ||
| val step = recoverySignal.runUntilRequested { | ||
| wait(workerFailureFollowUpDelayMillis) | ||
| } | ||
| ) { | ||
| AndroidDurableUploadSchedulingRecoveryStep.Completed -> { | ||
| workIdsToAwait.remove(workId) | ||
| recover() | ||
| } | ||
| is AndroidDurableUploadSchedulingRecoveryStep.Interrupted -> addRequests(step.batch) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| internal suspend fun awaitDurableUploadWorkToStopRunning( | ||
| workId: UUID, | ||
| retryDelayMillis: Long = 1_000L, | ||
| awaitWorkStopsRunning: suspend (UUID) -> Unit, | ||
| wait: suspend (Long) -> Unit, | ||
| ) { | ||
| require(retryDelayMillis > 0L) | ||
| while (true) { | ||
| try { | ||
| awaitWorkStopsRunning(workId) | ||
| return | ||
| } catch (cancelled: CancellationException) { | ||
| throw cancelled | ||
| } catch (_: Exception) { | ||
| wait(retryDelayMillis) | ||
| } | ||
| } | ||
|
veryCrunchy marked this conversation as resolved.
|
||
| } | ||
|
|
||
| internal suspend fun claimQueuedDurableUploadForExecution( | ||
| jobId: String, | ||
| coordinator: AndroidDurableUploadStartCoordinator = ANDROID_DURABLE_UPLOAD_START_COORDINATOR, | ||
| claim: suspend () -> AndroidDurableMultipartUploadJob?, | ||
| ): AndroidDurableMultipartUploadJob? = coordinator.withJob(jobId, claim) | ||
|
|
||
| internal suspend fun replaceDeferredDurableUploadWork( | ||
| expected: AndroidDurableMultipartUploadJob, | ||
| load: (String) -> AndroidDurableMultipartUploadJob?, | ||
| replace: suspend (AndroidDurableMultipartUploadJob) -> Unit, | ||
| coordinator: AndroidDurableUploadStartCoordinator = ANDROID_DURABLE_UPLOAD_START_COORDINATOR, | ||
| ): Boolean = coordinator.withJob(expected.id) { | ||
| val current = load(expected.id) | ||
| if ( | ||
| current == null || | ||
| current.accountId != expected.accountId || | ||
| current.state != DurableUploadState.Queued | ||
| ) { | ||
| return@withJob false | ||
| } | ||
| replace(current) | ||
| true | ||
| } | ||
|
|
||
| internal suspend fun constructAndReconcileQueuedDurableUploads( | ||
| createReconciler: () -> suspend () -> Boolean, | ||
| ): Boolean { | ||
| val reconcile = try { | ||
| createReconciler() | ||
| } catch (cancelled: CancellationException) { | ||
| throw cancelled | ||
| } catch (failure: Exception) { | ||
| throw AndroidDurableMultipartUploadRecoveryException(failure) | ||
| } | ||
| return reconcile() | ||
| } | ||
|
|
||
| internal suspend fun reconcileQueuedDurableUploads( | ||
| jobs: List<AndroidDurableMultipartUploadJob>, | ||
| allowQueuedScheduling: Boolean = true, | ||
| schedulerOwns: suspend (AndroidDurableMultipartUploadJob) -> Boolean = { false }, | ||
| cleanupCapability: suspend (AndroidDurableMultipartUploadJob) -> Unit, | ||
| schedule: suspend (AndroidDurableMultipartUploadJob) -> Unit, | ||
| ): Boolean { | ||
| var allScheduled = true | ||
| jobs.filter { job -> job.requiresSchedulingRecovery(allowQueuedScheduling) }.forEach { job -> | ||
| try { | ||
| if (job.capabilityCleanupPending) { | ||
| cleanupCapability(job) | ||
| } else if (!schedulerOwns(job)) { | ||
| schedule(job) | ||
| } | ||
| } catch (cancelled: CancellationException) { | ||
| throw cancelled | ||
| } catch (_: Exception) { | ||
| allScheduled = false | ||
| } | ||
| } | ||
| return allScheduled | ||
| } | ||
|
|
||
| private fun AndroidDurableMultipartUploadJob.requiresSchedulingRecovery( | ||
| allowQueuedScheduling: Boolean, | ||
| ): Boolean = capabilityCleanupPending || (allowQueuedScheduling && state == DurableUploadState.Queued) | ||
|
|
||
| internal suspend fun retryQueuedDurableUploadScheduling( | ||
| retryDelaysMillis: List<Long> = listOf(1_000L, 5_000L), | ||
| reconcile: suspend () -> Boolean, | ||
| wait: suspend (Long) -> Unit, | ||
| ): Boolean { | ||
| if (reconcile()) return true | ||
| retryDelaysMillis.forEach { delayMillis -> | ||
| require(delayMillis >= 0L) | ||
| wait(delayMillis) | ||
| if (reconcile()) return true | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| internal suspend fun keepRetryingQueuedDurableUploadScheduling( | ||
| retryDelaysMillis: List<Long> = listOf(1_000L, 5_000L), | ||
| followUpDelayMillis: Long = ANDROID_DURABLE_UPLOAD_SCHEDULING_FOLLOW_UP_DELAY_MILLIS, | ||
| reconcile: suspend () -> Boolean, | ||
| wait: suspend (Long) -> Unit, | ||
| recordRecoveryFailure: () -> Unit = {}, | ||
| ) { | ||
| require(followUpDelayMillis > 0L) | ||
| var recoveryFailureReported = false | ||
| while (true) { | ||
| val recovered = try { | ||
| retryQueuedDurableUploadScheduling(retryDelaysMillis, reconcile, wait) | ||
|
veryCrunchy marked this conversation as resolved.
|
||
| } catch (cancelled: CancellationException) { | ||
| throw cancelled | ||
| } catch (_: AndroidDurableMultipartUploadRecoveryException) { | ||
| false | ||
| } | ||
| if (recovered) return | ||
| if (!recoveryFailureReported) { | ||
| runCatching(recordRecoveryFailure) | ||
| recoveryFailureReported = true | ||
| } | ||
| wait(followUpDelayMillis) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Persists the upload before asking WorkManager to schedule it. WorkManager acceptance and its | ||
| * completion signal are not atomic, so a scheduling failure after persistence is ambiguous: the | ||
| * durable queued job must remain authoritative and can be scheduled again after process restart. | ||
| */ | ||
| internal suspend fun persistAndScheduleDurableUpload( | ||
| job: AndroidDurableMultipartUploadJob, | ||
| persist: (AndroidDurableMultipartUploadJob) -> Unit, | ||
| schedule: suspend (AndroidDurableMultipartUploadJob) -> Unit, | ||
| requestRecovery: () -> Unit = ::requestQueuedDurableUploadSchedulingRecovery, | ||
| ): DurableUploadEnqueueResult.Queued { | ||
| persist(job) | ||
| try { | ||
| schedule(job) | ||
| } catch (cancelled: CancellationException) { | ||
| runCatching(requestRecovery) | ||
| throw cancelled | ||
| } catch (_: Exception) { | ||
| runCatching(requestRecovery) | ||
| } | ||
| return DurableUploadEnqueueResult.Queued(job.status()) | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.