-
Notifications
You must be signed in to change notification settings - Fork 51
feat(database): CRUD benchmark domain with Postgres #274
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
HeyGarrison
wants to merge
4
commits into
master
Choose a base branch
from
devin/1785612029-database-crud-benchmark
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+622
−1
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| # Database CRUD benchmark | ||
|
|
||
| This benchmark measures a create → read → update → read → delete cycle against | ||
| each configured database provider. Postgres is currently the only provider. | ||
|
|
||
| ## Configuration | ||
|
|
||
| Required: | ||
|
|
||
| - `DATABASE_POSTGRES_URL` | ||
|
|
||
| Optional: | ||
|
|
||
| - `DATABASE_BENCH_TABLE` — table name, default `benchmark_crud` | ||
|
|
||
| Run a local Postgres target with: | ||
|
|
||
| ```bash | ||
| docker run --rm --name database-benchmark-postgres -p 5433:5432 \ | ||
| -e POSTGRES_PASSWORD=postgres -e POSTGRES_DB=benchmark postgres:16 | ||
| ``` | ||
|
|
||
| Then run the benchmark: | ||
|
|
||
| ```bash | ||
| DATABASE_POSTGRES_URL=postgresql://postgres:postgres@127.0.0.1:5433/benchmark \ | ||
| pnpm bench:database:postgres | ||
| ``` | ||
|
|
||
| The payload defaults to 1 KiB and can be changed with `--payload-size`, for | ||
| example `--payload-size 4096`. | ||
|
|
||
| Results are written to `results/database/<YYYY-MM-DD>.json` and | ||
| `results/database/latest.json`. | ||
|
|
||
| To add a provider, add one entry to `providers.ts` and implement its | ||
| `DatabaseClient` in a client module. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,155 @@ | ||
| import { writeFileSync } from 'node:fs'; | ||
| import os from 'node:os'; | ||
| import crypto from 'node:crypto'; | ||
| import { computeStats } from '../src/util/stats.js'; | ||
| import type { DatabaseClient, DatabaseDocument, DatabaseBenchmarkResult } from './types.js'; | ||
|
|
||
| interface StepContext { | ||
| step<R>(name: string, fn: () => Promise<R> | R): Promise<R>; | ||
| cleanup(fn: () => Promise<unknown> | unknown): Promise<unknown>; | ||
| } | ||
|
|
||
| function round(value: number): number { | ||
| return Math.round(value * 100) / 100; | ||
| } | ||
|
|
||
| function roundStats(stats: { median: number; p95: number; p99: number }) { | ||
| return { | ||
| median: round(stats.median), | ||
| p95: round(stats.p95), | ||
| p99: round(stats.p99), | ||
| }; | ||
| } | ||
|
|
||
| function randomId(): string { | ||
| return Math.random().toString(36).slice(2, 15); | ||
| } | ||
|
|
||
| function makePayload(size: number): string { | ||
| const bytes = Math.ceil(size * 3 / 4); | ||
| return Buffer.from(crypto.randomBytes(bytes)).toString('base64').slice(0, size); | ||
| } | ||
|
|
||
| function assertDocument(actual: DatabaseDocument | null, expected: DatabaseDocument, phase: string): void { | ||
| if ( | ||
| !actual || | ||
| actual.id !== expected.id || | ||
| actual.name !== expected.name || | ||
| actual.payload !== expected.payload || | ||
| actual.version !== expected.version | ||
| ) { | ||
| throw new Error(`${phase} verification failed`); | ||
| } | ||
| } | ||
|
|
||
| export async function runCrudCycle( | ||
| client: DatabaseClient, | ||
| ctx: StepContext, | ||
| payloadBytes: number, | ||
| ) { | ||
| const totalStart = performance.now(); | ||
| const id = `benchmark-${Date.now()}-${randomId()}`; | ||
| const payload = makePayload(payloadBytes); | ||
| const updatedPayload = makePayload(payloadBytes); | ||
| const document: DatabaseDocument = { id, name: 'database benchmark', payload, version: 1 }; | ||
| const updatedDocument: DatabaseDocument = { | ||
| ...document, | ||
| name: 'database benchmark updated', | ||
| payload: updatedPayload, | ||
| version: 2, | ||
| }; | ||
|
|
||
| try { | ||
| const createStart = performance.now(); | ||
| await ctx.step('create', () => client.create(document)); | ||
| const createMs = performance.now() - createStart; | ||
|
|
||
| const readStart = performance.now(); | ||
| const readDocument = await ctx.step('read', () => client.read(id)); | ||
| const readMs = performance.now() - readStart; | ||
| assertDocument(readDocument, document, 'create/read'); | ||
|
|
||
| const updateStart = performance.now(); | ||
| await ctx.step('update', () => client.update(id, { | ||
| name: updatedDocument.name, | ||
| payload: updatedDocument.payload, | ||
| version: updatedDocument.version, | ||
| })); | ||
| const updateMs = performance.now() - updateStart; | ||
|
|
||
| const readAfterUpdateStart = performance.now(); | ||
| const readAfterUpdateDocument = await ctx.step('read-after-update', () => client.read(id)); | ||
| const readAfterUpdateMs = performance.now() - readAfterUpdateStart; | ||
| assertDocument(readAfterUpdateDocument, updatedDocument, 'update/read'); | ||
|
|
||
| const deleteStart = performance.now(); | ||
| const deleted = await ctx.step('delete', () => client.delete(id)); | ||
| const deleteMs = performance.now() - deleteStart; | ||
| if (deleted !== 1) { | ||
| throw new Error(`delete verification failed: removed ${deleted} rows`); | ||
| } | ||
|
|
||
| return { | ||
| createMs, | ||
| readMs, | ||
| updateMs, | ||
| readAfterUpdateMs, | ||
| deleteMs, | ||
| totalMs: performance.now() - totalStart, | ||
| payloadBytes, | ||
| }; | ||
| } catch (error) { | ||
| try { | ||
| await ctx.cleanup(() => client.delete(id)); | ||
| } catch { | ||
| // Best-effort cleanup after a failed cycle. | ||
| } | ||
| throw error; | ||
| } | ||
| } | ||
|
|
||
| export async function writeDatabaseResultsJson( | ||
| results: DatabaseBenchmarkResult[], | ||
| outPath: string, | ||
| ): Promise<void> { | ||
| const cleanResults = results.map((result) => ({ | ||
| provider: result.provider, | ||
| mode: result.mode, | ||
| table: result.table, | ||
| payloadBytes: result.payloadBytes, | ||
| iterations: result.iterations.map((iteration) => ({ | ||
| createMs: round(iteration.createMs), | ||
| readMs: round(iteration.readMs), | ||
| updateMs: round(iteration.updateMs), | ||
| readAfterUpdateMs: round(iteration.readAfterUpdateMs), | ||
| deleteMs: round(iteration.deleteMs), | ||
| totalMs: round(iteration.totalMs), | ||
| payloadBytes: iteration.payloadBytes, | ||
| ...(iteration.error ? { error: iteration.error } : {}), | ||
| })), | ||
| summary: { | ||
| createMs: roundStats(result.summary.createMs), | ||
| readMs: roundStats(result.summary.readMs), | ||
| updateMs: roundStats(result.summary.updateMs), | ||
| readAfterUpdateMs: roundStats(result.summary.readAfterUpdateMs), | ||
| deleteMs: roundStats(result.summary.deleteMs), | ||
| totalMs: roundStats(result.summary.totalMs), | ||
| }, | ||
| ...(result.compositeScore !== undefined ? { compositeScore: round(result.compositeScore) } : {}), | ||
| ...(result.successRate !== undefined ? { successRate: round(result.successRate) } : {}), | ||
| ...(result.skipped ? { skipped: result.skipped, skipReason: result.skipReason } : {}), | ||
| })); | ||
|
|
||
| const output = { | ||
| version: '1.0', | ||
| timestamp: new Date().toISOString(), | ||
| environment: { node: process.version, platform: os.platform(), arch: os.arch() }, | ||
| config: { | ||
| iterations: results[0]?.iterations.length || 0, | ||
| timeoutMs: 30_000, | ||
| }, | ||
| results: cleanResults, | ||
| }; | ||
| writeFileSync(outPath, JSON.stringify(output, null, 2)); | ||
| console.log(`Results written to ${outPath}`); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| /** | ||
| * Database CRUD benchmark: create → read → update → read → delete cycles. | ||
| * Declarative — exports `config` + `task`; `bench run` owns the entrypoint. | ||
| * The custom `--payload-size` flag is scanned from argv here. | ||
| * | ||
| * bench run benchmarks/database/crud.bench.ts | ||
| * bench run benchmarks/database/crud.bench.ts --payload-size 4096 --iterations 10 | ||
| * bench run benchmarks/database/crud.bench.ts --payload-size 1024 --provider postgres | ||
| */ | ||
| import '../src/env.js'; | ||
| import path from 'node:path'; | ||
| import { fileURLToPath } from 'node:url'; | ||
| import { defineBenchmarkConfig, defineTask, TaskError } from '@benchsdk/runner'; | ||
| import { withTimeout } from '../src/util/timeout.js'; | ||
| import { formatError } from '../src/util/error.js'; | ||
| import { databaseProviders } from './providers.js'; | ||
| import { writeDatabaseLegacyResults } from './legacy-results.js'; | ||
| import { runCrudCycle } from './benchmark.js'; | ||
| import type { DatabaseClient, DatabaseProviderConfig } from './types.js'; | ||
|
|
||
| const __dirname = path.dirname(fileURLToPath(import.meta.url)); | ||
|
|
||
| function getArgValue(argv: string[], flag: string): string | undefined { | ||
| const idx = argv.indexOf(flag); | ||
| if (idx !== -1 && idx + 1 < argv.length) return argv[idx + 1]; | ||
| const equals = argv.find((arg) => arg.startsWith(`${flag}=`)); | ||
| return equals?.slice(flag.length + 1); | ||
| } | ||
|
|
||
| const payloadSizeArg = getArgValue(process.argv.slice(2), '--payload-size'); | ||
| const payloadBytes = payloadSizeArg === undefined ? 1024 : Number(payloadSizeArg); | ||
| if (!Number.isInteger(payloadBytes) || payloadBytes < 1) { | ||
| console.error(`Invalid --payload-size "${payloadSizeArg}". Provide a positive integer.`); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| const clients = new Map<string, DatabaseClient>(); | ||
| const setupPromises = new Map<string, Promise<void>>(); | ||
|
|
||
| export const config = defineBenchmarkConfig({ | ||
| benchmarkSlug: 'database-crud-local', | ||
| benchmarkName: 'Database CRUD (local)', | ||
| benchmarkKind: 'database', | ||
| iterations: 10, | ||
| concurrency: 1, | ||
| participants: databaseProviders, | ||
| onComplete: async (outcome) => { | ||
| await writeDatabaseLegacyResults(outcome.participants, { | ||
| resultsDir: path.resolve(__dirname, '../../results/database'), | ||
| providers: databaseProviders, | ||
| payloadBytes, | ||
| }); | ||
| await Promise.all([...clients.values()].map((client) => client.close())); | ||
| }, | ||
| }); | ||
|
|
||
| export const task = defineTask<DatabaseProviderConfig>(async (ctx) => { | ||
| const { participant, step } = ctx; | ||
| const timeout = participant.timeout ?? 30_000; | ||
| let client = clients.get(participant.name); | ||
| if (!client) { | ||
| client = participant.createClient(); | ||
| clients.set(participant.name, client); | ||
| } | ||
|
|
||
| try { | ||
| let setup = setupPromises.get(participant.name); | ||
| if (!setup) { | ||
| setup = withTimeout(client.setup(), timeout, 'Database setup timed out'); | ||
| setupPromises.set(participant.name, setup); | ||
| } | ||
| await setup; | ||
|
|
||
| const result = await runCrudCycle( | ||
| client, | ||
| { | ||
| step: (name, fn) => step(name, () => withTimeout(Promise.resolve(fn()), timeout, `${name} timed out`)), | ||
| cleanup: (fn) => withTimeout(Promise.resolve(fn()), 10_000, 'Delete timed out'), | ||
| }, | ||
| payloadBytes, | ||
| ); | ||
| return { data: result }; | ||
| } catch (error) { | ||
| const message = formatError(error); | ||
| throw new TaskError(message, { | ||
| code: 'DATABASE_ERROR', | ||
| data: { | ||
| createMs: 0, | ||
| readMs: 0, | ||
| updateMs: 0, | ||
| readAfterUpdateMs: 0, | ||
| deleteMs: 0, | ||
| totalMs: 0, | ||
| payloadBytes, | ||
| }, | ||
| }); | ||
| } | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| import { mkdirSync, copyFileSync } from 'node:fs'; | ||
| import path from 'node:path'; | ||
| import type { ParticipantRecords } from '@benchsdk/runner'; | ||
| import type { JsonObject } from '@benchsdk/client'; | ||
| import { byTaskIndex } from '../src/util/records.js'; | ||
| import { computeStats } from '../src/util/stats.js'; | ||
| import { computeDatabaseCompositeScores } from './scoring.js'; | ||
| import { writeDatabaseResultsJson } from './benchmark.js'; | ||
| import type { | ||
| DatabaseBenchmarkResult, | ||
| DatabaseProviderConfig, | ||
| DatabaseTimingResult, | ||
| } from './types.js'; | ||
|
|
||
| function num(value: unknown): number { | ||
| return typeof value === 'number' ? value : 0; | ||
| } | ||
|
|
||
| export function recordsToDatabaseResults( | ||
| participants: ParticipantRecords[], | ||
| opts: { payloadBytes: number; providers: DatabaseProviderConfig[] }, | ||
| ): DatabaseBenchmarkResult[] { | ||
| return participants.map((participant) => { | ||
| const provider = opts.providers.find((p) => p.name === participant.participant); | ||
| const iterations = byTaskIndex(participant.records).map((record): DatabaseTimingResult => { | ||
| const data = (record.data ?? {}) as JsonObject; | ||
| const base = { | ||
| createMs: num(data.createMs), | ||
| readMs: num(data.readMs), | ||
| updateMs: num(data.updateMs), | ||
| readAfterUpdateMs: num(data.readAfterUpdateMs), | ||
| deleteMs: num(data.deleteMs), | ||
| totalMs: num(data.totalMs), | ||
| payloadBytes: num(data.payloadBytes) || opts.payloadBytes, | ||
| }; | ||
| return record.status === 'error' | ||
| ? { ...base, error: record.errorCode ?? 'error' } | ||
| : base; | ||
| }); | ||
| const successful = iterations.filter((iteration) => !iteration.error); | ||
| const summary = { | ||
| createMs: computeStats(successful.map((i) => i.createMs)), | ||
| readMs: computeStats(successful.map((i) => i.readMs)), | ||
| updateMs: computeStats(successful.map((i) => i.updateMs)), | ||
| readAfterUpdateMs: computeStats(successful.map((i) => i.readAfterUpdateMs)), | ||
| deleteMs: computeStats(successful.map((i) => i.deleteMs)), | ||
| totalMs: computeStats(successful.map((i) => i.totalMs)), | ||
| }; | ||
| return { | ||
| provider: participant.participant, | ||
| mode: 'database', | ||
| table: provider?.table ?? 'benchmark_crud', | ||
| payloadBytes: opts.payloadBytes, | ||
| iterations, | ||
| summary, | ||
| }; | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Map records -> database results and write the dated and latest files. | ||
| * TEMPORARY BRIDGE until the platform read API exposes per-iteration data. | ||
| */ | ||
| export async function writeDatabaseLegacyResults( | ||
| participants: ParticipantRecords[], | ||
| opts: { resultsDir: string; providers: DatabaseProviderConfig[]; payloadBytes: number }, | ||
| ): Promise<void> { | ||
| const results = recordsToDatabaseResults(participants, opts); | ||
| computeDatabaseCompositeScores(results); | ||
| mkdirSync(opts.resultsDir, { recursive: true }); | ||
| const timestamp = new Date().toISOString().slice(0, 10); | ||
| const outPath = path.join(opts.resultsDir, `${timestamp}.json`); | ||
| await writeDatabaseResultsJson(results, outPath); | ||
| const latestPath = path.join(opts.resultsDir, 'latest.json'); | ||
| copyFileSync(outPath, latestPath); | ||
| console.log(`Copied latest: ${latestPath}`); | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 A failed database cycle can hang the whole benchmark forever during cleanup
The leftover record is removed (
client.delete(id)atbenchmarks/database/benchmark.ts:102) with no time limit after a failed cycle, so an unresponsive database leaves the benchmark stuck with no way to finish.Impact: A single hung cleanup call blocks the entire run indefinitely instead of failing the iteration and moving on.
Why the timeout protection is missing on this path
Every timed phase is wrapped by
withTimeoutthrough thestepshim inbenchmarks/database/crud.bench.ts:77, but the best-effort cleanup insiderunCrudCycle's catch block calls the client directly, bypassing that wrapper. The equivalent storage benchmark explicitly wraps its failure-path cleanup:benchmarks/storage/storage.bench.ts:107-108useswithTimeout(storage!.delete(key), 10_000, 'Delete timed out').Because the Postgres pool is configured with
max: 1(benchmarks/database/postgres.ts:24), a cleanup delete that never resolves also blocks the single connection for all remaining iterations.Prompt for agents
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch — fixed in b7ec88a. The failure-path cleanup now goes through a
ctx.cleanup()wrapper thatcrud.bench.tsimplements withwithTimeout(..., 10_000, 'Delete timed out'), matchingbenchmarks/storage/storage.bench.ts. Kept the helper out ofbenchmark.tsso the cycle stays runner-agnostic, and cleanup errors (including that timeout) are still swallowed so the original workload error is what gets rethrown.