Skip to content

benchmarks-website: duckdb conn pool#7850

Closed
a10y wants to merge 1 commit into
developfrom
aduffy/duckdb-pool
Closed

benchmarks-website: duckdb conn pool#7850
a10y wants to merge 1 commit into
developfrom
aduffy/duckdb-pool

Conversation

@a10y
Copy link
Copy Markdown
Contributor

@a10y a10y commented May 8, 2026

A single visit to the benchmarks website makes ~360 concurrent API requests to this axum server.

The previous Arc<Mutex<Connection>> was causing all of these requests to serialize (multiply by however many concurrent tabs had the benchmarks open).

This creates a connection pool of DuckDB conns, and adds a mutex on the ingestion endpoint to avoid attempting to open two concurrent DB connections for writing (which DuckDB does not allow and would cause an error).

Signed-off-by: Andrew Duffy <andrew@a10y.dev>
@a10y a10y requested a review from connortsui20 May 8, 2026 20:13
@a10y a10y added the changelog/skip Do not list PR in the changelog label May 8, 2026
pub db: DbHandle,
/// Serializes `/api/ingest` so concurrent ingests can't race on the same
/// rows and trigger a DuckDB write-write conflict. Reads are unaffected.
pub ingest_lock: Arc<Mutex<()>>,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

really calls out to a RwLock around the handle or something? put the valid states into the type system

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we actually want a separate Connection for the ingest endpoint. it's ok for there to be multiple readonly conns in the process but there can only be one writable. so actually i should make this an Arc<Mutex<Connection>> and leave the pool for the other one in place.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 8, 2026

Merging this PR will improve performance by 13.02%

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚡ 2 improved benchmarks
✅ 1206 untouched benchmarks

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation take_map[(0.1, 0.5)] 1,111.5 µs 983.5 µs +13.02%
Simulation take_map[(0.1, 1.0)] 1.8 ms 1.7 ms +11.01%

Comparing aduffy/duckdb-pool (f362091) with develop (ff12040)

Open in CodSpeed

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 8, 2026

Unable to generate the flame graphs

The performance report has correctly been generated, but there was an internal error while generating the flame graphs for this run. We're working on fixing the issue. Feel free to contact us on Discord or at support@codspeed.io if the issue persists.

@a10y
Copy link
Copy Markdown
Contributor Author

a10y commented May 8, 2026

Closing in favor of just doing this in the other benchmarks PR

@a10y a10y closed this May 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/skip Do not list PR in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants