* fix(images): backfill orientation for libraries that predate the fix (#1198) #1194 corrected the generators and every ingest path, but did nothing for photos already in the database. Those rows end up worse than untouched ones: before the fix a rotated photo was CONSISTENTLY wrong — a sideways image in a tile shaped to match — and afterwards the regenerated thumbnail is correct while photos.width/height still describe the raw sensor order, so masonry and justified size a portrait photo with a landscape ratio. The dimension repair cannot reach them: it only selects rows with a NULL dimension, and an affected row has both, just transposed. Its own job rather than a mode of that one. They look alike but are not the same operation: the repair FILLS missing values and touches nothing else, while this RECOMPUTES and invalidates the derived data generated against the old orientation. Sharing a lease would also mean one blocks the other. A first attempt at this was reverted from #1194 after review found five problems. All five are addressed here: - Originals are read through resolvePhotoStorageKey + withLocalCopy + withProcessableImage, so the job works on S3 installs and on RAW/DNG. The dimension repair's direct fs read does neither, which stops being an edge case in a job that walks the whole library. - The canonical preview is cleared BEFORE faces are requeued. ensurePreviewImage returns a cached preview whenever it is still a valid image, and a pre-fix unrotated one is perfectly valid — so requeueing alone made the rescan read unrotated pixels and scale those boxes by the corrected dimensions, which is worse than leaving the data alone. - Invalidation keys off the EXIF transform, not a dimension delta. Orientations 2, 3 and 4 move every pixel while leaving width and height unchanged, as does 5-8 on a square image; a delta check skips exactly those rows. - Archived events are excluded — archiving deletes the originals and keeps the rows, so every one of them would fail its read. - The dimension write and the invalidation share a transaction. Split, a failure between them leaves stale face data that no retry can fix, because the retry computes "already correct". Tier deletion stays outside the transaction on purpose: it touches storage, and a failed object delete must not roll back a correct database write. A leftover tier regenerates on next read; a rolled-back write is silent corruption. * fix(images): invalidate every stale rendition, fence the writes, and give the job a button (#1198) Three things from review, one of which mattered a lot. The invalidation was too narrow. Clearing only preview_path fixed the face data and left the gallery worse off: ensureThumbnail and ensureHeroImage return their cached file whenever it is merely VALID, and a pre-fix sideways thumbnail is perfectly valid — so a corrected row rendered the old sideways image inside a newly-corrected portrait tile. All three canonical renditions are cleared now, their stored objects deleted, and both responsive tier sets with them. The responsive tiers also needed handling rather than a hopeful catch. Their helpers swallow delete errors, and ensurePreviewImageAtWidth treats storage.stat(key) as a cache hit — so a tier that survived deletion keeps serving unrotated forever and never regenerates. The keys are re-checked after deletion and survivors are counted into the result, so a run that could not clear them does not report itself as clean. Writes are fenced on the identity that was measured, not just the id. replacePhoto swaps a new file under an existing row and rewrites path/filename, and it IS reachable — from the replace_by_name upload path in adminPhotos.js. A replacement landing while this job read the old original would otherwise have had the previous file's dimensions written over it and its fresh renditions cleared. And the job had no way to start it: the endpoint existed with no caller, so an upgrade would have left every affected library untouched unless an operator found the API themselves. It gets a Status card like its two neighbours, with strings in en/de/fr/sl. No backlog counter, because unlike the other two it cannot know how many rows need it without doing the work. * fix(images): make the backfill idempotent, and stop it lying about what it did (#1198) Six things from review round 2. The job was not idempotent, and the way it failed was expensive. Its trigger is the EXIF tag on the ORIGINAL, which correcting a photo never changes — so every re-run threw away the renditions it had just regenerated and requeued every completed face scan. On a face-enabled install, running it twice meant re-detecting the whole library for nothing. Migration 191 adds photos.orientation_checked_at, written in the same transaction as the work it records, with `force` as the escape hatch for an interrupted run. The candidate query selected preview_path but not thumbnail_path or hero_path, which the deletion loop reads — so those two pointers were cleared in the database while the objects stayed in storage, still reachable through previously issued URLs. watermark_path was missed entirely. gallery.js serves it ahead of the original when branding watermarking is on, which makes it the most visible rendition of the lot. (Its generator needed rotating too — that went into #1185, where the other three live.) storage.stat() RESOLVES with null for a missing key rather than rejecting, so counting "the promise settled" marked every deleted — and every never-created — tier as a survivor. A perfectly clean run told the operator to re-run. Now a null means gone, and a rejection counts as stuck, since a storage error is not proof the object went away. Face data is invalidated whenever the stored dimensions change, not only when the change came from rotation: boxes are scaled by photo.width at read time, so any dimension change strands them. And `corrected` now comes from the affected-row count. If the fence rejected the write because the file was replaced mid-run, the photo was not corrected and the run must not claim it was. * fix(images): stop the backfill doing unnecessary work, and make its retry advice true (#1198) Round 3, four points, all narrower than the last two rounds. It re-processed photos that were already correct. A 5-8 rotation changes the dimensions, so a tagged photo whose stored dimensions are ALREADY oriented must have been ingested after #1185 — its renditions are fine and clearing them deletes valid files and rescans a completed face detection for nothing. Those are now skipped and simply marked. Orientations 2, 3 and 4 (and 5-8 on a square image) leave the dimensions identical either way, so they carry no such evidence and are still done once. The retry advice was impossible to follow. When a responsive tier could not be deleted the row was still marked, so the ordinary re-run the UI recommends found nothing and the stale tier kept serving unrotated forever. The marker is withheld when a tier survives, which is what makes that message honest. Storage cleanup now only runs when a fenced write actually landed. If the file was replaced mid-run every update matched zero rows, but the deletion went ahead anyway and could destroy renditions belonging to the REPLACEMENT — watermarks especially, which are keyed by photo id and alias straight onto the new file. And the full-photo ETag includes the backfill's timestamp. It was built from the ORIGINAL's mtime plus the watermark settings hash, neither of which this job touches — so a guest holding a pre-fix ETag would go on getting 304 and their cached sideways image no matter how many times the backfill succeeded. --------- Co-authored-by: Paul Nothaft <[email protected]>
181 lines
7.1 KiB
JavaScript
181 lines
7.1 KiB
JavaScript
/**
|
|
* Shared run state for the photo maintenance sweeps (#1181).
|
|
*
|
|
* These jobs used to keep `{ isRunning, lastResult }` in a module-level
|
|
* variable. That is invisible to every other replica, so on a multi-replica
|
|
* install the status endpoint answers from whichever process the poll happens
|
|
* to reach and a second POST can start a duplicate pass over the whole
|
|
* library. Moving the state into the database makes both the claim and the
|
|
* reporting shared.
|
|
*
|
|
* The claim is a conditional UPDATE whose affected-row count is the answer —
|
|
* the same shape backgroundProcessor uses to hand a photo to exactly one
|
|
* worker (backgroundProcessor.js:110-116). Two replicas issuing it
|
|
* concurrently cannot both match: the row is locked for the duration of each
|
|
* UPDATE, so the loser sees is_running already true and gets 0 rows back.
|
|
*
|
|
* A lease that can be taken over needs fencing, which is what claim_token is
|
|
* for. Taking over a stale claim does not stop the old runner — it is a
|
|
* process nobody can signal, quite possibly still walking the library. So
|
|
* every write it attempts carries the token it was issued:
|
|
*
|
|
* - heartbeat() reports whether the renewal landed. It returns false once
|
|
* the claim has moved on, and the run loops treat that as "stop".
|
|
* - release() only clears the row if the token still matches, so a
|
|
* superseded runner finishing late cannot clear the new owner's flag or
|
|
* overwrite its result.
|
|
*
|
|
* Without both of those, a takeover produces two live runners and the loser
|
|
* ends up stomping the winner's state on its way out.
|
|
*
|
|
* Timestamps are written as ISO strings rather than Date objects. Production
|
|
* stores Dates fine, but inside jest the sqlite3 binding turns them into the
|
|
* literal string "[object Object]" (see CLAUDE.md), which would silently break
|
|
* every staleness comparison in the tests. ISO-8601 also compares correctly
|
|
* under SQLite's lexicographic text ordering, so the `<` below means the same
|
|
* thing on both engines.
|
|
*/
|
|
|
|
const os = require('os');
|
|
const crypto = require('crypto');
|
|
const { db } = require('../database/db');
|
|
const logger = require('../utils/logger');
|
|
|
|
const JOB_DIMENSION_REPAIR = 'photo_dimension_repair';
|
|
const JOB_CAPTURE_DATE_BACKFILL = 'photo_capture_date_backfill';
|
|
const JOB_ORIENTATION_BACKFILL = 'photo_orientation_backfill';
|
|
|
|
// How long a run may go without renewing its lease before another replica is
|
|
// allowed to take it over. Generous on purpose: these jobs walk the whole
|
|
// library and a single slow original on a stalled NAS mount can block the loop
|
|
// for a while. The cost of being too eager is a duplicate pass; the cost of
|
|
// being too patient is a button that stays disabled after a crash.
|
|
const DEFAULT_STALE_MS = 15 * 60 * 1000;
|
|
|
|
// How often a running job renews. Time-based, and comfortably inside the stale
|
|
// window: tying renewal to a photo counter meant a job whose photos were slow
|
|
// — a stalled mount, a handful of very large originals — could be declared
|
|
// abandoned while it was still working.
|
|
const HEARTBEAT_INTERVAL_MS = 60 * 1000;
|
|
|
|
const OWNER = `${os.hostname()}:${process.pid}`;
|
|
|
|
const nowIso = () => new Date().toISOString();
|
|
const cutoffIso = (staleAfterMs) => new Date(Date.now() - staleAfterMs).toISOString();
|
|
|
|
/**
|
|
* Try to become the one runner of `jobName`.
|
|
*
|
|
* Returns a claim token on success, or null when another replica holds it and
|
|
* is still renewing — the caller should answer 409. The token must be passed
|
|
* to every subsequent heartbeat/release for this run.
|
|
*/
|
|
async function claim(jobName, { staleAfterMs = DEFAULT_STALE_MS } = {}) {
|
|
const stamp = nowIso();
|
|
const cutoff = cutoffIso(staleAfterMs);
|
|
const token = crypto.randomBytes(16).toString('hex');
|
|
|
|
const claimed = await db('maintenance_jobs')
|
|
.where({ job_name: jobName })
|
|
.where(function () {
|
|
// Free, or held by a run that has stopped renewing. heartbeat_at is
|
|
// always written by the claim below, so a running job cannot have a null
|
|
// heartbeat — no third case to handle here.
|
|
this.where('is_running', false).orWhere('heartbeat_at', '<', cutoff);
|
|
})
|
|
.update({
|
|
is_running: true,
|
|
started_at: stamp,
|
|
heartbeat_at: stamp,
|
|
finished_at: null,
|
|
owner: OWNER,
|
|
claim_token: token,
|
|
});
|
|
|
|
return claimed > 0 ? token : null;
|
|
}
|
|
|
|
/**
|
|
* Renew the lease.
|
|
*
|
|
* Returns false when this run no longer owns the claim — it was declared stale
|
|
* and taken over. The caller must stop working at that point: the new owner is
|
|
* already walking the same rows, and two runners writing is exactly what the
|
|
* lock exists to prevent.
|
|
*/
|
|
async function heartbeat(jobName, token) {
|
|
try {
|
|
const renewed = await db('maintenance_jobs')
|
|
.where({ job_name: jobName, is_running: true, claim_token: token })
|
|
.update({ heartbeat_at: nowIso() });
|
|
return renewed > 0;
|
|
} catch (err) {
|
|
// A failed renewal query is not proof the claim is gone, and aborting a
|
|
// long sweep over one transient database blip is the worse trade. Say the
|
|
// claim still holds; if it really has moved on, the next renewal says so.
|
|
logger.warn(`maintenanceJobState: heartbeat failed for ${jobName}: ${err.message}`);
|
|
return true;
|
|
}
|
|
}
|
|
|
|
/**
|
|
* Give up the claim.
|
|
*
|
|
* Scoped to the token, so a runner that was superseded while it was working
|
|
* cannot clear the new owner's flag or overwrite its result on the way out.
|
|
* Returns false when the claim had already moved on.
|
|
*
|
|
* `result` is stored as the job's last outcome. Pass null (the "nothing to do"
|
|
* and error paths) to release without overwriting what the previous real run
|
|
* reported.
|
|
*/
|
|
async function release(jobName, token, result = null) {
|
|
const update = { is_running: false, finished_at: nowIso() };
|
|
if (result !== null && result !== undefined) {
|
|
update.last_result = JSON.stringify(result);
|
|
}
|
|
const released = await db('maintenance_jobs')
|
|
.where({ job_name: jobName, claim_token: token })
|
|
.update(update);
|
|
return released > 0;
|
|
}
|
|
|
|
/**
|
|
* Current state, in the shape the status endpoints hand to the frontend.
|
|
*
|
|
* A run whose lease has gone stale is reported as not running: the owning
|
|
* replica is gone, nothing is going to release the claim, and the operator
|
|
* needs the button back. The next claim() takes the row over on the same
|
|
* condition, so the two agree.
|
|
*/
|
|
async function read(jobName, { staleAfterMs = DEFAULT_STALE_MS } = {}) {
|
|
const row = await db('maintenance_jobs').where({ job_name: jobName }).first();
|
|
if (!row) return { isRunning: false, lastResult: null };
|
|
|
|
const alive = row.heartbeat_at && new Date(row.heartbeat_at).getTime() > Date.now() - staleAfterMs;
|
|
|
|
let lastResult = null;
|
|
if (row.last_result) {
|
|
try {
|
|
lastResult = JSON.parse(row.last_result);
|
|
} catch (err) {
|
|
// Never let a malformed row take the status endpoint down with it.
|
|
logger.warn(`maintenanceJobState: unreadable last_result for ${jobName}: ${err.message}`);
|
|
}
|
|
}
|
|
|
|
return { isRunning: Boolean(row.is_running) && Boolean(alive), lastResult };
|
|
}
|
|
|
|
module.exports = {
|
|
claim,
|
|
heartbeat,
|
|
release,
|
|
read,
|
|
JOB_DIMENSION_REPAIR,
|
|
JOB_CAPTURE_DATE_BACKFILL,
|
|
JOB_ORIENTATION_BACKFILL,
|
|
DEFAULT_STALE_MS,
|
|
HEARTBEAT_INTERVAL_MS,
|
|
};
|