diff --git a/lib/Backend/LocalBackend.php b/lib/Backend/LocalBackend.php index f2a61d5..1805768 100644 --- a/lib/Backend/LocalBackend.php +++ b/lib/Backend/LocalBackend.php @@ -33,11 +33,16 @@ final class LocalBackend implements IBackend { if ($out === false) { throw new BackendException('Cannot write ' . $path); } + $expected = StorageBackend::remaining($stream); if (stream_copy_to_stream($stream, $out) === false || !fflush($out)) { throw new BackendException('Write failed for ' . $path); } fclose($out); $out = false; + clearstatcache(true, $tmp); + if ($expected !== null && filesize($tmp) !== $expected) { + throw new BackendException('Incomplete write for ' . $path); + } if (!rename($tmp, $target)) { throw new BackendException('Cannot finalise ' . $path); } @@ -94,7 +99,7 @@ final class LocalBackend implements IBackend { $result = []; $it = new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($base, \FilesystemIterator::SKIP_DOTS)); foreach ($it as $file) { - if ($file->isFile() && !str_contains($file->getFilename(), '.part-')) { + if ($file->isFile() && !StorageBackend::isTemporary($file->getFilename())) { $result[] = ltrim(substr($file->getPathname(), strlen($this->root)), '/'); } } diff --git a/lib/Backend/StorageBackend.php b/lib/Backend/StorageBackend.php index 0719d8e..bed3dca 100644 --- a/lib/Backend/StorageBackend.php +++ b/lib/Backend/StorageBackend.php @@ -30,7 +30,13 @@ final class StorageBackend implements IBackend { } } + /** A temporary upload of put(): ".part-<8 hex>". */ + public static function isTemporary(string $name): bool { + return preg_match('/\.part-[0-9a-f]{8}$/', $name) === 1; + } + public function put(string $path, $stream): void { + $expected = self::remaining($stream); $target = $this->abs($path); $this->mkdirs(dirname($target)); $writeTo = $this->atomicPut ? $target : $target . '.part-' . bin2hex(random_bytes(4)); @@ -54,6 +60,10 @@ final class StorageBackend implements IBackend { if (is_resource($stream)) { fclose($stream); } + // Some adapters report success after a short write or a failed flush: check the size. + if ($expected !== null && $this->storage->filesize($writeTo) !== $expected) { + throw new BackendException('Incomplete write for ' . $path); + } if ($writeTo !== $target && !$this->storage->rename($writeTo, $target)) { throw new BackendException('Cannot finalise ' . $path); } @@ -132,19 +142,35 @@ final class StorageBackend implements IBackend { return; } while (($name = readdir($dh)) !== false) { - if ($name === '.' || $name === '..' || str_contains($name, '.part-')) { + if ($name === '.' || $name === '..') { continue; } $child = $dir . '/' . $name; if ($this->storage->is_dir($child)) { $this->listInto($child, $result); - } else { + } elseif (!self::isTemporary($name)) { $result[] = $child; } } closedir($dh); } + /** Bytes left to read in a seekable $stream, or null when it cannot tell. @param resource $stream */ + public static function remaining(mixed $stream): ?int { + if (!is_resource($stream)) { + return null; + } + if (!(stream_get_meta_data($stream)['seekable'] ?? false)) { + return null; + } + $stat = fstat($stream); + $pos = ftell($stream); + if ($stat === false || $pos === false) { + return null; + } + return max(0, $stat['size'] - $pos); + } + private function mkdirs(string $dir): void { if ($dir === '' || $dir === '.' || $this->storage->is_dir($dir)) { return; diff --git a/lib/BackgroundJob/BackupJob.php b/lib/BackgroundJob/BackupJob.php index b2e7e14..e8c7536 100644 --- a/lib/BackgroundJob/BackupJob.php +++ b/lib/BackgroundJob/BackupJob.php @@ -38,13 +38,19 @@ final class BackupJob extends TimedJob { $this->setTimeSensitivity(IJob::TIME_SENSITIVE); } + private const MAX_DAILY_ATTEMPTS = 3; + protected function run($argument): void { $budget = PHP_SAPI === 'cli' ? 240.0 : 20.0; $deadline = microtime(true) + $budget; $this->startScheduled(); foreach ($this->runs->findRunning(null, BackupService::KIND_FULL) as $run) { while (microtime(true) < $deadline && $run->getStatus() === 'running') { + $before = $run->getUpdatedAt(); $run = $this->backups->step($run, min($deadline, microtime(true) + $budget)); + if ($run->getUpdatedAt() === $before && $run->getStatus() === 'running') { + break; // another process holds this run's lease: leave it to that process + } } } $this->failStaleRestores(); @@ -67,10 +73,15 @@ final class BackupJob extends TimedJob { } } - /** Once a week (after Sunday's backup), apply the retention policy to non-append-only locations. */ + /** + * Once a week, apply the retention policy to non-append-only locations: from Sunday on, at the + * first moment no backup is running, so a long Sunday backup only delays it instead of skipping + * the week. + */ private function weeklyPrune(): void { $lastWeek = $this->appConfig->getValueString(Application::APP_ID, 'last_prune_week', ''); - if (date('N') !== '7' || $lastWeek === date('o-W') || $this->runs->findRunning(null, BackupService::KIND_FULL) !== []) { + $due = date('N') === '7' ? date('o-W') : date('o-W', (int)strtotime('last sunday')); + if (strcmp($lastWeek, $due) >= 0 || $this->runs->findRunning(null, BackupService::KIND_FULL) !== []) { return; } foreach ($this->targets->list() as $t) { @@ -83,7 +94,7 @@ final class BackupJob extends TimedJob { $this->logger->warning('NG Backup: cleanup of ' . $t->getName() . ' failed: ' . $e->getMessage()); } } - $this->appConfig->setValueString(Application::APP_ID, 'last_prune_week', date('o-W')); + $this->appConfig->setValueString(Application::APP_ID, 'last_prune_week', $due); } /** Daily schedule: appconfig ng_backup/schedule_time "HH:MM" (server time); empty = no schedule. */ @@ -92,7 +103,7 @@ final class BackupJob extends TimedJob { if (!preg_match('/^([01]\d|2[0-3]):[0-5]\d$/', $at)) { return; } - $todayAt = strtotime(date('Y-m-d') . ' ' . $at); + $todayAt = (int)strtotime(date('Y-m-d') . ' ' . $at); if (time() < $todayAt) { return; } @@ -101,6 +112,12 @@ final class BackupJob extends TimedJob { if ($this->runs->findRunning($t->getId(), BackupService::KIND_FULL) !== [] || ($last !== null && $last->getStartedAt() >= $todayAt)) { continue; } + // After a failure: retry at most MAX_DAILY_ATTEMPTS times a day, an hour apart, instead + // of starting (and notifying about) a new full run every five minutes all day. + $today = $this->runs->startedSince($t->getId(), BackupService::KIND_FULL, $todayAt); + if (count($today) >= self::MAX_DAILY_ATTEMPTS || ($today !== [] && time() - (int)$today[0]->getFinishedAt() < 3600)) { + continue; + } try { $this->backups->start($t, 'scheduled'); } catch (\Throwable $e) { diff --git a/lib/Command/RunBackup.php b/lib/Command/RunBackup.php index 5e5f40e..b5aa06e 100644 --- a/lib/Command/RunBackup.php +++ b/lib/Command/RunBackup.php @@ -75,6 +75,13 @@ HELP); }); if ($run->getStatus() === Run::DONE) { $output->writeln(sprintf(' done in %.0fs', microtime(true) - $start)); + $f = json_decode($run->getStats() ?? '{}', true)['files'] ?? []; + if (($f['unreadable'] ?? 0) > 0) { + $output->writeln(sprintf(' warning: %d unreadable files or folders are missing from this backup (see the server log)', $f['unreadable'])); + } + if (($f['changing'] ?? 0) > 0) { + $output->writeln(sprintf(' warning: %d files kept changing and were saved as read (see the server log)', $f['changing'])); + } } else { $output->writeln(' ' . $run->getStatus() . ': ' . $run->getError() . ''); $failed++; diff --git a/lib/Db/RunMapper.php b/lib/Db/RunMapper.php index e700370..9799259 100644 --- a/lib/Db/RunMapper.php +++ b/lib/Db/RunMapper.php @@ -43,6 +43,17 @@ final class RunMapper extends QBMapper { return $this->findEntities($qb); } + /** @return list runs of $kind started at or after $since, newest first */ + public function startedSince(int $targetId, string $kind, int $since): array { + $qb = $this->db->getQueryBuilder(); + $qb->select('*')->from($this->getTableName()) + ->where($qb->expr()->eq('target_id', $qb->createNamedParameter($targetId, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->eq('kind', $qb->createNamedParameter($kind))) + ->andWhere($qb->expr()->gte('started_at', $qb->createNamedParameter($since, IQueryBuilder::PARAM_INT))) + ->orderBy('id', 'DESC'); + return $this->findEntities($qb); + } + public function lastFinished(int $targetId, string $kind): ?Run { $qb = $this->db->getQueryBuilder(); $qb->select('*')->from($this->getTableName()) diff --git a/lib/Job/BackupRun.php b/lib/Job/BackupRun.php index 86ef1e5..7edb3f4 100644 --- a/lib/Job/BackupRun.php +++ b/lib/Job/BackupRun.php @@ -49,9 +49,11 @@ final class BackupRun { public static function step(Repository $repo, array $state, float $deadline): array { $state['steps']++; if ($state['phase'] === 'files') { - $walker = new TreeWalker($state['roots'], $state['exclude']); - $parent = new ParentCursor($repo, $state['parent'], $state['parentPos']); $stats = $state['stats']; + $walker = new TreeWalker($state['roots'], $state['exclude'], function (string $logical) use (&$state, &$stats): void { + self::recordUnreadable($state, $stats, $logical); + }); + $parent = new ParentCursor($repo, $state['parent'], $state['parentPos']); $lines = []; $finishedWalk = true; @@ -68,15 +70,20 @@ final class BackupRun { $stats['reused']++; } else { $cur = $state['cur']; - if ($cur === null || $cur['path'] !== $logical || $cur['version'] !== $version) { - if ($cur !== null && $cur['path'] === $logical) { - $stats['retried'] = ($stats['retried'] ?? 0) + 1; // changed since the previous step + if ($cur !== null && $cur['path'] === $logical && $cur['version'] !== $version) { + // Changed since the previous step: start over, unless it keeps changing. + if (self::countRetry($state, $stats, $logical)) { + $cur['version'] = $version; + } else { + $cur = null; } + } + if ($cur === null || $cur['path'] !== $logical) { $cur = ['path' => $logical, 'offset' => 0, 'blobs' => [], 'version' => $version]; } - $fh = fopen($path, 'rb'); + $fh = @fopen($path, 'rb'); if ($fh === false) { - $stats['unreadable'] = ($stats['unreadable'] ?? 0) + 1; + self::recordUnreadable($state, $stats, $logical); $state['cur'] = null; continue; } @@ -91,12 +98,17 @@ final class BackupRun { } } fclose($fh); - if (Repository::fileVersion($path) !== $cur['version']) { - // Changed while reading: read it again from the start (now or next step). - $stats['retried'] = ($stats['retried'] ?? 0) + 1; - $state['cur'] = null; - $finishedWalk = false; - break; + $now = Repository::fileVersion($path); + if ($now !== $cur['version']) { + if (!self::countRetry($state, $stats, $logical)) { + // Changed while reading: read it again from the start (now or next step). + $state['cur'] = null; + $finishedWalk = false; + break; + } + // Keeps changing (a live database, VM image or log): keep what was read, + // like any copy of a file in use, and report it instead of retrying forever. + $cur['version'] = $now; } if (!$complete) { $state['cur'] = $cur; @@ -104,6 +116,7 @@ final class BackupRun { break; } $blobs = $cur['blobs']; + $size = $cur['offset']; $state['cur'] = null; $stats['read']++; } @@ -135,7 +148,8 @@ final class BackupRun { $segments = $state['segments']; $repo->putSnapshot($state['snapshot'], ['time' => $state['started'], 'parent' => $state['parent'], 'label' => $state['label'], - 'roots' => array_keys($state['roots']), 'stats' => $state['stats']] + $state['meta'], + 'roots' => array_keys($state['roots']), 'stats' => $state['stats'], + 'warnings' => self::warnings($state)] + $state['meta'], (function () use ($repo, $segments) { foreach ($segments as $blobs) { yield from $repo->readLines($blobs); @@ -146,6 +160,52 @@ final class BackupRun { return $state; } + /** + * A file or directory that could not be read: it is missing from this snapshot. Remembered (first + * 1000, deduplicated across steps) so the run can report it instead of silently succeeding. + */ + private static function recordUnreadable(array &$state, array &$stats, string $logical): void { + if (isset($state['unreadable'][$logical])) { + return; + } + if (count($state['unreadable'] ?? []) < 1000) { + $state['unreadable'][$logical] = true; + } + $stats['unreadable'] = ($stats['unreadable'] ?? 0) + 1; + } + + /** @return array{unreadable: list, changing: list} examples for the report (first 50 each) */ + public static function warnings(array $state): array { + return [ + 'unreadable' => array_slice(array_map('strval', array_keys($state['unreadable'] ?? [])), 0, 50), + 'changing' => array_slice(array_map('strval', array_values($state['changing'] ?? [])), 0, 50), + ]; + } + + /** Retries of one file that changes while it is read, before its last read is kept anyway. */ + public const MAX_RETRIES = 3; + + /** + * Count a retry of $logical. Returns true once it has changed MAX_RETRIES times: the caller then + * keeps the data it read, and the file is listed in $state['changing'] (first 50) for the report. + */ + private static function countRetry(array &$state, array &$stats, string $logical): bool { + $stats['retried'] = ($stats['retried'] ?? 0) + 1; + $n = (($state['retryPath'] ?? null) === $logical ? (int)($state['retryCount'] ?? 0) : 0) + 1; + $state['retryPath'] = $logical; + $state['retryCount'] = $n; + if ($n < self::MAX_RETRIES) { + return false; + } + if ($n === self::MAX_RETRIES) { + $stats['changing'] = ($stats['changing'] ?? 0) + 1; + if (count($state['changing'] ?? []) < 50) { + $state['changing'][] = $logical; + } + } + return true; + } + private static function readFull($fh, int $length): string { $buffer = ''; while (strlen($buffer) < $length && !feof($fh)) { diff --git a/lib/Job/TreeWalker.php b/lib/Job/TreeWalker.php index a685058..ce342b4 100644 --- a/lib/Job/TreeWalker.php +++ b/lib/Job/TreeWalker.php @@ -23,6 +23,8 @@ final class TreeWalker { public function __construct( private array $roots, private array $exclude = [], + /** Called with the logical path of a directory that cannot be listed. */ + private ?\Closure $onUnreadable = null, ) { } @@ -57,6 +59,9 @@ final class TreeWalker { } $names = @scandir($abs); if ($names === false) { + if ($this->onUnreadable !== null) { + ($this->onUnreadable)($logical === '' ? '/' : $logical); + } return; } $names = array_values(array_diff($names, ['.', '..'])); diff --git a/lib/Notification/Notifier.php b/lib/Notification/Notifier.php index e5cf2e8..df81fa3 100644 --- a/lib/Notification/Notifier.php +++ b/lib/Notification/Notifier.php @@ -42,6 +42,10 @@ final class Notifier implements INotifier { : $l->t('Backup to %s failed', [$p['target']])); $notification->setParsedMessage((string)($p['error'] ?? '')); break; + case 'run_warnings': + $notification->setParsedSubject($l->t('Backup to %1$s finished with warnings: %2$s unreadable, %3$s changed while being read', [$p['target'], $p['unreadable'], $p['changing']])); + $notification->setParsedMessage($l->t('Unreadable files and folders are missing from this backup (check their permissions). Files that kept changing were saved as they were while being read. Examples: %s', [$p['examples'] ?? ''])); + break; case 'quota_warning': $notification->setParsedSubject($l->t('Backup location %1$s is at %2$s%% of its limit (%3$s of %4$s)', [$p['target'], $p['percent'], $p['used'], $p['limit']])); $notification->setParsedMessage($l->t('Backups stop when the limit is reached. Raise the limit, or let the retention policy remove older backups.')); @@ -62,6 +66,8 @@ final class Notifier implements INotifier { 'catalog_trusted' => $l->t('%1$s accepted an older state of backup location %2$s (catalog generation %3$s)', [$who, $p['target'] ?? '', $p['generation'] ?? '']), 'slot_added' => $l->t('%1$s added backup passphrase slot %2$s (%3$s); download and confirm the new recovery kit', [$who, $p['slot'] ?? '', $p['label'] ?? '']), 'slot_replaced' => $l->t('%1$s set a new passphrase for backup key slot %2$s (%3$s); download and confirm the new recovery kit', [$who, $p['slot'] ?? '', $p['label'] ?? '']), + 'user_export_retention_changed' => $l->t('%1$s changed how many exports per user are kept to %2$s', [$who, $p['keep'] ?? '']), + 'user_exports_forgotten' => $l->t('%1$s removed %2$s per-user exports from %3$s', [$who, $p['count'] ?? '', $p['target'] ?? '']), 'slot_removed' => $l->t('%1$s removed backup passphrase slot %2$s; download and confirm the new recovery kit', [$who, $p['slot'] ?? '']), default => $l->t('NG Backup security event'), }); diff --git a/lib/Repository/Catalog.php b/lib/Repository/Catalog.php index 44fb6c4..b8a3c47 100644 --- a/lib/Repository/Catalog.php +++ b/lib/Repository/Catalog.php @@ -88,6 +88,11 @@ final class Catalog { /** Write the next generation after the in-memory state was changed. */ public function write(): void { $next = $this->gen + 1; + // Two writers starting from the same generation would otherwise overwrite each other + // silently and break the chain for whichever server anchored the lost write. + if ($this->backend->exists($this->path($next))) { + throw new RepositoryException("Catalog generation $next was written by another process at the same time; try again"); + } $plain = json_encode(['repository' => $this->repositoryId, 'gen' => $next, 'time' => gmdate('c'), 'prev' => $this->hash, 'snapshots' => array_keys($this->snapshots), 'trash' => array_keys($this->trash), 'userExports' => array_keys($this->userExports)], JSON_THROW_ON_ERROR); diff --git a/lib/Repository/Repository.php b/lib/Repository/Repository.php index 5c6ccc3..767964c 100644 --- a/lib/Repository/Repository.php +++ b/lib/Repository/Repository.php @@ -236,8 +236,13 @@ final class Repository { unlink($tmp); throw new RepositoryException('Size mismatch restoring ' . $entry['p']); } - rename($tmp, $dest); - touch($dest, $entry['m']); + if (!rename($tmp, $dest)) { + @unlink($tmp); + throw new RepositoryException('Cannot put the restored file in place: ' . $entry['p']); + } + if (!touch($dest, $entry['m'])) { + throw new RepositoryException('Cannot set the modification time of ' . $entry['p']); + } $count++; } return $count; @@ -375,28 +380,39 @@ final class Repository { // must be decrypted and re-encrypted under the new path, not moved as raw bytes. $dbInTrash = 'trash/' . $meta['db']; $this->putObject($dbInTrash, $this->getObject($meta['db'])); - $this->backend->delete($meta['db']); } - // The snapshot object is bound to its id (not its path), so it can be moved as raw bytes. - $this->backend->move('snapshots/' . $snapshotId, 'trash/snapshots/' . $snapshotId); + // Copy first, switch the catalog, delete the originals last: a crash at any point leaves + // either the old state or the new one plus unreferenced leftovers, never a catalog entry + // pointing at a missing object. The snapshot object is bound to its id (not its path), + // so it can be copied as raw bytes. + $this->backend->put('trash/snapshots/' . $snapshotId, $this->backend->get('snapshots/' . $snapshotId)); $this->putObject('trash/info/' . $snapshotId, json_encode(['forgottenAt' => time(), 'by' => $by, 'db' => $meta['db'] ?? null, 'dbInTrash' => $dbInTrash, 'time' => $meta['time'] ?? null, 'label' => $meta['label'] ?? ''], JSON_THROW_ON_ERROR)); unset($this->catalog->snapshots[$snapshotId]); $this->catalog->trash[$snapshotId] = true; $this->catalog->write(); + if ($dbInTrash !== null) { + $this->backend->delete($meta['db']); + } + $this->backend->delete('snapshots/' . $snapshotId); } /** Undo forget(): move a snapshot back from the trash. */ public function untrash(string $snapshotId): void { $info = $this->trashInfo($snapshotId); - if ($info['dbInTrash'] !== null && $this->backend->exists($info['dbInTrash'])) { + $restoreDb = $info['dbInTrash'] !== null && $this->backend->exists($info['dbInTrash']); + if ($restoreDb) { $this->putObject($info['db'], $this->getObject($info['dbInTrash'])); - $this->backend->delete($info['dbInTrash']); } - $this->backend->move('trash/snapshots/' . $snapshotId, 'snapshots/' . $snapshotId); + // Same order as forget(): copy, switch the catalog, then delete the trash copies. + $this->backend->put('snapshots/' . $snapshotId, $this->backend->get('trash/snapshots/' . $snapshotId)); $this->catalog->snapshots[$snapshotId] = true; unset($this->catalog->trash[$snapshotId]); $this->catalog->write(); + if ($restoreDb) { + $this->backend->delete($info['dbInTrash']); + } + $this->backend->delete('trash/snapshots/' . $snapshotId); $this->backend->delete('trash/info/' . $snapshotId); } @@ -530,7 +546,10 @@ final class Repository { } } } - foreach ($this->backend->list('users') as $path) { + // listUserExports() throws when a recorded export's manifest is missing: then stop, since + // freeing its blobs would turn a tampering or storage alarm into irreversible deletion. + $exports = array_unique(array_merge($this->listUserExports(), $this->backend->list('users'))); + foreach ($exports as $path) { foreach (json_decode($this->getObject($path), true, 512, JSON_THROW_ON_ERROR)['entries'] as $e) { foreach ($e['b'] ?? [] as $b) { $used[$b] = true; diff --git a/lib/Service/AlertService.php b/lib/Service/AlertService.php index f5051ae..817e78a 100644 --- a/lib/Service/AlertService.php +++ b/lib/Service/AlertService.php @@ -54,6 +54,13 @@ final class AlertService { ['kind' => $run->getKind(), 'target' => $targetName, 'error' => mb_strimwidth((string)$run->getError(), 0, 300, '…')]); } + /** A backup that finished but is missing unreadable files, or holds files that changed while read. */ + public function runWarnings(Run $run, string $targetName, int $unreadable, int $changing, array $examples): void { + $this->notifyAdmins('run_warnings', 'run-' . $run->getId(), ['target' => $targetName, + 'unreadable' => (string)$unreadable, 'changing' => (string)$changing, + 'examples' => mb_strimwidth(implode(', ', array_merge($examples['unreadable'], $examples['changing'])), 0, 300, '…')]); + } + /** * Remember how much is stored on a location (for the settings page) and warn administrators * once a day when it is at 80% of its limit or more. diff --git a/lib/Service/BackupService.php b/lib/Service/BackupService.php index 3649383..702cf78 100644 --- a/lib/Service/BackupService.php +++ b/lib/Service/BackupService.php @@ -14,6 +14,7 @@ use OCA\NgBackup\Db\Snapshot; use OCA\NgBackup\Db\SnapshotMapper; use OCA\NgBackup\Db\Target; use OCA\NgBackup\Job\BackupRun; +use OCA\NgBackup\Repository\RepositoryException; use OCP\App\IAppManager; use OCP\IConfig; use OCP\IDBConnection; @@ -151,16 +152,25 @@ final class BackupService { // Catalog write under the catalog lock, from a repository opened inside it: a per-user // export or restore may have written the catalog since this step opened $repo. try { - $this->leases->with(PruneService::catalogLockKey($target), LeaseService::EXCLUSIVE, 30, + $this->leases->with(PruneService::catalogLockKey($target), LeaseService::EXCLUSIVE, 300, fn () => $this->targets->repository($target)->recordSnapshot((string)$state['files']['snapshot'])); $state['files']['phase'] = 'done'; - } catch (LockedException) { - // Another catalog write is in progress: the next step retries this commit. + } catch (LockedException|RepositoryException) { + // Another catalog write is in progress (or just raced us to the next + // generation): the next step retries this commit on the fresh catalog. } } if ($state['files']['phase'] === 'done') { $this->recordSnapshot($run, $state['files']); $this->alerts->storedBytes($target, $repo->storedBytes()); + $stats = $state['files']['stats']; + if (($stats['unreadable'] ?? 0) > 0 || ($stats['changing'] ?? 0) > 0) { + $warnings = BackupRun::warnings($state['files']); + $this->logger->warning(sprintf('NG Backup run %d to %s: %d unreadable (missing from the snapshot): %s; %d changed while being read: %s', + $run->getId(), $target->getName(), $stats['unreadable'] ?? 0, implode(', ', $warnings['unreadable']), + $stats['changing'] ?? 0, implode(', ', $warnings['changing']))); + $this->alerts->runWarnings($run, $target->getName(), (int)($stats['unreadable'] ?? 0), (int)($stats['changing'] ?? 0), $warnings); + } $run->setStatus(Run::DONE); $run->setPhase('done'); $run->setFinishedAt(time()); diff --git a/lib/Service/UserBackupService.php b/lib/Service/UserBackupService.php index 4c5bf5c..0974b66 100644 --- a/lib/Service/UserBackupService.php +++ b/lib/Service/UserBackupService.php @@ -58,7 +58,7 @@ final class UserBackupService { private function withCatalogLock(Target $target, callable $fn): mixed { try { - return $this->leases->with(PruneService::catalogLockKey($target), LeaseService::EXCLUSIVE, 30, $fn); + return $this->leases->with(PruneService::catalogLockKey($target), LeaseService::EXCLUSIVE, 300, $fn); } catch (LockedException) { throw new \RuntimeException('Another catalog write for ' . $target->getName() . ' is in progress; try again'); } diff --git a/lib/Service/UserRestoreService.php b/lib/Service/UserRestoreService.php index eff3621..6fccfc1 100644 --- a/lib/Service/UserRestoreService.php +++ b/lib/Service/UserRestoreService.php @@ -129,7 +129,7 @@ final class UserRestoreService { private function withCatalogLock(Target $target, callable $fn): mixed { try { - return $this->leases->with(PruneService::catalogLockKey($target), LeaseService::EXCLUSIVE, 30, $fn); + return $this->leases->with(PruneService::catalogLockKey($target), LeaseService::EXCLUSIVE, 300, $fn); } catch (LockedException) { throw new \RuntimeException('Another catalog write for ' . $target->getName() . ' is in progress; try again'); } diff --git a/psalm-baseline.xml b/psalm-baseline.xml index ab61ac7..2718b1c 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -224,14 +224,9 @@ - - - - - diff --git a/tests/Unit/Repository/CrashSafetyTest.php b/tests/Unit/Repository/CrashSafetyTest.php new file mode 100644 index 0000000..a50978f --- /dev/null +++ b/tests/Unit/Repository/CrashSafetyTest.php @@ -0,0 +1,106 @@ +tempDir(); + $backend = new LocalBackend($root); + mkdir("$root/users/john.part-time", 0700, true); + file_put_contents("$root/users/john.part-time/20261006T120000Z-aaaa0001", 'x'); + file_put_contents("$root/users/upload.part-deadbeef", 'partial'); + $this->assertSame(['users/john.part-time/20261006T120000Z-aaaa0001'], $backend->list('users')); + } + + public function testPruneStopsWhenARecordedExportManifestIsMissing(): void { + $root = $this->tempDir(); + $backend = new LocalBackend($root); + $keys = KeyRing::generate(); + $repo = Repository::initWithKey($backend, $keys, $keys->wrap('pw')); + $stats = []; + $blob = $repo->storeData(random_bytes(1000), $stats); + $repo->flushPacks(); + $path = 'users/alice/20261006T120000Z-aaaa0001'; + $repo->putObject($path, json_encode(['uid' => 'alice', 'entries' => ['f.txt' => ['t' => 'f', 'b' => [$blob], 's' => 1000]]])); + $repo->recordUserExport($path); + unlink("$root/$path"); + + $packsBefore = $backend->list('packs'); + try { + Repository::openWithKey($backend, $keys)->prune(); + $this->fail('prune must refuse while a recorded export is missing'); + } catch (RepositoryException $e) { + $this->assertStringContainsString('missing on the location', $e->getMessage()); + } + $this->assertSame($packsBefore, $backend->list('packs'), 'no pack was deleted'); + } + + public function testForgetAndUntrashKeepTheSnapshotRestorable(): void { + $src = $this->tempDir(); + file_put_contents("$src/a.txt", 'hello'); + $root = $this->tempDir(); + $backend = new LocalBackend($root); + $keys = KeyRing::generate(); + Repository::initWithKey($backend, $keys, $keys->wrap('pw')); + $state = BackupRun::start($src); + while ($state['phase'] !== 'commit') { + $state = BackupRun::step(Repository::openWithKey($backend, $keys), $state, microtime(true) + 5); + } + Repository::openWithKey($backend, $keys)->recordSnapshot($state['snapshot']); + $id = $state['snapshot']; + + Repository::openWithKey($backend, $keys)->forget($id, 'test'); + $repo = Repository::openWithKey($backend, $keys); + $this->assertSame([], $repo->listSnapshots()); + $this->assertArrayHasKey($id, $repo->trash()); + $this->assertFalse($backend->exists('snapshots/' . $id), 'original removed after the catalog switch'); + + $repo->untrash($id); + $repo = Repository::openWithKey($backend, $keys); + $this->assertSame([$id], $repo->listSnapshots()); + $this->assertSame([], $repo->trash()); + $target = $this->tempDir(); + $repo->restore($id, $target); + $this->assertSame('hello', file_get_contents("$target/a.txt")); + } + + public function testFileThatKeepsChangingDoesNotStallTheRun(): void { + $src = $this->tempDir(); + file_put_contents("$src/live.db", random_bytes(3 * Repository::BLOB_SIZE)); + $root = $this->tempDir(); + $backend = new LocalBackend($root); + $keys = KeyRing::generate(); + Repository::initWithKey($backend, $keys, $keys->wrap('pw')); + + $state = BackupRun::start($src); + $n = 0; + for ($i = 0; $i < 50 && $state['phase'] !== 'commit'; $i++) { + $state = BackupRun::step(Repository::openWithKey($backend, $keys), $state, microtime(true) + 0.001); + // Rewrite the file after every step: it never stays the same between two steps. + $fh = fopen("$src/live.db", 'r+b'); + fwrite($fh, random_bytes(16)); + fclose($fh); + touch("$src/live.db", time() + ++$n); + clearstatcache(); + } + $this->assertSame('commit', $state['phase'], 'the run finishes instead of retrying forever'); + $this->assertSame(1, $state['stats']['changing'] ?? 0); + $this->assertSame(['live.db'], BackupRun::warnings($state)['changing']); + } +}