nodes/ComfyAssets-ComfyUI_PromptManager
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(jobs): refuse legacy record-changing operations while a job runs

Browse Source 
The job registry gains admission classes. A records job is refused
while any records or guarded job runs, a guarded job while a records
job runs, and read jobs share only the one-job-per-kind rule.
record_changing=True stays as the spelling of records. Job summaries
add cancellable and admission, and a non-cancellable job ignores
cancel.

JobRegistry.hold() registers a running, non-cancellable job for a
legacy-page operation and ends it when the request does. The legacy
restore, maintenance, prompt cleanup, duplicate image delete, LoRA
import and batch auto-tag routes hold as records; thumbnail generation
(both routes) and thumbnail clear hold as guarded. A maintenance run
of only read checks holds as read. So a restore during a scan, or a
scan during a legacy restore, is refused with the running job named
instead of interleaving; this closes the gap noted on #269.

Refusals keep each route's shape: the JSON routes return the legacy
409 "<title> already running", and the thumbnail and LoRA streams
send their existing error event.
This commit is contained in:
Vito Sansevero
2026-10-22 19:17:11 -07:00
parent 9193373eac
commit ebebf0d69c
11 changed files with 918 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files
py/api/__init__.py
+20 -1
View File
@@ -34,7 +34,7 @@ from .images import ImageRoutesMixin
from .admin import AdminRoutesMixin
from .job_routes import JobRoutesMixin
from .image_tag_jobs import ImageTagJobsMixin
from .jobs import JobRegistry
from .jobs import JobBusy, JobRegistry
from .logging_routes import LoggingRoutesMixin
from .autotag_routes import AutotagRoutesMixin
from .lora_integration import LoraIntegrationMixin
@@ -361,6 +361,25 @@ class PromptManagerAPI(
"""409 envelope for a long job that is already running."""
return _json_error(f"{label} already running", 409)
async def _run_held(self, kind, title, admission, operation):
"""Run a legacy-page operation under a registry hold (D80).
While ``operation()`` runs, the registry lists it as a running,
non-cancellable job, so workspace jobs its admission class conflicts
with are refused, naming it. When the registry refuses the hold, the
legacy 409 envelope names the running job and nothing is done. A
response of 400 or above ends the hold ``failed``.
"""
try:
held = self._jobs.hold(kind, title, admission=admission)
except JobBusy as busy:
return self._long_job_busy_response(busy.running.title)
async with held as job:
response = await operation()
if getattr(response, "status", 200) >= 400:
job.finish("failed")
return response
async def _run_in_executor(self, func, *args, **kwargs):
"""Run a blocking function in the default thread pool executor.
py/api/admin.py
+67 -7
View File
@@ -22,6 +22,9 @@ from .jobs import (
SCAN_GROUP_NAMES,
SCAN_GROUPS,
SCAN_SCOPES,
DUPLICATE_CLEANUP_JOB_KIND,
MAINTENANCE_JOB_KIND,
RESTORE_JOB_KIND,
Job,
JobBusy,
)
@@ -696,7 +699,15 @@ class AdminRoutesMixin:
job.finish()
async def cleanup_duplicates_endpoint(self, request):
"""Cleanup duplicate prompts endpoint."""
"""Cleanup duplicate prompts endpoint (holds the registry: records)."""
return await self._run_held(
MAINTENANCE_JOB_KIND,
"Prompt cleanup (admin page)",
"records",
self._cleanup_duplicates,
)
async def _cleanup_duplicates(self):
try:
removed_count = await self._run_in_executor(self.db.cleanup_duplicates)
@@ -811,12 +822,26 @@ class AdminRoutesMixin:
return hash_sha256.hexdigest()
async def delete_duplicate_images_endpoint(self, request):
"""Delete duplicate image files from disk (single flight with the scan)."""
job = self._acquire_long_job(self.LONG_JOB_DUPLICATES)
if job is None:
"""Delete duplicate image files from disk.
Single flight with the duplicate scan, and a records hold on the
registry, so no scan, restore or tagging job runs meanwhile.
"""
guard = self._acquire_long_job(self.LONG_JOB_DUPLICATES)
if guard is None:
return self._long_job_busy_response("Duplicate scan or delete")
async with job:
return await self._delete_duplicate_images(request)
async def delete():
# No await since the check above: the lock is still free.
async with guard:
return await self._delete_duplicate_images(request)
return await self._run_held(
DUPLICATE_CLEANUP_JOB_KIND,
"Duplicate image delete (gallery page)",
"records",
delete,
)
async def _delete_duplicate_images(self, request):
try:
@@ -1268,6 +1293,11 @@ class AdminRoutesMixin:
"vacuum",
"cleanup_orphaned_images",
)
# Operations that only read; a run of only these holds the registry as
# "read", every other run as "records".
READ_ONLY_MAINTENANCE_OPERATIONS = frozenset(
{"check_hash_duplicates", "statistics", "check_consistency"}
)
async def run_maintenance(self, request):
"""Perform database maintenance operations and report each outcome."""
@@ -1280,7 +1310,27 @@ class AdminRoutesMixin:
operations = data.get(
"operations", list(self.DEFAULT_MAINTENANCE_OPERATIONS)
)
except Exception as e:
self.logger.error(f"Maintenance error: {e}", exc_info=True)
return web.json_response(
{"success": False, "error": "Maintenance failed"}, status=500
)
writes = not (
isinstance(operations, list)
and all(
isinstance(name, str) and name in self.READ_ONLY_MAINTENANCE_OPERATIONS
for name in operations
)
)
return await self._run_held(
MAINTENANCE_JOB_KIND,
"Maintenance (admin page)",
"records" if writes else "read",
lambda: self._run_maintenance(operations),
)
async def _run_maintenance(self, operations):
try:
results = await self._run_in_executor(
self._run_maintenance_operations, operations
)
@@ -1434,8 +1484,18 @@ class AdminRoutesMixin:
The upload is streamed to a temp file (capped at restore_max_bytes),
verified with PromptModel.verify_database_file and only then copied
over the live database, which is backed up first.
over the live database, which is backed up first. The whole request
holds the registry (records): it is refused while a scan or another
record-changing job runs, and those are refused while it runs.
"""
return await self._run_held(
RESTORE_JOB_KIND,
"Database restore (admin page)",
"records",
lambda: self._restore_database(request),
)
async def _restore_database(self, request):
max_bytes = getattr(self, "restore_max_bytes", 100 * 1024 * 1024)
temp_path = None
try:
py/api/autotag_routes.py
+22
View File
@@ -8,6 +8,7 @@ from pathlib import Path
from typing import Optional
from urllib.parse import quote
from .jobs import AUTOTAG_BATCH_JOB_KIND, JobBusy
from .prompts import thumbnail_url_for
from .. import model_gate
@@ -440,6 +441,27 @@ class AutotagRoutesMixin:
"message": _INTERNAL_ERROR,
}
# The batch writes prompt tags: a records hold on the registry, so it
# never runs beside a scan, restore or the workspace tagging job.
try:
held = self._jobs.hold(
AUTOTAG_BATCH_JOB_KIND,
"Auto-tag images (legacy page)",
admission="records",
)
except JobBusy as busy:
if busy.running.kind == AUTOTAG_BATCH_JOB_KIND: # unchanged payload
return web.json_response(
{"success": False, "error": _BUSY_ERROR}, status=409
)
return self._long_job_busy_response(busy.running.title)
async with held as job:
response = await self._start_autotag_batch(request, opts, events)
if response.status >= 400:
job.finish("failed")
return response
async def _start_autotag_batch(self, request, opts, events):
if not _autotag_batch_lock.acquire(blocking=False):
return web.json_response(
{"success": False, "error": _BUSY_ERROR}, status=409
py/api/images.py
+47 -5
View File
@@ -19,6 +19,7 @@ from typing import Callable, NamedTuple
from aiohttp import web
from PIL import Image, UnidentifiedImageError
from .jobs import THUMBNAILS_JOB_KIND, JobBusy
from .prompts import bad_request, parse_page_params, publish_image_paths
from .prompts import thumbnail_url_for
from .prompts import safe_error_message as _safe_error
@@ -104,6 +105,10 @@ def _card_header(path: Path) -> tuple:
return inspection.width, inspection.height, records
# The legacy gallery page's thumbnail generation, as Activity lists it (D80).
THUMBNAIL_GENERATION_TITLE = "Thumbnail generation (gallery page)"
class _CardFacts(NamedTuple):
"""One image file's card facts, derived from its header alone.
@@ -1736,7 +1741,19 @@ class ImageRoutesMixin:
return cls.THUMBNAIL_SIZES.get(quality, cls.THUMBNAIL_SIZES["medium"])
async def generate_thumbnails(self, request):
"""Generate every thumbnail in one executor job (POST, blocking)."""
"""Generate every thumbnail in one executor job (POST, blocking).
Holds the registry as guarded work: refused while a record-changing
job runs, and such jobs are refused while it runs.
"""
return await self._run_held(
THUMBNAILS_JOB_KIND,
THUMBNAIL_GENERATION_TITLE,
"guarded",
lambda: self._generate_thumbnails(request),
)
async def _generate_thumbnails(self, request):
try:
data, error = await _json_body_or_error(request)
if error is not None:
@@ -1935,14 +1952,31 @@ class ImageRoutesMixin:
"Connection": "keep-alive",
},
)
await response.prepare(request)
try:
held = self._jobs.hold(
THUMBNAILS_JOB_KIND, THUMBNAIL_GENERATION_TITLE, admission="guarded"
)
except JobBusy as busy:
# The gallery reads this stream: refuse with its error event.
await response.prepare(request)
message = f"{busy.running.title} already running"
await self._emit_sse(
response, "error", {"error": message, "message": message}
)
return response
async with held:
await response.prepare(request)
await self._thumbnail_progress_stream(response, thumbnail_size)
return response
async def _thumbnail_progress_stream(self, response, thumbnail_size):
try:
output_dirs = self._get_all_output_dirs()
if not output_dirs:
await self._emit_sse(
response, "error", {"error": "ComfyUI output directory not found"}
)
return response
return
await self._emit_sse(
response,
@@ -1970,7 +2004,6 @@ class ImageRoutesMixin:
"message": "Thumbnail generation failed. Check server logs.",
},
)
return response
async def _stream_thumbnails(
self, response, targets, thumbnail_size, skipped=0, remaining=0
@@ -2179,8 +2212,17 @@ class ImageRoutesMixin:
"""Clear generated thumbnails across gallery roots, preserving originals.
Invalid thumbnail directories return 400; unavailable gallery roots
return 404, and unexpected filesystem failures return 500.
return 404, and unexpected filesystem failures return 500. Holds the
registry as guarded work, like generation.
"""
return await self._run_held(
THUMBNAILS_JOB_KIND,
"Thumbnail clear (gallery page)",
"guarded",
self._clear_thumbnails,
)
async def _clear_thumbnails(self) -> web.Response:
try:
output_dirs = self._get_all_output_dirs()
if not output_dirs:
py/api/jobs.py
+147 -25
View File
@@ -9,9 +9,21 @@ as a snapshot and then every event published after it, until the job ends.
Jobs live only in server memory and are gone after a restart. The registry
keeps the running jobs and this session's last ``MAX_FINISHED_JOBS`` finished
ones. Record-changing jobs (the metadata scan, bulk image tagging, and later
maintenance, cleanup and restore) never run together, and one job of a kind
runs at a time.
ones. One job of a kind runs at a time, and each job has an admission class
(``ADMISSIONS``) that decides what may run beside it:
- ``records`` changes library records or files (the metadata scan, bulk image
tagging, and the legacy restore, maintenance, cleanup, duplicate delete,
LoRA import and batch auto-tag routes). It is refused while any
``records`` or ``guarded`` job runs.
- ``guarded`` must not overlap record changes but may overlap other guarded
work (thumbnail generation and clearing, later backup). It is refused while
a ``records`` job runs.
- ``read`` only reads; it is refused only by a running job of its own kind.
A short legacy-page operation takes a ``hold``: a running, non-cancellable
job that lasts as long as the request, so the Activity page lists it and a
workspace job is refused while it runs (and the other way round).
"""
import asyncio
@@ -60,6 +72,41 @@ SCAN_GROUPS = {
IMAGE_TAG_JOB_KIND = "image-tags"
IMAGE_TAG_GROUP_NAMES = ("tagged", "unchanged", "skipped", "errors")
ADMISSIONS = ("read", "guarded", "records")
# What each admission class is refused beside (besides its own kind).
_REFUSED_BESIDE = {
"read": frozenset(),
"guarded": frozenset({"records"}),
"records": frozenset({"records", "guarded"}),
}
# Kinds of the legacy-page operations that hold the registry (D80; titles in
# their handlers end in "(admin page)" or "(gallery page)").
RESTORE_JOB_KIND = "restore"
MAINTENANCE_JOB_KIND = "maintenance"
DUPLICATE_CLEANUP_JOB_KIND = "duplicate_cleanup"
THUMBNAILS_JOB_KIND = "thumbnails"
LORA_IMPORT_JOB_KIND = "lora_import"
AUTOTAG_BATCH_JOB_KIND = "autotag"
def resolve_admission(admission=None, record_changing=None) -> str:
"""The admission class for a start call; ``record_changing`` is an alias.
``record_changing=True`` means ``records`` and ``False`` means ``read``;
with neither given the class is ``read``. Raises ValueError for an
unknown class or one that contradicts ``record_changing``.
"""
if admission is None:
return "records" if record_changing else "read"
if admission not in ADMISSIONS:
raise ValueError(f"Unknown job admission {admission!r}")
if record_changing is not None and record_changing != (admission == "records"):
raise ValueError(
f"record_changing={record_changing!r} contradicts admission {admission!r}"
)
return admission
def _relative_path(path) -> str:
"""``path`` as a root-relative POSIX path; an absolute one keeps its name."""
@@ -85,13 +132,23 @@ class Job:
"""One tracked operation: state, counters, outcomes, log and subscribers."""
def __init__(
self, kind, title, *, record_changing=False, scope=None, groups=(), clock=None
self,
kind,
title,
*,
admission=None,
record_changing=None,
cancellable=True,
scope=None,
groups=(),
clock=None,
):
self._clock = clock or time.time
self.id = uuid.uuid4().hex
self.kind = kind
self.title = title
self.record_changing = record_changing
self.admission = resolve_admission(admission, record_changing)
self.cancellable = bool(cancellable)
self.scope = dict(scope or {})
self.state = "running"
self.started_at = self._clock()
@@ -110,6 +167,10 @@ class Job:
self.task = None
self._subscribers = set()
@property
def record_changing(self):
return self.admission == "records"
@property
def running(self):
return self.state == "running"
@@ -240,6 +301,9 @@ class Job:
# What detail() lists, so a view re-reads it only when it changed.
"listed": {group: len(rows) for group, rows in self.outcomes.items()},
"log_length": len(self.log),
# A view offers Cancel only when the server can stop the job.
"cancellable": self.cancellable,
"admission": self.admission,
}
def detail(self):
@@ -291,33 +355,76 @@ class JobRegistry:
self._logger = logger_ or logger
self._jobs = {}
def start(self, kind, title, *, record_changing, scope, runner, groups=()) -> Job:
def start(
self,
kind,
title,
*,
admission=None,
record_changing=None,
cancellable=True,
scope=None,
runner,
groups=(),
) -> Job:
"""Create a job and run ``runner(job)`` as a task.
Raises JobBusy, naming the job in the way, when a job of ``kind`` is
running or when ``record_changing`` and another record-changing job
runs. Must be called from the event loop.
``admission`` is one of ``ADMISSIONS`` (default ``read``);
``record_changing=True`` is the older spelling of ``records``. Raises
JobBusy, naming the job in the way, when the admission class refuses
it (see the module docstring). Must be called from the event loop.
"""
blocker = self.blocker(kind, record_changing=record_changing)
if blocker is not None:
raise JobBusy(blocker)
self._prune()
job = Job(
kind, title, record_changing=record_changing, scope=scope, groups=groups
job = self._admit(
kind,
title,
admission=admission,
record_changing=record_changing,
cancellable=cancellable,
scope=scope,
groups=groups,
)
self._jobs[job.id] = job
job.task = asyncio.ensure_future(self._run(job, runner))
return job
def blocker(self, kind, *, record_changing):
"""The running job a new ``kind`` job would be refused for, or None."""
def hold(self, kind, title, *, admission, scope=None):
"""Register a running, non-cancellable job for a legacy-page operation.
Returns an async context manager yielding the job; the job ends when
the block exits: ``finished``, or ``failed`` when it raised (unless
the body already ended it). Raises JobBusy at once, without waiting,
when the admission class refuses it. Use it directly in ``async with``
so the job cannot outlive the operation.
"""
job = self._admit(
kind, title, admission=admission, cancellable=False, scope=scope
)
return _Hold(job)
def _admit(self, kind, title, *, admission, record_changing=None, **job_args):
resolved = resolve_admission(admission, record_changing)
blocker = self.blocker(kind, admission=resolved)
if blocker is not None:
raise JobBusy(blocker)
self._prune()
job = Job(kind, title, admission=resolved, **job_args)
self._jobs[job.id] = job
return job
def blocker(self, kind, *, admission=None, record_changing=None):
"""The running job a new ``kind`` job would be refused for, or None.
The same kind refuses first; then the oldest running job whose class
the new job's admission class may not run beside.
"""
same_kind = self.active(kind)
if same_kind is not None:
return same_kind
if record_changing:
for job in self._jobs.values():
if job.running and job.record_changing:
return job
refused = _REFUSED_BESIDE[resolve_admission(admission, record_changing)]
running = [job for job in self._jobs.values() if job.running]
running.sort(key=lambda job: job._started_seq)
for job in running:
if job.admission in refused:
return job
return None
async def _run(self, job, runner):
@@ -362,11 +469,26 @@ class JobRegistry:
def cancel(self, job_id) -> bool:
"""Ask a running job to stop at its next safe boundary.
Returns whether a cancellation was requested: False for a finished or
unknown job, so a cancel after the end is a no-op.
Returns whether a cancellation was requested: False for a finished,
unknown or non-cancellable job, so a cancel after the end is a no-op.
"""
job = self._jobs.get(job_id)
if job is None or job.finished:
if job is None or job.finished or not job.cancellable:
return False
job.cancelled = True
return True
class _Hold:
"""The async context manager ``JobRegistry.hold`` returns."""
def __init__(self, job):
self._job = job
async def __aenter__(self):
return self._job
async def __aexit__(self, exc_type, exc, tb):
if self._job.running:
self._job.finish("finished" if exc_type is None else "failed")
return False
py/api/lora_integration.py
+26 -6
View File
@@ -6,6 +6,7 @@ from pathlib import Path
from aiohttp import web
from ..config_store import ConfigUnreadable, RevisionConflict, get_config_store
from .jobs import LORA_IMPORT_JOB_KIND, JobBusy
# Flags of POST /lora/enable; each must be a JSON boolean when present.
_LORA_FLAGS = ("enabled", "trigger_words_enabled", "clear_civitai_api_key")
@@ -273,19 +274,38 @@ class LoraIntegrationMixin:
reason="OK",
headers={"Content-Type": "text/event-stream"},
)
await response.prepare(request)
async def send_progress(data):
line = f"data: {json.dumps(data)}\n\n"
await response.write(line.encode("utf-8"))
# The import replaces prompts and images: a records hold, so it
# never runs beside a scan, restore or tagging job.
try:
await self._lora_scan_stream(allowed, send_progress)
except Exception as e:
self.logger.exception("LoRA scan failed")
await send_progress({"type": "error", "message": self._public_error(e)})
held = self._jobs.hold(
LORA_IMPORT_JOB_KIND,
"LoRA import (admin page)",
admission="records",
)
except JobBusy as busy:
await response.prepare(request)
message = f"{busy.running.title} already running"
await send_progress({"type": "error", "message": message})
await response.write_eof()
return response
await response.write_eof()
async with held as job:
await response.prepare(request)
try:
await self._lora_scan_stream(allowed, send_progress)
except Exception as e:
self.logger.exception("LoRA scan failed")
job.finish("failed")
await send_progress(
{"type": "error", "message": self._public_error(e)}
)
await response.write_eof()
return response
except Exception as e: