fix: produce worker claim — SECURITY DEFINER fn fixes 08P01 poll crash + stale-lease reclaim (040)
The store's hand-built claim interpolated the lane predicate into the SQL text while still passing a bind parameter — node-postgres sent 1 param for a 0-param statement (08P01) on every poll, so the live produce worker store-errored every ~2s and never claimed. Migration 040 replaces the inline claim with produce_job_claim(p_lane, p_stale_after): SECURITY DEFINER (the 035/oidc_refresh_lookup pattern — the poller is lane-global, FORCE RLS hides every row without a tenant context), atomic FOR UPDATE SKIP LOCKED with the queued→transcoding bump as the lease, and stale- transcoding reclaim so a killed worker's rows return to the queue. produce_job_workspace(p_job_id) discovers the owning tenant for the follow-up writes, which stay RLS-bound as sp_app. pg-store rewired; store errors now surface err.message instead of err.name; pg-store tests added. 38/38 producer tests green.
This commit is contained in:
parent
d9e8cdf789
commit
ee8d5c6e76
5 changed files with 512 additions and 91 deletions
|
|
@ -1,16 +1,16 @@
|
|||
/**
|
||||
* Postgres produce job store: the poll/claim/complete half of the produce worker.
|
||||
*
|
||||
* - Claim: `UPDATE ... SET state='transcoding' WHERE id IN (SELECT id ... FOR UPDATE
|
||||
* SKIP LOCKED) RETURNING` — one row, one statement, lane-filtered, so concurrent
|
||||
* workers never race and a claimed row is invisible to everyone else until its
|
||||
* owner finishes it. The `queued → transcoding` bump inside SKIP LOCKED is the
|
||||
* lease: there is no separate lease column to renew, because a claimed row is
|
||||
* owned until its owner writes complete/failed.
|
||||
* - Every write happens inside the job's tenant transaction (`app.workspace_id`),
|
||||
* because both tables are FORCE ROW LEVEL SECURITY with a `tenant_isolation`
|
||||
* policy (009). The claim reads the workspace off the row first, then re-claims
|
||||
* inside that tenant's scope.
|
||||
* - Claim: `SELECT * FROM produce_job_claim(lane, stale_after)` — a SECURITY
|
||||
* DEFINER function (040, the 035 submission_box_lookup / 017 invite_link_for_claim
|
||||
* pattern) because the worker polls with NO tenant context while
|
||||
* video_processing_jobs is FORCE ROW LEVEL SECURITY (009 tenant_isolation). The
|
||||
* definer claim is the one statement that may cross tenants: it bumps
|
||||
* `queued → transcoding` inside FOR UPDATE SKIP LOCKED as the lease (no separate
|
||||
* lease column) and requeues rows whose lease has gone stale, then returns the
|
||||
* claimed row WITH its workspace_id.
|
||||
* - Every write after the claim happens inside the job's tenant transaction
|
||||
* (`SET LOCAL app.workspace_id`), so those statements stay RLS-bound as sp_app.
|
||||
* - A row whose `input_json` or source asset fails re-validation is FAILED by THE
|
||||
* CLAIM ITSELF (never thrown out of `claim`), so an undecodable row can never sit
|
||||
* in `transcoding` forever holding its lease.
|
||||
|
|
@ -23,19 +23,20 @@ import type { ClaimedProduceJob, ProduceJobStore, ProduceOutput, ProduceSourceAs
|
|||
const KINDS: readonly ProduceJobKind[] = ["compose", "mix"];
|
||||
|
||||
/**
|
||||
* Lane → which rows this worker may claim (039 lanes: default worker = local or NULL,
|
||||
* i.e. anything but an explicit `lane:'remote'`; a remote worker takes only remote).
|
||||
* A claimed `transcoding` row older than this is assumed ownerless (the worker
|
||||
* died mid-encode) and is requeued by the claim itself. Long enough that no real
|
||||
* encode loses its lease to the sweep: the runner reports progress at every stage
|
||||
* (claimed/downloaded/encoded/published) and each write bumps `updated_at`, so a
|
||||
* live lease is touched far more often than this window.
|
||||
*/
|
||||
export function lanePredicate(lane: Lane): { laneSql: string; params: [boolean] } {
|
||||
// $1 binds the boolean flag: a local/default worker skips remote rows, a remote
|
||||
// worker claims ONLY remote rows. The predicate never matches a local row to a
|
||||
// remote worker, so nothing is locked and then released.
|
||||
return {
|
||||
laneSql: lane === "remote"
|
||||
? "(j.input_json->>'lane' = 'remote')"
|
||||
: "(j.input_json->>'lane' IS DISTINCT FROM 'remote')",
|
||||
params: [lane === "remote"],
|
||||
};
|
||||
export const PRODUCE_LEASE_STALE_AFTER = "45 minutes";
|
||||
|
||||
interface ClaimRow {
|
||||
id: string;
|
||||
workspace_id: string;
|
||||
kind: string;
|
||||
input_json: unknown;
|
||||
video_asset_id: string;
|
||||
}
|
||||
|
||||
function uuid(value: unknown, field: string): string {
|
||||
|
|
@ -76,6 +77,8 @@ export class PostgresProduceJobStore implements ProduceJobStore {
|
|||
const client = await this.pool.connect();
|
||||
try {
|
||||
await client.query("BEGIN");
|
||||
// SET LOCAL: the tenant context dies with this transaction, so a pooled
|
||||
// client can never leak one workspace's scope into the next borrower.
|
||||
await client.query("SELECT set_config('app.workspace_id', $1, true)", [workspaceId]);
|
||||
const result = await fn(client);
|
||||
await client.query("COMMIT");
|
||||
|
|
@ -89,69 +92,56 @@ export class PostgresProduceJobStore implements ProduceJobStore {
|
|||
}
|
||||
|
||||
async claim(lane: Lane): Promise<ClaimedProduceJob | null> {
|
||||
// Lane filter in the sub-select, so a worker never locks a row it would have to
|
||||
// release: a remote worker only ever locks remote rows, the default worker only
|
||||
// ever locks non-remote rows.
|
||||
const { laneSql, params } = lanePredicate(lane);
|
||||
const claimed = await this.pool.query<{ id: string; workspace_id: string }>(
|
||||
`UPDATE video_processing_jobs j
|
||||
SET state = 'transcoding', progress_percent = 5, error = NULL, updated_at = now()
|
||||
WHERE j.id IN (
|
||||
SELECT id FROM video_processing_jobs
|
||||
WHERE kind IN ('compose', 'mix')
|
||||
AND state = 'queued'
|
||||
AND ${laneSql}
|
||||
ORDER BY created_at, id
|
||||
FOR UPDATE SKIP LOCKED
|
||||
LIMIT 1
|
||||
)
|
||||
RETURNING j.id, j.workspace_id`,
|
||||
params,
|
||||
// Lane and the stale-lease window are typed bind parameters of the definer
|
||||
// function (040): never interpolated into the SQL text, so the parameter list
|
||||
// and the statement can no longer disagree (the 08P01 bind-message bug).
|
||||
const claimed = await this.pool.query<ClaimRow>(
|
||||
"SELECT id, workspace_id, kind, input_json, video_asset_id FROM produce_job_claim($1, $2::interval)",
|
||||
[lane, PRODUCE_LEASE_STALE_AFTER],
|
||||
);
|
||||
if (claimed.rowCount === 0) return null;
|
||||
const row = claimed.rows[0]!;
|
||||
const workspaceId = uuid(row.workspace_id, "workspace id");
|
||||
const kind = row.kind === "mix" ? "mix" : row.kind === "compose" ? "compose" : null;
|
||||
let request = null;
|
||||
if (kind !== null) {
|
||||
try {
|
||||
request = parseProduceRequest(row.input_json, kind);
|
||||
} catch {
|
||||
request = null;
|
||||
}
|
||||
}
|
||||
if (kind === null || request === null) {
|
||||
// A row the worker cannot honour must FAIL here — never strand in `transcoding`.
|
||||
// A null return (not a throw) commits the failure instead of rolling it back.
|
||||
await this.tenant(workspaceId, (client) =>
|
||||
client.query(
|
||||
`UPDATE video_processing_jobs
|
||||
SET state = 'failed', progress_percent = 100, error = $2, updated_at = now()
|
||||
WHERE id = $1 AND state = 'transcoding'`,
|
||||
[row.id, kind === null ? "produce job store: unknown kind" : "produce invalid request"],
|
||||
));
|
||||
return null;
|
||||
}
|
||||
|
||||
return this.tenant<ClaimedProduceJob | null>(workspaceId, async (client) => {
|
||||
// Re-claim inside the tenant's RLS scope (the pool-level row above is outside
|
||||
// it; claiming again is the cheap way to stay correct under FORCE RLS).
|
||||
const mine = await client.query(
|
||||
`UPDATE video_processing_jobs
|
||||
SET state = 'transcoding', progress_percent = 5, error = NULL, updated_at = now()
|
||||
WHERE id = $1 AND state IN ('queued', 'transcoding')
|
||||
RETURNING id, kind, input_json, video_asset_id`,
|
||||
[row.id],
|
||||
);
|
||||
if (mine.rowCount === 0) return null;
|
||||
const job = mine.rows[0]! as Record<string, unknown>;
|
||||
const kind = job.kind === "mix" ? "mix" : job.kind === "compose" ? "compose" : null;
|
||||
let request = null;
|
||||
if (kind !== null) {
|
||||
try {
|
||||
request = parseProduceRequest(job.input_json, kind);
|
||||
} catch {
|
||||
request = null;
|
||||
}
|
||||
}
|
||||
if (kind === null || request === null) {
|
||||
// A row the worker cannot honour must FAIL here — never strand in `transcoding`.
|
||||
// Written inside this transaction and COMMITTED by returning null (not thrown:
|
||||
// a throw would roll the failure back and re-strand the row).
|
||||
await this.failInvalid(client, job, kind === null ? "produce job store: unknown kind" : "produce invalid request");
|
||||
return null;
|
||||
}
|
||||
|
||||
const assets = await client.query(
|
||||
`SELECT id, storage_key, mime_type, size_bytes, width, height, duration_ms, frame_rate, audio_codec
|
||||
FROM video_assets WHERE id = $1`,
|
||||
[job.video_asset_id],
|
||||
[row.video_asset_id],
|
||||
);
|
||||
if (assets.rowCount === 0) {
|
||||
await this.failInvalid(client, job, "produce asset unavailable");
|
||||
// Same rule as an undecodable payload: fail the row, release the lease.
|
||||
await client.query(
|
||||
`UPDATE video_processing_jobs
|
||||
SET state = 'failed', progress_percent = 100, error = 'produce asset unavailable', updated_at = now()
|
||||
WHERE id = $1 AND state = 'transcoding'`,
|
||||
[row.id],
|
||||
);
|
||||
return null;
|
||||
}
|
||||
return {
|
||||
jobId: uuid(job.id, "job id"),
|
||||
jobId: uuid(row.id, "job id"),
|
||||
workspaceId,
|
||||
kind,
|
||||
request,
|
||||
|
|
@ -161,20 +151,6 @@ export class PostgresProduceJobStore implements ProduceJobStore {
|
|||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Fail a just-claimed row whose payload cannot be honoured, inside the claim's
|
||||
* own transaction so the failure commits with it. Callers then return `null`:
|
||||
* the row is done (failed), there is simply nothing for this tick to encode.
|
||||
*/
|
||||
private async failInvalid(client: PoolClient, job: Record<string, unknown>, reason: string): Promise<void> {
|
||||
await client.query(
|
||||
`UPDATE video_processing_jobs
|
||||
SET state = 'failed', progress_percent = 100, error = $2, updated_at = now()
|
||||
WHERE id = $1 AND state = 'transcoding'`,
|
||||
[job.id, reason],
|
||||
);
|
||||
}
|
||||
|
||||
async progress(jobId: string, percent: number): Promise<void> {
|
||||
await this.tenant(await this.workspaceOf(jobId), async (client) => {
|
||||
await client.query(
|
||||
|
|
@ -231,14 +207,21 @@ export class PostgresProduceJobStore implements ProduceJobStore {
|
|||
});
|
||||
}
|
||||
|
||||
/** A claimed job's workspace, for the tenant-scoped follow-up writes. */
|
||||
/**
|
||||
* A claimed job's workspace, for the tenant-scoped follow-up writes: the 040
|
||||
* definer lookup (012's live_monitor_source_workspace shape), because an
|
||||
* ordinary SELECT as sp_app with no tenant context sees no rows at all under
|
||||
* FORCE RLS. `job not found` here is exactly what it means: the row is gone
|
||||
* (tenant-deleted) or the lease was reclaimed and the job finished elsewhere.
|
||||
*/
|
||||
private async workspaceOf(jobId: string): Promise<string> {
|
||||
const rows = await this.pool.query<{ workspace_id: string }>(
|
||||
`SELECT workspace_id FROM video_processing_jobs WHERE id = $1`,
|
||||
"SELECT produce_job_workspace($1) AS workspace_id",
|
||||
[jobId],
|
||||
);
|
||||
if (rows.rowCount === 0) throw new Error(`produce job store: job ${jobId} not found`);
|
||||
return uuid(rows.rows[0]!.workspace_id, "workspace id");
|
||||
const workspaceId = rows.rows[0]?.workspace_id;
|
||||
if (workspaceId === undefined || workspaceId === null) throw new Error(`produce job store: job ${jobId} not found`);
|
||||
return uuid(workspaceId, "workspace id");
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue