Compare commits

..

2 commits

Author SHA1 Message Date
dlawler489
099e6d24b4 Merge the tagging deadlock fix
All checks were successful
build-and-push / docker (push) Successful in 22s
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-16 14:56:28 +10:00
dlawler489
8a2ad5785c Stop tagging deadlocking, and retry a transaction that loses one
Two transactions tagging overlapping models could each hold a tag row the other
wanted. tagId used ON CONFLICT DO UPDATE, which rewrites the row and therefore
write-locks it even though the name never changes, and each caller iterated in
whatever order it happened to have. That is what killed "Mushroom Clicker"
during AI run 21, while the collection watcher was importing and rule-tagging
the same tags.

tagId no longer rewrites unchanged rows, and applyAiResult, replaceRuleTags and
setTags all tag in sorted order, so locks are taken in the same sequence every
time. db.transaction retries a deadlock victim up to three times as well, which
covers the paths this doesn't — the victim rolled back and changed nothing, so
running it again is safe.

Measured: the old pattern deadlocks on demand (40P01), and the same collision
run 48 times through the new code fails zero times.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-16 14:56:28 +10:00
2 changed files with 34 additions and 14 deletions

View file

@ -21,7 +21,12 @@ function query(text, params) {
}
// Run a function inside a transaction, passing it a dedicated client.
async function transaction(fn) {
// Postgres breaks a deadlock by killing one of the transactions. The victim
// rolled back and changed nothing, so running it again is safe — and usually
// works, because the transaction it collided with has since finished.
const RETRY_CODES = new Set(['40P01', '40001']); // deadlock_detected, serialization_failure
async function transaction(fn, attempt = 0) {
const client = await pool.connect();
try {
await client.query('BEGIN');
@ -29,11 +34,14 @@ async function transaction(fn) {
await client.query('COMMIT');
return result;
} catch (err) {
await client.query('ROLLBACK');
throw err;
await client.query('ROLLBACK').catch(() => {}); // the connection may be gone
if (!RETRY_CODES.has(err.code) || attempt >= 3) throw err;
console.warn(`Database ${err.code}, retrying the transaction (attempt ${attempt + 2}).`);
} finally {
client.release();
client.release(); // before the pause below, so a retry doesn't hold a connection
}
await new Promise((r) => setTimeout(r, 50 * (attempt + 1) ** 2));
return transaction(fn, attempt + 1);
}
module.exports = { pool, query, transaction, init };

View file

@ -301,11 +301,25 @@ async function insertFile(client, modelId, file) {
}
// ---------------------------------------------------------------- tags
// Always tag in the same order. Two transactions touching the same tags in
// opposite orders each hold a row the other wants, which Postgres resolves by
// killing one of them ("deadlock detected").
const tagOrder = (names) => [...new Set((names || []).map((n) => String(n).trim()).filter(Boolean))].sort();
// The tag's id, creating it if it's new. Deliberately not ON CONFLICT DO UPDATE:
// that rewrites the row and so write-locks it for the rest of the transaction,
// even though the name never changes — which is what made tagging deadlock.
async function tagId(client, name) {
const { rows } = await client.query(
`INSERT INTO tags (name) VALUES ($1)
ON CONFLICT (name) DO UPDATE SET name = EXCLUDED.name RETURNING id`, [name]);
return rows[0].id;
for (let attempt = 0; attempt < 3; attempt++) {
const { rows: found } = await client.query('SELECT id FROM tags WHERE name = $1', [name]);
if (found.length) return found[0].id;
const { rows } = await client.query(
'INSERT INTO tags (name) VALUES ($1) ON CONFLICT (name) DO NOTHING RETURNING id', [name]);
if (rows.length) return rows[0].id;
// Someone else is inserting this very tag and hasn't committed yet.
await new Promise((r) => setTimeout(r, 20 * (attempt + 1)));
}
throw new Error(`Could not get an id for the tag “${name}”`);
}
// Sets a model's tags from an edit. A tag that stays keeps where it came from
@ -314,9 +328,7 @@ async function setTags(client, modelId, tagNames) {
const { rows: before } = await client.query('SELECT tag_id, source FROM model_tags WHERE model_id = $1', [modelId]);
const sourceOf = new Map(before.map((r) => [r.tag_id, r.source]));
await client.query('DELETE FROM model_tags WHERE model_id = $1', [modelId]);
for (const raw of tagNames) {
const name = String(raw).trim();
if (!name) continue;
for (const name of tagOrder(tagNames)) {
const id = await tagId(client, name);
await client.query(
'INSERT INTO model_tags (model_id, tag_id, source) VALUES ($1,$2,$3) ON CONFLICT DO NOTHING',
@ -329,7 +341,7 @@ async function setTags(client, modelId, tagNames) {
async function replaceRuleTags(modelId, tagNames) {
await db.transaction(async (client) => {
const ids = [];
for (const name of tagNames) ids.push(await tagId(client, name));
for (const name of tagOrder(tagNames)) ids.push(await tagId(client, name));
await client.query(
`DELETE FROM model_tags WHERE model_id = $1 AND source = 'rule' AND NOT (tag_id = ANY($2::int[]))`, [modelId, ids]);
for (const id of ids) {
@ -350,8 +362,8 @@ async function removeTagsFrom(source) {
// markTagged records that the model has had AI tags and a description.
async function applyAiResult(modelId, { tags, description, overwriteDescription, name = null, markTagged = true }) {
await db.transaction(async (client) => {
for (const name of tags) {
const id = await tagId(client, name);
for (const tagName of tagOrder(tags)) { // not `name`: that's this function's rename parameter
const id = await tagId(client, tagName);
await client.query(`INSERT INTO model_tags (model_id, tag_id, source) VALUES ($1,$2,'ai') ON CONFLICT DO NOTHING`,
[modelId, id]);
}