feat(maintenance): one kept pre-migration backup and nothing-to-do refusal

Re-review fixes R2, R3, R4, R5 and R6 (job side): - R3: Apply refuses a plan with nothing to rewrite (409 nothing-to-do, no job, no backup). Only the first apply keeps a pre-workflow-storage backup; later applies (after a Cancel, a rerun) take a pooled safety backup, forced on whatever the policy, so one copy is never pruned. - R4: a second-connection writer test runs against the live job loop; with its pause removed the writer hits "database is locked" (4 of 5 links). - R5: the digest tool loads only database/workflow_documents.py by path (no package start-up, no log handler) and opens a stopped database with immutable=1, so it leaves no -wal/-shm; a live one is read through its WAL. - R6: a backup that fails on a damaged document says which one in the job line. - R2: README's changelog says images recorded by this version use the new form from the first start, and the way back is the earliest pre-workflow-storage backup (or a pre-update backup if the job never ran).
2026-10-24 06:13:06 -07:00
parent 46fdf71e34
commit 8d61eda691
6 changed files with 391 additions and 50 deletions
@@ -58,9 +58,11 @@ from .jobs import MAINTENANCE_JOB_KIND, JobBusy
 from .maintenance_plans import PlanExpired, PlanUsed, ScopeTooLarge

 try:
+    from ...database.workflow_storage import PRE_MIGRATION_LABEL, pre_migration_backups
    from ...database.health import readonly_uri
    from ...database.maintenance import MAINTENANCE_OPERATIONS, PlanChanged
 except ImportError:
+    from database.workflow_storage import PRE_MIGRATION_LABEL, pre_migration_backups
    from database.health import readonly_uri
    from database.maintenance import MAINTENANCE_OPERATIONS, PlanChanged

@@ -69,8 +71,6 @@ PREVIEW_BODY_MAX = 4096
 DUPLICATE_PREVIEW_BODY_MAX = 1024 * 1024
 DUPLICATE_FILES = "duplicate_files"
 WORKFLOW_STORAGE = "workflow_storage"  # applied by py/api/workflow_storage.py
-# Its backup's name, outside the pruned ``<db>.backup_*`` pool.
-PRE_MIGRATION_LABEL = "pre-workflow-storage"
 PREVIEW_OPERATIONS = MAINTENANCE_OPERATIONS + (DUPLICATE_FILES,)
 # Room left over beyond what a step writes, so a disk is never filled to 0.
 DISK_MARGIN_BYTES = 16 * 2**20
@@ -93,12 +93,28 @@ PLAN_CHANGED = (
    "Review the plan again."
 )
 APPLY_FAILED = "Maintenance failed; nothing was changed."
+NOTHING_TO_DO = "Every image record already stores its workflow once; nothing to do."
 NO_BACKUP = "No safety backup"
 _NAME_ATTEMPTS = 1000


 class SafetyBackupFailed(Exception):
-    """The safety backup could not be written, verified or put in place."""
+    """The safety backup could not be written, verified or put in place.
+
+    ``reason`` is the verify reason when the copy did not verify.
+    """
+
+    def __init__(self, message, reason=None):
+        super().__init__(message)
+        self.reason = reason
+
+
+def backup_failed_message(error):
+    """The job line for a failed backup, naming a damaged workflow document."""
+    reason = getattr(error, "reason", None) or ""
+    if reason.startswith("Workflow document "):
+        return f"{BACKUP_FAILED} {reason}"
+    return BACKUP_FAILED


 def _now():
@@ -409,9 +425,9 @@ def take_safety_backup(model, label="backup"):
        if not model.backup_database(temp):
            raise SafetyBackupFailed("The backup could not be written")
        _standalone(temp)
-        ok, _reason = model.verify_database_file(temp)
+        ok, reason = model.verify_database_file(temp)
        if not ok:
-            raise SafetyBackupFailed("The backup did not verify")
+            raise SafetyBackupFailed("The backup did not verify", reason)
        _fsync(temp)
        if os.name != "nt":
            os.chmod(temp, 0o600)
@@ -530,6 +546,7 @@ class MaintenanceRoutesMixin:
                "targets": draft.targets,
                "roots": roots,
                "protected": draft.protected["count"],
+                "rows": draft.scope["count"],
            },
        )

@@ -551,8 +568,11 @@ class MaintenanceRoutesMixin:
        policy = self._safety_backup_policy()
        destination = "backup"
        if operation == WORKFLOW_STORAGE:
-            # Always backed up first, whatever the policy (the way back).
-            policy, destination = "required", PRE_MIGRATION_LABEL
+            # Always backed up first, whatever the policy (the way back);
+            # one kept pre-migration backup ever, pooled ones after it.
+            policy = "required"
+            kept = await self._run_in_executor(pre_migration_backups, db.model.db_path)
+            destination = "backup" if kept else PRE_MIGRATION_LABEL
        try:
            disk, _short, size = await self._run_in_executor(
                disk_plan,
@@ -611,6 +631,9 @@ class MaintenanceRoutesMixin:
                409,
                "storage-changed",
            )
