Skip to content

Add http/https vs fetch transport benchmark for the Node.js client#779

Draft
Copilot wants to merge 2 commits into
mainfrom
copilot/replace-http-https-modules
Draft

Add http/https vs fetch transport benchmark for the Node.js client#779
Copilot wants to merge 2 commits into
mainfrom
copilot/replace-http-https-modules

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 29, 2026

Summary

The issue proposes replacing the Node.js client's legacy http/https transport with fetch/undici for performance, and the discussion asks for trustworthy comparison data before committing to a rewrite. This PR adds a reproducible benchmark that pits the current client against a trivial fetch() (undici) stub over identical requests — no transport rewrite yet, just the data to justify one.

  • benchmarks/transport/ — new benchmark:
    • clients.ts — a TransportClient abstraction with two impls driven through the same scenarios: SdkTransportClient (@clickhouse/client, uses exec() to isolate transport from row parsing) and FetchTransportClient (minimal fetch() stub).
    • index.ts — runner for three use cases with warmup + configurable iterations and p50/p90/p99 latency: single-request latency (SELECT 1), download throughput (large result set), upload throughput (POST to null() so no table setup is needed). Tunable via CLICKHOUSE_URL, LATENCY_REQUESTS, DOWNLOAD_ROWS, UPLOAD_ROWS, ITERATIONS, WARMUP.
    • stats.ts — dependency-free latency/throughput helpers.
    • README.md — run instructions, config, interpretation, sample local run.
  • Tooling fixes to make benchmarks buildable/lintable:
    • benchmarks/tsconfig.json extended a non-existent ../tsconfig.json → now ../tsconfig.base.json; registers transport/**/*.ts.
    • Added benchmarks/eslint.config.mjs so benchmark files have a discoverable flat config (ESLint v10 + lint-staged otherwise error on staged benchmark files).

Indicative single local run (sandbox, Node 24, loopback):

Scenario @clickhouse/client (http/https) fetch (undici) stub
SELECT 1 latency (mean) 2.49 ms 1.74 ms
Download throughput 273 MiB/s 68 MiB/s
Upload throughput 40 MiB/s 40 MiB/s

Early signal is a trade-off rather than a clear win: fetch has lower small-request latency, the existing streaming path drains large downloads faster, and uploads are server-bound for both. Numbers are environment-specific and meant to be re-run per workload.

docker-compose up -d
tsc --project benchmarks/tsconfig.json \
  && node benchmarks/dist/benchmarks/transport/index.js

Checklist

  • A human-readable description of the changes was provided to include in CHANGELOG

@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copilot AI changed the title [WIP] Replace legacy http/https with fetch API for improved performance Add http/https vs fetch transport benchmark for the Node.js client May 29, 2026
Copilot AI requested a review from peter-leonov-ch May 29, 2026 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Performance] Replace legacy http/https modules with fetch API or better yet with undici low level api for better performance

3 participants