Skip to content

feat: make ClickHouse connection limit configurable - #4492

Open
arthurbragaa wants to merge 2 commits into
umami-software:devfrom
arthurbragaa:configurable-clickhouse-connection-limit
Open

feat: make ClickHouse connection limit configurable#4492
arthurbragaa wants to merge 2 commits into
umami-software:devfrom
arthurbragaa:configurable-clickhouse-connection-limit

Conversation

@arthurbragaa

@arthurbragaa arthurbragaa commented Aug 26, 2026

Copy link
Copy Markdown

Summary

  • add optional CLICKHOUSE_MAX_OPEN_CONNECTIONS configuration
  • validate positive integers during database preflight
  • preserve the ClickHouse client default when unset
  • document the default and per-instance pool sizing

Closes #4491.

Verification

  • pnpm test — 95 files, 748 tests
  • pnpm exec tsc --noEmit
  • pnpm build
  • changed-file Biome lint
  • production Docker image build
  • Docker Compose integration: default peaked at 10 connections; configured 25 peaked at 25 with 50/50 successful /api/send requests; invalid 0 exited before health startup

Full pnpm lint remains blocked by pre-existing diagnostics on dev; the changed files pass Biome.

AI assistance disclosure: This contribution was implemented and tested with AI assistance and reviewed by Arthur Braga Alfredo.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Implemented and tested with AI assistance; reviewed by Arthur Braga.
@vercel

vercel Bot commented Aug 26, 2026

Copy link
Copy Markdown

@arthurbragaa is attempting to deploy a commit to the Umami Software Team on Vercel.

A member of the Team first needs to authorize it.

@greptile-apps

greptile-apps Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR adds a validated CLICKHOUSE_MAX_OPEN_CONNECTIONS setting, passes it to the ClickHouse client when configured, and documents per-instance pool sizing.

  • Preserves the client’s default connection limit when the setting is unset.
  • Adds runtime and database-preflight validation for positive safe integers.
  • Adds tests for configured, unset, and invalid values.

Confidence Score: 4/5

The PR should not merge until ClickHouse-specific validation is gated so PostgreSQL-only deployments cannot be broken by an irrelevant environment value.

The new module-scope and preflight checks reject CLICKHOUSE_MAX_OPEN_CONNECTIONS before determining whether ClickHouse is enabled, making an unused setting capable of breaking route loading, builds, and container startup.

Files Needing Attention: src/lib/clickhouse.ts, scripts/check-db.js

Important Files Changed

Filename Overview
src/lib/clickhouse.ts Adds the client pool option, but validates it at module load even when ClickHouse is disabled.
scripts/check-db.js Adds matching preflight validation before both ClickHouse applicability and the SKIP_DB_CHECK bypass.
src/lib/clickhouse.test.ts Covers valid, unset, and malformed values but not the disabled-ClickHouse or SKIP_DB_CHECK cases.
README.md Documents the option, client default, and aggregate pool sizing across replicas.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  Env[Environment] --> URL{CLICKHOUSE_URL set?}
  Env --> Limit[Parse CLICKHOUSE_MAX_OPEN_CONNECTIONS]
  Limit -->|Invalid| Fail[Throw or exit]
  URL -->|Yes| CH[Create ClickHouse client]
  URL -->|No| PG[Use PostgreSQL]
  Limit -->|Valid or unset| CH
  Limit -. currently evaluated even here .-> PG
Loading

Reviews (1): Last reviewed commit: "feat: make ClickHouse connection limit c..." | Re-trigger Greptile

Comment thread src/lib/clickhouse.ts Outdated
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.

1 participant