+        if plan.operation == WORKFLOW_STORAGE and not plan.payload.get("rows"):
+            store.discard(token)  # no job, no backup: nothing to rewrite
+            return _failure(NOTHING_TO_DO, 409, "nothing-to-do")
        backup = self._backup_promised(plan)
        try:
            _disk, short, _size = await self._run_in_executor(
@@ -698,9 +721,9 @@ class MaintenanceRoutesMixin:
        job.log_line("Taking a safety backup of the database")
        try:
            backup = await self._run_in_executor(take_safety_backup, db.model)
-        except SafetyBackupFailed:
+        except SafetyBackupFailed as error:
            self.logger.error("Safety backup failed", exc_info=True)
-            _fail_job(job, "backup-failed", BACKUP_FAILED)
+            _fail_job(job, "backup-failed", backup_failed_message(error))
            return False, None
        job.log_line(f"Safety backup {backup} verified")
        return True, backup
@@ -32,10 +32,10 @@ import sqlite3

 from .jobs import MAINTENANCE_JOB_KIND, JobBusy
 from .maintenance import (
-    BACKUP_FAILED,
    MAINTENANCE_GROUPS,
    PRE_MIGRATION_LABEL,
    SafetyBackupFailed,
+    backup_failed_message,
    _fail_job,
    _megabytes,
    disk_plan,
@@ -61,8 +61,7 @@ RECLAIM_FAILED = "Reclaim space failed; the database is unchanged."
 ROW_KEPT = "could not be rewritten; it keeps its inline text and reads as before"


-def _backup_part(backup):
-    return f"Pre-migration backup {backup}"
+NOTHING_LEFT = "Every image record already stores its workflow once"


 def _plural(count, one, many=None):
@@ -97,34 +96,57 @@ class WorkflowStorageRoutesMixin:
        )

    async def _run_workflow_storage(self, job, db, _backup_on):
-        # Always backed up first, whatever the policy: the way back.
-        backup = await self._pre_migration_backup(job, db)
+        # Always backed up first, whatever the policy: the way back. The first
+        # apply keeps a pre-migration backup; later ones (after a Cancel, a
+        # rerun) take a pooled safety backup, so only one copy is never pruned.
+        counts = await self._run_in_executor(db.workflow_storage_status)
+        if not counts["rows"]:
+            job.log_line(NOTHING_LEFT)
+            job.update(progress=100, status_line=NOTHING_LEFT)
+            return
+        kept = await self._run_in_executor(
+            workflow_storage.pre_migration_backups, db.model.db_path
+        )
+        label = "backup" if kept else PRE_MIGRATION_LABEL
+        backup = await self._backup_before_rewrite(job, db, label, kept)
        if backup is None:
            return
-        if await self._rewrite_workflows(job, db, backup):
+        try:
+            done = await self._rewrite_workflows(job, db, backup)
+        finally:
+            if kept:
+                await self._prune_backup_pool(job, db)
+        if done:
            job.finish("finished")
            self._start_reclaim_space(job, db)

-    async def _pre_migration_backup(self, job, db):
-        """The verified ``<db>.pre-workflow-storage_<stamp>`` copy, or None.
+    async def _backup_before_rewrite(self, job, db, label, kept):
+        """The verified backup's status note, or None (the job failed).

-        Outside the pruned ``<db>.backup_*`` pool: it stays until the user
-        removes it. A failure ends the job ``backup-failed``, nothing changed.
+        ``label`` ``pre-workflow-storage``: kept until the user removes it.
+        ``backup``: a pooled safety backup, while the earliest kept
+        pre-migration backup (``kept[0]``) stays. A failure ends the job
+        ``backup-failed``, nothing changed.
        """
-        job.update(status_line="Taking the pre-migration backup")
-        job.log_line("Taking the pre-migration backup of the database")
+        noun = "safety backup" if kept else "pre-migration backup"
+        job.update(status_line=f"Taking the {noun}")
+        job.log_line(f"Taking the {noun} of the database")
        try:
-            backup = await self._run_in_executor(
-                take_safety_backup, db.model, PRE_MIGRATION_LABEL
-            )
-        except SafetyBackupFailed:
-            self.logger.error("Pre-migration backup failed", exc_info=True)
-            _fail_job(job, "backup-failed", BACKUP_FAILED)
+            name = await self._run_in_executor(take_safety_backup, db.model, label)
+        except SafetyBackupFailed as error:
+            self.logger.error("Backup before the rewrite failed", exc_info=True)
+            _fail_job(job, "backup-failed", backup_failed_message(error))
            return None
+        if kept:
+            job.log_line(
+                f"Safety backup {name} verified; the pre-migration backup "
+                f"{kept[0]} is kept"
+            )
+            return f"Safety backup {name}"
        job.log_line(
-            f"Pre-migration backup {backup} verified; it is kept until you remove it"
+            f"Pre-migration backup {name} verified; it is kept until you remove it"
        )
-        return backup
+        return f"Pre-migration backup {name}"

    async def _rewrite_workflows(self, job, db, backup):
        """Rewrite every old-form row; True when every row was looked at."""
@@ -185,7 +207,7 @@ class WorkflowStorageRoutesMixin:
        job.log_line(line)
        if removed:
            job.log_line(f"Removed {_plural(removed, 'unused workflow document')}")
-        job.update(progress=100, status_line=f"{line} · {_backup_part(backup)}")
+        job.update(progress=100, status_line=f"{line} · {backup}")
        return True

    def _stopped(self, job, rows, backup):
@@ -194,7 +216,7 @@ class WorkflowStorageRoutesMixin:
            "run it again to continue"
        )
        job.log_line(line)
-        job.update(status_line=f"{line} · {_backup_part(backup)}")
+        job.update(status_line=f"{line} · {backup}")
        job.finish("cancelled")
        return False