Skip to content

fix(clickhouse): retire the native events pool when its host changes - #120295

Closed
vdekrijger wants to merge 1 commit into
masterfrom
vasco/native-events-pool-retirement
Closed

vdekrijger wants to merge 1 commit into
masterfrom
vasco/native-events-pool-retirement

Conversation

@vdekrijger

Copy link
Copy Markdown
Contributor

Problem

Staff can change the native events cluster host at runtime through an instance setting. Each host change caches one more ClickHouse connection pool per worker, and the old pools keep their sockets open indefinitely.

Changes

  • Changing the native host retires the previous host's pool.
  • Idle connections close at once. Each in-flight connection closes when its query finishes, so running queries are not interrupted.
  • Other workloads keep the existing cached pool factory.

How did you test this code?

Test rationale: test_native_events_pool_is_retired_when_the_host_changes in the existing connection tests switches the host, checks that a new pool serves it, and checks that a client returned to the old pool disconnects.

Release status

  • No feature flag controls this change

Docs update

None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted).
Agent: Claude Code, model claude-opus-5-5.

Split out of #116838 so each piece of the native events cluster rollout reviews on its own. Skills: stacking-prs, writing-tests, writing-code-comments, writing-pr-descriptions.

@vdekrijger vdekrijger self-assigned this Oct 20, 2026
@trunk-io

trunk-io Bot commented Oct 20, 2026

Copy link
Copy Markdown

Merging to master in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Trunk lane — backend Python lane

This PR is assigned to the backend Python lane. It runs backend Python tests and may merge in parallel with PRs in other lanes.

✅ Duplication (Python) — clean

New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

✅ Duplication (TypeScript) — clean

New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

@trunk-io

trunk-io Bot commented Oct 20, 2026

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

View Full Report ↗︎ ⋅ Docs

@vdekrijger vdekrijger added the posthog-review Run a Standard PostHog Review label Oct 20, 2026

@posthog posthog Bot left a comment

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.

PostHog Review · Standard · reviewed 18f21cd: no findings.

This Standard review started from the posthog-review label. Comment @posthog review --deep for a Deep review, or @posthog help to see all commands.

@posthog posthog Bot removed the posthog-review Run a Standard PostHog Review label Oct 20, 2026
@vdekrijger

Copy link
Copy Markdown
Contributor Author

Superseded by #122489, which merged on Oct 26 and routes events_json reads to the events cluster with one rule instead of the per-query opt-in this PR carried. Closing; the branch stays.

@vdekrijger vdekrijger closed this Oct 27, 2026
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