Nemcio/NodeBB
1
0
Fork 0
mirror of https://github.com/NodeBB/NodeBB.git synced 2026-07-06 23:59:46 +02:00
Code Issues Packages Projects Releases Wiki Activity

Fix the saving of the statistics on PosgreSQL #14124 (#14129)

Browse Source 
* fix: deduplicate postgres sorted set bulk ops to prevent pkey violation

sortedSetIncrByBulk and sortedSetAddBulk did not deduplicate (key, value)
pairs before INSERT, causing "duplicate key value violates unique constraint
legacy_zset_pkey" errors since PostgreSQL ON CONFLICT only resolves against
existing table rows, not within-statement duplicates.

Also adds missing pageviews:ap metrics to analyticsKeys sorted set.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: use upsert with RETURNING to prevent postgres analytics write failures

Replace the INSERT ON CONFLICT DO NOTHING + separate SELECT verification
pattern with INSERT ON CONFLICT DO UPDATE RETURNING. The old pattern had
an unreliable gap between INSERT and SELECT causing random "failed to
insert keys for objects" errors that blocked all analytics writes.

The no-op upsert (DO UPDATE SET type = existing type) guarantees every
row is returned via RETURNING, eliminating the need for a separate SELECT
and the "missing keys" check entirely. Also deduplicates the keys array
to prevent "cannot affect row a second time" errors with DO UPDATE.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Michele Di Maria
2026-03-28 18:24:34 +01:00
committed by GitHub
parent 6c4e928482
commit b8fd88fba9
4 changed files with 38 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
src/analytics.js
View File

@@ -189,6 +189,12 @@ Analytics.writeData = async function () {
incrByBulk.push(['analytics:pageviews:ap', total.apPageViews, today.getTime()]);
incrByBulk.push(['analytics:pageviews:ap:month', total.apPageViews, month.getTime()]);
total.apPageViews = 0;
if (!metrics.includes('pageviews:ap')) {
metrics.push('pageviews:ap');
}
if (!metrics.includes('pageviews:ap:month')) {
metrics.push('pageviews:ap:month');
}
}
if (total.uniquevisitors > 0) {

44
src/database/postgres/helpers.js
View File

@@ -27,31 +27,25 @@ DELETE FROM "legacy_object"
AND "expireAt" <= CURRENT_TIMESTAMP`,
});
await db.query({
name: 'ensureLegacyObjectType1',
const res = await db.query({
name: 'ensureLegacyObjectType_upsert',
text: `
INSERT INTO "legacy_object" ("_key", "type")
VALUES ($1::TEXT, $2::TEXT::LEGACY_OBJECT_TYPE)
ON CONFLICT
DO NOTHING`,
ON CONFLICT ("_key")
DO UPDATE SET "type" = "legacy_object"."type"
RETURNING "type"`,
values: [key, type],
});
const res = await db.query({
name: 'ensureLegacyObjectType2',
text: `
SELECT "type"
FROM "legacy_object_live"
WHERE "_key" = $1::TEXT`,
values: [key],
});
if (res.rows[0].type !== type) {
throw new Error(`database: cannot insert ${JSON.stringify(key)} as ${type} because it already exists as ${res.rows[0].type}`);
}
};
helpers.ensureLegacyObjectsType = async function (db, keys, type) {
keys = [...new Set(keys)];
await db.query({
name: 'ensureLegacyObjectTypeBefore',
text: `
@@ -60,38 +54,24 @@ DELETE FROM "legacy_object"
AND "expireAt" <= CURRENT_TIMESTAMP`,
});
await db.query({
name: 'ensureLegacyObjectsType1',
const res = await db.query({
name: 'ensureLegacyObjectsType_upsert',
text: `
INSERT INTO "legacy_object" ("_key", "type")
SELECT k, $2::TEXT::LEGACY_OBJECT_TYPE
FROM UNNEST($1::TEXT[]) k
ON CONFLICT
DO NOTHING`,
ON CONFLICT ("_key")
DO UPDATE SET "type" = "legacy_object"."type"
RETURNING "_key", "type"`,
values: [keys, type],
});
const res = await db.query({
name: 'ensureLegacyObjectsType2',
text: `
SELECT "_key", "type"
FROM "legacy_object_live"
WHERE "_key" = ANY($1::TEXT[])`,
values: [keys],
});
const invalid = res.rows.filter(r => r.type !== type);
if (invalid.length) {
const parts = invalid.map(r => `${JSON.stringify(r._key)} is ${r.type}`);
throw new Error(`database: cannot insert multiple objects as ${type} because they already exist: ${parts.join(', ')}`);
}
const missing = keys.filter(k => !res.rows.some(r => r._key === k));
if (missing.length) {
throw new Error(`database: failed to insert keys for objects: ${JSON.stringify(missing)}`);
}
};
helpers.noop = function () {};

21
src/database/postgres/sorted.js
View File

@@ -551,16 +551,29 @@ RETURNING "score" s`,
return [];
}
// Deduplicate by (key, value) pair, summing increments for duplicates
const seen = new Map();
const deduped = [];
data.forEach(([key, increment, value]) => {
value = helpers.valueToString(value);
increment = parseFloat(increment);
const mapKey = `${key}\0${value}`;
if (seen.has(mapKey)) {
deduped[seen.get(mapKey)][1] += increment;
} else {
seen.set(mapKey, deduped.length);
deduped.push([key, increment, value]);
}
});
return await module.transaction(async (client) => {
await helpers.ensureLegacyObjectsType(client, data.map(item => item[0]), 'zset');
await helpers.ensureLegacyObjectsType(client, deduped.map(item => item[0]), 'zset');
const values = [];
const queryParams = [];
let paramIndex = 1;
data.forEach(([key, increment, value]) => {
value = helpers.valueToString(value);
increment = parseFloat(increment);
deduped.forEach(([key, increment, value]) => {
values.push(key, value, increment);
queryParams.push(`($${paramIndex}::TEXT, $${paramIndex + 1}::TEXT, $${paramIndex + 2}::NUMERIC)`);
paramIndex += 3;

4
src/database/postgres/sorted/add.js
View File

@@ -114,8 +114,10 @@ INSERT INTO "legacy_zset" ("_key", "value", "score")
}
keys.push(item[0]);
scores.push(item[1]);
values.push(item[2]);
values.push(helpers.valueToString(item[2]));
});
const compositeKeys = keys.map((k, i) => `${k}\0${values[i]}`);
helpers.removeDuplicateValues(compositeKeys, keys, values, scores);
await module.transaction(async (client) => {
await helpers.ensureLegacyObjectsType(client, keys, 'zset');
await client.query({