From b21a243c13703bb7f0e1ac12fa5f46c45c2a305b Mon Sep 17 00:00:00 2001 From: Dan Connolly Date: Wed, 21 Jul 2021 15:36:28 -0500 Subject: [PATCH] fix(swingset): delete unused snapshots In addition to maintaining a mapping from vatID to snapshotID, vatKeeper maintains a reverse mapping. After `commitCrank()`, the kernel calls `vatWarehouse.pruneSnapshots()`, which 1. calls `kernelKeeper.getUnusedSnapshots()`, 2. tries to `snapStore.delete()` each of them, and 3. reports the results using `kernelKeeper.forgetUnusedSnapshots()`. fixes #3374 --- packages/SwingSet/src/kernel/kernel.js | 4 ++ .../SwingSet/src/kernel/state/kernelKeeper.js | 33 +++++++++++++ .../SwingSet/src/kernel/state/vatKeeper.js | 48 +++++++++++++++++++ .../src/kernel/vatManager/vat-warehouse.js | 24 ++++++++++ .../test/vat-warehouse/test-warehouse.js | 29 +++++++++++ packages/xsnap/src/snapStore.js | 10 +++- 6 files changed, 147 insertions(+), 1 deletion(-) diff --git a/packages/SwingSet/src/kernel/kernel.js b/packages/SwingSet/src/kernel/kernel.js index 1d9169258d8f..ebb6f8d106f9 100644 --- a/packages/SwingSet/src/kernel/kernel.js +++ b/packages/SwingSet/src/kernel/kernel.js @@ -752,6 +752,10 @@ export default function buildKernel( kernelKeeper.saveStats(); commitCrank(); kernelKeeper.incrementCrankNumber(); + if (snapStore) { + // eslint-disable-next-line no-use-before-define + vatWarehouse.pruneSnapshots(snapStore); + } } finally { processQueueRunning = undefined; } diff --git a/packages/SwingSet/src/kernel/state/kernelKeeper.js b/packages/SwingSet/src/kernel/state/kernelKeeper.js index bef40c3be70d..a006a848b251 100644 --- a/packages/SwingSet/src/kernel/state/kernelKeeper.js +++ b/packages/SwingSet/src/kernel/state/kernelKeeper.js @@ -59,6 +59,8 @@ const enableKernelGC = true; // m$NN.remaining = $NN // remaining capacity (in computrons) or 'unlimited' // m$NN.threshold = $NN // notify when .remaining first drops below this +// snapshot.$id = [vatID, ...] + // d$NN.o.nextID = $NN // d$NN.c.$kernelSlot = $deviceSlot = o-$NN/d+$NN/d-$NN // d$NN.c.$deviceSlot = $kernelSlot = ko$NN/kd$NN @@ -629,6 +631,35 @@ export default function makeKernelKeeper( return kernelPromisesToReject; } + function getUnusedSnapshots() { + /** @type { string[] } */ + const found = []; + for (const k of kvStore.getKeys(`snapshot.`, `snapshot/`)) { + const consumers = JSON.parse(kvStore.get(k)); + assert(Array.isArray(consumers)); + if (consumers.length === 0) { + const snapshotID = k.slice(`snapshot.`.length); + found.push(snapshotID); + } + } + return found; + } + + /** + * @param {string[]} allegedlyUnused + */ + function forgetUnusedSnapshots(allegedlyUnused) { + for (const snapshotID of allegedlyUnused) { + const k = `snapshot.${snapshotID}`; + const consumersJSON = kvStore.get(k); + assert(consumersJSON); + const consumers = JSON.parse(consumersJSON); + assert(Array.isArray(consumers)); + assert(consumers.length === 0); + kvStore.delete(k); + } + } + function addMessageToPromiseQueue(kernelSlot, msg) { insistKernelType('promise', kernelSlot); insistMessage(msg); @@ -1310,6 +1341,8 @@ export default function makeKernelKeeper( evictVatKeeper, closeVatTranscript, cleanupAfterTerminatedVat, + getUnusedSnapshots, + forgetUnusedSnapshots, addDynamicVatID, getDynamicVats, getStaticVats, diff --git a/packages/SwingSet/src/kernel/state/vatKeeper.js b/packages/SwingSet/src/kernel/state/vatKeeper.js index 71f7b51250b8..ae2a2df24e23 100644 --- a/packages/SwingSet/src/kernel/state/vatKeeper.js +++ b/packages/SwingSet/src/kernel/state/vatKeeper.js @@ -452,6 +452,49 @@ export function makeVatKeeper( return { totalEntries, snapshottedEntries }; } + /** + * Add vatID to consumers of a snapshot. + * + * @param {string} snapshotID + */ + function addToSnapshot(snapshotID) { + const key = `snapshot.${snapshotID}`; + const consumersJSON = kvStore.get(key); + const consumers = + consumersJSON !== undefined ? JSON.parse(consumersJSON) : []; + assert(Array.isArray(consumers)); + + // We can't completely rule out the possibility that + // a vat will use the same snapshot twice in a row. + // + // PERFORMANCE NOTE: we assume consumer lists are short; + // usually length 1. So O(n) search here is better + // than keeping the list sorted. + if (!consumers.includes(vatID)) { + consumers.push(vatID); + kvStore.set(key, JSON.stringify(consumers)); + // console.log('addToSnapshot result:', { vatID, snapshotID, consumers }); + } + } + + /** + * Remove vatID from consumers of a snapshot. + * + * @param {string} snapshotID + */ + function removeFromSnapshot(snapshotID) { + const key = `snapshot.${snapshotID}`; + const consumersJSON = kvStore.get(key); + assert(consumersJSON, X`cannot remove ${vatID}: ${key} key not defined`); + const consumers = JSON.parse(consumersJSON); + assert(Array.isArray(consumers)); + const ix = consumers.indexOf(vatID); + assert(ix >= 0); + consumers.splice(ix, 1); + // console.log('removeFromSnapshot done:', { vatID, snapshotID, consumers }); + kvStore.set(key, JSON.stringify(consumers)); + } + /** * Store a snapshot, if given a snapStore. * @@ -464,11 +507,16 @@ export function makeVatKeeper( } const snapshotID = await manager.makeSnapshot(snapStore); + const old = getLastSnapshot(); + if (old) { + removeFromSnapshot(old.snapshotID); + } const endPosition = getTranscriptEndPosition(); kvStore.set( `${vatID}.lastSnapshot`, JSON.stringify({ snapshotID, startPos: endPosition }), ); + addToSnapshot(snapshotID); return true; } diff --git a/packages/SwingSet/src/kernel/vatManager/vat-warehouse.js b/packages/SwingSet/src/kernel/vatManager/vat-warehouse.js index ceea90ec5081..7314a8b62977 100644 --- a/packages/SwingSet/src/kernel/vatManager/vat-warehouse.js +++ b/packages/SwingSet/src/kernel/vatManager/vat-warehouse.js @@ -302,6 +302,29 @@ export function makeVatWarehouse(kernelKeeper, vatLoader, policyOptions) { return true; } + /** + * Delete unused snapshots. + * + * WARNING: caller is responsible to call this only + * when all records of snapshot consumers are durably + * stored; for example, after commitCrank(). + * + * @param { SnapStore } snapStore + */ + function pruneSnapshots(snapStore) { + const todo = kernelKeeper.getUnusedSnapshots(); + const done = []; + for (const snapshotID of todo) { + try { + snapStore.delete(snapshotID); + done.push(snapshotID); + } catch (_ignored) { + // better luck next time... + } + } + kernelKeeper.forgetUnusedSnapshots(done); + } + /** * @param {string} vatID * @param {unknown[]} kd @@ -363,6 +386,7 @@ export function makeVatWarehouse(kernelKeeper, vatLoader, policyOptions) { kernelDeliveryToVatDelivery, deliverToVat, maybeSaveSnapshot, + pruneSnapshots, // mostly for testing? activeVatsInfo: () => diff --git a/packages/SwingSet/test/vat-warehouse/test-warehouse.js b/packages/SwingSet/test/vat-warehouse/test-warehouse.js index 6b42c7dc0334..066a3309a801 100644 --- a/packages/SwingSet/test/vat-warehouse/test-warehouse.js +++ b/packages/SwingSet/test/vat-warehouse/test-warehouse.js @@ -103,6 +103,26 @@ test('4 vats in warehouse with 2 online', async t => { await runSteps(c, t); }); +function unusedSnapshotsOnDisk(kvStore, snapstorePath) { + const inUse = []; + for (const k of kvStore.getKeys(`snapshot.`, `snapshot/`)) { + const consumers = JSON.parse(kvStore.get(k)); + if (consumers.length > 0) { + const id = k.slice(`snapshot.`.length); + inUse.push(id); + } + } + const onDisk = fs.readdirSync(snapstorePath); + const extra = []; + for (const snapshotPath of onDisk) { + const id = snapshotPath.slice(0, -'.gz'.length); + if (!inUse.includes(id)) { + extra.push(id); + } + } + return { inUse, onDisk, extra }; +} + test('snapshot after deliveries', async t => { const snapstorePath = path.resolve(__dirname, './fixture-test-warehouse/'); fs.mkdirSync(snapstorePath, { recursive: true }); @@ -115,7 +135,16 @@ test('snapshot after deliveries', async t => { warehousePolicy: { maxVatsOnline, snapshotInterval: 1 }, }); t.teardown(c.shutdown); + await runSteps(c, t); + + const { kvStore } = hostStorage; + const { inUse, onDisk, extra } = unusedSnapshotsOnDisk( + kvStore, + snapstorePath, + ); + t.log({ inUse, onDisk, extra }); + t.deepEqual(extra, [], `inUse: ${inUse}, onDisk: ${onDisk}`); }); test('LRU eviction', t => { diff --git a/packages/xsnap/src/snapStore.js b/packages/xsnap/src/snapStore.js index 6887d15bf476..1b43fbe76c65 100644 --- a/packages/xsnap/src/snapStore.js +++ b/packages/xsnap/src/snapStore.js @@ -130,5 +130,13 @@ export function makeSnapStore( }, `${hash}-load`); } - return freeze({ load, save }); + /** + * @param {string} hash + * @throws { Error } if there is no such snapshot + */ + function deleteSnapshot(hash) { + unlink(resolve(root, `${hash}.gz`)); + } + + return freeze({ load, save, delete: deleteSnapshot }); }