diff --git a/Cargo.lock b/Cargo.lock index 9e30ce91dedc..6e069ac13edb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10001,6 +10001,7 @@ name = "turbo-persistence" version = "0.1.0" dependencies = [ "anyhow", + "auto-hash-map", "bitfield", "byteorder", "codspeed-criterion-compat", diff --git a/lerna.json b/lerna.json index cf21c39263fc..df580516daaf 100644 --- a/lerna.json +++ b/lerna.json @@ -15,5 +15,5 @@ "registry": "https://registry.npmjs.org/" } }, - "version": "16.3.1-canary.20" + "version": "16.3.1-canary.21" } \ No newline at end of file diff --git a/packages/create-next-app/package.json b/packages/create-next-app/package.json index 3a43db49f65f..9a7b8c3bcf95 100644 --- a/packages/create-next-app/package.json +++ b/packages/create-next-app/package.json @@ -1,6 +1,6 @@ { "name": "create-next-app", - "version": "16.3.1-canary.20", + "version": "16.3.1-canary.21", "keywords": [ "react", "next", diff --git a/packages/devlow-bench/package.json b/packages/devlow-bench/package.json index 49b768cf52ec..908624afab97 100644 --- a/packages/devlow-bench/package.json +++ b/packages/devlow-bench/package.json @@ -1,7 +1,7 @@ { "name": "@vercel/devlow-bench", "private": true, - "version": "16.3.1-canary.20", + "version": "16.3.1-canary.21", "description": "Benchmarking tool for the developer workflow", "repository": { "type": "git", diff --git a/packages/eslint-config-next/package.json b/packages/eslint-config-next/package.json index a96f9ab1e129..a2dc24447148 100644 --- a/packages/eslint-config-next/package.json +++ b/packages/eslint-config-next/package.json @@ -1,6 +1,6 @@ { "name": "eslint-config-next", - "version": "16.3.1-canary.20", + "version": "16.3.1-canary.21", "description": "ESLint configuration used by Next.js.", "license": "MIT", "repository": { @@ -12,7 +12,7 @@ "dist" ], "dependencies": { - "@next/eslint-plugin-next": "16.3.1-canary.20", + "@next/eslint-plugin-next": "16.3.1-canary.21", "eslint-import-resolver-node": "^0.3.6", "eslint-import-resolver-typescript": "^3.5.2", "eslint-plugin-import": "^2.32.0", diff --git a/packages/eslint-plugin-internal/package.json b/packages/eslint-plugin-internal/package.json index 0c49b385f8c2..d95189976da3 100644 --- a/packages/eslint-plugin-internal/package.json +++ b/packages/eslint-plugin-internal/package.json @@ -1,7 +1,7 @@ { "name": "@next/eslint-plugin-internal", "private": true, - "version": "16.3.1-canary.20", + "version": "16.3.1-canary.21", "description": "ESLint plugin for working on Next.js.", "exports": { ".": "./src/eslint-plugin-internal.js" diff --git a/packages/eslint-plugin-next/package.json b/packages/eslint-plugin-next/package.json index c4a31224c223..7ce45055e463 100644 --- a/packages/eslint-plugin-next/package.json +++ b/packages/eslint-plugin-next/package.json @@ -1,6 +1,6 @@ { "name": "@next/eslint-plugin-next", - "version": "16.3.1-canary.20", + "version": "16.3.1-canary.21", "description": "ESLint plugin for Next.js.", "main": "dist/index.js", "types": "dist/index.d.ts", diff --git a/packages/font/package.json b/packages/font/package.json index 958fcd25d630..9e36c766581c 100644 --- a/packages/font/package.json +++ b/packages/font/package.json @@ -1,7 +1,7 @@ { "name": "@next/font", "private": true, - "version": "16.3.1-canary.20", + "version": "16.3.1-canary.21", "repository": { "url": "vercel/next.js", "directory": "packages/font" diff --git a/packages/next-bundle-analyzer/package.json b/packages/next-bundle-analyzer/package.json index 2e2f74df5d19..647e861080c6 100644 --- a/packages/next-bundle-analyzer/package.json +++ b/packages/next-bundle-analyzer/package.json @@ -1,6 +1,6 @@ { "name": "@next/bundle-analyzer", - "version": "16.3.1-canary.20", + "version": "16.3.1-canary.21", "main": "index.js", "types": "index.d.ts", "license": "MIT", diff --git a/packages/next-codemod/package.json b/packages/next-codemod/package.json index c94381b66b24..67fcaf119d66 100644 --- a/packages/next-codemod/package.json +++ b/packages/next-codemod/package.json @@ -1,6 +1,6 @@ { "name": "@next/codemod", - "version": "16.3.1-canary.20", + "version": "16.3.1-canary.21", "license": "MIT", "repository": { "type": "git", diff --git a/packages/next-env/package.json b/packages/next-env/package.json index 92a8e63573cd..0256f24f7705 100644 --- a/packages/next-env/package.json +++ b/packages/next-env/package.json @@ -1,6 +1,6 @@ { "name": "@next/env", - "version": "16.3.1-canary.20", + "version": "16.3.1-canary.21", "keywords": [ "react", "next", diff --git a/packages/next-mdx/package.json b/packages/next-mdx/package.json index b312d905edca..78d6ef8ffd4d 100644 --- a/packages/next-mdx/package.json +++ b/packages/next-mdx/package.json @@ -1,6 +1,6 @@ { "name": "@next/mdx", - "version": "16.3.1-canary.20", + "version": "16.3.1-canary.21", "main": "index.js", "license": "MIT", "repository": { diff --git a/packages/next-playwright/package.json b/packages/next-playwright/package.json index 25efa7322b87..a2c21a496fd3 100644 --- a/packages/next-playwright/package.json +++ b/packages/next-playwright/package.json @@ -1,6 +1,6 @@ { "name": "@next/playwright", - "version": "16.3.1-canary.20", + "version": "16.3.1-canary.21", "repository": { "url": "vercel/next.js", "directory": "packages/next-playwright" diff --git a/packages/next-plugin-storybook/package.json b/packages/next-plugin-storybook/package.json index 8c676c8da684..3341d527b8b2 100644 --- a/packages/next-plugin-storybook/package.json +++ b/packages/next-plugin-storybook/package.json @@ -1,6 +1,6 @@ { "name": "@next/plugin-storybook", - "version": "16.3.1-canary.20", + "version": "16.3.1-canary.21", "repository": { "url": "vercel/next.js", "directory": "packages/next-plugin-storybook" diff --git a/packages/next-polyfill-module/package.json b/packages/next-polyfill-module/package.json index e8472b39a9d3..a955c79cf004 100644 --- a/packages/next-polyfill-module/package.json +++ b/packages/next-polyfill-module/package.json @@ -1,6 +1,6 @@ { "name": "@next/polyfill-module", - "version": "16.3.1-canary.20", + "version": "16.3.1-canary.21", "description": "A standard library polyfill for ES Modules supporting browsers (Edge 16+, Firefox 60+, Chrome 61+, Safari 10.1+)", "main": "dist/polyfill-module.js", "license": "MIT", diff --git a/packages/next-polyfill-nomodule/package.json b/packages/next-polyfill-nomodule/package.json index 88def75eccc7..9cfe87cd75d1 100644 --- a/packages/next-polyfill-nomodule/package.json +++ b/packages/next-polyfill-nomodule/package.json @@ -1,6 +1,6 @@ { "name": "@next/polyfill-nomodule", - "version": "16.3.1-canary.20", + "version": "16.3.1-canary.21", "description": "A polyfill for non-dead, nomodule browsers.", "main": "dist/polyfill-nomodule.js", "license": "MIT", diff --git a/packages/next-routing/package.json b/packages/next-routing/package.json index 0288a679a1b0..2aed89179ffa 100644 --- a/packages/next-routing/package.json +++ b/packages/next-routing/package.json @@ -1,6 +1,6 @@ { "name": "@next/routing", - "version": "16.3.1-canary.20", + "version": "16.3.1-canary.21", "keywords": [ "react", "next", diff --git a/packages/next-rspack/package.json b/packages/next-rspack/package.json index c926d17650d0..b4867f29c123 100644 --- a/packages/next-rspack/package.json +++ b/packages/next-rspack/package.json @@ -1,6 +1,6 @@ { "name": "next-rspack", - "version": "16.3.1-canary.20", + "version": "16.3.1-canary.21", "repository": { "url": "vercel/next.js", "directory": "packages/next-rspack" diff --git a/packages/next-swc/package.json b/packages/next-swc/package.json index 147318c06049..db2cd0a99a23 100644 --- a/packages/next-swc/package.json +++ b/packages/next-swc/package.json @@ -1,6 +1,6 @@ { "name": "@next/swc", - "version": "16.3.1-canary.20", + "version": "16.3.1-canary.21", "private": true, "files": [ "native/" diff --git a/packages/next/package.json b/packages/next/package.json index 4d076a378dd6..82d3ff650bda 100644 --- a/packages/next/package.json +++ b/packages/next/package.json @@ -1,6 +1,6 @@ { "name": "next", - "version": "16.3.1-canary.20", + "version": "16.3.1-canary.21", "description": "The React Framework", "main": "./dist/server/next.js", "license": "MIT", @@ -100,7 +100,7 @@ ] }, "dependencies": { - "@next/env": "16.3.1-canary.20", + "@next/env": "16.3.1-canary.21", "@swc/helpers": "0.5.23", "baseline-browser-mapping": "^2.9.19", "caniuse-lite": "^1.0.30001579", @@ -164,11 +164,11 @@ "@modelcontextprotocol/sdk": "1.18.1", "@mswjs/interceptors": "0.42.0", "@napi-rs/triples": "1.2.0", - "@next/font": "16.3.1-canary.20", - "@next/polyfill-module": "16.3.1-canary.20", - "@next/polyfill-nomodule": "16.3.1-canary.20", - "@next/react-refresh-utils": "16.3.1-canary.20", - "@next/swc": "16.3.1-canary.20", + "@next/font": "16.3.1-canary.21", + "@next/polyfill-module": "16.3.1-canary.21", + "@next/polyfill-nomodule": "16.3.1-canary.21", + "@next/react-refresh-utils": "16.3.1-canary.21", + "@next/swc": "16.3.1-canary.21", "@opentelemetry/api": "1.6.0", "@playwright/test": "1.61.0", "@rspack/core": "1.6.7", diff --git a/packages/next/src/server/app-render/action-async-storage-instance.ts b/packages/next/src/server/app-render/action-async-storage-instance.ts index 64aae6f9b720..7212566beac7 100644 --- a/packages/next/src/server/app-render/action-async-storage-instance.ts +++ b/packages/next/src/server/app-render/action-async-storage-instance.ts @@ -1,5 +1,5 @@ import type { ActionAsyncStorage } from './action-async-storage.external' -import { createAsyncLocalStorage } from './async-local-storage' +import { getOrCreateGlobalAsyncLocalStorage } from './async-local-storage' export const actionAsyncStorageInstance: ActionAsyncStorage = - createAsyncLocalStorage() + getOrCreateGlobalAsyncLocalStorage('action-async-storage') diff --git a/packages/next/src/server/app-render/after-task-async-storage-instance.ts b/packages/next/src/server/app-render/after-task-async-storage-instance.ts index 0e5ecbeb2606..f5e54a97e13e 100644 --- a/packages/next/src/server/app-render/after-task-async-storage-instance.ts +++ b/packages/next/src/server/app-render/after-task-async-storage-instance.ts @@ -1,5 +1,5 @@ import type { AfterTaskAsyncStorage } from './after-task-async-storage.external' -import { createAsyncLocalStorage } from './async-local-storage' +import { getOrCreateGlobalAsyncLocalStorage } from './async-local-storage' export const afterTaskAsyncStorageInstance: AfterTaskAsyncStorage = - createAsyncLocalStorage() + getOrCreateGlobalAsyncLocalStorage('after-task-async-storage') diff --git a/packages/next/src/server/app-render/async-local-storage.ts b/packages/next/src/server/app-render/async-local-storage.ts index 664337b51456..804be49c0862 100644 --- a/packages/next/src/server/app-render/async-local-storage.ts +++ b/packages/next/src/server/app-render/async-local-storage.ts @@ -45,6 +45,43 @@ export function createAsyncLocalStorage< return new FakeAsyncLocalStorage() } +/** + * Returns the storage registered under `name`, and creates it on first use. + * + * These storages must be singletons within a realm. A store that is entered + * through one reference to a storage must be readable through every other + * reference to it. If it is not, code that runs inside the scope sees no store + * at all. Module identity does not guarantee this. A realm can evaluate the + * same `next` file more than once if the package is reachable through more than + * one path, and then each evaluation creates a storage of its own. A global + * symbol keeps the singleton intact for any number of copies. Worker threads + * and edge sandboxes still get separate storages, because each of them has its + * own `globalThis`. + * + * Module identity broke this way in `next dev`. A bug in Node's + * `fs.realpathSync` can return a path with its symlinks unresolved, and the + * module loader keys the module cache on that path. On a pnpm install it then + * resolves `next/dist/...` through the `node_modules/next` symlink instead of + * the real path, so the file is evaluated a second time. See + * https://github.com/nodejs/node/pull/65113. Node versions without that fix + * stay affected. + * + * The key includes the Next.js version, so two different versions of Next.js in + * the same realm keep separate storages. Their store shapes might not be + * compatible. + */ +export function getOrCreateGlobalAsyncLocalStorage( + name: string +): AsyncLocalStorage { + const key = Symbol.for(`@next/${name}@${process.env.__NEXT_VERSION}`) + + const globalStore = globalThis as typeof globalThis & { + [key: symbol]: AsyncLocalStorage | undefined + } + + return (globalStore[key] ??= createAsyncLocalStorage()) +} + export function bindSnapshot( // WARNING: Don't pass a named function to this argument! See: https://github.com/facebook/react/pull/34911 fn: T diff --git a/packages/next/src/server/app-render/console-async-storage-instance.ts b/packages/next/src/server/app-render/console-async-storage-instance.ts index 7ef5d8871288..e507f153992c 100644 --- a/packages/next/src/server/app-render/console-async-storage-instance.ts +++ b/packages/next/src/server/app-render/console-async-storage-instance.ts @@ -1,5 +1,5 @@ -import { createAsyncLocalStorage } from './async-local-storage' +import { getOrCreateGlobalAsyncLocalStorage } from './async-local-storage' import type { ConsoleAsyncStorage } from './console-async-storage.external' export const consoleAsyncStorageInstance: ConsoleAsyncStorage = - createAsyncLocalStorage() + getOrCreateGlobalAsyncLocalStorage('console-async-storage') diff --git a/packages/next/src/server/app-render/dynamic-access-async-storage-instance.ts b/packages/next/src/server/app-render/dynamic-access-async-storage-instance.ts index 4364e4447ae2..fa8ed7d81412 100644 --- a/packages/next/src/server/app-render/dynamic-access-async-storage-instance.ts +++ b/packages/next/src/server/app-render/dynamic-access-async-storage-instance.ts @@ -1,5 +1,5 @@ -import { createAsyncLocalStorage } from './async-local-storage' +import { getOrCreateGlobalAsyncLocalStorage } from './async-local-storage' import type { DynamicAccessStorage } from './dynamic-access-async-storage.external' export const dynamicAccessAsyncStorageInstance: DynamicAccessStorage = - createAsyncLocalStorage() + getOrCreateGlobalAsyncLocalStorage('dynamic-access-async-storage') diff --git a/packages/next/src/server/app-render/work-async-storage-instance.ts b/packages/next/src/server/app-render/work-async-storage-instance.ts index b3e352daa09e..6722b3e222a4 100644 --- a/packages/next/src/server/app-render/work-async-storage-instance.ts +++ b/packages/next/src/server/app-render/work-async-storage-instance.ts @@ -1,5 +1,5 @@ import type { WorkAsyncStorage } from './work-async-storage.external' -import { createAsyncLocalStorage } from './async-local-storage' +import { getOrCreateGlobalAsyncLocalStorage } from './async-local-storage' export const workAsyncStorageInstance: WorkAsyncStorage = - createAsyncLocalStorage() + getOrCreateGlobalAsyncLocalStorage('work-async-storage') diff --git a/packages/next/src/server/app-render/work-unit-async-storage-instance.ts b/packages/next/src/server/app-render/work-unit-async-storage-instance.ts index c30ae013142e..acfa603b2573 100644 --- a/packages/next/src/server/app-render/work-unit-async-storage-instance.ts +++ b/packages/next/src/server/app-render/work-unit-async-storage-instance.ts @@ -1,5 +1,5 @@ -import { createAsyncLocalStorage } from './async-local-storage' +import { getOrCreateGlobalAsyncLocalStorage } from './async-local-storage' import type { WorkUnitAsyncStorage } from './work-unit-async-storage.external' export const workUnitAsyncStorageInstance: WorkUnitAsyncStorage = - createAsyncLocalStorage() + getOrCreateGlobalAsyncLocalStorage('work-unit-async-storage') diff --git a/packages/next/src/server/lib/trace/request-insights-identity.ts b/packages/next/src/server/lib/trace/request-insights-identity.ts index 2da05e0fea48..d931eef43d92 100644 --- a/packages/next/src/server/lib/trace/request-insights-identity.ts +++ b/packages/next/src/server/lib/trace/request-insights-identity.ts @@ -1,6 +1,6 @@ import type { AsyncLocalStorage } from 'async_hooks' import type { RequestInsightKind } from '../../../next-devtools/shared/request-insights' -import { createAsyncLocalStorage } from '../../app-render/async-local-storage' +import { getOrCreateGlobalAsyncLocalStorage } from '../../app-render/async-local-storage' export type RequestInsightsIdentity = { requestId: string @@ -12,17 +12,8 @@ export type RequestInsightsIdentity = { // This storage covers the part of BaseServer request handling that runs before // App Render creates workAsyncStorage. Once available, workStore remains the // primary identity source for locally recorded spans. -const REQUEST_INSIGHTS_IDENTITY_STORAGE_KEY = Symbol.for( - '@next/request-insights-identity-storage' -) - function getRequestInsightsIdentityStorage(): AsyncLocalStorage { - const globalStore = globalThis as typeof globalThis & { - [REQUEST_INSIGHTS_IDENTITY_STORAGE_KEY]?: AsyncLocalStorage - } - - return (globalStore[REQUEST_INSIGHTS_IDENTITY_STORAGE_KEY] ??= - createAsyncLocalStorage()) + return getOrCreateGlobalAsyncLocalStorage('request-insights-identity-storage') } export function runWithRequestInsightsIdentity( diff --git a/packages/react-refresh-utils/package.json b/packages/react-refresh-utils/package.json index 1e2017c6c945..679dd4614698 100644 --- a/packages/react-refresh-utils/package.json +++ b/packages/react-refresh-utils/package.json @@ -1,6 +1,6 @@ { "name": "@next/react-refresh-utils", - "version": "16.3.1-canary.20", + "version": "16.3.1-canary.21", "description": "An experimental package providing utilities for React Refresh.", "repository": { "url": "vercel/next.js", diff --git a/packages/third-parties/package.json b/packages/third-parties/package.json index 97efa240ca6f..eb07f494448b 100644 --- a/packages/third-parties/package.json +++ b/packages/third-parties/package.json @@ -1,6 +1,6 @@ { "name": "@next/third-parties", - "version": "16.3.1-canary.20", + "version": "16.3.1-canary.21", "repository": { "url": "vercel/next.js", "directory": "packages/third-parties" @@ -26,7 +26,7 @@ "third-party-capital": "1.0.20" }, "devDependencies": { - "next": "16.3.1-canary.20", + "next": "16.3.1-canary.21", "outdent": "0.8.0", "prettier": "2.5.1", "typescript": "6.0.2" diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f6bb46ede3e4..80d598182025 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1024,7 +1024,7 @@ importers: packages/eslint-config-next: dependencies: '@next/eslint-plugin-next': - specifier: 16.3.1-canary.20 + specifier: 16.3.1-canary.21 version: link:../eslint-plugin-next eslint: specifier: '>=9.0.0' @@ -1107,7 +1107,7 @@ importers: packages/next: dependencies: '@next/env': - specifier: 16.3.1-canary.20 + specifier: 16.3.1-canary.21 version: link:../next-env '@swc/helpers': specifier: 0.5.23 @@ -1228,19 +1228,19 @@ importers: specifier: 1.2.0 version: 1.2.0 '@next/font': - specifier: 16.3.1-canary.20 + specifier: 16.3.1-canary.21 version: link:../font '@next/polyfill-module': - specifier: 16.3.1-canary.20 + specifier: 16.3.1-canary.21 version: link:../next-polyfill-module '@next/polyfill-nomodule': - specifier: 16.3.1-canary.20 + specifier: 16.3.1-canary.21 version: link:../next-polyfill-nomodule '@next/react-refresh-utils': - specifier: 16.3.1-canary.20 + specifier: 16.3.1-canary.21 version: link:../react-refresh-utils '@next/swc': - specifier: 16.3.1-canary.20 + specifier: 16.3.1-canary.21 version: link:../next-swc '@opentelemetry/api': specifier: 1.6.0 @@ -1983,7 +1983,7 @@ importers: version: 1.0.20 devDependencies: next: - specifier: 16.3.1-canary.20 + specifier: 16.3.1-canary.21 version: link:../next outdent: specifier: 0.8.0 diff --git a/test/development/app-dir/cache-components-dev-warmup/dev-warmup.util.ts b/test/development/app-dir/cache-components-dev-warmup/dev-warmup.util.ts index 338b8bbf9d8b..fb91224ae6d3 100644 --- a/test/development/app-dir/cache-components-dev-warmup/dev-warmup.util.ts +++ b/test/development/app-dir/cache-components-dev-warmup/dev-warmup.util.ts @@ -25,7 +25,7 @@ export function runDevWarmupTests({ : 'fixtures/without-prefetch-config' describe(`cache-components-dev-warmup - ${description}`, () => { - const { next, isTurbopack } = nextTestSetup({ + const { next } = nextTestSetup({ files: nodePath.join(__dirname, fixturePath), }) @@ -102,19 +102,6 @@ export function runDevWarmupTests({ 'AbortError: This operation was aborted' ) - if (isTurbopack) { - // FIXME: - // In Turbopack, requests to the /revalidate route seem to occasionally crash - // due to some HMR or compilation issue. `revalidatePath` throws this error: - // - // Invariant: static generation store missing in revalidatePath - // - // This is unrelated to the logic being tested here, so for now, we skip the assertions - // that require us to revalidate. - console.log('WARNING: skipping revalidation assertions in turbopack') - return - } - // After a revalidation the subsequent render must discard the stale cache // entries. This should not affect the environment labels once the caches // are warm again. @@ -171,19 +158,6 @@ export function runDevWarmupTests({ 'AbortError: This operation was aborted' ) - if (isTurbopack) { - // FIXME: - // In Turbopack, requests to the /revalidate route seem to occasionally crash - // due to some HMR or compilation issue. `revalidatePath` throws this error: - // - // Invariant: static generation store missing in revalidatePath - // - // This is unrelated to the logic being tested here, so for now, we skip the assertions - // that require us to revalidate. - console.log('WARNING: skipping revalidation assertions in turbopack') - return - } - // After a revalidation the subsequent render must discard the stale cache // entries. This should not affect the environment labels once the caches // are warm again. @@ -443,58 +417,52 @@ export function runDevWarmupTests({ }) }) - // FIXME: it seems like in Turbopack we sometimes get two instances of `workUnitAsyncStorage` -- - // `app-render` gets a second, newer instance, different from `io()`. - // Thus, `io()` gets an undefined `workUnitStore` and does nothing, so sync IO does not get tracked at all. - // This is likely caused by the same bug that breaks `/revalidate` (see other FIXME above), - // where a route crashes due to a missing `workStore`. - if (!isTurbopack) { - it('sync IO in the static phase', async () => { - const path = '/sync-io/static' + it('sync IO in the static phase', async () => { + const path = '/sync-io/static' - const assertLogs = async (browser: Playwright) => { - const logs = await browser.log() - - assertLog(logs, 'after first cache', 'Prerender') - // sync IO in the static stage errors and advances to Server. - assertLog(logs, 'after sync io', 'Server') - assertLog(logs, 'after cache read - page', 'Server') - } + const assertLogs = async (browser: Playwright) => { + const logs = await browser.log() - if (isInitialLoad) { - await testInitialLoad(path, assertLogs) - } else { - await testNavigation(path, assertLogs) - } - }) + assertLog(logs, 'after first cache', 'Prerender') + // sync IO in the static stage errors and advances to Server. + assertLog(logs, 'after sync io', 'Server') + assertLog(logs, 'after cache read - page', 'Server') + } - it('sync IO in the runtime phase', async () => { - const path = '/sync-io/runtime' + if (isInitialLoad) { + await testInitialLoad(path, assertLogs) + } else { + await testNavigation(path, assertLogs) + } + }) - const assertLogs = async (browser: Playwright) => { - const logs = await browser.log() + it('sync IO in the runtime phase', async () => { + const path = '/sync-io/runtime' - assertLog(logs, 'after first cache', 'Prerender') - assertLog(logs, 'after cookies', 'Prefetch') - if (hasRuntimePrefetch || partialPrefetching) { - // in partialPrefetching (via per-segment config or global flag), - // sync IO in the runtime stage errors and advances to Server. - assertLog(logs, 'after sync io', 'Server') - assertLog(logs, 'after cache read - page', 'Server') - } else { - // if runtime prefetching is not on, sync IO in the runtime stage does nothing. - assertLog(logs, 'after sync io', 'Prefetch') - assertLog(logs, 'after cache read - page', 'Prefetch') - } - } + const assertLogs = async (browser: Playwright) => { + const logs = await browser.log() - if (isInitialLoad) { - await testInitialLoad(path, assertLogs) + assertLog(logs, 'after first cache', 'Prerender') + assertLog(logs, 'after cookies', 'Prefetch') + if (hasRuntimePrefetch || partialPrefetching) { + // in partialPrefetching (via per-segment config or global flag), + // sync IO in the runtime stage errors and advances to Server. + assertLog(logs, 'after sync io', 'Server') + assertLog(logs, 'after cache read - page', 'Server') } else { - await testNavigation(path, assertLogs) + // if runtime prefetching is not on, sync IO in the runtime stage + // does nothing. + assertLog(logs, 'after sync io', 'Prefetch') + assertLog(logs, 'after cache read - page', 'Prefetch') } - }) - } + } + + if (isInitialLoad) { + await testInitialLoad(path, assertLogs) + } else { + await testNavigation(path, assertLogs) + } + }) }) }) } diff --git a/test/development/app-dir/cache-components-tasks/cache-components-tasks.test.ts b/test/development/app-dir/cache-components-tasks/cache-components-tasks.test.ts index 9c2415db41a8..25d3010d9b51 100644 --- a/test/development/app-dir/cache-components-tasks/cache-components-tasks.test.ts +++ b/test/development/app-dir/cache-components-tasks/cache-components-tasks.test.ts @@ -16,7 +16,7 @@ describe.each([ ])( 'cache-components-tasks - $description', ({ fixturePath, hasRuntimePrefetch }) => { - const { next, isTurbopack, isNextDev } = nextTestSetup({ + const { next } = nextTestSetup({ files: nodePath.join(__dirname, fixturePath), }) @@ -79,19 +79,6 @@ describe.each([ await retry(() => assertLogs(browser)) assertNoUnexpectedErrorsInCli() - if (isNextDev && isTurbopack) { - // FIXME: - // In Turbopack, requests to the /revalidate route seem to occasionally crash - // due to some HMR or compilation issue. `revalidatePath` throws this error: - // - // Invariant: static generation store missing in revalidatePath - // - // This is unrelated to the logic being tested here, so for now, we skip the assertions - // that require us to revalidate. - console.log('WARNING: skipping revalidation assertions in turbopack') - return - } - // After a revalidation the subsequent warmup render must discard stale // cache entries. // This should not affect the environment labels. @@ -119,19 +106,6 @@ describe.each([ await retry(() => assertLogs(browser)) assertNoUnexpectedErrorsInCli() - if (isNextDev && isTurbopack) { - // FIXME: - // In Turbopack, requests to the /revalidate route seem to occasionally crash - // due to some HMR or compilation issue. `revalidatePath` throws this error: - // - // Invariant: static generation store missing in revalidatePath - // - // This is unrelated to the logic being tested here, so for now, we skip the assertions - // that require us to revalidate. - console.log('WARNING: skipping revalidation assertions in turbopack') - return - } - // After a revalidation the subsequent warmup render must discard stale // cache entries. // This should not affect the environment labels. diff --git a/turbopack/crates/turbo-persistence/Cargo.toml b/turbopack/crates/turbo-persistence/Cargo.toml index b70c73ccf1fb..ea5d8475a679 100644 --- a/turbopack/crates/turbo-persistence/Cargo.toml +++ b/turbopack/crates/turbo-persistence/Cargo.toml @@ -14,6 +14,7 @@ verbose_log = [] [dependencies] anyhow = { workspace = true } +auto-hash-map = { workspace = true } bitfield = { workspace = true } byteorder = { workspace = true } crc32fast = { workspace = true } diff --git a/turbopack/crates/turbo-persistence/README.md b/turbopack/crates/turbo-persistence/README.md index 7934eac48162..8b979bf2dbf4 100644 --- a/turbopack/crates/turbo-persistence/README.md +++ b/turbopack/crates/turbo-persistence/README.md @@ -29,7 +29,10 @@ Therefore there are these value types: - SMALL: Values 9–4096 bytes packed into shared value blocks within `*.sst` files. - MEDIUM: Values 4097 bytes – 64 MB stored in dedicated value blocks within `*.sst` files. - BLOB: Values > 64 MB stored in separate `*.blob` files. -- DELETED: Values that are deleted. (Tombstone) +- KEY DELETED: Every value for the key is deleted. (Key tombstone) +- KEY-VALUE DELETED: Only one named key → value pair is deleted, leaving other values for the same + key intact. (Key-value tombstone) Only meaningful for `MultiValue` families; see + [Key-value tombstones](#key-value-tombstones). - Future: - MERGE: An application specific update operation that is applied on the old value. @@ -138,7 +141,7 @@ Depending on the `type` field entry has a different format: - 8 bytes key hash (if block type 1) - key data - 4 bytes sequence number -- 2: deleted key / tombstone (no data) +- 2: deleted key / key tombstone (no data) - 8 bytes key hash (if block type 1) - key data - 3: normal key (medium sized value) @@ -150,25 +153,62 @@ Depending on the `type` field entry has a different format: - 2 byte block index - 3 bytes size - 4 bytes position in block -- 8..255: inlined value (currently only values ≤8 bytes are inlined, though the format supports up to 247) +- 8..=16: inlined value, size = type - 8 (the format supports up to 247, but `MAX_INLINE_VALUE_SIZE` + currently caps it at 8) - 8 bytes key hash (if block type 1) - key data - (type - 8) bytes value data (inline, no separate value block) +- 17..=25: key-value tombstone, deleted value size = type - 17 (mirrors the inline range and shifts + with `MAX_INLINE_VALUE_SIZE`) + - 8 bytes key hash (if block type 1) + - key data + - (type - 17) bytes of the deleted value, stored inline + +Both ranged kinds are open-ended, so a decoder must test the key-value tombstone range **before** +the inline range. The entries are sorted by key hash and key. +##### Key-value tombstones + +A key-value tombstone names the exact pair to remove, so it must carry a copy of the deleted +value's bytes. Since the value lives inline in the key block and its length is encoded in the type +byte, only inline-sized values can be deleted this way — hence `MAX_INLINE_VALUE_SIZE` bounds +`delete_value`. + +The size limit is a consequence of that encoding, not of the comparison logic: matching is a plain +byte comparison and does not care how a value is stored. Supporting larger deleted values is +therefore possible but unmotivated — the tombstone stores a second copy of the value, so the cost +of deleting approaches the cost of the value itself, and reclaiming space is the whole point. + +If it is ever needed, the natural encoding is a dedicated is-tombstone bit (e.g. the top bit) on the +entry type, making "deleted" orthogonal to storage class rather than a parallel type range. That +would also collapse the current duplication where the tombstone representation mirrors the inline +one at every layer. Note that blob-backed values need a separate design: comparing against a blob +means reading it, which would put unbounded I/O in the compaction path, and blob liveness +accounting would have to handle a tombstone holding a blob reference. + #### Key Block (fixed-size) -Used when all entries in a block have the same key size and value type. Eliminates the per-entry offset table, enabling direct arithmetic indexing during binary search. +Used when all entries in a block have the same key size, and either the same value type or at least +the same value size. Eliminates the per-entry offset table, enabling direct arithmetic indexing +during binary search. - 1 byte block type (3: fixed-size with hash, 4: fixed-size without hash) - 3 bytes entry count - 1 byte key size (uniform across all entries) -- 1 byte value type (shared by all entries, same encoding as variable-size type field) +- 1 byte value type (shared by all entries, same encoding as variable-size type field), or + `FIXED_KEY_BLOCK_MIXED_VALUE_TYPE` (4) when entries share a value size but not a value type +- 1 byte value size — only present when the value type is `FIXED_KEY_BLOCK_MIXED_VALUE_TYPE` - foreach entry (packed at stride = hash_len + key_size + val_size): - 8 bytes key hash (if block type 3) - key data (key_size bytes) - - value data (size determined by value type) + - 1 byte value type — only present when the block is mixed-type + - value data (size determined by the block's or the entry's value type) + +The mixed-type form exists so that same-sized inline values and key-value tombstones can share a +fixed-size block: they have equal value sizes but different type bytes. Tag 4 is available as the +mixed marker because it is not itself a valid entry type. Entry position for index `i` is computed as `header_size + i * stride` with no indirection. The writer automatically selects fixed-size format when all entries in a block qualify; otherwise falls back to the variable-size format above. @@ -321,3 +361,7 @@ Configuration options for compactions are: - fsync! - (this also deleted enqueued files) + +## Compatibility + +Currently, the database does not support cross version compatibility. Therefore all updates should be considered breaking changes and the only approach is to rewrite the databases. diff --git a/turbopack/crates/turbo-persistence/src/bin/sst_inspect.rs b/turbopack/crates/turbo-persistence/src/bin/sst_inspect.rs index f359cb743528..b7f4c5708bac 100644 --- a/turbopack/crates/turbo-persistence/src/bin/sst_inspect.rs +++ b/turbopack/crates/turbo-persistence/src/bin/sst_inspect.rs @@ -3,12 +3,11 @@ //! This tool inspects SST files to report entry type statistics per family, //! useful for verifying that inline value optimization is being used. //! -//! Entry types: -//! - 0: Small value (stored in value block) -//! - 1: Blob reference -//! - 2: Deleted/tombstone -//! - 3: Medium value -//! - 8-255: Inline value where (type - 8) = value byte count +//! Entry types are the `KEY_BLOCK_ENTRY_TYPE_*` constants in +//! [`turbo_persistence::static_sorted_file`]; the `--help` output lists them with their current +//! values. The two ranged kinds encode a size in the type byte: an inline value's byte count is +//! `type - KEY_BLOCK_ENTRY_TYPE_INLINE_MIN`, and a key-value tombstone's deleted byte count is +//! `type - KEY_BLOCK_ENTRY_TYPE_KEY_VALUE_DELETED_MIN`. use std::{ collections::{BTreeMap, HashSet}, @@ -21,15 +20,17 @@ use fs_err::{self as fs, File}; use lzzzz::lz4::decompress; use memmap2::Mmap; use turbo_persistence::{ - BLOCK_HEADER_SIZE, checksum_block, + BLOCK_HEADER_SIZE, MAX_INLINE_VALUE_SIZE, checksum_block, meta_file::MetaFile, mmap_helper::advise_mmap_for_persistence, read_current_version, sst_filter::SstFilter, static_sorted_file::{ BLOCK_TYPE_FIXED_KEY_NO_HASH, BLOCK_TYPE_FIXED_KEY_WITH_HASH, BLOCK_TYPE_KEY_NO_HASH, - BLOCK_TYPE_KEY_WITH_HASH, KEY_BLOCK_ENTRY_TYPE_BLOB, KEY_BLOCK_ENTRY_TYPE_DELETED, - KEY_BLOCK_ENTRY_TYPE_INLINE_MIN, KEY_BLOCK_ENTRY_TYPE_MEDIUM, KEY_BLOCK_ENTRY_TYPE_SMALL, + BLOCK_TYPE_KEY_WITH_HASH, FIXED_KEY_BLOCK_MIXED_VALUE_TYPE, KEY_BLOCK_ENTRY_TYPE_BLOB, + KEY_BLOCK_ENTRY_TYPE_INLINE_MIN, KEY_BLOCK_ENTRY_TYPE_KEY_DELETED, + KEY_BLOCK_ENTRY_TYPE_KEY_VALUE_DELETED_MIN, KEY_BLOCK_ENTRY_TYPE_MEDIUM, + KEY_BLOCK_ENTRY_TYPE_SMALL, }, }; @@ -96,10 +97,11 @@ struct SstStats { /// Value sizes by type (inline values track actual bytes) inline_value_bytes: u64, - small_value_refs: u64, // Count of references to value blocks - medium_value_refs: u64, // Count of references to medium values - blob_refs: u64, // Count of blob references - deleted_count: u64, // Count of deleted entries + small_value_refs: u64, // Count of references to value blocks + medium_value_refs: u64, // Count of references to medium values + blob_refs: u64, // Count of blob references + key_deleted_count: u64, // Count of key tombstones + key_value_deleted_count: u64, // Count of key-value tombstones /// File size in bytes file_size: u64, @@ -121,7 +123,8 @@ impl SstStats { self.small_value_refs += other.small_value_refs; self.medium_value_refs += other.medium_value_refs; self.blob_refs += other.blob_refs; - self.deleted_count += other.deleted_count; + self.key_deleted_count += other.key_deleted_count; + self.key_value_deleted_count += other.key_value_deleted_count; self.file_size += other.file_size; } } @@ -144,12 +147,16 @@ fn track_entry_type(stats: &mut SstStats, entry_type: u8) { KEY_BLOCK_ENTRY_TYPE_BLOB => { stats.blob_refs += 1; } - KEY_BLOCK_ENTRY_TYPE_DELETED => { - stats.deleted_count += 1; + KEY_BLOCK_ENTRY_TYPE_KEY_DELETED => { + stats.key_deleted_count += 1; } KEY_BLOCK_ENTRY_TYPE_MEDIUM => { stats.medium_value_refs += 1; } + // Must precede the inline arm: both are open-ended and the tombstone range sits above it. + ty if ty >= KEY_BLOCK_ENTRY_TYPE_KEY_VALUE_DELETED_MIN => { + stats.key_value_deleted_count += 1; + } ty if ty >= KEY_BLOCK_ENTRY_TYPE_INLINE_MIN => { let inline_size = (ty - KEY_BLOCK_ENTRY_TYPE_INLINE_MIN) as u64; stats.inline_value_bytes += inline_size; @@ -162,8 +169,13 @@ fn entry_type_description(ty: u8) -> String { match ty { KEY_BLOCK_ENTRY_TYPE_SMALL => "small value (in value block)".to_string(), KEY_BLOCK_ENTRY_TYPE_BLOB => "blob reference".to_string(), - KEY_BLOCK_ENTRY_TYPE_DELETED => "deleted/tombstone".to_string(), + KEY_BLOCK_ENTRY_TYPE_KEY_DELETED => "key tombstone".to_string(), KEY_BLOCK_ENTRY_TYPE_MEDIUM => "medium value".to_string(), + // Must precede the inline arm: both are open-ended and the tombstone range sits above it. + ty if ty >= KEY_BLOCK_ENTRY_TYPE_KEY_VALUE_DELETED_MIN => { + let size = ty - KEY_BLOCK_ENTRY_TYPE_KEY_VALUE_DELETED_MIN; + format!("key-value tombstone ({size} byte value)") + } ty if ty >= KEY_BLOCK_ENTRY_TYPE_INLINE_MIN => { let inline_size = ty - KEY_BLOCK_ENTRY_TYPE_INLINE_MIN; format!("inline {} bytes", inline_size) @@ -368,9 +380,23 @@ fn parse_key_block_indices(index_block: &[u8]) -> HashSet { } /// Parsed header of a key block. +#[derive(Clone, Copy)] enum KeyBlockHeader { - Variable { entry_count: u32 }, - Fixed { entry_count: u32, value_type: u8 }, + Variable { + entry_count: u32, + }, + Fixed { + entry_count: u32, + value_type: u8, + }, + /// Fixed-size layout whose entries share a value size but not a value type, so each carries + /// its own type byte between its key and its value. + FixedMixedType { + entry_count: u32, + hash_len: usize, + key_size: usize, + stride: usize, + }, } /// Parses the header of a key block from the full decompressed block data. @@ -384,10 +410,28 @@ fn parse_key_block_header(block: &[u8]) -> Result { } BLOCK_TYPE_FIXED_KEY_WITH_HASH | BLOCK_TYPE_FIXED_KEY_NO_HASH => { assert!(block.len() >= 6, "Fixed key block header too small"); - Ok(KeyBlockHeader::Fixed { - entry_count, - value_type: block[5], - }) + if block[5] == FIXED_KEY_BLOCK_MIXED_VALUE_TYPE { + assert!(block.len() >= 7, "Mixed-type key block header too small"); + let hash_len = if block_type == BLOCK_TYPE_FIXED_KEY_WITH_HASH { + 8 + } else { + 0 + }; + let key_size = block[4] as usize; + let val_size = block[6] as usize; + Ok(KeyBlockHeader::FixedMixedType { + entry_count, + hash_len, + key_size, + // +1 for the per-entry type byte. + stride: hash_len + key_size + val_size + 1, + }) + } else { + Ok(KeyBlockHeader::Fixed { + entry_count, + value_type: block[5], + }) + } } _ => bail!("Invalid key block type: {block_type}"), } @@ -395,27 +439,32 @@ fn parse_key_block_header(block: &[u8]) -> Result { /// Iterates over entry type bytes in a key block. /// -/// For variable-size key blocks, reads byte 0 of each 4-byte offset table entry. -/// For fixed-size key blocks, yields the single `value_type` repeated `entry_count` times. +/// For variable-size key blocks, reads byte 0 of each 4-byte offset table entry. For fixed-size +/// key blocks, yields the single `value_type` repeated `entry_count` times, or reads the per-entry +/// type byte when the block has mixed types. fn iter_key_block_entry_types( header: KeyBlockHeader, block: &[u8], ) -> impl Iterator + '_ { - let (entry_count, fixed_type) = match header { - KeyBlockHeader::Variable { entry_count } => (entry_count, None), - KeyBlockHeader::Fixed { - entry_count, - value_type, - } => (entry_count, Some(value_type)), + let entry_count = match header { + KeyBlockHeader::Variable { entry_count } + | KeyBlockHeader::Fixed { entry_count, .. } + | KeyBlockHeader::FixedMixedType { entry_count, .. } => entry_count, }; - (0..entry_count).map(move |i| { - if let Some(vt) = fixed_type { - vt - } else { - // Variable block: offset table starts at byte 4 (after 1B type + 3B count), - // each entry is 4 bytes, first byte is the entry type. - let header_offset = KEY_BLOCK_HEADER_SIZE + i as usize * 4; - block[header_offset] + (0..entry_count).map(move |i| match header { + // Variable block: offset table starts at byte 4 (after 1B type + 3B count), + // each entry is 4 bytes, first byte is the entry type. + KeyBlockHeader::Variable { .. } => block[KEY_BLOCK_HEADER_SIZE + i as usize * 4], + KeyBlockHeader::Fixed { value_type, .. } => value_type, + KeyBlockHeader::FixedMixedType { + hash_len, + key_size, + stride, + .. + } => { + // Entry data starts after the 7-byte mixed-type header; the type byte sits between + // the entry's key and its value. + block[7 + i as usize * stride + hash_len + key_size] } }) } @@ -502,7 +551,7 @@ fn analyze_sst_file(db_path: &Path, info: &SstInfo) -> Result { raw.was_compressed, ); } - KeyBlockHeader::Fixed { .. } => { + KeyBlockHeader::Fixed { .. } | KeyBlockHeader::FixedMixedType { .. } => { stats.fixed_key_blocks.add( raw.compressed_size, raw.actual_size, @@ -645,11 +694,18 @@ fn print_value_storage(stats: &SstStats, prefix: &str) { format_number(stats.blob_refs) ); } - if stats.deleted_count > 0 { + if stats.key_deleted_count > 0 { + println!( + "{} Key tombstones: {} entries", + prefix, + format_number(stats.key_deleted_count) + ); + } + if stats.key_value_deleted_count > 0 { println!( - "{} Deleted: {} entries", + "{} Key-value tombstones: {} entries", prefix, - format_number(stats.deleted_count) + format_number(stats.key_value_deleted_count) ); } } @@ -830,14 +886,32 @@ fn main() -> Result<()> { eprintln!(" -v, --verbose Show per-SST file details (default: family totals only)"); eprintln!(); eprintln!("Entry types:"); - eprintln!(" 0: Small value (stored in separate value block)"); - eprintln!(" 1: Blob reference"); - eprintln!(" 2: Deleted/tombstone"); - eprintln!(" 3: Medium value"); - eprintln!(" 8+: Inline value (size = type - 8)"); + eprintln!( + " {KEY_BLOCK_ENTRY_TYPE_SMALL}: Small value (stored in separate value block)" + ); + eprintln!(" {KEY_BLOCK_ENTRY_TYPE_BLOB}: Blob reference"); + eprintln!( + " {KEY_BLOCK_ENTRY_TYPE_KEY_DELETED}: Key tombstone (deletes all values for the \ + key)" + ); + eprintln!(" {KEY_BLOCK_ENTRY_TYPE_MEDIUM}: Medium value"); + eprintln!( + " {KEY_BLOCK_ENTRY_TYPE_INLINE_MIN}-{}: Inline value (size = type - \ + {KEY_BLOCK_ENTRY_TYPE_INLINE_MIN})", + KEY_BLOCK_ENTRY_TYPE_INLINE_MIN + MAX_INLINE_VALUE_SIZE as u8 + ); + eprintln!( + " {KEY_BLOCK_ENTRY_TYPE_KEY_VALUE_DELETED_MIN}-{}: Key-value tombstone (deleted \ + value size = type - {KEY_BLOCK_ENTRY_TYPE_KEY_VALUE_DELETED_MIN})", + KEY_BLOCK_ENTRY_TYPE_KEY_VALUE_DELETED_MIN + MAX_INLINE_VALUE_SIZE as u8 + ); eprintln!(); eprintln!("For TaskCache (family 3), values are 4-byte TaskIds."); - eprintln!("Expected entry type is 12 (8 + 4) for inline optimization."); + eprintln!( + "Expected entry type is {} ({KEY_BLOCK_ENTRY_TYPE_INLINE_MIN} + 4) for inline \ + optimization.", + KEY_BLOCK_ENTRY_TYPE_INLINE_MIN + 4 + ); std::process::exit(1); } }; diff --git a/turbopack/crates/turbo-persistence/src/collector.rs b/turbopack/crates/turbo-persistence/src/collector.rs index 80c144494bba..7ae553e298f7 100644 --- a/turbopack/crates/turbo-persistence/src/collector.rs +++ b/turbopack/crates/turbo-persistence/src/collector.rs @@ -4,7 +4,8 @@ use crate::{ FamilyKind, ValueBuffer, collector_entry::{CollectorEntry, CollectorEntryValue, EntryKey, TINY_VALUE_THRESHOLD}, constants::{ - DATA_THRESHOLD_PER_INITIAL_FILE, MAX_ENTRIES_PER_INITIAL_FILE, MAX_SMALL_VALUE_SIZE, + DATA_THRESHOLD_PER_INITIAL_FILE, MAX_ENTRIES_PER_INITIAL_FILE, MAX_INLINE_VALUE_SIZE, + MAX_SMALL_VALUE_SIZE, }, key::{StoreKey, hash_key}, value_block_count_tracker::ValueBlockCountTracker, @@ -86,7 +87,7 @@ impl Collector { }); } - /// Adds a tombstone pair to the collector. + /// Adds a tombstone pair to the collector. This deletes *all* values for `key`. pub fn delete(&mut self, key: K) { let key = EntryKey { hash: hash_key(&key), @@ -95,7 +96,39 @@ impl Collector { self.total_key_size += key.len(); self.entries.push(CollectorEntry { key, - value: CollectorEntryValue::Deleted, + value: CollectorEntryValue::KeyDeleted, + }); + } + + /// Adds a key-value tombstone to the collector: deletes only the single `key` -> `value` pair, + /// leaving any other values for `key` intact. + /// + /// Only meaningful for [`FamilyKind::MultiValue`] families, where a key can map to several + /// values and [`Collector::delete`] is too coarse: it would drop the unrelated values too. + /// The motivating case is the task cache, which is keyed by a hash and so holds more than one + /// value whenever two tasks collide. Removing one task must leave the colliding task's entry + /// readable, which requires naming the exact pair to delete. + /// + /// Deleting a pair written in the same batch is not supported; see + /// [`WriteBatch::delete_value`][crate::WriteBatch]. + /// + /// `value` must be at most [`MAX_INLINE_VALUE_SIZE`] bytes; callers must validate this. Larger + /// values could be supported in the future but there is currently no usecase. + pub fn delete_value(&mut self, key: K, value: &[u8]) { + debug_assert!(value.len() <= MAX_INLINE_VALUE_SIZE); + let key = EntryKey { + hash: hash_key(&key), + data: key, + }; + self.total_key_size += key.len(); + let mut data = [0u8; MAX_INLINE_VALUE_SIZE]; + data[..value.len()].copy_from_slice(value); + self.entries.push(CollectorEntry { + key, + value: CollectorEntryValue::KeyValueDeleted { + value: data, + len: value.len() as u8, + }, }); } @@ -110,19 +143,19 @@ impl Collector { self.entries.push(entry); } - /// Sorts entries by key. Tombstones are placed last within each key group. + /// Sorts entries by key. Within a key group, key-value tombstones are placed first and key + /// tombstones last (see [`CollectorEntryValue::sort_rank`]). /// This method does not deduplicate entries. /// /// In debug builds, asserts that SingleValue families have no duplicate keys. pub fn sorted(&mut self, family_kind: FamilyKind) -> (&[CollectorEntry], usize) { - // Sort by (hash, key) with tombstones placed last within each key group. // We can use unstable sort because the relative order of equal elements // doesn't matter — duplicates are either disallowed (SingleValue) or // allowed without deduplication (MultiValue). self.entries.sort_unstable_by(|a, b| { a.key .cmp(&b.key) - .then_with(|| a.value.is_deleted().cmp(&b.value.is_deleted())) + .then_with(|| a.value.sort_rank().cmp(&b.value.sort_rank())) }); #[cfg(debug_assertions)] diff --git a/turbopack/crates/turbo-persistence/src/collector_entry.rs b/turbopack/crates/turbo-persistence/src/collector_entry.rs index 13e7a2ee26e5..19de3e849bb3 100644 --- a/turbopack/crates/turbo-persistence/src/collector_entry.rs +++ b/turbopack/crates/turbo-persistence/src/collector_entry.rs @@ -32,17 +32,24 @@ pub enum CollectorEntryValue { Large { blob: u32, }, - Deleted, + KeyDeleted, + /// Key-value tombstone: deletes only this one value from the key's group. MultiValue only. + /// The deleted value is stored inline, so it is capped at [`MAX_INLINE_VALUE_SIZE`]. + KeyValueDeleted { + value: [u8; MAX_INLINE_VALUE_SIZE], + len: u8, + }, } impl CollectorEntryValue { pub fn len(&self) -> usize { match self { - CollectorEntryValue::Tiny { len, .. } => *len as usize, + CollectorEntryValue::KeyValueDeleted { len, .. } + | CollectorEntryValue::Tiny { len, .. } => *len as usize, CollectorEntryValue::Small { value } => value.len(), CollectorEntryValue::Medium { value } => value.len(), CollectorEntryValue::Large { blob: _ } => 0, - CollectorEntryValue::Deleted => 0, + CollectorEntryValue::KeyDeleted => 0, } } @@ -51,6 +58,22 @@ impl CollectorEntryValue { matches!(self, CollectorEntryValue::Medium { .. }) } + /// The value bytes, or `None` for variants that carry no value data of their own (blob + /// references and key tombstones). + #[cfg(feature = "verify_sst_content")] + pub fn as_bytes(&self) -> Option<&[u8]> { + match self { + // Separate arms: the inline buffers have different sizes, so they cannot be bound by + // a single or-pattern. + CollectorEntryValue::Tiny { value, len } => Some(&value[..*len as usize]), + CollectorEntryValue::KeyValueDeleted { value, len } => Some(&value[..*len as usize]), + CollectorEntryValue::Small { value } | CollectorEntryValue::Medium { value } => { + Some(value) + } + CollectorEntryValue::Large { .. } | CollectorEntryValue::KeyDeleted => None, + } + } + /// Returns the value size if it will be packed into a small value block, or 0 otherwise. pub fn small_value_size(&self) -> usize { match self { @@ -62,9 +85,21 @@ impl CollectorEntryValue { } } - /// Returns true if this value is a deletion tombstone. - pub fn is_deleted(&self) -> bool { - matches!(self, CollectorEntryValue::Deleted) + /// Sort rank within a key group. The two tombstone kinds sit at opposite ends: + /// + /// - Key-value tombstones (rank 0) go **first**, so a reader collects them before the values + /// they filter and can apply them in one forward pass. + /// - Values (rank 1) go in the middle. + /// - Key tombstones (rank 2) go **last**, because they shadow only entries older than + /// themselves — including entries in this same SST. A batch doing `put(A); delete; put(B)` + /// must keep A and B, so a reader that stops at the first key tombstone it sees still returns + /// the same-batch values it already collected. + pub fn sort_rank(&self) -> u8 { + match self { + CollectorEntryValue::KeyValueDeleted { .. } => 0, + CollectorEntryValue::KeyDeleted => 2, + _ => 1, + } } } @@ -133,7 +168,10 @@ impl Entry for CollectorEntry { } CollectorEntryValue::Medium { value } => EntryValue::Medium { value }, CollectorEntryValue::Large { blob } => EntryValue::Large { blob: *blob }, - CollectorEntryValue::Deleted => EntryValue::Deleted, + CollectorEntryValue::KeyDeleted => EntryValue::KeyDeleted, + CollectorEntryValue::KeyValueDeleted { value, len } => EntryValue::KeyValueDeleted { + value: &value[..*len as usize], + }, } } } diff --git a/turbopack/crates/turbo-persistence/src/db.rs b/turbopack/crates/turbo-persistence/src/db.rs index afd60c8c8a54..20e55ebbe944 100644 --- a/turbopack/crates/turbo-persistence/src/db.rs +++ b/turbopack/crates/turbo-persistence/src/db.rs @@ -2,6 +2,7 @@ use std::{ borrow::Cow, collections::HashSet, fmt::Display, + hash::BuildHasherDefault, io::{BufWriter, ErrorKind, Write}, mem::take, ops::RangeInclusive, @@ -13,6 +14,7 @@ use std::{ }; use anyhow::{Context, Result, bail}; +use auto_hash_map::AutoSet; use byteorder::{BE, ReadBytesExt, WriteBytesExt}; use dashmap::DashSet; use fs_err::{self as fs, File, OpenOptions, ReadDir}; @@ -20,6 +22,7 @@ use jiff::Timestamp; use memmap2::Mmap; use nohash_hasher::BuildNoHashHasher; use parking_lot::{Mutex, RwLock}; +use rustc_hash::FxHasher; use serde::{Deserialize, Serialize}; use smallvec::SmallVec; use tracing::span::EnteredSpan; @@ -1524,6 +1527,7 @@ impl TurboPersistence .parallel_scheduler .parallel_map_collect_owned::<_, _, Result>>(merge_jobs, |indices| { let _span = span.clone().entered(); + if indices.len() == 1 { // If we only have one file, we can just move it let index = indices[0]; @@ -1547,6 +1551,39 @@ impl TurboPersistence }); } + // A tombstone is dead if no older SST contains a matching key. + // Returns `true`` if the tombstone is definitely dead (no false + // positives), if `false` is returned then the tomstone is only likely + // to be alive since the amqf may have false positive match for the + let tombstone_is_dead = { + // Filters of every SST older than this job. + // + // A tombstone only suppresses values older than itself, and within + // the job `MergeIter` yields + // newest-first so the loop below already drops + // those. What remains is everything older than the job's oldest + // member. + let oldest_index_in_job = indices + .iter() + .copied() + .min() + .expect("merge jobs are not empty"); + let older_filters = ssts_with_ranges[..oldest_index_in_job] + .iter() + .map(|sst| { + let entry = + meta_files[sst.meta_index].entry(sst.index_in_meta); + (entry.min_hash(), entry.max_hash(), entry.amqf()) + }) + .collect::>(); + move |hash: u64| { + !older_filters.iter().any(|(min, max, amqf)| { + hash >= *min + && hash <= *max + && amqf.contains_fingerprint(hash) + }) + } + }; // Open SST files independently for compaction. // Uses MADV_SEQUENTIAL for better OS page management // and avoids caching mmaps on MetaEntry's OnceLock. @@ -1675,6 +1712,13 @@ impl TurboPersistence // - MultiValue: skip all older entries after encountering a tombstone // (which signals deletion of all prior values for this key) let mut skip_remaining_for_this_key = false; + // Values deleted by key-value tombstones in the current key group. + // Reset at each key boundary. + let mut deleted_values_for_this_key: AutoSet< + RcBytes, + BuildHasherDefault, + 1, + > = AutoSet::default(); let family_config = &self.config.family_configs[family as usize]; for entry in iter { @@ -1682,8 +1726,27 @@ impl TurboPersistence if current_key.as_ref() != Some(&entry.key) { // we changed keys so undo this flag skip_remaining_for_this_key = false; + deleted_values_for_this_key.clear(); current_key = Some(entry.key.clone()); } + // Key-value tombstones sort first within a group, so each is + // recorded before the values it might delete. + // See: `crate::collector_entry::sort_rank` + if let IterValue::KeyValueDeleted { value } = &entry.value { + deleted_values_for_this_key.insert(value.clone()); + // Applied to this job's values above; keep it only if an SST + // outside the job could still hold a matching key. + if tombstone_is_dead(entry.hash) { + continue; + } + } else if !deleted_values_for_this_key.is_empty() + // Deleted values cannot match blobs, just normal payloads. + && let IterValue::Slice { value } = &entry.value + && deleted_values_for_this_key.contains(value) + { + // Deleted by a key-value tombstone seen earlier in this group. + continue; + } if !skip_remaining_for_this_key { let is_used = used_key_hashes .as_ref() @@ -1696,8 +1759,9 @@ impl TurboPersistence match family_config.kind { FamilyKind::MultiValue => { // For MultiValue families we only skip remaining if we - // see a tombstone - if matches!(entry.value, IterValue::Deleted) { + // see a key tombstone. Key-value tombstones are + // handled above and never reach here. + if matches!(entry.value, IterValue::KeyDeleted) { skip_remaining_for_this_key = true; } } @@ -1707,6 +1771,12 @@ impl TurboPersistence skip_remaining_for_this_key = true; } } + // If this is a tombstone, see if we need to retain it or not. + if matches!(entry.value, IterValue::KeyDeleted) + && tombstone_is_dead(entry.hash) + { + continue; + } collector.add_entry( entry, path, @@ -1944,6 +2014,12 @@ impl TurboPersistence #[cfg(feature = "stats")] let mut found_in_sst = false; + // Values deleted by key-value tombstones seen so far. Because we walk meta files newest + // first, and tombstones sort first within a key group, every tombstone that could apply to + // a value has already been seen by the time we reach that value. + let mut deleted_values: AutoSet, 1> = + AutoSet::default(); + let mut size = 0; for meta in inner.meta_files.iter().rev() { @@ -1973,20 +2049,18 @@ impl TurboPersistence found_in_sst = true; } inner.accessed_key_hashes[family].insert(hash); - // Process values. Tombstones sort last within a key group, - // so when we see a tombstone, we can return immediately. for value in values { match value { - LookupValue::Deleted => { + LookupValue::KeyDeleted => { #[cfg(feature = "stats")] self.stats.hits_deleted.fetch_add(1, Ordering::Relaxed); if !FIND_ALL { span.record("result_size", "deleted"); return Ok(SmallVec::new()); } - // Tombstone is last in key group. Return accumulated - // values (from this SST and newer layers). Stop - // searching older SSTs. + // A key tombstone deletes every older value for this + // key. Return what we accumulated from this SST and newer + // layers and stop searching older SSTs. if output.is_empty() { span.record("result_size", "deleted"); } else { @@ -1994,9 +2068,19 @@ impl TurboPersistence } return Ok(output); } + LookupValue::KeyValueDeleted { value } => { + #[cfg(feature = "stats")] + self.stats.hits_deleted.fetch_add(1, Ordering::Relaxed); + // Cannot terminate the search: older layers may hold other + // values for the same key. + deleted_values.insert(value); + } LookupValue::Slice { value } => { #[cfg(feature = "stats")] self.stats.hits_small.fetch_add(1, Ordering::Relaxed); + if deleted_values.contains(&value) { + continue; + } if !FIND_ALL { span.record("result_size", value.len()); return Ok(SmallVec::from_buf([value])); @@ -2008,6 +2092,9 @@ impl TurboPersistence #[cfg(feature = "stats")] self.stats.hits_blob.fetch_add(1, Ordering::Relaxed); let blob = self.read_blob(sequence_number)?; + if deleted_values.iter().any(|d| **d == *blob) { + continue; + } if !FIND_ALL { span.record("result_size", blob.len()); return Ok(SmallVec::from_buf([blob])); @@ -2120,12 +2207,20 @@ impl TurboPersistence if let Some(result) = result { inner.accessed_key_hashes[family].insert(hash); let result = match result { - LookupValue::Deleted => { + LookupValue::KeyDeleted => { #[cfg(feature = "stats")] self.stats.hits_deleted.fetch_add(1, Ordering::Relaxed); deleted += 1; None } + LookupValue::KeyValueDeleted { .. } => { + // Key-value tombstones are only written to MultiValue families, and + // `batch_get` rejects those above. + bail!( + "unexpected key-value tombstone in SingleValue family {}", + self.config.family_configs[family].name + ) + } LookupValue::Slice { value } => { #[cfg(feature = "stats")] self.stats.hits_small.fetch_add(1, Ordering::Relaxed); diff --git a/turbopack/crates/turbo-persistence/src/lib.rs b/turbopack/crates/turbo-persistence/src/lib.rs index ebc5e52f1daa..fa0e9bd484e7 100644 --- a/turbopack/crates/turbo-persistence/src/lib.rs +++ b/turbopack/crates/turbo-persistence/src/lib.rs @@ -75,6 +75,9 @@ impl Default for DbConfig { } } } +/// The largest value that [`WriteBatch::delete_value`] can delete, since the tombstone stores +/// a copy of the value inline. +pub use constants::MAX_INLINE_VALUE_SIZE; pub use key::{KeyBase, QueryKey, StoreKey, hash_key}; pub use meta_file::MetaEntryFlags; pub use parallel_scheduler::{ParallelScheduler, SerialScheduler}; diff --git a/turbopack/crates/turbo-persistence/src/lookup_entry.rs b/turbopack/crates/turbo-persistence/src/lookup_entry.rs index 108625ed872c..a7fa17544ac3 100644 --- a/turbopack/crates/turbo-persistence/src/lookup_entry.rs +++ b/turbopack/crates/turbo-persistence/src/lookup_entry.rs @@ -11,7 +11,10 @@ use crate::{ #[derive(PartialEq)] pub enum LookupValue { /// The value was deleted. - Deleted, + KeyDeleted, + /// A single value was deleted from this key's group (MultiValue families only). Other values + /// for the same key are unaffected. The bytes are the deleted value. + KeyValueDeleted { value: B }, /// The value is stored in the SST file. /// /// The bytes will be pointing either at a keyblock or a value block in the SST @@ -24,7 +27,9 @@ pub enum LookupValue { /// non-atomic refcounting). pub enum IterValue { /// The value was deleted. - Deleted, + KeyDeleted, + /// A single value was deleted from this key's group (MultiValue families only). + KeyValueDeleted { value: RcBytes }, /// The value is stored in the SST file. Slice { value: RcBytes }, /// The value is stored in a blob file. @@ -39,7 +44,8 @@ pub enum IterValue { impl From> for IterValue { fn from(v: LookupValue) -> Self { match v { - LookupValue::Deleted => IterValue::Deleted, + LookupValue::KeyDeleted => IterValue::KeyDeleted, + LookupValue::KeyValueDeleted { value } => IterValue::KeyValueDeleted { value }, LookupValue::Slice { value } => IterValue::Slice { value }, LookupValue::Blob { sequence_number } => IterValue::Blob { sequence_number }, } @@ -70,7 +76,8 @@ impl Entry for LookupEntry { fn value(&self) -> EntryValue<'_> { match &self.value { - IterValue::Deleted => EntryValue::Deleted, + IterValue::KeyDeleted => EntryValue::KeyDeleted, + IterValue::KeyValueDeleted { value } => EntryValue::KeyValueDeleted { value }, IterValue::Slice { value } => { if value.len() <= MAX_INLINE_VALUE_SIZE { EntryValue::Inline { value } diff --git a/turbopack/crates/turbo-persistence/src/meta_file.rs b/turbopack/crates/turbo-persistence/src/meta_file.rs index dbde5538cf3b..5a8ba6ff0cf2 100644 --- a/turbopack/crates/turbo-persistence/src/meta_file.rs +++ b/turbopack/crates/turbo-persistence/src/meta_file.rs @@ -49,6 +49,9 @@ impl Display for MetaEntryFlags { } } +/// Magic number identifying a `.meta` file. +pub(crate) const META_FILE_MAGIC: u32 = 0xFE4ADA4A; + /// On-disk layout of a single entry header in the `.meta` file. /// /// Fields are big-endian to match the existing wire format written by [`MetaFileBuilder`]. @@ -139,6 +142,10 @@ impl MetaEntry { self.amqf_data_offset.end - self.amqf_data_offset.start } + pub fn amqf(&self) -> &qfilter::FilterRef<'static> { + &self.amqf + } + /// Returns the raw serialized AMQF bytes from the mmap. pub fn raw_amqf<'l>(&self, amqf_data: &'l [u8]) -> &'l [u8] { &amqf_data[self.amqf_data_offset.start as usize..self.amqf_data_offset.end as usize] @@ -276,7 +283,7 @@ impl MetaFile { // Parse the header from the mmap via ReadBytesExt on &[u8]. let mut reader: &[u8] = &mmap; let magic = reader.read_u32::()?; - if magic != 0xFE4ADA4A { + if magic != META_FILE_MAGIC { bail!("Invalid magic number"); } let family = reader.read_u32::()?; @@ -480,10 +487,12 @@ impl MetaFile { // Return immediately with the first result return Ok(MetaLookupResult::SstLookup(SstLookupResult::Found(values))); } - // Check for tombstone — stops search across older SSTs within this meta file. - // Since tombstones sort last within a key group, if the last value is Deleted, - // we have a tombstone. - let has_tombstone = values.last().is_some_and(|v| *v == LookupValue::Deleted); + // A key tombstone stops the search across older SSTs within this meta file. + // It sorts last within a key group, so it is the last value if present. + // Key-value tombstones do not stop the search: they delete a single value, + // and older SSTs may hold others for this key. + let has_tombstone = + values.last().is_some_and(|v| *v == LookupValue::KeyDeleted); all_results.extend(values); if has_tombstone { return Ok(MetaLookupResult::SstLookup(SstLookupResult::Found( diff --git a/turbopack/crates/turbo-persistence/src/meta_file_builder.rs b/turbopack/crates/turbo-persistence/src/meta_file_builder.rs index 7d7d8c244c56..af2baaae2b45 100644 --- a/turbopack/crates/turbo-persistence/src/meta_file_builder.rs +++ b/turbopack/crates/turbo-persistence/src/meta_file_builder.rs @@ -9,7 +9,10 @@ use fs_err::File; use qfilter::Filter; use zerocopy::IntoBytes; -use crate::{meta_file::EntryHeader, static_sorted_file_builder::StaticSortedFileBuilderMeta}; +use crate::{ + meta_file::{EntryHeader, META_FILE_MAGIC}, + static_sorted_file_builder::StaticSortedFileBuilderMeta, +}; pub struct MetaFileBuilder<'a> { family: u32, @@ -54,7 +57,7 @@ impl<'a> MetaFileBuilder<'a> { // Wrap the writer to count the bytes written, so callers can accumulate written-byte totals // without stat'ing the file afterwards. let mut file = CountingWriter::new(BufWriter::new(File::create(file)?)); - file.write_u32::(0xFE4ADA4A)?; // Magic number + file.write_u32::(META_FILE_MAGIC)?; // Magic number file.write_u32::(self.family)?; self.obsolete_sst_files.sort(); diff --git a/turbopack/crates/turbo-persistence/src/rc_bytes.rs b/turbopack/crates/turbo-persistence/src/rc_bytes.rs index f9f2097cf652..234ddb828f14 100644 --- a/turbopack/crates/turbo-persistence/src/rc_bytes.rs +++ b/turbopack/crates/turbo-persistence/src/rc_bytes.rs @@ -1,6 +1,7 @@ use std::{ borrow::Borrow, fmt::{self, Debug, Formatter}, + hash::{Hash, Hasher}, ops::{Deref, Range}, rc::Rc, }; @@ -75,6 +76,12 @@ impl Debug for RcBytes { impl Eq for RcBytes {} +impl Hash for RcBytes { + fn hash(&self, state: &mut H) { + Hash::hash(self.deref(), state); + } +} + impl SharedBytes for RcBytes { type MmapHandle = Rc; diff --git a/turbopack/crates/turbo-persistence/src/static_sorted_file.rs b/turbopack/crates/turbo-persistence/src/static_sorted_file.rs index 4bd1a233a3c8..93c3985fc55b 100644 --- a/turbopack/crates/turbo-persistence/src/static_sorted_file.rs +++ b/turbopack/crates/turbo-persistence/src/static_sorted_file.rs @@ -40,16 +40,30 @@ pub const BLOCK_TYPE_FIXED_KEY_WITH_HASH: u8 = 3; /// The block header for a fixed-size key block without hash. pub const BLOCK_TYPE_FIXED_KEY_NO_HASH: u8 = 4; +/// Written in a fixed-size key block header's value type field when entries share a value size but +/// not a value type. Each entry then carries its own type byte ahead of its value. +pub const FIXED_KEY_BLOCK_MIXED_VALUE_TYPE: u8 = 4; + /// The tag for a small-sized value. pub const KEY_BLOCK_ENTRY_TYPE_SMALL: u8 = 0; /// The tag for the blob value. pub const KEY_BLOCK_ENTRY_TYPE_BLOB: u8 = 1; -/// The tag for the deleted value. -pub const KEY_BLOCK_ENTRY_TYPE_DELETED: u8 = 2; +/// The tag for the deleted value. This is a *key* tombstone: it deletes every value for the key. +pub const KEY_BLOCK_ENTRY_TYPE_KEY_DELETED: u8 = 2; /// The tag for a medium-sized value. pub const KEY_BLOCK_ENTRY_TYPE_MEDIUM: u8 = 3; /// The minimum tag for inline values. The actual size is (tag - INLINE_MIN). pub const KEY_BLOCK_ENTRY_TYPE_INLINE_MIN: u8 = 8; +/// The minimum tag for a key-value tombstone, which deletes only the one value it carries and +/// leaves other values for the same key intact. Only meaningful for +/// [`FamilyKind::MultiValue`][crate::FamilyKind::MultiValue] families. +/// +/// This mirrors the inline value range: the deleted value is stored inline in the key block and +/// its size is (tag - KEY_VALUE_DELETED_MIN). Only inline-sized values can be deleted this way — +/// a tombstone for a larger value would have to store a second copy of it, costing more than the +/// value it reclaims. +pub const KEY_BLOCK_ENTRY_TYPE_KEY_VALUE_DELETED_MIN: u8 = + KEY_BLOCK_ENTRY_TYPE_INLINE_MIN + MAX_INLINE_VALUE_SIZE as u8 + 1; /// Encoded size of a small value reference: 2B block index + 2B size + 4B offset. pub(crate) const SMALL_VALUE_REF_SIZE: usize = 8; @@ -58,12 +72,13 @@ pub(crate) const MEDIUM_VALUE_REF_SIZE: usize = 2; /// Encoded size of a blob value reference: 4B blob id. pub(crate) const BLOB_VALUE_REF_SIZE: usize = 4; /// Encoded size of a deleted (tombstone) value reference. -pub(crate) const DELETED_VALUE_REF_SIZE: usize = 0; +pub(crate) const KEY_DELETED_REF_SIZE: usize = 0; -// Static assertion: MAX_INLINE_VALUE_SIZE must fit in the key type encoding. -// Key types 8-255 encode inline values of size 0-247, so max is 255 - 8 = 247. +// Static assertion: both the inline range and the key-value tombstone range that follows it must +// fit in the key type byte. The tombstone range starts after the inline range and is the same +// width, so the tombstone range's top is the binding constraint. const _: () = assert!( - MAX_INLINE_VALUE_SIZE <= (u8::MAX - KEY_BLOCK_ENTRY_TYPE_INLINE_MIN) as usize, + MAX_INLINE_VALUE_SIZE <= (u8::MAX - KEY_BLOCK_ENTRY_TYPE_KEY_VALUE_DELETED_MIN) as usize, "MAX_INLINE_VALUE_SIZE exceeds what can be encoded in key type byte" ); @@ -369,19 +384,21 @@ impl StaticSortedFile { ensure!(block.len() >= 6, "fixed key block too short"); let entry_count = be::read_u24(&block[1..]) as usize; let key_size = be::read_u8(&block[4..]) as usize; - let value_type = be::read_u8(&block[5..]); - let val_size = entry_val_size(value_type)?; + let header_type = be::read_u8(&block[5..]); + let FixedValueLayout { + value_type, + val_size, + header_size, + } = fixed_value_layout(&block, header_type)?; let stride = hash_len as usize + key_size + val_size; - let entries = &block[6..]; + let entries = &block[header_size..]; ensure!( entries.len() == entry_count * stride, "fixed key block for {entry_count} entries must is the wrong size" ); self.lookup_block_inner::(&block, entry_count, key_hash, key, reader, |i| { - Ok(get_fixed_key_entry( - entries, i, hash_len, key_size, value_type, stride, - )) + get_fixed_key_entry(entries, i, hash_len, key_size, value_type, stride) }) } @@ -422,10 +439,9 @@ impl StaticSortedFile { return Ok(SstLookupResult::Found(SmallVec::from_buf([result]))); } // FIND_ALL (MultiValue) mode: collect all values for this key. - // Tombstones (Deleted) sort last within each key group, so we - // scan backward to find the start of the key group, then forward - // to collect all entries. The tombstone, if present, will be the - // last entry in the results. + // Within a key group, key-value tombstones sort first and key tombstones + // last. We scan backward to find the start of the key group, then forward to + // collect all entries. let mut results = SmallVec::new(); for i in (l..m).rev() { let GetKeyEntryResult { @@ -439,12 +455,10 @@ impl StaticSortedFile { } results.push(self.handle_key_match(ty, val, block, reader)?); } - // Technically we could `.reverse()` the items collected by the backwards - // scan, but the only ordering constraint we need to maintain for single - // sst multivalue reads is that a deleted token, if it exists comes last. - // Because all the backwards scan items are strictly before the found item - // we know they don't contain the _last_ item. So we don't care about - // their order. + // Restore on-disk order: callers depend on both ends of the key group, with + // key-value tombstones preceding the values they filter and a key tombstone + // landing last. + results.reverse(); // Add the entry at `m` results.push(self.handle_key_match(ty, val, block, reader)?); @@ -712,7 +726,14 @@ fn handle_key_match_generic( let sequence_number = be::read_u32(val); LookupValue::Blob { sequence_number } } - KEY_BLOCK_ENTRY_TYPE_DELETED => LookupValue::Deleted, + KEY_BLOCK_ENTRY_TYPE_KEY_DELETED => LookupValue::KeyDeleted, + // Must precede the inline arm: both are open-ended and the tombstone range sits above it. + ty if ty >= KEY_BLOCK_ENTRY_TYPE_KEY_VALUE_DELETED_MIN => { + // The deleted value is stored inline, so `val` is already the correct slice. + // SAFETY: val points into key_block's data + let value = unsafe { key_block.slice_from_subslice(val) }; + LookupValue::KeyValueDeleted { value } + } _ => { // Inline value — val is already the correct slice // SAFETY: val points into key_block's data @@ -747,11 +768,12 @@ pub struct StaticSortedFileIter { enum CurrentKeyBlockKind { /// Variable-size entries with an offset table for random access. Variable { offsets: RcBytes, hash_len: u8 }, - /// Fixed-size entries with uniform key size and value type (no offset table). + /// Fixed-size entries with uniform key size and value size (no offset table). Fixed { hash_len: u8, key_size: usize, - value_type: u8, + /// The type shared by every entry, or `None` if each entry carries its own type byte. + value_type: Option, stride: usize, }, } @@ -865,11 +887,13 @@ impl StaticSortedFileIter { 0 }; let key_size = data[4] as usize; - let value_type = data[5]; - let val_size = entry_val_size(value_type)?; + let FixedValueLayout { + value_type, + val_size, + header_size, + } = fixed_value_layout(data, data[5])?; let stride = hash_len as usize + key_size + val_size; - // Header is 6 bytes for fixed-size blocks - let entries_range = 6..block.len(); + let entries_range = header_size..block.len(); let entries = block.slice(entries_range); Ok(CurrentKeyBlock { kind: CurrentKeyBlockKind::Fixed { @@ -912,7 +936,7 @@ impl StaticSortedFileIter { *key_size, *value_type, *stride, - ), + )?, }; let full_hash = if hash.is_empty() { crate::key::hash_key(&key) @@ -1017,7 +1041,11 @@ fn entry_val_size(ty: u8) -> Result { KEY_BLOCK_ENTRY_TYPE_SMALL => Ok(SMALL_VALUE_REF_SIZE), KEY_BLOCK_ENTRY_TYPE_MEDIUM => Ok(MEDIUM_VALUE_REF_SIZE), KEY_BLOCK_ENTRY_TYPE_BLOB => Ok(BLOB_VALUE_REF_SIZE), - KEY_BLOCK_ENTRY_TYPE_DELETED => Ok(DELETED_VALUE_REF_SIZE), + KEY_BLOCK_ENTRY_TYPE_KEY_DELETED => Ok(KEY_DELETED_REF_SIZE), + // Must precede the inline arm: both are open-ended and the tombstone range sits above it. + ty if ty >= KEY_BLOCK_ENTRY_TYPE_KEY_VALUE_DELETED_MIN => { + Ok((ty - KEY_BLOCK_ENTRY_TYPE_KEY_VALUE_DELETED_MIN) as usize) + } ty if ty >= KEY_BLOCK_ENTRY_TYPE_INLINE_MIN => { Ok((ty - KEY_BLOCK_ENTRY_TYPE_INLINE_MIN) as usize) } @@ -1067,20 +1095,58 @@ fn get_key_entry<'l>( /// /// All entries have the same key size and value type, so positions are computed /// arithmetically with no offset table indirection. +/// How a fixed-size key block encodes its entry values, decoded from the block header. +struct FixedValueLayout { + /// The type shared by every entry, or `None` if each entry carries its own type byte. + value_type: Option, + /// Value bytes per entry, including any per-entry type byte. + val_size: usize, + /// Total header size, which the entry data follows. + header_size: usize, +} + +/// Decodes the value layout from a fixed-size key block header. +fn fixed_value_layout(block: &[u8], header_type: u8) -> Result { + if header_type == FIXED_KEY_BLOCK_MIXED_VALUE_TYPE { + // Mixed-type block: the value size follows the header's type byte, and each entry + // carries its own type. + ensure!(block.len() >= 7, "mixed-type fixed key block too short"); + Ok(FixedValueLayout { + value_type: None, + // +1 for the per-entry type byte, which is part of the stride. + val_size: be::read_u8(&block[6..]) as usize + 1, + header_size: 7, + }) + } else { + Ok(FixedValueLayout { + value_type: Some(header_type), + val_size: entry_val_size(header_type)?, + header_size: 6, + }) + } +} + fn get_fixed_key_entry<'l>( entries: &'l [u8], index: usize, hash_len: u8, key_size: usize, - value_type: u8, + value_type: Option, stride: usize, -) -> GetKeyEntryResult<'l> { +) -> Result> { let hash_len_usize = hash_len as usize; let start = index * stride; - GetKeyEntryResult { - hash: &entries[start..start + hash_len_usize], - key: &entries[start + hash_len_usize..start + hash_len_usize + key_size], - ty: value_type, - val: &entries[start + hash_len_usize + key_size..(index + 1) * stride], - } + let key_start = start + hash_len_usize; + let key_end = key_start + key_size; + // In a mixed-type block the entry's type byte sits between its key and its value. + let (ty, val_start) = match value_type { + Some(ty) => (ty, key_end), + None => (be::read_u8(&entries[key_end..]), key_end + 1), + }; + Ok(GetKeyEntryResult { + hash: &entries[start..key_start], + key: &entries[key_start..key_end], + ty, + val: &entries[val_start..(index + 1) * stride], + }) } diff --git a/turbopack/crates/turbo-persistence/src/static_sorted_file_builder.rs b/turbopack/crates/turbo-persistence/src/static_sorted_file_builder.rs index 66be2750686f..f4e25667622f 100644 --- a/turbopack/crates/turbo-persistence/src/static_sorted_file_builder.rs +++ b/turbopack/crates/turbo-persistence/src/static_sorted_file_builder.rs @@ -15,9 +15,11 @@ use crate::{ meta_file::MetaEntryFlags, static_sorted_file::{ BLOB_VALUE_REF_SIZE, BLOCK_TYPE_FIXED_KEY_NO_HASH, BLOCK_TYPE_FIXED_KEY_WITH_HASH, - BLOCK_TYPE_INDEX, BLOCK_TYPE_KEY_NO_HASH, BLOCK_TYPE_KEY_WITH_HASH, DELETED_VALUE_REF_SIZE, - KEY_BLOCK_ENTRY_TYPE_BLOB, KEY_BLOCK_ENTRY_TYPE_DELETED, KEY_BLOCK_ENTRY_TYPE_INLINE_MIN, - KEY_BLOCK_ENTRY_TYPE_MEDIUM, KEY_BLOCK_ENTRY_TYPE_SMALL, MEDIUM_VALUE_REF_SIZE, + BLOCK_TYPE_INDEX, BLOCK_TYPE_KEY_NO_HASH, BLOCK_TYPE_KEY_WITH_HASH, + FIXED_KEY_BLOCK_MIXED_VALUE_TYPE, KEY_BLOCK_ENTRY_TYPE_BLOB, + KEY_BLOCK_ENTRY_TYPE_INLINE_MIN, KEY_BLOCK_ENTRY_TYPE_KEY_DELETED, + KEY_BLOCK_ENTRY_TYPE_KEY_VALUE_DELETED_MIN, KEY_BLOCK_ENTRY_TYPE_MEDIUM, + KEY_BLOCK_ENTRY_TYPE_SMALL, KEY_DELETED_REF_SIZE, MEDIUM_VALUE_REF_SIZE, SMALL_VALUE_REF_SIZE, }, }; @@ -65,6 +67,12 @@ const MIN_KEY_SIZE_FOR_COMPRESSION: usize = 16; /// fall back to variable-size layout. const MAX_FIXED_KEY_LEN: usize = u8::MAX as usize; +/// Maximum value size that can use fixed-size key block layout. +/// +/// Mixed-type fixed blocks store the value size in a single header byte, since it can no longer be +/// derived from a single shared entry type. +const MAX_FIXED_VAL_SIZE: usize = u8::MAX as usize; + /// Newtype for the key block entry type byte. /// /// This encodes what kind of value reference an entry has (small, medium, blob, deleted, or @@ -74,33 +82,46 @@ struct EntryType(u8); /// Tracks whether a key block's entries are uniform enough for fixed-size layout. /// +/// Fixed layout needs a uniform *stride*, which requires a uniform key length and a uniform value +/// size. A uniform value *type* is a stronger condition that additionally lets the type be hoisted +/// into the block header; when types differ but sizes agree, the type is stored per entry instead +/// (1 byte, still cheaper than the 4-byte offset table entry a variable block would need). +/// /// State transitions: -/// - `Unknown` → first entry → `Fixed { key_len, value_type }` -/// - `Fixed` + matching entry → stays `Fixed` -/// - `Fixed` + mismatched key_len or value_type → `Variable` +/// - `Unknown` → first entry → `Fixed` +/// - `Fixed` + matching key_len and value type → stays `Fixed` +/// - `Fixed` + matching key_len and value *size* → `Fixed` with `value_type: None` +/// - `Fixed` + mismatched key_len or value size → `Variable` /// - `Variable` → stays `Variable` #[derive(Clone, Copy)] enum KeyBlockFormat { /// No entries yet — format undetermined. Unknown, - /// All entries so far have uniform key length and value type. - Fixed { key_len: u8, value_type: EntryType }, - /// Entries have mixed key lengths or value types; must use offset table. + /// All entries so far have uniform key length and value size. + Fixed { + key_len: u8, + val_size: u8, + /// The shared entry type, or `None` if entries have differing types of the same size. + value_type: Option, + }, + /// Entries have mixed key lengths or value sizes; must use offset table. Variable, } impl KeyBlockFormat { /// Updates the format after seeing an entry with the given key length and value type. /// - /// A `Fixed` state is only reachable when all entries have matching key length and value type, + /// A `Fixed` state is only reachable when all entries have matching key length and value size, /// and the key length fits in a u8 (required by the on-disk header). fn update(&mut self, key_len: usize, value_type: EntryType) { + let val_size = value_type_val_size(value_type); *self = match *self { KeyBlockFormat::Unknown => { - if key_len <= MAX_FIXED_KEY_LEN { + if key_len <= MAX_FIXED_KEY_LEN && val_size <= MAX_FIXED_VAL_SIZE { KeyBlockFormat::Fixed { key_len: key_len as u8, - value_type, + val_size: val_size as u8, + value_type: Some(value_type), } } else { KeyBlockFormat::Variable @@ -108,10 +129,13 @@ impl KeyBlockFormat { } KeyBlockFormat::Fixed { key_len: k, + val_size: s, value_type: v, - } if k as usize == key_len && v == value_type => KeyBlockFormat::Fixed { + } if k as usize == key_len && s as usize == val_size => KeyBlockFormat::Fixed { key_len: k, - value_type: v, + val_size: s, + // Collapse to `None` as soon as two entries disagree on type. + value_type: v.filter(|v| *v == value_type), }, KeyBlockFormat::Fixed { .. } | KeyBlockFormat::Variable => KeyBlockFormat::Variable, }; @@ -250,7 +274,11 @@ pub enum EntryValue<'l> { /// Large-sized value. They are stored in a blob file. Large { blob: u32 }, /// Tombstone. The value was removed. - Deleted, + KeyDeleted, + /// Key-value tombstone. Only the one carried value was removed; other values for the same key + /// survive. MultiValue families only. The value must be at most [`MAX_INLINE_VALUE_SIZE`] + /// bytes. + KeyValueDeleted { value: &'l [u8] }, } #[derive(Debug, Clone)] @@ -392,7 +420,13 @@ enum ValueRef { /// Large blob stored externally. Blob { blob_id: u32 }, /// Tombstone. - Deleted, + KeyDeleted, + /// Key-value tombstone: deletes only the carried value from the key's group. The value is + /// stored inline, exactly like [`ValueRef::Inline`]. + KeyValueDeleted { + data: [u8; MAX_INLINE_VALUE_SIZE], + len: u8, + }, } impl ValueRef { @@ -403,7 +437,10 @@ impl ValueRef { ValueRef::Medium { .. } => KEY_BLOCK_ENTRY_TYPE_MEDIUM, ValueRef::Inline { len, .. } => KEY_BLOCK_ENTRY_TYPE_INLINE_MIN + *len, ValueRef::Blob { .. } => KEY_BLOCK_ENTRY_TYPE_BLOB, - ValueRef::Deleted => KEY_BLOCK_ENTRY_TYPE_DELETED, + ValueRef::KeyDeleted => KEY_BLOCK_ENTRY_TYPE_KEY_DELETED, + ValueRef::KeyValueDeleted { len, .. } => { + KEY_BLOCK_ENTRY_TYPE_KEY_VALUE_DELETED_MIN + *len + } }) } @@ -437,7 +474,10 @@ impl ValueRef { BE::write_u32(&mut scratch, *blob_id); buffer.extend(scratch); } - ValueRef::Deleted => { /* no value bytes */ } + ValueRef::KeyDeleted => { /* no value bytes */ } + ValueRef::KeyValueDeleted { data, len } => { + buffer.extend(&data[..*len as usize]); + } ValueRef::PendingSmall { .. } => { unreachable!("PendingSmall should have been resolved"); } @@ -703,7 +743,17 @@ impl StreamingSstWriter { } } EntryValue::Large { blob } => ValueRef::Blob { blob_id: blob }, - EntryValue::Deleted => ValueRef::Deleted, + EntryValue::KeyDeleted => ValueRef::KeyDeleted, + EntryValue::KeyValueDeleted { value } => { + // Enforced by `WriteBatch::delete_value`, which rejects oversized values. + debug_assert!(value.len() <= MAX_INLINE_VALUE_SIZE); + let mut data = [0u8; MAX_INLINE_VALUE_SIZE]; + data[..value.len()].copy_from_slice(value); + ValueRef::KeyValueDeleted { + data, + len: value.len() as u8, + } + } }; self.push_pending_key_entry(entry, value_ref); @@ -843,6 +893,7 @@ impl StreamingSstWriter { if let KeyBlockFormat::Fixed { key_len: key_size, + val_size, value_type, } = info.format { @@ -851,6 +902,7 @@ impl StreamingSstWriter { entry_count as u32, has_hash, key_size, + val_size, value_type, ); for i in start..end { @@ -1123,13 +1175,18 @@ impl<'l> KeyBlockBuilder<'l> { // --------------------------------------------------------------------------- /// The size of the fixed-size key block header (block type + entry count + key size + value type). +/// Mixed-type blocks append one more byte for the value size. const FIXED_KEY_BLOCK_HEADER_SIZE: usize = 6; -/// Builder for a fixed-size key block where all entries share the same key size and value type. +/// Builder for a fixed-size key block where all entries share the same key size and value size. /// -/// No offset table is written — entry positions are computed arithmetically from the stride. +/// No offset table is written — entry positions are computed arithmetically from the stride. When +/// entries share a value size but not a value type, the header records +/// [`FIXED_KEY_BLOCK_MIXED_VALUE_TYPE`] and each entry carries its own type byte before its value. struct FixedKeyBlockBuilder<'l> { buffer: &'l mut Vec, + /// Whether each entry writes its own type byte (set for mixed-type blocks). + per_entry_type: bool, } impl<'l> FixedKeyBlockBuilder<'l> { @@ -1138,11 +1195,12 @@ impl<'l> FixedKeyBlockBuilder<'l> { entry_count: u32, has_hash: bool, key_size: u8, - value_type: EntryType, + val_size: u8, + value_type: Option, ) -> Self { let hash_len: usize = if has_hash { 8 } else { 0 }; - let val_size = value_type_val_size(value_type); - let stride = hash_len + key_size as usize + val_size; + let per_entry_type = value_type.is_none(); + let stride = hash_len + key_size as usize + val_size as usize + usize::from(per_entry_type); buffer.reserve(FIXED_KEY_BLOCK_HEADER_SIZE + entry_count as usize * stride); let block_type = if has_hash { @@ -1156,19 +1214,30 @@ impl<'l> FixedKeyBlockBuilder<'l> { (entry_count >> 8) as u8, entry_count as u8, key_size, - value_type.0, + value_type.map_or(FIXED_KEY_BLOCK_MIXED_VALUE_TYPE, |ty| ty.0), ]); + // Mixed-type blocks cannot derive the value size from the header's type byte, so it is + // written explicitly. + if per_entry_type { + buffer.push(val_size); + } - Self { buffer } + Self { + buffer, + per_entry_type, + } } - /// Writes a single entry (hash + key + value data) to the block. + /// Writes a single entry (hash + key + optional type byte + value data) to the block. fn put(&mut self, entry: &E, value_ref: &ValueRef, has_hash: bool) { if has_hash { self.buffer .extend_from_slice(&entry.key_hash().to_be_bytes()); } entry.write_key_to(self.buffer); + if self.per_entry_type { + self.buffer.push(value_ref.entry_type().0); + } value_ref.write_value_to(self.buffer); } @@ -1186,7 +1255,11 @@ fn value_type_val_size(ty: EntryType) -> usize { KEY_BLOCK_ENTRY_TYPE_SMALL => SMALL_VALUE_REF_SIZE, KEY_BLOCK_ENTRY_TYPE_MEDIUM => MEDIUM_VALUE_REF_SIZE, KEY_BLOCK_ENTRY_TYPE_BLOB => BLOB_VALUE_REF_SIZE, - KEY_BLOCK_ENTRY_TYPE_DELETED => DELETED_VALUE_REF_SIZE, + KEY_BLOCK_ENTRY_TYPE_KEY_DELETED => KEY_DELETED_REF_SIZE, + // Must precede the inline arm: both are open-ended and the tombstone range sits above it. + ty if ty >= KEY_BLOCK_ENTRY_TYPE_KEY_VALUE_DELETED_MIN => { + (ty - KEY_BLOCK_ENTRY_TYPE_KEY_VALUE_DELETED_MIN) as usize + } ty if ty >= KEY_BLOCK_ENTRY_TYPE_INLINE_MIN => { (ty - KEY_BLOCK_ENTRY_TYPE_INLINE_MIN) as usize } @@ -1260,7 +1333,8 @@ mod tests { /// Already-formatted block with `uncompressed_size = 0` (stored as-is). MediumRaw(Vec), Blob(u32), - Deleted, + KeyDeleted, + KeyValueDeleted(Vec), } impl TestEntry { @@ -1292,7 +1366,7 @@ mod tests { } fn deleted(key: &[u8]) -> Self { - Self::new(key, TestValueKind::Deleted) + Self::new(key, TestValueKind::KeyDeleted) } fn medium_raw(key: &[u8], value: &[u8]) -> Self { @@ -1335,7 +1409,8 @@ mod tests { block: v, }, TestValueKind::Blob(id) => EntryValue::Large { blob: *id }, - TestValueKind::Deleted => EntryValue::Deleted, + TestValueKind::KeyDeleted => EntryValue::KeyDeleted, + TestValueKind::KeyValueDeleted(v) => EntryValue::KeyValueDeleted { value: v }, } } } @@ -1409,8 +1484,22 @@ mod tests { }; assert_eq!(*sequence_number, *expected_id); } - (TestValueKind::Deleted, SstLookupResult::Found(values)) - if values.len() == 1 && matches!(values[0], LookupValue::Deleted) => {} + (TestValueKind::KeyDeleted, SstLookupResult::Found(values)) + if values.len() == 1 && matches!(values[0], LookupValue::KeyDeleted) => {} + (TestValueKind::KeyValueDeleted(expected), SstLookupResult::Found(values)) + if values.len() == 1 + && matches!(values[0], LookupValue::KeyValueDeleted { .. }) => + { + let LookupValue::KeyValueDeleted { value } = &values[0] else { + unreachable!() + }; + assert_eq!( + value.as_ref(), + expected.as_slice(), + "tombstone value mismatch for key {:?}", + std::str::from_utf8(&entry.key) + ); + } _ => { panic!( "Unexpected lookup result for key {:?}", @@ -1710,7 +1799,7 @@ mod tests { std::str::from_utf8(&entry.key) ); } - (LookupValue::Deleted, LookupValue::Deleted) => {} + (LookupValue::KeyDeleted, LookupValue::KeyDeleted) => {} ( LookupValue::Blob { sequence_number: s1, @@ -1775,6 +1864,108 @@ mod tests { Ok(()) } + /// Reads the first key block of an SST, returning its raw (uncompressed) bytes. + /// + /// Block 0 is always a key block; the block offset table sits at the end of the file. + fn read_first_block(dir: &Path, seq: u32, block_count: u16) -> Result> { + let data = fs_err::read(dir.join(format!("{seq:08}.sst")))?; + let offsets_start = data.len() - block_count as usize * size_of::(); + let end = BE::read_u32(&data[offsets_start..]) as usize; + let raw = &data[..end]; + // Each block is prefixed by BLOCK_HEADER_SIZE bytes: 4B uncompressed size + 4B checksum. + // An uncompressed size of 0 means the block is stored as-is. + let uncompressed_size = BE::read_u32(raw) as usize; + let body = &raw[BLOCK_HEADER_SIZE..]; + Ok(if uncompressed_size == 0 { + body.to_vec() + } else { + let mut out = vec![0u8; uncompressed_size]; + lzzzz::lz4::decompress(body, &mut out)?; + out + }) + } + + /// A tombstone and a value of the same size keep the block in fixed layout. + /// + /// This is what makes tombstones cheap for uniform-key families like the task cache: without + /// the mixed-type layout, one tombstone would demote its whole block to the variable format + /// and add a 4-byte offset table entry for every entry in it. + #[test] + fn fixed_layout_survives_mixed_value_types_of_equal_size() -> Result<()> { + let dir = tempfile::tempdir()?; + + // Uniform 8-byte keys, uniform 4-byte values, but two different entry types. + let mut entries: Vec = (0..64u64) + .map(|i| { + let key = format!("k-{i:06}"); + if i % 4 == 0 { + TestEntry::new( + key.as_bytes(), + TestValueKind::KeyValueDeleted(vec![0xAAu8; 4]), + ) + } else { + TestEntry::inline(key.as_bytes(), &[0xBBu8; 4]) + } + }) + .collect(); + sort_entries(&mut entries); + + let meta = write_sst(dir.path(), 1, &entries, MetaEntryFlags::default())?; + let block = read_first_block(dir.path(), 1, meta.block_count)?; + + assert!( + block[0] == BLOCK_TYPE_FIXED_KEY_WITH_HASH || block[0] == BLOCK_TYPE_FIXED_KEY_NO_HASH, + "mixed value types of equal size should stay in fixed layout, got block type {}", + block[0] + ); + assert_eq!( + block[5], FIXED_KEY_BLOCK_MIXED_VALUE_TYPE, + "block should be marked mixed-type" + ); + assert_eq!(block[6], 4, "value size should be recorded in the header"); + + // The layout is only useful if it still reads back correctly. + let sst = open_sst(dir.path(), 1, &meta)?; + let kc = make_cache(); + let vc = make_cache(); + for entry in &entries { + assert_lookup(&sst, entry, &kc, &vc)?; + } + Ok(()) + } + + /// Differing value *sizes* cannot share a stride, so the block must fall back to variable + /// layout rather than silently misreading entries. + #[test] + fn mixed_value_sizes_fall_back_to_variable_layout() -> Result<()> { + let dir = tempfile::tempdir()?; + + let mut entries: Vec = (0..64u64) + .map(|i| { + let key = format!("k-{i:06}"); + let len = if i % 4 == 0 { 2 } else { 4 }; + TestEntry::inline(key.as_bytes(), &vec![0xCCu8; len]) + }) + .collect(); + sort_entries(&mut entries); + + let meta = write_sst(dir.path(), 1, &entries, MetaEntryFlags::default())?; + let block = read_first_block(dir.path(), 1, meta.block_count)?; + assert!( + block[0] == BLOCK_TYPE_KEY_WITH_HASH || block[0] == BLOCK_TYPE_KEY_NO_HASH, + "differing value sizes should use variable layout, got block type {}", + block[0] + ); + + let sst = open_sst(dir.path(), 1, &meta)?; + let kc = make_cache(); + let vc = make_cache(); + for entry in &entries { + assert_lookup(&sst, entry, &kc, &vc)?; + } + Ok(()) + } + #[test] fn single_medium_raw_entry() -> Result<()> { let dir = tempfile::tempdir()?; diff --git a/turbopack/crates/turbo-persistence/src/tests.rs b/turbopack/crates/turbo-persistence/src/tests.rs index da007139c160..6bb30c27bed0 100644 --- a/turbopack/crates/turbo-persistence/src/tests.rs +++ b/turbopack/crates/turbo-persistence/src/tests.rs @@ -5,9 +5,11 @@ use rayon::iter::{IntoParallelIterator, ParallelIterator}; use crate::{ DbConfig, FamilyConfig, FamilyKind, - constants::{MAX_MEDIUM_VALUE_SIZE, MAX_SMALL_VALUE_SIZE}, + constants::{MAX_INLINE_VALUE_SIZE, MAX_MEDIUM_VALUE_SIZE, MAX_SMALL_VALUE_SIZE}, db::{CompactConfig, TurboPersistence, read_current_version}, + lookup_entry::IterValue, parallel_scheduler::ParallelScheduler, + static_sorted_file::{StaticSortedFileIter, StaticSortedFileMetaData}, write_batch::WriteBatch, }; @@ -2261,6 +2263,564 @@ fn current_file_is_json_with_commit_time() -> Result<()> { "commit_time {} outside [{before}, {after}]", version.commit_time ); + Ok(()) +} + +/// A key-value tombstone deletes only the pair it names, leaving other values for the same key. +#[test] +fn valued_tombstone_deletes_only_its_pair() -> Result<()> { + let tempdir = tempfile::tempdir()?; + let path = tempdir.path(); + let key = vec![1u8]; + + let db = TurboPersistence::<_, 1>::open_with_config_and_parallel_scheduler( + path.to_path_buf(), + multi_value_config(), + RayonParallelScheduler, + )?; + + // Three values under one key, each in its own SST. + for v in [10u32, 20, 30] { + let batch = db.write_batch()?; + batch.put(0, key.clone(), v.to_be_bytes().to_vec().into())?; + db.commit_write_batch(batch)?; + } + + // Delete just the middle one. + let batch = db.write_batch()?; + batch.delete_value(0, key.clone(), 20u32.to_be_bytes().to_vec().into())?; + db.commit_write_batch(batch)?; + + let mut results = db + .get_multiple(0, &key.as_slice())? + .iter() + .map(|v| u32::from_be_bytes((**v).try_into().unwrap())) + .collect::>(); + results.sort(); + assert_eq!(results, vec![10, 30], "only the named pair should be gone"); + + db.shutdown()?; + Ok(()) +} + +/// A partial compaction must NOT drop a key-value tombstone: an unmerged older SST may still hold a +/// matching value, and dropping the tombstone would resurrect it. +#[test] +fn valued_tombstone_survives_partial_compaction() -> Result<()> { + let tempdir = tempfile::tempdir()?; + let path = tempdir.path(); + + let db = TurboPersistence::<_, 1>::open_with_config_and_parallel_scheduler( + path.to_path_buf(), + multi_value_config(), + RayonParallelScheduler, + )?; + + // Enough distinct keys that each SST spans a real slice of the hash space. The compaction + // selector estimates duplication by scaling sizes against that spread, so a couple of keys + // sharing one hash makes the estimate degenerate and no merge is ever chosen — which would + // leave this test passing without compacting anything. + const KEYS: u32 = 2000; + + // Oldest layer holds the values that the tombstones must keep suppressing. + let batch = db.write_batch()?; + for k in 0..KEYS { + batch.put( + 0, + k.to_be_bytes().to_vec(), + 42u32.to_be_bytes().to_vec().into(), + )?; + } + db.commit_write_batch(batch)?; + + // Newer layers so compaction has overlapping candidates to merge partially. + for v in [1u32, 2, 3] { + let batch = db.write_batch()?; + for k in 0..KEYS { + batch.put(0, k.to_be_bytes().to_vec(), v.to_be_bytes().to_vec().into())?; + } + db.commit_write_batch(batch)?; + } + + let batch = db.write_batch()?; + for k in 0..KEYS { + batch.delete_value( + 0, + k.to_be_bytes().to_vec(), + 42u32.to_be_bytes().to_vec().into(), + )?; + } + db.commit_write_batch(batch)?; + + // Compact repeatedly; whatever coverage the selector chooses, 42 must stay deleted. + for round in 0..3 { + db.compact(&CompactConfig { + min_merge_count: 2, + optimal_merge_count: 2, + min_merge_duplication_bytes: 1, + optimal_merge_duplication_bytes: 1, + ..Default::default() + })?; + for k in [0u32, KEYS / 2, KEYS - 1] { + let results = db + .get_multiple(0, &k.to_be_bytes().to_vec().as_slice())? + .iter() + .map(|v| u32::from_be_bytes((**v).try_into().unwrap())) + .collect::>(); + assert!( + !results.contains(&42), + "42 was resurrected for key {k} in round {round}: {results:?}" + ); + } + } + + db.shutdown()?; + Ok(()) +} + +/// Deletes must survive a reopen: the tombstone is persisted, not just held in memory. +#[test] +fn valued_tombstone_persists_across_reopen() -> Result<()> { + let tempdir = tempfile::tempdir()?; + let path = tempdir.path(); + let key = vec![3u8]; + + { + let db = TurboPersistence::<_, 1>::open_with_config_and_parallel_scheduler( + path.to_path_buf(), + multi_value_config(), + RayonParallelScheduler, + )?; + let batch = db.write_batch()?; + batch.put(0, key.clone(), 100u32.to_be_bytes().to_vec().into())?; + batch.put(0, key.clone(), 200u32.to_be_bytes().to_vec().into())?; + db.commit_write_batch(batch)?; + + let batch = db.write_batch()?; + batch.delete_value(0, key.clone(), 100u32.to_be_bytes().to_vec().into())?; + db.commit_write_batch(batch)?; + db.shutdown()?; + } + + { + let db = TurboPersistence::<_, 1>::open_with_config_and_parallel_scheduler( + path.to_path_buf(), + multi_value_config(), + RayonParallelScheduler, + )?; + let results = db + .get_multiple(0, &key.as_slice())? + .iter() + .map(|v| u32::from_be_bytes((**v).try_into().unwrap())) + .collect::>(); + assert_eq!(results, vec![200], "tombstone lost across reopen"); + db.shutdown()?; + } + + Ok(()) +} + +/// A key tombstone still deletes everything, including values a key-value tombstone left alone. +#[test] +fn whole_key_tombstone_still_deletes_all_values() -> Result<()> { + let tempdir = tempfile::tempdir()?; + let path = tempdir.path(); + let key = vec![5u8]; + + let db = TurboPersistence::<_, 1>::open_with_config_and_parallel_scheduler( + path.to_path_buf(), + multi_value_config(), + RayonParallelScheduler, + )?; + + let batch = db.write_batch()?; + batch.put(0, key.clone(), 1u32.to_be_bytes().to_vec().into())?; + batch.put(0, key.clone(), 2u32.to_be_bytes().to_vec().into())?; + db.commit_write_batch(batch)?; + + let batch = db.write_batch()?; + batch.delete_value(0, key.clone(), 1u32.to_be_bytes().to_vec().into())?; + db.commit_write_batch(batch)?; + + let batch = db.write_batch()?; + batch.delete(0, key.clone())?; + db.commit_write_batch(batch)?; + + assert!( + db.get_multiple(0, &key.as_slice())?.is_empty(), + "key tombstone should remove everything" + ); + + db.shutdown()?; + Ok(()) +} + +/// Counts tombstone entries (both kinds) across every live SST, by reading the files directly. +/// Tombstone counts are not tracked in the meta file, so there is nothing cheaper to read. +fn count_tombstones( + path: &Path, + db: &TurboPersistence, +) -> Result { + let mut count = 0; + for meta in db.meta_info()? { + for entry in &meta.entries { + let sst = StaticSortedFileMetaData { + sequence_number: entry.sequence_number, + block_count: entry.block_count, + }; + for item in StaticSortedFileIter::open(path, sst)? { + if matches!( + item?.value, + IterValue::KeyDeleted | IterValue::KeyValueDeleted { .. } + ) { + count += 1; + } + } + } + } + Ok(count) +} + +/// Compaction reclaims tombstones once no *older* SST outside the job can still hold the key. +/// Without this, tombstones accumulate forever. +#[test] +fn compaction_reclaims_tombstones_when_no_older_sst_has_the_key() -> Result<()> { + let tempdir = tempfile::tempdir()?; + let path = tempdir.path(); + + let db = TurboPersistence::<_, 1>::open_with_config_and_parallel_scheduler( + path.to_path_buf(), + multi_value_config(), + RayonParallelScheduler, + )?; + + // Enough distinct keys that the SSTs span a real hash range: the compaction selector + // estimates duplication by scaling sizes against the key-space spread, and a handful of + // keys sharing one hash makes that estimate degenerate. + const KEYS: u32 = 2000; + + let batch = db.write_batch()?; + for k in 0..KEYS { + batch.put( + 0, + k.to_be_bytes().to_vec(), + 1u32.to_be_bytes().to_vec().into(), + )?; + batch.put( + 0, + k.to_be_bytes().to_vec(), + 2u32.to_be_bytes().to_vec().into(), + )?; + } + db.commit_write_batch(batch)?; + + // Delete one of the two values for every key. + let batch = db.write_batch()?; + for k in 0..KEYS { + batch.delete_value( + 0, + k.to_be_bytes().to_vec(), + 1u32.to_be_bytes().to_vec().into(), + )?; + } + db.commit_write_batch(batch)?; + + assert_eq!( + count_tombstones(path, &db)?, + KEYS as usize, + "tombstones should be on disk before compaction" + ); + db.compact(&CompactConfig { + min_merge_count: 2, + optimal_merge_count: 2, + min_merge_duplication_bytes: 1, + optimal_merge_duplication_bytes: 1, + ..Default::default() + })?; + + // No older SST holds these keys, so every tombstone is dead weight and must be gone. Assert + // on the tombstone entries themselves: total file size would shrink from dropping the deleted + // values alone, so it cannot distinguish a working probe from one that never fires. + assert_eq!( + count_tombstones(path, &db)?, + 0, + "compaction should have reclaimed every tombstone" + ); + + // ...and the deletes must still hold after reclamation. + for k in [0u32, KEYS / 2, KEYS - 1] { + let results = db + .get_multiple(0, &k.to_be_bytes().to_vec().as_slice())? + .iter() + .map(|v| u32::from_be_bytes((**v).try_into().unwrap())) + .collect::>(); + assert_eq!(results, vec![2], "value 1 must stay deleted for key {k}"); + } + + db.shutdown()?; + Ok(()) +} + +/// When an older SST *outside* the compaction job still holds the key, the tombstone must be +/// kept. Dropping it would resurrect the value. +#[test] +fn compaction_keeps_tombstone_when_older_sst_has_the_key() -> Result<()> { + let tempdir = tempfile::tempdir()?; + let path = tempdir.path(); + + let db = TurboPersistence::<_, 1>::open_with_config_and_parallel_scheduler( + path.to_path_buf(), + multi_value_config(), + RayonParallelScheduler, + )?; + + const KEYS: u32 = 2000; + + // Oldest layer: the values that must stay suppressed. + let batch = db.write_batch()?; + for k in 0..KEYS { + batch.put( + 0, + k.to_be_bytes().to_vec(), + 1u32.to_be_bytes().to_vec().into(), + )?; + } + db.commit_write_batch(batch)?; + + // Newer layers: an unrelated value per key, then a tombstone for the old one. + let batch = db.write_batch()?; + for k in 0..KEYS { + batch.put( + 0, + k.to_be_bytes().to_vec(), + 2u32.to_be_bytes().to_vec().into(), + )?; + } + db.commit_write_batch(batch)?; + + let batch = db.write_batch()?; + for k in 0..KEYS { + batch.delete_value( + 0, + k.to_be_bytes().to_vec(), + 1u32.to_be_bytes().to_vec().into(), + )?; + } + db.commit_write_batch(batch)?; + + // Compact repeatedly. Whatever subset each job picks, value 1 must never come back: while an + // older SST still holds it, the probe has to keep the tombstone alive. + for round in 0..4 { + db.compact(&CompactConfig { + min_merge_count: 2, + optimal_merge_count: 2, + min_merge_duplication_bytes: 1, + optimal_merge_duplication_bytes: 1, + ..Default::default() + })?; + + for k in [0u32, KEYS / 2, KEYS - 1] { + let mut results = db + .get_multiple(0, &k.to_be_bytes().to_vec().as_slice())? + .iter() + .map(|v| u32::from_be_bytes((**v).try_into().unwrap())) + .collect::>(); + results.sort(); + assert_eq!( + results, + vec![2], + "value 1 resurrected for key {k} in round {round}" + ); + } + } + + db.shutdown()?; + Ok(()) +} + +/// A compaction job's SSTs need not be contiguous: the selector picks members by hash-range +/// overlap and skips already-claimed candidates, so an SST can sit "between" two job members +/// without belonging to the job. A tombstone must still be kept for it. +/// +/// This is the case a sequence-number threshold gets wrong — it would treat the skipped SST as +/// part of the job and drop a tombstone that is still load-bearing. +#[test] +fn compaction_keeps_tombstone_when_skipped_sst_has_the_key() -> Result<()> { + let tempdir = tempfile::tempdir()?; + let path = tempdir.path(); + + let db = TurboPersistence::<_, 1>::open_with_config_and_parallel_scheduler( + path.to_path_buf(), + multi_value_config(), + RayonParallelScheduler, + )?; + + const KEYS: u32 = 2000; + + // Every SST spans a wide, overlapping slice of the hash space. That is what lets the selector + // form jobs that skip over an intervening file: with narrow or identical ranges it only ever + // picks contiguous runs, and the bug this guards against stays hidden. + let key_for = |round: u32, i: u32| { + let mut k = vec![0u8; 8]; + k[..4].copy_from_slice(&i.to_be_bytes()); + k[4..].copy_from_slice(&round.to_be_bytes()); + k + }; + + // Oldest layer: the values that must stay suppressed once deleted. + let batch = db.write_batch()?; + for i in 0..KEYS { + batch.put(0, key_for(0, i), 1u32.to_be_bytes().to_vec().into())?; + } + db.commit_write_batch(batch)?; + + // Several more layers touching the same keys, so the selector has many overlapping candidates. + for round in 1..7u32 { + let batch = db.write_batch()?; + for i in 0..KEYS { + batch.put(0, key_for(0, i), (round + 1).to_be_bytes().to_vec().into())?; + batch.put(0, key_for(round, i), 9u32.to_be_bytes().to_vec().into())?; + } + db.commit_write_batch(batch)?; + } + + // Delete the oldest value for every key in the first layer. + let batch = db.write_batch()?; + for i in 0..KEYS { + batch.delete_value(0, key_for(0, i), 1u32.to_be_bytes().to_vec().into())?; + } + db.commit_write_batch(batch)?; + + // More layers *after* the tombstone, so it sits in the middle of the stack rather than at the + // newest end. A job containing the tombstone's SST can then skip over older SSTs that still + // hold value 1 — those are only caught by probing every older SST the job does not merge, not + // just the ones below the job's oldest member. + for round in 7..12u32 { + let batch = db.write_batch()?; + for i in 0..KEYS { + batch.put(0, key_for(round, i), 9u32.to_be_bytes().to_vec().into())?; + } + db.commit_write_batch(batch)?; + } + + for round in 0..6 { + db.compact(&CompactConfig { + min_merge_count: 2, + max_merge_count: 3, + optimal_merge_count: 2, + min_merge_duplication_bytes: 1, + optimal_merge_duplication_bytes: 1, + ..Default::default() + })?; + + for i in [0u32, KEYS / 2, KEYS - 1] { + let results = db + .get_multiple(0, &key_for(0, i).as_slice())? + .iter() + .map(|v| u32::from_be_bytes((**v).try_into().unwrap())) + .collect::>(); + assert!( + !results.contains(&1), + "deleted value resurrected for key {i} in round {round}: {results:?}" + ); + } + } + + db.shutdown()?; + Ok(()) +} + +/// Tombstones carry the deleted value inline, so every size up to the inline limit round-trips. +/// The boundary sizes matter: the tag range is packed directly above the inline value range, so an +/// off-by-one in either bound would decode a tombstone as a value or vice versa. +#[test] +fn valued_tombstone_supports_all_inline_value_sizes() -> Result<()> { + let tempdir = tempfile::tempdir()?; + let path = tempdir.path(); + + let db = TurboPersistence::<_, 1>::open_with_config_and_parallel_scheduler( + path.to_path_buf(), + multi_value_config(), + RayonParallelScheduler, + )?; + + // One key per value size, each holding a deleted value of that size plus a survivor. + for len in 0..=MAX_INLINE_VALUE_SIZE { + let key = vec![len as u8]; + let doomed = vec![0xAAu8; len]; + + let batch = db.write_batch()?; + batch.put(0, key.clone(), doomed.clone().into())?; + batch.put(0, key.clone(), vec![0xBBu8; 3].into())?; + db.commit_write_batch(batch)?; + + let batch = db.write_batch()?; + batch.delete_value(0, key.clone(), doomed.clone().into())?; + db.commit_write_batch(batch)?; + + let results = db.get_multiple(0, &key.as_slice())?; + assert_eq!( + results.iter().map(|v| v.to_vec()).collect::>(), + vec![vec![0xBBu8; 3]], + "{len}-byte value should have been deleted, and only it" + ); + } + + db.shutdown()?; + Ok(()) +} + +/// Values too large to store inline are rejected rather than silently truncated: the tombstone +/// carries a copy of the value, so deleting a large value would cost more than it reclaims. +#[test] +fn valued_tombstone_rejects_values_larger_than_inline() -> Result<()> { + let tempdir = tempfile::tempdir()?; + let path = tempdir.path(); + + let db = TurboPersistence::<_, 1>::open_with_config_and_parallel_scheduler( + path.to_path_buf(), + multi_value_config(), + RayonParallelScheduler, + )?; + + let batch = db.write_batch()?; + let too_big = vec![0u8; MAX_INLINE_VALUE_SIZE + 1]; + let err = batch + .delete_value(0, vec![1u8], too_big.into()) + .expect_err("oversized value should be rejected"); + assert!( + err.to_string().contains("at most"), + "unexpected error: {err}" + ); + + db.shutdown()?; + Ok(()) +} + +/// Key-value tombstones are meaningless in a SingleValue family, where `delete` already removes +/// the single value exactly. Rejecting at the API keeps the tombstone off disk, where it would +/// otherwise only surface as an error at read time. +#[test] +fn valued_tombstone_rejects_single_value_families() -> Result<()> { + let tempdir = tempfile::tempdir()?; + let path = tempdir.path(); + + let db = TurboPersistence::<_, 1>::open_with_config_and_parallel_scheduler( + path.to_path_buf(), + DbConfig::<1>::default(), + RayonParallelScheduler, + )?; + + let batch = db.write_batch()?; + let err = batch + .delete_value(0, vec![1u8], 1u32.to_be_bytes().to_vec().into()) + .expect_err("SingleValue family should be rejected"); + assert!( + err.to_string().contains("MultiValue"), + "unexpected error: {err}" + ); + + db.shutdown()?; Ok(()) } diff --git a/turbopack/crates/turbo-persistence/src/write_batch.rs b/turbopack/crates/turbo-persistence/src/write_batch.rs index 42e65aed7aef..1c837e3e18f4 100644 --- a/turbopack/crates/turbo-persistence/src/write_batch.rs +++ b/turbopack/crates/turbo-persistence/src/write_batch.rs @@ -6,7 +6,7 @@ use std::{ sync::atomic::{AtomicU32, AtomicU64, Ordering}, }; -use anyhow::{Context, Result}; +use anyhow::{Context, Result, bail}; use byteorder::{BE, WriteBytesExt}; use either::Either; use fs_err::File; @@ -15,11 +15,11 @@ use smallvec::SmallVec; use thread_local::ThreadLocal; use crate::{ - FamilyConfig, ValueBuffer, + FamilyConfig, FamilyKind, ValueBuffer, collector::Collector, collector_entry::CollectorEntry, compression::{checksum_block, compress_into_buffer}, - constants::{MAX_MEDIUM_VALUE_SIZE, THREAD_LOCAL_SIZE_SHIFT}, + constants::{MAX_INLINE_VALUE_SIZE, MAX_MEDIUM_VALUE_SIZE, THREAD_LOCAL_SIZE_SHIFT}, db::WriteOperationGuard, key::StoreKey, meta_file::MetaEntryFlags, @@ -245,7 +245,11 @@ impl<'db, K: StoreKey + Send + Sync, S: ParallelScheduler, const FAMILIES: usize Ok(()) } - /// Puts a delete operation into the write batch. + /// Puts a delete operation into the write batch. This deletes *all* values for `key`. + /// + /// Combining this with a [`WriteBatch::put`] of the same key in the same batch is **not + /// supported**: which one wins is undefined, and callers are expected to resolve the intent + /// themselves before writing. pub fn delete(&self, family: u32, key: K) -> Result<()> { let state = self.thread_local_state(); let collector = self.thread_local_collector_mut(state, family)?; @@ -253,6 +257,39 @@ impl<'db, K: StoreKey + Send + Sync, S: ParallelScheduler, const FAMILIES: usize Ok(()) } + /// Deletes a single key-value pair, leaving any other values for `key` intact. + /// + /// Only valid for [`FamilyKind::MultiValue`] families: in a `SingleValue` family a key has one + /// value and [`WriteBatch::delete`] already removes it exactly. + /// + /// Deleting a pair that is written in the same batch — by this or any other operation on the + /// key — is **not supported**, for the reason given on [`WriteBatch::delete`]: which one wins + /// is undefined, and it is the caller's job to resolve that before writing. + /// + /// Only values of at most [`MAX_INLINE_VALUE_SIZE`] bytes can be deleted this way. This is a + /// simplifying limitation that could be relaxed if needed. Of course in general the storage + /// overhead of deleting large values by value makes it apriori inefficient. + pub fn delete_value(&self, family: u32, key: K, value: ValueBuffer<'_>) -> Result<()> { + let family_config = &self.family_configs[usize_from_u32(family)]; + if family_config.kind != FamilyKind::MultiValue { + bail!( + "delete_value is only valid for MultiValue families, but family {} is SingleValue", + family_config.name + ); + } + if value.len() > MAX_INLINE_VALUE_SIZE { + bail!( + "delete_value only supports values of at most {MAX_INLINE_VALUE_SIZE} bytes, got \ + {} bytes", + value.len() + ); + } + let state = self.thread_local_state(); + let collector = self.thread_local_collector_mut(state, family)?; + collector.delete_value(key, &value); + Ok(()) + } + /// Flushes a family of the write batch, reducing the amount of buffered memory used. /// Does not commit any data persistently. /// @@ -547,10 +584,22 @@ impl<'db, K: StoreKey + Send + Sync, S: ParallelScheduler, const FAMILIES: usize "we wrote a blob but did not read it" ); } - CollectorEntryValue::Deleted => assert!( - values.first() == Some(&LookupValue::Deleted), - "we wrote a deleted tombstone but it was not first in results" + // Key tombstones sort last within a key group, so a same-batch + // `put(K, v); delete(K)` reads back as [v, KeyDeleted]. + CollectorEntryValue::KeyDeleted => assert!( + values.last() == Some(&LookupValue::KeyDeleted), + "we wrote a key tombstone but it was not last in results" ), + CollectorEntryValue::KeyValueDeleted { value, len } => { + let expected = &value[..*len as usize]; + assert!( + values.iter().any(|lv| matches!( + lv, + LookupValue::KeyValueDeleted { value } if &**value == expected + )), + "we wrote a key-value tombstone but did not read it back" + ) + } v => { assert!( values.into_iter().any(|lv| {