diigo-cache: dispose RPC stubs after each fetch; one fetch per miss
Two bugs in page()'s miss path, found while building the fill job: - the webapp stubs (app, pathjoin, query) were never disposed, which kept the DO from being evicted (evictDurableObject hung after a single miss); they are now disposed once the page is in hand - a miss on page n > 1 also fetched page 1 and threw it away, doubling diigo calls; every page carries `total`, so only page n is fetched Regression test: a miss, then eviction, then a cache hit. tsconfig adds the ESNext.Disposable lib for Symbol.dispose. Generated-by: Claude Opus 5.5
This commit is contained in:
1 parent
cf5ccce4e2
commit
530942f5d9
3 files changed
+30
-13
No files matched your search
+18
-12
@@ -72,6 +72,12 @@ export type FillStatus = {
|
|||||||
error?: string;
|
error?: string;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
/** release an RPC stub, if it is one (plain objects are left alone) */
|
||||||
|
function disposeStub(stub: unknown): void {
|
||||||
|
const dispose = (stub as { [Symbol.dispose]?: () => void } | null)?.[Symbol.dispose];
|
||||||
|
if (typeof dispose === "function") dispose.call(stub);
|
||||||
|
}
|
||||||
|
|
||||||
function sleep(ms: number): Promise<void> {
|
function sleep(ms: number): Promise<void> {
|
||||||
return new Promise((r) => setTimeout(r, ms));
|
return new Promise((r) => setTimeout(r, ms));
|
||||||
}
|
}
|
||||||
@@ -145,8 +151,8 @@ abstract class PagedCache extends DurableObject<CacheEnv> {
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* read page `pageNum` (`count` per page, diigo's updated-desc order):
|
* read page `pageNum` (`count` per page, diigo's updated-desc order):
|
||||||
* storage first; on miss, fetch through the webapp instance (etching
|
* storage first; on miss, fetch that one page through the webapp
|
||||||
* page 1 alongside — it carries `total`) and etch. Cache provenance
|
* instance (every page carries `total`) and etch. Cache provenance
|
||||||
* rides in the result (`cached`).
|
* rides in the result (`cached`).
|
||||||
*/
|
*/
|
||||||
async page(pageNum: number, count: number): Promise<CachePage> {
|
async page(pageNum: number, count: number): Promise<CachePage> {
|
||||||
@@ -170,17 +176,17 @@ abstract class PagedCache extends DurableObject<CacheEnv> {
|
|||||||
return hit;
|
return hit;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// RPC stubs (the app and each pipelined step) are disposed once the
|
||||||
|
// page is in hand: undisposed stubs keep this DO from being evicted
|
||||||
const app = await this.env.WEBAPP.at(init.webappId);
|
const app = await this.env.WEBAPP.at(init.webappId);
|
||||||
const fetchPage = (n: number): Promise<{ total: number; items: Item[] }> =>
|
const joined = app.pathjoin("load_user_items");
|
||||||
app
|
const queried = joined.query({ sort: "updated", count, page_num: pageNum });
|
||||||
.pathjoin("load_user_items")
|
let data: { total: number; items: Item[] };
|
||||||
.query({ sort: "updated", count, page_num: n })
|
try {
|
||||||
.get()
|
data = JSON.parse(await queried.get()) as { total: number; items: Item[] };
|
||||||
.then((t) => JSON.parse(t)) as Promise<{ total: number; items: Item[] }>;
|
} finally {
|
||||||
|
for (const stub of [queried, joined, app]) disposeStub(stub);
|
||||||
// a miss on page n>1 etches page 1 first (it carries `total`)
|
}
|
||||||
const first = await fetchPage(1);
|
|
||||||
const data = pageNum === 1 ? first : await fetchPage(pageNum);
|
|
||||||
const kept = this.keep(toRows(data.items, console));
|
const kept = this.keep(toRows(data.items, console));
|
||||||
// fetched vs kept: the public cache's drop of private rows shows here
|
// fetched vs kept: the public cache's drop of private rows shows here
|
||||||
console.log({ event: "cache", kind: this.kind, page: pageNum, outcome: "miss", fetched: data.items.length, kept: kept.length });
|
console.log({ event: "cache", kind: this.kind, page: pageNum, outcome: "miss", fetched: data.items.length, kept: kept.length });
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
import { env } from "cloudflare:workers";
|
import { env } from "cloudflare:workers";
|
||||||
import { runInDurableObject } from "cloudflare:test";
|
import { evictDurableObject, runInDurableObject } from "cloudflare:test";
|
||||||
import { describe, expect, it, vi } from "vitest";
|
import { describe, expect, it, vi } from "vitest";
|
||||||
import worker from "../src/index.js";
|
import worker from "../src/index.js";
|
||||||
import { toWebkey } from "../src/webkeys.js";
|
import { toWebkey } from "../src/webkeys.js";
|
||||||
@@ -68,6 +68,16 @@ describe("PublicCache", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("eviction", () => {
|
||||||
|
it("a cache miss leaves no undisposed RPC stubs to block eviction", async () => {
|
||||||
|
const stub = testEnv.CACHE_DO.get(testEnv.CACHE_DO.newUniqueId());
|
||||||
|
await stub.setup({ webappId: WEBAPP_ID });
|
||||||
|
await stub.page(1, 24);
|
||||||
|
await evictDurableObject(stub);
|
||||||
|
expect((await stub.page(1, 24)).cached).toBe(true);
|
||||||
|
}, 3000);
|
||||||
|
});
|
||||||
|
|
||||||
describe("DiigoCache", () => {
|
describe("DiigoCache", () => {
|
||||||
it("keeps every item of a page, each under its own link_id", async () => {
|
it("keeps every item of a page, each under its own link_id", async () => {
|
||||||
const stub = testEnv.CACHE_DO.get(testEnv.CACHE_DO.newUniqueId());
|
const stub = testEnv.CACHE_DO.get(testEnv.CACHE_DO.newUniqueId());
|
||||||
|
|||||||
@@ -7,6 +7,7 @@
|
|||||||
"esModuleInterop": true,
|
"esModuleInterop": true,
|
||||||
"verbatimModuleSyntax": true,
|
"verbatimModuleSyntax": true,
|
||||||
"skipLibCheck": true,
|
"skipLibCheck": true,
|
||||||
|
"lib": ["ES2022", "ESNext.Disposable"],
|
||||||
"noEmit": true,
|
"noEmit": true,
|
||||||
"types": []
|
"types": []
|
||||||
},
|
},
|
||||||
|
|||||||
Reference in new issue
Block a user