fix: manifest cache not updated after backup causes NAS Archive = 1 and backup loop

- writeManifest now updates NASManifestCache after every successful NAS write;
  previously the fast-path reconcile would read the pre-backup cache entry
  (e.g. 1 entry) and overwrite nasArchiveTotal / alreadySafe with stale values
- BackupEngine seeds NASManifestCache when it loads the manifest so the cache
  is always at least as fresh as what the engine sees at backup-start time
- Remove snapshot.nasArchiveTotal += uploaded from refreshAfterBackup; the
  correct total comes from the merged manifest written to NAS, not an additive
  delta from a potentially-wrong baseline
- Fix JSONDecoder() → JSONDecoder.kisani throughout manifest load paths so
  ISO8601 dates decode correctly instead of silently failing and returning nil
- Add retry with exponential backoff (max 3 attempts, 2 s / 4 s intervals)
  to the upload loop; permanently-failed items are now marked .failed only
  after all retries are exhausted

Co-Authored-By: Kutesir <kutesir@provoc.ug>
Co-Authored-By: Sentry <sentry@provoc.ug>
This commit is contained in:
Robin Kutesa
2026-05-18 00:03:10 +03:00
parent a98822d7b2
commit c7555a2001
2 changed files with 100 additions and 59 deletions

View File

