From dcd8cd317c91fd42cb0718716c3536bce30d3bbf Mon Sep 17 00:00:00 2001 From: boomzero Date: Mon, 27 Jul 2026 14:20:26 +0800 Subject: [PATCH 1/3] Stop the std_list cache drifting from the database The KV cache kept losing problems that exist in std_answer. Three separate defects combined to make it look intermittent. The KV write on the upload path was a floating promise. The D1 insert was awaited and committed, `this.kv.put` was not, and the handler returned immediately after. The fetch handler receives an execution context but never uses it - waitUntil appears nowhere outside the cron - so a pending KV write had nothing extending its lifetime past the response and was not guaranteed to complete. Every upload also did get, string-concat, put of the entire list. Two uploads landing close together both read the same base list, each appended only its own id, and the second put overwrote the first. KV has no compare-and-set to prevent it, and reads are eventually consistent, so even non-concurrent uploads could read a stale list and clobber it. The branch meant to backfill a missing entry was dead code. `split('\n')` yields strings and CheckParams guarantees ProblemID is a number, so `d === Data["ProblemID"]` was strict equality across types - always false, for every input. The condition was also inverted: it appended when the id was already present and did nothing when it was missing. Two bugs cancelling into a no-op, which is why nothing ever healed. So writes dropped entries and nothing put them back. Drift was one-directional and permanent. Rebuild the cache wholesale from the database instead of patching it. GetStdList stays a pure KV read costing no database rows, since that is why the cache exists. Uploads are bounded at one per problem ever, so the insert path can afford a rebuild. The already-uploaded path stays cheap: it rebuilds only when the cached list is genuinely missing the problem. A daily rebuild in the existing cron bounds any remaining drift to 24 hours rather than forever, which is the part that does not depend on every write path staying correct. Also fixes GetStdList returning a spurious trailing 0 from the trailing newline, and throwing outright if the key were ever unset. Co-Authored-By: Claude Opus 5 --- Source/Process.ts | 49 +++++++++++--- Source/index.ts | 8 ++- test/process.test.js | 156 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 201 insertions(+), 12 deletions(-) diff --git a/Source/Process.ts b/Source/Process.ts index ebc8f2e..8929369 100644 --- a/Source/Process.ts +++ b/Source/Process.ts @@ -45,6 +45,34 @@ function sleep(time: number) { return new Promise((resolve) => setTimeout(resolve, time)); } +// The KV key holding the list of problems that have a std answer. It is a +// cache of `SELECT problem_id FROM std_answer`, kept so that GetStdList - a +// hot read - costs no database rows. +export const StdListKey = "std_list"; + +// Tolerates the legacy trailing-newline format, a missing key, and any blank +// lines left behind by earlier writes. +export const ParseStdList = (Cached: string | null | undefined): Array => { + if (!Cached) { + return []; + } + return Cached.split("\n") + .filter((Entry) => Entry.trim() !== "") + .map(Number); +}; + +// Rewrites the cache from the database. Rebuilding wholesale rather than +// patching an entry in means a dropped or racing write can only ever cost +// freshness until the next rebuild, never permanent drift. +export const RebuildStdList = async (XMOJDatabase: Database, kv: KVNamespace): Promise => { + const Rows = ThrowErrorIfFailed( + await XMOJDatabase.Select("std_answer", ["problem_id"]) + ) as Array>; + const List = Rows.map((Row) => Row["problem_id"]).join("\n"); + await kv.put(StdListKey, List); + return List; +}; + export class Process { private AdminUserList: Array = ["chenlangning", "shanwenxiao", "zhuchenrui2","liushangchen"]; // noinspection JSMismatchedCollectionQueryUpdate @@ -1192,13 +1220,12 @@ export class Process { if (ThrowErrorIfFailed(await this.XMOJDatabase.GetTableSize("std_answer", { problem_id: ProblemID }))["TableSize"] !== 0) { - let currentStdList = await this.kv.get("std_list"); - console.log(currentStdList.toString().indexOf(Data["ProblemID"].toString())); - if (currentStdList.split('\n').some(d => d === Data["ProblemID"])) { - currentStdList = currentStdList + Data["ProblemID"] + "\n"; - this.kv.put("std_list", currentStdList); + // This is the hot path - the script calls UploadStd for problems that + // already have a std. Only touch the database when the cache is + // actually missing this problem, which is the drift we are repairing. + if (!ParseStdList(await this.kv.get(StdListKey)).includes(ProblemID)) { + await RebuildStdList(this.XMOJDatabase, this.kv); } - console.log("ProblemID: " + ProblemID + " already has a std answer, skipping upload."); return new Result(true, "此题已经有人上传标程"); } if (await this.GetProblemScoreChecker(ProblemID) !== 100) { @@ -1289,9 +1316,11 @@ export class Process { problem_id: Data["ProblemID"], std_code: StdCode })); - let currentStdList = await this.kv.get("std_list"); - currentStdList = currentStdList + Data["ProblemID"] + "\n"; - this.kv.put("std_list", currentStdList); + // Rebuild from the database rather than appending to the cached string: + // an append races with concurrent uploads (KV has no compare-and-set) and + // silently loses entries. Uploads are bounded at one per problem ever, so + // the extra scan is affordable here in a way it would not be on reads. + await RebuildStdList(this.XMOJDatabase, this.kv); return new Result(true, "标程上传成功"); }, GetStdList: async (Data: object): Promise => { @@ -1299,7 +1328,7 @@ export class Process { const ResponseData = { StdList: new Array() }; - ResponseData.StdList = (await this.kv.get("std_list")).split("\n").map(Number); + ResponseData.StdList = ParseStdList(await this.kv.get(StdListKey)); return new Result(true, "获得标程列表成功", ResponseData); }, GetStd: async (Data: object): Promise => { diff --git a/Source/index.ts b/Source/index.ts index 9df6af4..0eb486b 100644 --- a/Source/index.ts +++ b/Source/index.ts @@ -15,7 +15,7 @@ * along with XMOJ-bbs. If not, see . */ -import {Process} from "./Process"; +import {Process, RebuildStdList} from "./Process"; import {Database} from "./Database"; import {NotificationManager} from "./NotificationManager"; import type {D1Database, KVNamespace, AnalyticsEngineDataset, DurableObjectNamespace, Ai} from "@cloudflare/workers-types"; @@ -152,7 +152,7 @@ export default { let Processor = new Process(RequestData, Environment); return addCorsHeaders(await Processor.Process(), origin); }, - async scheduled(Event: any, Environment: { DB: D1Database; }, Context: { + async scheduled(Event: any, Environment: { DB: D1Database; kv: KVNamespace; }, Context: { waitUntil: (arg0: Promise) => void; }) { let XMOJDatabase = new Database(Environment.DB); @@ -173,6 +173,10 @@ export default { "Value": new Date().getTime() - 1000 * 60 * 60 * 24 * 5 } }); + // Reconcile the std list cache against the database. One scan per day + // bounds any drift - from a dropped KV write or two uploads racing - to + // 24 hours, instead of it persisting forever as it does today. + await RebuildStdList(XMOJDatabase, Environment.kv); Resolve(); })); }, diff --git a/test/process.test.js b/test/process.test.js index c4785b2..c2cb558 100644 --- a/test/process.test.js +++ b/test/process.test.js @@ -664,3 +664,159 @@ test('GetPost clears mentions for the reader', async () => { assert.deepStrictEqual(deleted, [{ table: 'bbs_mention', where: { post_id: 1, to_user_id: 'testuser' } }]); }); + +// --- std_list KV cache --------------------------------------------------- +// The cache is a denormalised copy of `SELECT problem_id FROM std_answer`. +// It is rebuilt wholesale from the database rather than patched incrementally, +// so a dropped or racing write cannot leave it permanently out of sync. + +const { RebuildStdList } = require('../Source/Process.ts'); + +function kvStub(initial) { + const store = { std_list: initial }; + const puts = []; + return { + store, + puts, + get: async (key) => (key in store ? store[key] : null), + put: async (key, value) => { store[key] = value; puts.push(value); }, + }; +} + +test('RebuildStdList writes every problem_id from the database', async () => { + const kv = kvStub('stale\n'); + const db = { + Select: async (table, columns) => { + assert.strictEqual(table, 'std_answer'); + assert.deepStrictEqual(columns, ['problem_id']); + return new Result(true, '', [ + { problem_id: 1000 }, { problem_id: 1001 }, { problem_id: 1002 } + ]); + } + }; + + const list = await RebuildStdList(db, kv); + + assert.strictEqual(list, '1000\n1001\n1002'); + assert.strictEqual(kv.store.std_list, '1000\n1001\n1002'); +}); + +test('RebuildStdList writes an empty cache when the table is empty', async () => { + const kv = kvStub('1000\n1001\n'); + const db = { Select: async () => new Result(true, '', []) }; + + await RebuildStdList(db, kv); + + assert.strictEqual(kv.store.std_list, ''); +}); + +test('RebuildStdList heals a cache that has drifted from the database', async () => { + // 1001 was dropped by a lost write; 9999 was never in the table. + const kv = kvStub('1000\n9999\n'); + const db = { + Select: async () => new Result(true, '', [ + { problem_id: 1000 }, { problem_id: 1001 }, { problem_id: 1002 } + ]) + }; + + await RebuildStdList(db, kv); + + assert.strictEqual(kv.store.std_list, '1000\n1001\n1002'); +}); + +const STD_CODE_MARKER = '/' + '*'.repeat(62); + +// Minimal pages that satisfy the XMOJ scraper in UploadStd. +function stdScraperFetch() { + const statusPage = ` + + + + + +
#SIDuser
11someone
2555std
[NEXT]`; + const sourcePage = `int main(){}\n${STD_CODE_MARKER}\ntrailer\n`; + return async (url) => new Response( + String(url).includes('getsource.php') ? sourcePage : statusPage + ); +} + +test('UploadStd rebuilds and awaits the cache after inserting a std', async () => { + const kv = kvStub('1000\n'); + const proc = createProcess({ + db: { + GetTableSize: async () => new Result(true, '', { TableSize: 0 }), + Insert: async () => new Result(true, '', { InsertID: 1 }), + Select: async () => new Result(true, '', [{ problem_id: 1000 }, { problem_id: 1234 }]), + } + }); + proc.kv = kv; + proc.GetProblemScoreChecker = async () => 100; + proc.Fetch = stdScraperFetch(); + + const result = await proc.ProcessFunctions['UploadStd']({ ProblemID: 1234 }); + + assert.ok(result.Success, result.Message); + assert.strictEqual(kv.puts.length, 1, 'cache written exactly once'); + assert.strictEqual(kv.store.std_list, '1000\n1234', + 'cache must reflect the database once UploadStd resolves'); +}); + +test('UploadStd repairs a cache missing an already-uploaded problem', async () => { + // The DB already has a std for 1234 but the cache lost it. Re-uploading + // must put it back rather than silently doing nothing. + const kv = kvStub('1000\n'); + const proc = createProcess({ + db: { + GetTableSize: async () => new Result(true, '', { TableSize: 1 }), + Select: async () => new Result(true, '', [{ problem_id: 1000 }, { problem_id: 1234 }]), + } + }); + proc.kv = kv; + + const result = await proc.ProcessFunctions['UploadStd']({ ProblemID: 1234 }); + + assert.ok(result.Success); + assert.strictEqual(result.Message, '此题已经有人上传标程'); + assert.strictEqual(kv.store.std_list, '1000\n1234'); +}); + +test('UploadStd touches neither database nor cache when already in sync', async () => { + // The hot path: the script re-uploads a problem that already has a std and + // is already cached. This must cost zero database rows and zero KV writes. + const kv = kvStub('1000\n1234\n'); + const select = test.mock.fn(async () => new Result(true, '', [])); + const proc = createProcess({ + db: { + GetTableSize: async () => new Result(true, '', { TableSize: 1 }), + Select: select, + } + }); + proc.kv = kv; + + await proc.ProcessFunctions['UploadStd']({ ProblemID: 1234 }); + + assert.strictEqual(select.mock.calls.length, 0, 'no database read on the hot path'); + assert.strictEqual(kv.puts.length, 0, 'no cache write on the hot path'); + assert.strictEqual(kv.store.std_list, '1000\n1234\n', 'cache left untouched'); +}); + +test('GetStdList returns no spurious trailing zero', async () => { + const proc = createProcess(); + proc.kv = kvStub('1000\n1001\n1002\n'); // legacy trailing-newline format + + const result = await proc.ProcessFunctions['GetStdList']({}); + + assert.ok(result.Success); + assert.deepStrictEqual(result.Data.StdList, [1000, 1001, 1002]); +}); + +test('GetStdList handles an unset cache key', async () => { + const proc = createProcess(); + proc.kv = kvStub(undefined); + + const result = await proc.ProcessFunctions['GetStdList']({}); + + assert.ok(result.Success); + assert.deepStrictEqual(result.Data.StdList, []); +}); From 5f0ac9189b79aa15bdc4fbd396e0ed9bf10ded3d Mon Sep 17 00:00:00 2001 From: boomzero Date: Mon, 27 Jul 2026 14:29:42 +0800 Subject: [PATCH 2/3] Hoist the RebuildStdList require to the top of the test file Matches the convention of the other requires rather than sitting mid-file. Co-Authored-By: Claude Opus 5 --- test/process.test.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/process.test.js b/test/process.test.js index c2cb558..4e8b29c 100644 --- a/test/process.test.js +++ b/test/process.test.js @@ -1,6 +1,6 @@ const test = require('node:test'); const assert = require('node:assert'); -const { Process } = require('../Source/Process.ts'); +const { Process, RebuildStdList } = require('../Source/Process.ts'); const { Result } = require('../Source/Result.ts'); function createProcess(mocks = {}) { @@ -670,8 +670,6 @@ test('GetPost clears mentions for the reader', async () => { // It is rebuilt wholesale from the database rather than patched incrementally, // so a dropped or racing write cannot leave it permanently out of sync. -const { RebuildStdList } = require('../Source/Process.ts'); - function kvStub(initial) { const store = { std_list: initial }; const puts = []; From f8378f20773da4bc4e58bead1b837b76bfb9b114 Mon Sep 17 00:00:00 2001 From: boomzero Date: Mon, 27 Jul 2026 14:32:42 +0800 Subject: [PATCH 3/3] Propagate cron failures and distinguish a missing cache from an empty one Two problems from review. The scheduled handler passed an async function as a Promise executor. The constructor discards the executor's returned promise, so a throw from ThrowErrorIfFailed - which both Database.Delete and RebuildStdList route through - rejected a promise nobody held while the outer one stayed pending forever. waitUntil would hang until the runtime killed the invocation, and the failed run was never reported. Demonstrated: current shape: Context.waitUntil(new Promise(async (Resolve) => ...)) [unhandled rejection: DB failure] outer promise: STILL PENDING -> waitUntil hangs, failure never reported proposed shape: Context.waitUntil((async () => ...)()) outer promise: REJECTED (DB failure) Hand waitUntil the async call's promise directly. The pattern predates the cache rebuild, but the rebuild added another throwing path into it. Separately, ParseStdList treated a missing key and an empty string alike, so GetStdList answered [] when the cache had never been built - telling clients no problem has a std answer. Rather than raise an error and leave the endpoint broken until the next cron run, fill the cache from the database on a miss. An empty string remains a valid empty cache and is served without a database read, so this costs a scan only on a genuine miss, which the daily rebuild keeps rare. Co-Authored-By: Claude Opus 5 --- Source/Process.ts | 23 ++++++++++++++-------- Source/index.ts | 10 +++++++--- test/process.test.js | 45 +++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 64 insertions(+), 14 deletions(-) diff --git a/Source/Process.ts b/Source/Process.ts index 8929369..5f793ca 100644 --- a/Source/Process.ts +++ b/Source/Process.ts @@ -50,12 +50,10 @@ function sleep(time: number) { // hot read - costs no database rows. export const StdListKey = "std_list"; -// Tolerates the legacy trailing-newline format, a missing key, and any blank -// lines left behind by earlier writes. -export const ParseStdList = (Cached: string | null | undefined): Array => { - if (!Cached) { - return []; - } +// Tolerates the legacy trailing-newline format and any blank lines left behind +// by earlier writes. An empty string is a legitimately empty cache; a missing +// key is not, and callers must handle that before getting here. +export const ParseStdList = (Cached: string): Array => { return Cached.split("\n") .filter((Entry) => Entry.trim() !== "") .map(Number); @@ -1223,7 +1221,8 @@ export class Process { // This is the hot path - the script calls UploadStd for problems that // already have a std. Only touch the database when the cache is // actually missing this problem, which is the drift we are repairing. - if (!ParseStdList(await this.kv.get(StdListKey)).includes(ProblemID)) { + const Cached = await this.kv.get(StdListKey); + if (Cached === null || Cached === undefined || !ParseStdList(Cached).includes(ProblemID)) { await RebuildStdList(this.XMOJDatabase, this.kv); } return new Result(true, "此题已经有人上传标程"); @@ -1328,7 +1327,15 @@ export class Process { const ResponseData = { StdList: new Array() }; - ResponseData.StdList = ParseStdList(await this.kv.get(StdListKey)); + // A missing key is not an empty list - it means the cache has never been + // built, and answering [] would tell the client that no problem has a std + // answer. Fill it from the database instead. An empty string is a real + // empty cache and is served as-is, so this costs a scan only on a genuine + // miss, which the daily rebuild keeps rare. + const Cached = await this.kv.get(StdListKey); + ResponseData.StdList = ParseStdList( + Cached ?? await RebuildStdList(this.XMOJDatabase, this.kv) + ); return new Result(true, "获得标程列表成功", ResponseData); }, GetStd: async (Data: object): Promise => { diff --git a/Source/index.ts b/Source/index.ts index 0eb486b..5875d7f 100644 --- a/Source/index.ts +++ b/Source/index.ts @@ -156,7 +156,12 @@ export default { waitUntil: (arg0: Promise) => void; }) { let XMOJDatabase = new Database(Environment.DB); - Context.waitUntil(new Promise(async (Resolve) => { + // An async function passed as a Promise executor swallows its own + // rejection - the constructor discards the returned promise, so a throw + // from ThrowErrorIfFailed would leave this pending forever and waitUntil + // would hang instead of reporting the failed run. Hand waitUntil the async + // call's promise directly so errors propagate. + Context.waitUntil((async () => { await XMOJDatabase.Delete("short_message", { "send_time": { "Operator": "<=", @@ -177,7 +182,6 @@ export default { // bounds any drift - from a dropped KV write or two uploads racing - to // 24 hours, instead of it persisting forever as it does today. await RebuildStdList(XMOJDatabase, Environment.kv); - Resolve(); - })); + })()); }, }; diff --git a/test/process.test.js b/test/process.test.js index 4e8b29c..ae803be 100644 --- a/test/process.test.js +++ b/test/process.test.js @@ -809,12 +809,51 @@ test('GetStdList returns no spurious trailing zero', async () => { assert.deepStrictEqual(result.Data.StdList, [1000, 1001, 1002]); }); -test('GetStdList handles an unset cache key', async () => { - const proc = createProcess(); - proc.kv = kvStub(undefined); +test('GetStdList fills the cache from the database when the key is unset', async () => { + // An unset key is not an empty list - answering [] would tell the client + // that no problem has a std answer at all. + const kv = kvStub(undefined); + const proc = createProcess({ + db: { + Select: async () => new Result(true, '', [ + { problem_id: 1000 }, { problem_id: 1001 } + ]) + } + }); + proc.kv = kv; + + const result = await proc.ProcessFunctions['GetStdList']({}); + + assert.ok(result.Success); + assert.deepStrictEqual(result.Data.StdList, [1000, 1001]); + assert.strictEqual(kv.store.std_list, '1000\n1001', 'cache filled for next time'); +}); + +test('GetStdList serves an empty cache without touching the database', async () => { + // An empty string is a legitimately empty cache (no stds uploaded yet) and + // must be distinguished from a missing key. + const select = test.mock.fn(async () => new Result(true, '', [])); + const proc = createProcess({ db: { Select: select } }); + proc.kv = kvStub(''); const result = await proc.ProcessFunctions['GetStdList']({}); assert.ok(result.Success); assert.deepStrictEqual(result.Data.StdList, []); + assert.strictEqual(select.mock.calls.length, 0, 'empty cache is valid, no rebuild'); +}); + +test('UploadStd rebuilds when the cache key is unset entirely', async () => { + const kv = kvStub(undefined); + const proc = createProcess({ + db: { + GetTableSize: async () => new Result(true, '', { TableSize: 1 }), + Select: async () => new Result(true, '', [{ problem_id: 1234 }]), + } + }); + proc.kv = kv; + + await proc.ProcessFunctions['UploadStd']({ ProblemID: 1234 }); + + assert.strictEqual(kv.store.std_list, '1234'); });