Backups to SMB no longer crawl; add occ backup:cancel

Each 30-second step opened the location again and read its list of index files. Over SMB that cost a request per file, so on a large repository opening took longer than the step: each step stored one 4 MiB pack and the list grew further. A first backup of nc01 to a Storage Box over SMB managed 12 GiB in 35 hours. occ backup:run and system cron now keep the repository open for the whole run (reopened when another process took a step or the location was edited), opening no longer counts towards the step's time, and SMB folders are listed in one request. occ backup:cancel stops a running backup: no new step starts and the step in progress finishes first. A run of an earlier version, which does not hold back its next step, is marked cancelled after two minutes unless it is recording its snapshot.
2026-10-14 12:15:27 +02:00
parent 92e693cfe6
commit dfd3f38d8c
9 changed files with 336 additions and 10 deletions
@@ -223,12 +223,23 @@ final class StorageBackend implements IBackend {
 		if (!$this->storage->is_dir($abs)) {
 			return []; // not created yet
 		}
+		$cut = $this->base === '' ? 0 : strlen($this->base) + 1;
+		$names = $this->smbNames($abs);
+		if ($names !== null) {
+			$result = [];
+			foreach ($names as $name) {
+				if (!self::isTemporary($name)) {
+					$result[] = substr($abs . '/' . $name, $cut);
+				}
+			}
+			sort($result);
+			return $result;
+		}
 		$dh = $this->storage->opendir($abs);
 		if ($dh === false) {
 			// An existing folder read as empty would look like a new repository.
 			throw new BackendException('Cannot list ' . $dir);
 		}
-		$cut = $this->base === '' ? 0 : strlen($this->base) + 1;
 		$result = [];
 		while (($name = readdir($dh)) !== false) {
 			if ($name !== '.' && $name !== '..' && !self::isTemporary($name)) {
@@ -240,6 +251,46 @@ final class StorageBackend implements IBackend {
 		return $result;
 	}

+	/**
+	 * The names in an SMB folder, read in one listing; null for other storages, with access
+	 * control lists on, or when the storage's internals are not as expected. The SMB storage's
+	 * opendir() asks the server for the attributes of each entry to leave out hidden ones: with
+	 * thousands of index files that took minutes. Hidden entries are still left out: names
+	 * starting with a dot, and, after asking, any name a repository does not use (letters,
+	 * digits, - and _), such as desktop.ini or Thumbs.db. The repository's own files have such
+	 * names: they are not checked one by one, which was the cost.
+	 *
+	 * @return list<string>|null
+	 */
+	private function smbNames(string $abs): ?array {
+		if (!$this->storage instanceof \OCA\Files_External\Lib\Storage\SMB) {
+			return null;
+		}
+		try {
+			/** @var list<string>|null */
+			return (function (string $path): ?array {
+				/** @psalm-suppress UndefinedThisPropertyFetch */
+				if ($this->checkAcl) {
+					return null;
+				}
+				$names = [];
+				/** @psalm-suppress UndefinedThisPropertyFetch, UndefinedMethod */
+				foreach ($this->share->dir(ltrim($this->buildPath($path), '/')) as $info) {
+					$name = (string)$info->getName();
+					/** @psalm-suppress UndefinedThisPropertyFetch */
+					if (!$this->showHidden && (str_starts_with($name, '.')
+						|| (preg_match('/^[A-Za-z0-9_-]+$/', $name) !== 1 && $info->isHidden()))) {
+						continue;
+					}
+					$names[] = $name;
+				}
+				return $names;
+			})->call($this->storage, $abs);
+		} catch (\Throwable) {
+			return null; // another SMB storage version, or an error: the usual way reports it
+		}
+	}
+
 	/**
 	 * Run $op; when it throws, or $failed says its result may come from a dead connection, and
 	 * the SFTP connection turns out to be dead, run it once more on a new connection.
@@ -0,0 +1,74 @@
+<?php
+
+declare(strict_types=1);
+
+// SPDX-FileCopyrightText: 2026 Dillard Blom
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+namespace OCA\NgBackup\Command;
+
+use OCA\NgBackup\Db\Run;
+use OCA\NgBackup\Db\RunMapper;
+use OCA\NgBackup\Service\BackupService;
+use OCP\AppFramework\Db\DoesNotExistException;
+use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Input\InputArgument;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
+
+final class CancelRun extends Command {
+	public function __construct(
+		private RunMapper $runs,
+		private BackupService $backups,
+	) {
+		parent::__construct();
+	}
+
+	protected function configure(): void {
+		$this->setName('backup:cancel')
+			->setDescription('Stop a running backup')
+			->addArgument('run', InputArgument::REQUIRED, 'Run number, as shown by backup:status')
+			->setHelp("No new step of the run starts; the step in progress finishes first (usually within a
+"
+				. "minute), so a backup that is just being completed is not cut off. The process working on the
+"
+				. "run (occ backup:run, or cron) then stops. A backup started by an earlier version is marked
+"
+				. "cancelled after two minutes and stops at the end of its step. What it already stored is
+"
+				. "removed by the next cleanup.");
+	}
+
+	protected function execute(InputInterface $input, OutputInterface $output): int {
+		$id = (string)$input->getArgument('run');
+		if (!ctype_digit($id)) {
+			$output->writeln('<error>Give the run number, as shown by backup:status.</error>');
+			return 1;
+		}
+		try {
+			$run = $this->runs->find((int)$id);
+		} catch (DoesNotExistException) {
+			$output->writeln("<error>There is no run $id.</error>");
+			return 1;
+		}
+		if ($run->getKind() !== BackupService::KIND_FULL) {
+			$output->writeln("<error>Run $id is not a backup; only backups can be cancelled.</error>");
+			return 1;
+		}
+		if ($run->getStatus() === Run::RUNNING) {
+			$output->writeln("Waiting for the current step of run $id to finish...");
+		}
+		try {
+			$run = $this->backups->cancel($run);
+		} catch (\RuntimeException $e) {
+			$output->writeln('<error>' . $e->getMessage() . '</error>');
+			return 1;
+		}
+		if ($run->getStatus() !== Run::CANCELLED) {
+			$output->writeln("Run $id is not running (status: {$run->getStatus()}); nothing to cancel.");
+			return 0;
+		}
+		$output->writeln("Run $id is cancelled.");
+		return 0;
+	}
+}
@@ -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\Repository;
 use OCA\NgBackup\Repository\RepositoryException;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\App\IAppManager;
@@ -34,6 +35,9 @@ use Psr\Log\LoggerInterface;
 final class BackupService {
 	public const KIND_FULL = 'full';

+	/** @var \WeakMap<Repository, string> the location settings each kept repository was opened with */
+	private \WeakMap $openedWith;
+
 	public function __construct(
 		private TargetService $targets,
 		private KeyService $keys,
@@ -47,6 +51,9 @@ final class BackupService {
 		private AlertService $alerts,
 		private LeaseService $leases,
 	) {
+		/** @var \WeakMap<Repository, string> $opened */
+		$opened = new \WeakMap();
+		$this->openedWith = $opened;
 	}

 	public function start(Target $target, string $label = ''): Run {
@@ -144,13 +151,22 @@ final class BackupService {
 	/**
 	 * Do one step of a run until $deadline. Safe to call concurrently: a second caller finds the
 	 * run locked and returns immediately.
+	 *
+	 * @param Repository|null $repo the location's repository, opened by an earlier step of this run
+	 *                              in this process (see runToCompletion()); set when this step opens it
 	 */
-	public function step(Run $run, float $deadline): Run {
+	public function step(Run $run, float $deadline, ?Repository &$repo = null): Run {
 		// Leases expire on their own: a step killed by a time limit or OOM blocks nothing for long.
 		// A step can run well past its deadline (the commit of a large snapshot writes its tree and
 		// index), so the lease holds at least 15 minutes; should it still expire and another process
 		// take the run over, the checks below keep the two from overwriting each other.
 		$ttl = (int)min(3600, max(900, $deadline - microtime(true) + 600));
+		// No new step once a cancel has begun: it waits for the step in progress, if any.
+		try {
+			$this->leases->release($this->leases->acquire(self::cancelKey($run->getId()), LeaseService::SHARED, 60));
+		} catch (LockedException) {
+			return $run;
+		}
 		try {
 			$runLease = $this->leases->acquire('run/' . $run->getId(), LeaseService::EXCLUSIVE, $ttl);
 		} catch (LockedException) {
@@ -164,6 +180,7 @@ final class BackupService {
 			return $run;
 		}
 		$loadedAt = null;
+		$savedBefore = $run->getUpdatedAt();
 		try {
 			$run = $this->runs->find($run->getId()); // fresh state after acquiring the lock
 			if ($run->getStatus() !== Run::RUNNING || $run->getKind() !== self::KIND_FULL) {
@@ -171,7 +188,19 @@ final class BackupService {
 			}
 			$loadedAt = $run->getUpdatedAt();
 			$target = $this->targets->get((string)$run->getTargetId());
-			$repo = $this->targets->repository($target);
+			// Reopen when another process took a step since ours (its packs are not in our copy of
+			// the index) or the location was edited (password, folder, size limit).
+			$settings = json_encode([$target->getBackend(), $target->getAuth(), $target->getOptions(),
+				$target->getBasePath(), $target->getRepositoryId(), $target->getMaxBytes()], JSON_THROW_ON_ERROR);
+			if ($repo === null || $loadedAt !== $savedBefore || ($this->openedWith[$repo] ?? null) !== $settings) {
+				// Opening reads the index listing, which takes minutes on a slow location with many
+				// packs: that time does not count, or each step would store one blob and stop.
+				$opening = microtime(true);
+				$repo = null;
+				$repo = $this->targets->repository($target);
+				$this->openedWith[$repo] = $settings;
+				$deadline += microtime(true) - $opening;
+			}
 			$state = json_decode($run->getState(), true, 512, JSON_THROW_ON_ERROR);

 			if ($run->getPhase() === 'db') {
@@ -234,8 +263,14 @@ final class BackupService {
 			$run->setStats(json_encode(['db' => $state['dbStats'] ?? null, 'files' => $state['files']['stats']], JSON_THROW_ON_ERROR));
 			// Another process may have taken the run over after this step's lease expired: then its
 			// writes stand and this step's are dropped.
-			return $this->runs->updateIfUnchanged($run, $loadedAt) ?? $this->runs->find($run->getId());
+			$saved = $this->runs->updateIfUnchanged($run, $loadedAt);
+			if ($saved === null) {
+				$repo = null; // the run is another process's now: its packs are not in our index
+				return $this->runs->find($run->getId());
+			}
+			return $saved;
 		} catch (\Throwable $e) {
+			$repo = null; // may hold blobs of a half-written pack: the next step opens it afresh
 			if ($loadedAt === null) {
 				// The run could not even be loaded: there is nothing fresh to mark as failed, and
 				// the caller's copy may be stale.
@@ -266,14 +301,16 @@ final class BackupService {

 	/** Run to completion in this process (occ, system cron), reporting progress. */
 	public function runToCompletion(Run $run, ?callable $progress = null, float $stepSeconds = 30): Run {
+		$repo = null; // opened once, by the first step, and kept for the next ones
 		while ($run->getStatus() === Run::RUNNING) {
 			$before = $run->getUpdatedAt();
-			$run = $this->step($run, microtime(true) + $stepSeconds);
+			$run = $this->step($run, microtime(true) + $stepSeconds, $repo);
 			if ($run->getStatus() === Run::RUNNING && $run->getUpdatedAt() === $before) {
 				// Step was not possible (another process holds the run, or a crashed step's lease has
 				// not expired yet): wait instead of spinning.
 				sleep(5);
 				$run = $this->runs->find($run->getId());
+				$repo = null; // another process may have stored packs in the meantime
 			}
 			if ($progress !== null) {
 				$progress($run);
@@ -282,11 +319,70 @@ final class BackupService {
 		return $run;
 	}

-	public function cancel(Run $run): Run {
-		$run->setStatus(Run::CANCELLED);
-		$run->setFinishedAt(time());
-		$run->setUpdatedAt(time());
-		return $this->runs->update($run);
+	/**
+	 * Stop a running backup. First no new step may start, then this waits (up to $wait seconds)
+	 * for the step in progress, so a step that is recording the snapshot finishes first. A process
+	 * running the backup then finds it cancelled and stops. The packs it stored stay until the
+	 * next cleanup.
+	 *
+	 * A process of an earlier version does not hold back its next step and may never leave a gap:
+	 * after $wait the run is marked cancelled while it works, unless it is recording the snapshot.
+	 * Its step then fails to save and the process stops.
+	 */
+	public function cancel(Run $run, int $wait = 120): Run {
+		$id = $run->getId();
+		$until = time() + $wait;
+		$hold = null;
+		for ($try = 0; $hold === null; $try++) {
+			try {
+				$hold = $this->leases->acquire(self::cancelKey($id), LeaseService::EXCLUSIVE, $wait + 300);
+			} catch (LockedException) {
+				if ($try >= 20) {
+					throw new \RuntimeException("Run $id is being cancelled already");
+				}
+				usleep(100000); // a step checking whether it may start
+			}
+		}
+		try {
+			$lease = null;
+			while ($lease === null && time() < $until) {
+				try {
+					$lease = $this->leases->acquire('run/' . $id, LeaseService::EXCLUSIVE, 60);
+				} catch (LockedException) {
+					sleep(2);
+				}
+			}
+			try {
+				for ($try = 0; $try < 10; $try++) {
+					$run = $this->runs->find($id);
+					if ($run->getStatus() !== Run::RUNNING) {
+						return $run;
+					}
+					$state = json_decode($run->getState(), true) ?: [];
+					if ($lease === null && ($state['files']['phase'] ?? '') === 'commit') {
+						throw new \RuntimeException("Run $id is recording its snapshot; try again in a minute");
+					}
+					$loadedAt = $run->getUpdatedAt();
+					$run->setStatus(Run::CANCELLED);
+					$run->setFinishedAt(time());
+					if (($cancelled = $this->runs->updateIfUnchanged($run, $loadedAt)) !== null) {
+						return $cancelled;
+					}
+					usleep(200000); // a step of another process saved just now
+				}
+				throw new \RuntimeException("Run $id keeps changing; try again");
+			} finally {
+				if ($lease !== null) {
+					$this->leases->release($lease);
+				}
+			}
+		} finally {
+			$this->leases->release($hold);
+		}
+	}
+
+	private static function cancelKey(int $runId): string {
+		return 'run/' . $runId . '/cancel';
 	}

 	private function recordSnapshot(Run $run, array $files): void {