@@ -99,16 +99,22 @@ final class BackupEngine: ObservableObject {
let manifestPath = "\(connection.remotePath)/\(BackupManifest.remoteFilename)" let manifestPath = "\(connection.remotePath)/\(BackupManifest.remoteFilename)"
let spManifest = signposter.beginInterval("ManifestLoad") let spManifest = signposter.beginInterval("ManifestLoad")
let manifestData = try? await transfer.downloadData(at: manifestPath) let manifestData = try? await transfer.downloadData(at: manifestPath)
let index: ManifestIndex = await Task.detached(priority: .userInitiated) { let (index, loadedManifest): (ManifestIndex, BackupManifest?) = await Task.detached(priority: .userInitiated) {
guard let data = manifestData, guard let data = manifestData,
let manifest = try? JSONDecoder().decode(BackupManifest.self, from: data) let manifest = try? JSONDecoder.kisani.decode(BackupManifest.self, from: data)
else { return ManifestIndex(nasListing: []) } else { return (ManifestIndex(nasListing: []), nil) }
return ManifestIndex(manifest: manifest) return (ManifestIndex(manifest: manifest), manifest)
}.value }.value
signposter.endInterval("ManifestLoad", spManifest, signposter.endInterval("ManifestLoad", spManifest,
"\(index.totalCount) entries, filename-only=\(index.isFilenameOnly)") "\(index.totalCount) entries, filename-only=\(index.isFilenameOnly)")
logger.info("Manifest: \(index.totalCount) entries, filename-only=\(index.isFilenameOnly)") logger.info("Manifest: \(index.totalCount) entries, filename-only=\(index.isFilenameOnly)")
// Seed the local cache so BackupStatusService's fast-path reconcile reflects
// the current NAS state before the upload loop begins.
if let m = loadedManifest {
await NASManifestCache.shared.update(m)
}
try Task.checkCancellation() try Task.checkCancellation()
// 4. Build pending queue off the main actor // 4. Build pending queue off the main actor
@@ -156,6 +162,8 @@ final class BackupEngine: ObservableObject {
let spUpload = signposter.beginInterval("UploadLoop", "\(pendingAssets.count) files") let spUpload = signposter.beginInterval("UploadLoop", "\(pendingAssets.count) files")
let maxRetries = 3
for (assetIdx, asset) in pendingAssets.enumerated() { for (assetIdx, asset) in pendingAssets.enumerated() {
if isCancelled { break } if isCancelled { break }
while case .paused = job.status { while case .paused = job.status {
@@ -164,7 +172,6 @@ final class BackupEngine: ObservableObject {
let remotePath = "\(connection.remotePath)/\(asset.filename)" let remotePath = "\(connection.remotePath)/\(asset.filename)"
// Mark as uploading
if assetIdx < queueItems.count { if assetIdx < queueItems.count {
queueItems[assetIdx].status = .uploading queueItems[assetIdx].status = .uploading
} }
@@ -180,60 +187,87 @@ final class BackupEngine: ObservableObject {
continue continue
} }
do { var uploadSucceeded = false
let spFile = signposter.beginInterval("UploadFile", var lastUploadError: Error?
"\(asset.filename, privacy: .public)")
let localURL = try await photoService.exportAsset(asset)
defer { try? FileManager.default.removeItem(at: localURL) }
let fileSize = (try? localURL.resourceValues(forKeys: [.fileSizeKey]).fileSize) for attempt in 0..<maxRetries {
.flatMap { Int64($0) } ?? 0 if isCancelled { break }
let bytesSoFar = totalBytes
let capturedIdx = assetIdx
if capturedIdx < queueItems.count { if attempt > 0 {
queueItems[capturedIdx].totalBytes = fileSize // Exponential backoff: 2s, 4s before retries 2 and 3
let backoffNs = UInt64(pow(2.0, Double(attempt)) * 1_000_000_000)
try? await Task.sleep(nanoseconds: backoffNs)
if assetIdx < queueItems.count {
queueItems[assetIdx].retryCount = attempt
queueItems[assetIdx].status = .uploading
}
logger.info("Retry \(attempt)/\(maxRetries - 1) for \(asset.filename, privacy: .public)")
} }
try await transfer.upload(localURL: localURL, remotePath: remotePath) { sent, _ in do {
let speed = speedTracker.update(bytesSent: sent) let spFile = signposter.beginInterval("UploadFile",
guard throttle.shouldUpdate() else { return } "\(asset.filename, privacy: .public)")
Task { @MainActor [weak self] in let localURL = try await photoService.exportAsset(asset)
guard let self else { return } defer { try? FileManager.default.removeItem(at: localURL) }
self.job.updateProgress(
fileName: asset.filename, let fileSize = (try? localURL.resourceValues(forKeys: [.fileSizeKey]).fileSize)
fileSize: fileSize, .flatMap { Int64($0) } ?? 0
bytesTransferred: bytesSoFar + sent, let bytesSoFar = totalBytes
speed: speed let capturedIdx = assetIdx
)
if capturedIdx < self.queueItems.count { if capturedIdx < queueItems.count {
self.queueItems[capturedIdx].bytesUploaded = sent queueItems[capturedIdx].totalBytes = fileSize
self.queueItems[capturedIdx].speedBytesPerSec = speed queueItems[capturedIdx].bytesUploaded = 0
}
try await transfer.upload(localURL: localURL, remotePath: remotePath) { sent, _ in
let speed = speedTracker.update(bytesSent: sent)
guard throttle.shouldUpdate() else { return }
Task { @MainActor [weak self] in
guard let self else { return }
self.job.updateProgress(
fileName: asset.filename,
fileSize: fileSize,
bytesTransferred: bytesSoFar + sent,
speed: speed
)
if capturedIdx < self.queueItems.count {
self.queueItems[capturedIdx].bytesUploaded = sent
self.queueItems[capturedIdx].speedBytesPerSec = speed
}
} }
} }
signposter.endInterval("UploadFile", spFile, "\(fileSize) bytes")
totalBytes += fileSize
job.fileCompleted(skipped: false)
uploaded += 1
uploadSucceeded = true
if assetIdx < queueItems.count {
queueItems[assetIdx].status = .uploaded
queueItems[assetIdx].bytesUploaded = fileSize
queueItems[assetIdx].completedAt = Date()
}
manifestEntries.append(ManifestEntry(
localIdentifier: asset.localIdentifier,
filename: asset.filename,
creationDate: asset.creationDate,
fileSize: fileSize,
remotePath: remotePath,
uploadedAt: Date()
))
} catch {
lastUploadError = error
logger.warning("Upload attempt \(attempt + 1)/\(maxRetries) failed for \(asset.filename, privacy: .public): \(error.localizedDescription, privacy: .public)")
} }
signposter.endInterval("UploadFile", spFile, "\(fileSize) bytes") if uploadSucceeded { break }
totalBytes += fileSize }
job.fileCompleted(skipped: false)
uploaded += 1
if assetIdx < queueItems.count { if !uploadSucceeded, let error = lastUploadError {
queueItems[assetIdx].status = .uploaded logger.error("Upload permanently failed for \(asset.filename, privacy: .public): \(error.localizedDescription, privacy: .public)")
queueItems[assetIdx].bytesUploaded = fileSize
queueItems[assetIdx].completedAt = Date()
}
manifestEntries.append(ManifestEntry(
localIdentifier: asset.localIdentifier,
filename: asset.filename,
creationDate: asset.creationDate,
fileSize: fileSize,
remotePath: remotePath,
uploadedAt: Date()
))
} catch {
logger.error("Upload failed for \(asset.filename, privacy: .public): \(error.localizedDescription, privacy: .public)")
job.fileFailed() job.fileFailed()
failed += 1 failed += 1
if assetIdx < queueItems.count { if assetIdx < queueItems.count {

View File

@@ -68,7 +68,8 @@ final class BackupStatusService: NSObject, ObservableObject {
func refreshAfterBackup(entries: [ManifestEntry], connection: NASConnection) { func refreshAfterBackup(entries: [ManifestEntry], connection: NASConnection) {
let uploaded = entries.count let uploaded = entries.count
snapshot.alreadySafe = min(snapshot.phoneTotal, snapshot.alreadySafe + uploaded) snapshot.alreadySafe = min(snapshot.phoneTotal, snapshot.alreadySafe + uploaded)
snapshot.nasArchiveTotal += uploaded // nasArchiveTotal is NOT updated optimistically writeManifest will update
// NASManifestCache with the full merged count, and refresh(force: true) reads it.
saveSnapshot() saveSnapshot()
log.info("Post-backup optimistic update: +\(uploaded) safe") log.info("Post-backup optimistic update: +\(uploaded) safe")
Task { Task {
@@ -234,7 +235,7 @@ final class BackupStatusService: NSObject, ObservableObject {
let data = try await transfer.downloadData(at: path) let data = try await transfer.downloadData(at: path)
// Decode and build index off main actor JSONDecoder is not cheap for large manifests. // Decode and build index off main actor JSONDecoder is not cheap for large manifests.
let decoded: (ManifestIndex, BackupManifest)? = await Task.detached(priority: .utility) { let decoded: (ManifestIndex, BackupManifest)? = await Task.detached(priority: .utility) {
guard let manifest = try? JSONDecoder().decode(BackupManifest.self, from: data) guard let manifest = try? JSONDecoder.kisani.decode(BackupManifest.self, from: data)
else { return nil } else { return nil }
return (ManifestIndex(manifest: manifest), manifest) return (ManifestIndex(manifest: manifest), manifest)
}.value }.value
@@ -332,23 +333,29 @@ final class BackupStatusService: NSObject, ObservableObject {
let existingData = try? await transfer.downloadData(at: manifestPath) let existingData = try? await transfer.downloadData(at: manifestPath)
// Merge + JSON encode off main actor can be slow for large manifests. // Merge + JSON encode off main actor can be slow for large manifests.
let encoded: Data? = await Task.detached(priority: .utility) { let result: (data: Data, manifest: BackupManifest)? = await Task.detached(priority: .utility) {
var manifest: BackupManifest var manifest: BackupManifest
if let data = existingData, if let data = existingData,
let existing = try? JSONDecoder().decode(BackupManifest.self, from: data) { let existing = try? JSONDecoder.kisani.decode(BackupManifest.self, from: data) {
manifest = existing manifest = existing
} else { } else {
manifest = BackupManifest() manifest = BackupManifest()
} }
manifest.merge(entries: entries) manifest.merge(entries: entries)
let encoder = JSONEncoder() guard let encoded = try? JSONEncoder.kisani.encode(manifest) else { return nil }
encoder.dateEncodingStrategy = .iso8601 return (encoded, manifest)
return try? encoder.encode(manifest)
}.value }.value
guard let encoded else { return }
guard let (encoded, mergedManifest) = result else { return }
try await transfer.writeData(encoded, to: manifestPath) try await transfer.writeData(encoded, to: manifestPath)
log.info("Manifest written — \(entries.count) new entries")
// Update local cache immediately the next reconcile's fast path will read
// this fresh cache instead of re-downloading from NAS, preventing the stale-
// cache bug where nasArchiveTotal resets to the pre-backup count.
await NASManifestCache.shared.update(mergedManifest)
lastManifest = mergedManifest
log.info("Manifest written — \(entries.count) new entries, total \(mergedManifest.entries.count)")
} catch { } catch {
log.error("Manifest write failed (non-fatal): \(error.localizedDescription)") log.error("Manifest write failed (non-fatal): \(error.localizedDescription)")
} }