From ec371ac40007f1db8ea3f449e056a41503c884d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ullrich=20Sch=C3=A4fer?= Date: Sat, 23 May 2026 20:49:09 +0200 Subject: [PATCH] Simplify: typed status, skip-write guard, remove redundant comment - Use ImportBatchStatus type instead of raw string literals in sweep job - Add a COUNT pre-check so the sweep UPDATE is skipped when no stale batches exist (avoids an unconditional write every minute) - Remove comment in disconnect route that explained what the code does Co-Authored-By: Claude Sonnet 4.6 --- apps/journal/app/jobs/import-batches-sweep.ts | 30 +++++++++++-------- .../routes/api.sync.disconnect.$provider.ts | 2 -- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/apps/journal/app/jobs/import-batches-sweep.ts b/apps/journal/app/jobs/import-batches-sweep.ts index c94c63e..2cb405d 100644 --- a/apps/journal/app/jobs/import-batches-sweep.ts +++ b/apps/journal/app/jobs/import-batches-sweep.ts @@ -1,11 +1,11 @@ import type { JobDefinition } from "@trails-cool/jobs"; -import { and, lt, inArray } from "drizzle-orm"; +import { and, lt, inArray, count } from "drizzle-orm"; import { getDb } from "../lib/db.ts"; -import { importBatches } from "@trails-cool/db/schema/journal"; +import { importBatches, type ImportBatchStatus } from "@trails-cool/db/schema/journal"; import { logger } from "../lib/logger.server.ts"; -// Batches stuck longer than this without completing are considered stale. const STALE_MS = 10 * 60 * 1000; +const STALE_STATUSES: ImportBatchStatus[] = ["pending", "running"]; export const importBatchesSweepJob: JobDefinition = { name: "import-batches-sweep", @@ -15,24 +15,28 @@ export const importBatchesSweepJob: JobDefinition = { async handler() { const db = getDb(); const cutoff = new Date(Date.now() - STALE_MS); + const staleFilter = and( + inArray(importBatches.status, STALE_STATUSES), + lt(importBatches.startedAt, cutoff), + ); + + // Skip the write when nothing is stale to avoid an unconditional UPDATE every minute. + const rows = await db + .select({ staleCount: count() }) + .from(importBatches) + .where(staleFilter); + if ((rows[0]?.staleCount ?? 0) === 0) return; const result = await db .update(importBatches) .set({ - status: "failed", + status: "failed" satisfies ImportBatchStatus, errorMessage: "Import timed out — the server may have restarted mid-import. Click 'Run again' to retry.", completedAt: new Date(), }) - .where( - and( - inArray(importBatches.status, ["pending", "running"]), - lt(importBatches.startedAt, cutoff), - ), - ) + .where(staleFilter) .returning({ id: importBatches.id }); - if (result.length > 0) { - logger.info({ count: result.length }, "import-batches-sweep: marked stale batches as failed"); - } + logger.info({ count: result.length }, "import-batches-sweep: marked stale batches as failed"); }, }; diff --git a/apps/journal/app/routes/api.sync.disconnect.$provider.ts b/apps/journal/app/routes/api.sync.disconnect.$provider.ts index 47d05e8..6bc31a1 100644 --- a/apps/journal/app/routes/api.sync.disconnect.$provider.ts +++ b/apps/journal/app/routes/api.sync.disconnect.$provider.ts @@ -17,8 +17,6 @@ export async function action({ params, request }: Route.ActionArgs) { const referer = request.headers.get("referer"); const url = referer ? new URL(referer) : null; - // If coming from a provider-specific settings page, go up to /settings/connections. - // Otherwise return to wherever the user was within /settings. let back = "/settings/connections"; if (url?.pathname.startsWith("/settings") && !url.pathname.startsWith("/settings/connections/")) { back = url.pathname;