Clean-up existing cache entries on overwrite (#9774)

* Clean-up existing cache entries on overwrite

* Add tests for not leaving trash behind

* Improve test quality

* Implement clean-up of dangling node references

* Update with fixes to large blob writes

* Fix null checking issue

---------

Co-authored-by: pyamada (Remote Dev Environment) <[email protected]>

packages/core/cache/src/LMDBCache.js

@@ -141,6 +141,12 @@ export class LMDBCache implements Cache {
  contents: Buffer | string,
  options?: {|signal?: AbortSignal|},
  ): Promise<void> {
+ const previousEntry = await this.get<LargeBlobEntry>(key);
+ if (previousEntry) {
+ await this.store.remove(key);
+ await this.fsCache.deleteLargeBlob(previousEntry.largeBlobKey);
+ }
+
  // $FlowFixMe flow libs are outdated but we only support node>16 so randomUUID is present
  const largeBlobKey = `${key}_${crypto.randomUUID()}`;
  await this.fsCache.setLargeBlob(largeBlobKey, contents, options);

packages/core/cache/test/LMDBCache.test.js

@@ -2,14 +2,16 @@
 
 import * as mkdirp from 'mkdirp';
 import * as tempy from 'tempy';
+import fs from 'fs';
 import assert from 'assert';
 import path from 'path';
 import {LMDBCache, type Cache} from '../src';
 
 describe('LMDBCache', () => {
+ let tmpDir: string;
  let lmdbCache: Cache;
  beforeEach(async () => {
- const tmpDir = path.join(tempy.directory(), 'LMDBCache');
+ tmpDir = path.join(tempy.directory(), 'LMDBCache');
  mkdirp.sync(tmpDir);
  lmdbCache = new LMDBCache(tmpDir);
  await lmdbCache.ensure();
@@ -58,4 +60,23 @@ describe('LMDBCache', () => {
  'LMDB did not store a buffer',
  );
  });
+
+ it('setting multiple large blobs and then removing them will leave the disk empty', async () => {
+ assert(!(await lmdbCache.has('test-key')), 'LMDB did not start empty');
+
+ const filesAtStart = fs.readdirSync(tmpDir);
+
+ const buffer = Buffer.from([1, 2, 3, 4]);
+ await lmdbCache.setLargeBlob('test-key', buffer);
+ await lmdbCache.setLargeBlob('test-key', buffer);
+ await lmdbCache.setLargeBlob('test-key', buffer);
+ await lmdbCache.setLargeBlob('other-key', buffer);
+ await lmdbCache.setLargeBlob('other-key', buffer);
+ await lmdbCache.setLargeBlob('other-key', buffer);
+ await lmdbCache.deleteLargeBlob('test-key');
+ await lmdbCache.deleteLargeBlob('other-key');
+
+ const filesAtEnd = fs.readdirSync(tmpDir);
+ assert.deepEqual(filesAtStart, filesAtEnd);
+ });
 });

packages/core/core/src/RequestTracker.js

@@ -57,7 +57,7 @@ import type {Cache} from '@parcel/cache';
 import {getConfigKeyContentHash} from './requests/ConfigRequest';
 import {
  storeRequestTrackerCacheInfo,
- clearRequestTrackerCacheInfo,
+ clearRequestTrackerCache,
 } from './RequestTrackerCacheInfo';
 import type {AssetGraphRequestResult} from './requests/AssetGraphRequest';
 import type {PackageRequestResult} from './requests/PackageRequest';
@@ -1393,9 +1393,9 @@ export default class RequestTracker {
  let serialisedGraph = this.graph.serialize();
 
  // Delete an existing request graph cache, to prevent invalid states
- await clearRequestTrackerCacheInfo(this.options.cache);
- await this.options.cache.deleteLargeBlob(requestGraphKey);
+ await clearRequestTrackerCache(this.options.cache);
 
+ const allLargeBlobKeys = new Set<string>();
  let total = 0;
  const serialiseAndSet = async (
  key: string,
@@ -1406,6 +1406,7 @@ export default class RequestTracker {
  throw new Error('Serialization was aborted');
  }
 
+ allLargeBlobKeys.add(key);
  await this.options.cache.setLargeBlob(
  key,
  serialize(contents),
@@ -1521,6 +1522,7 @@ export default class RequestTracker {
  }
 
  await storeRequestTrackerCacheInfo(this.options.cache, {
+ allLargeBlobKeys: Array.from(allLargeBlobKeys),
  requestGraphKey,
  snapshotKey,
  timestamp: Date.now(),

packages/core/core/src/RequestTrackerCacheInfo.js

@@ -10,6 +10,14 @@ export type RequestTrackerCacheInfo = {|
  requestGraphKey: string,
  snapshotKey: string,
  timestamp: number,
+ /**
+ * All the entries associated with this cache instance, including the
+ * `requestGraphKey`. These will all be cleared when the cache
+ * `clearRequestTrackerCacheInfo` is called.
+ *
+ * Nullable for backwards compatibility only. Added on 05-06-2024.
+ */
+ allLargeBlobKeys?: string[],
 |};
 
 /**
@@ -18,7 +26,7 @@ export type RequestTrackerCacheInfo = {|
  * Non-hex strings will fail silently. That is a leaky abstraction and therefore
  * this function is required here to fix it.
  */
-function toFsCacheKey(key: string): string {
+export function toFsCacheKey(key: string): string {
  let result = '';
  for (let i = 0; i < key.length; i += 1) {
  result += key.charCodeAt(i).toString(16);
@@ -67,3 +75,24 @@ export async function storeRequestTrackerCacheInfo(
 export async function clearRequestTrackerCacheInfo(cache: Cache) {
  await cache.set(toFsCacheKey('RequestTrackerCacheInfo'), null);
 }
+
+/**
+ * Clear the current request tracker cache including all nodes and related
+ * files. This is transactional and can't lead to an invalid state.
+ *
+ * This also cleans-up all the large blobs on disk, including dangling node
+ * entries.
+ */
+export async function clearRequestTrackerCache(cache: Cache) {
+ const requestTrackerCacheInfo = await getRequestTrackerCacheInfo(cache);
+ await clearRequestTrackerCacheInfo(cache);
+
+ if (!requestTrackerCacheInfo) {
+ return;
+ }
+
+ await cache.deleteLargeBlob(requestTrackerCacheInfo.requestGraphKey);
+ for (const largeBlobKey of requestTrackerCacheInfo.allLargeBlobKeys ?? []) {
+ await cache.deleteLargeBlob(largeBlobKey);
+ }
+}

packages/core/core/test/RequestTrackerCacheInfo.test.js

@@ -4,12 +4,15 @@ import type {Cache} from '@parcel/types';
 import {FSCache, LMDBCache} from '@parcel/cache';
 import * as tempy from 'tempy';
 import {
+ clearRequestTrackerCache,
  clearRequestTrackerCacheInfo,
  getRequestTrackerCacheInfo,
  storeRequestTrackerCacheInfo,
+ toFsCacheKey,
 } from '../src/RequestTrackerCacheInfo';
 import assert from 'assert';
 import {NodeFS} from '@parcel/fs';
+import type {RequestTrackerCacheInfo} from '../src/RequestTrackerCacheInfo';
 
 type CacheImplementation = {|
  name: string,
@@ -31,6 +34,8 @@ describe('RequestTrackerCacheInfo', () => {
  cacheImplementations.forEach(cacheImplementation => {
  describe(`When using ${cacheImplementation.name}`, () => {
  let cache: Cache;
+ const requestGraphKey = toFsCacheKey('request-graph-key');
+
  beforeEach(async () => {
  cache = cacheImplementation.build();
  await cache.ensure();
@@ -49,7 +54,7 @@ describe('RequestTrackerCacheInfo', () => {
  const expectedEntry = {
  snapshotKey: 'snapshot-key',
  timestamp: Date.now(),
- requestGraphKey: 'request-graph-key',
+ requestGraphKey: requestGraphKey,
  };
  await storeRequestTrackerCacheInfo(cache, expectedEntry);
  {
@@ -62,13 +67,37 @@ describe('RequestTrackerCacheInfo', () => {
  const expectedEntry = {
  snapshotKey: 'snapshot-key',
  timestamp: Date.now(),
- requestGraphKey: 'request-graph-key',
+ requestGraphKey: requestGraphKey,
+ allLargeBlobKeys: [],
  };
  await storeRequestTrackerCacheInfo(cache, expectedEntry);
  await clearRequestTrackerCacheInfo(cache);
  const entry = await getRequestTrackerCacheInfo(cache);
  assert(entry === null);
  });
+
+ it('request-graph and large blob entries are cleared', async () => {
+ const otherKey = toFsCacheKey('other-key');
+
+ await cache.setLargeBlob(requestGraphKey, '1234');
+ await cache.setLargeBlob(otherKey, '5678');
+ assert.equal(await cache.getLargeBlob(requestGraphKey), '1234');
+ assert.equal(await cache.getLargeBlob(otherKey), '5678');
+
+ const expectedEntry: RequestTrackerCacheInfo = {
+ snapshotKey: 'snapshot-key',
+ timestamp: Date.now(),
+ requestGraphKey: requestGraphKey,
+ allLargeBlobKeys: [otherKey],
+ };
+ await storeRequestTrackerCacheInfo(cache, expectedEntry);
+ await clearRequestTrackerCache(cache);
+ const entry = await getRequestTrackerCacheInfo(cache);
+
+ assert(entry === null);
+ assert.equal(await cache.hasLargeBlob(requestGraphKey), false);
+ assert.equal(await cache.hasLargeBlob(otherKey), false);
+ });
  });
  });
 });