[vuanhphung]

What type of PR is this?

• Bug Fix
• Other (performance)

Description

On use_kernel=True the kernel exports telemetry itself, and is_telemetry_enabled already returns False for kernel connections. Every kernel connect() still registered a no-op client with TelemetryClientFactory under its global lock, and the first one also started the flush thread, executor, and excepthook. Concurrent connects serialize on that lock behind the GIL. In a 200-thread cold burst, initialize_telemetry_client took p50 45 ms / max 120 ms on main (p50 692 ms / max 1.7 s before #1024).

Registering also caused a bug. When a kernel connection was opened first, its no-op client stayed registered for the host, so a later Thrift connection to that host reused it and never sent Python telemetry. When a Thrift connection was opened first, kernel connections sent their connection log through the Thrift client and overwrote its connection parameters.

Kernel connections now use a NoopTelemetryClient directly and skip the factory on close. Connection-failure telemetry is unchanged (still suppressed when the kernel was attempted, still sent for Thrift). Errors raised with host_url still go through the host-keyed lookup in exc.Error, so in a process that also has a Thrift connection to the same host, a kernel connection's error logs can still go out through the Thrift client. That behavior is unchanged from main and out of scope here.

How is this tested?

• Unit tests
• E2E Tests
• Manually

New unit tests cover: a kernel connect/close leaves the factory untouched, it doesn't change the refcount of a Thrift client for the same host, and a Thrift client created after a kernel connection is a real client. All three fail on main. The proxy assertions in test_kernel_http_client_lifecycle are replaced, since kernel connections no longer build Python connection parameters. Full unit suite (-m "not realkernel"), the realkernel sets, and tests/e2e/test_kernel_backend.py pass.

Perf, main (0c1b9e8) vs this branch: c800.py, 1 process × 200 sessions × 60 s against a local mock SEA server (40 ms per statement), alternating order, n=3, median [min–max]:

	main	this PR
connect p50	488 ms [459–765]	411 ms [376–427]
connect p99	903 ms [822–1136]	767 ms [542–788]
all sessions open	0.98 s [0.97–1.33]	0.82 s [0.74–0.89]
avg QPS	1559 [1547–1577]	1572 [1559–1579]
peak QPS	1643 [1638–1675]	1663 [1654–1683]
steady QPS	1583 [1562–1591]	1587 [1573–1593]
CPU per query	1.203 ms	1.191 ms

Live runs against a shared Reyden warehouse (1 × 200 × 60 s, n=3) showed no regression, but warehouse load from other clients varied too much to resolve a gain this small.

Live benchmark (staging Large Reyden warehouse)

TPC-H Q6 on a Large Real-Time warehouse, 800 sessions (4 processes x 200 threads), cold start, each session querying back to back for 120 s, 16 client cores. Isolated A/B: only this change vs connector main 0c1b9e8 + kernel 1.4.0, 3 interleaved rounds, nothing else on the warehouse. Measured at 508709b. Median (min-max):

	main	this PR
Average QPS (setup included)	2146 (1998-2220)	2188 (2160-2259)
Steady QPS (after all sessions open)	2183 (2043-2260)	2234 (2200-2304)
Query latency p50 (ms)	354 (342-404)	348 (337-353)
Client CPU per query (ms)	1.21 (1.20-1.25)	1.21 (1.15-1.30)
Peak RSS per process (MB)	405 (393-407)	395 (389-396)

No measurable QPS or CPU change; ~10 MB less per process.

SELECT 1 on an X-Large Real-Time warehouse (driver-bound). Q6 above is limited by the warehouse, so the same harness also ran SELECT 1, where the client is the limit: X-Large warehouse, 4 processes x 200 threads, 120 s, 16 client cores, this change vs connector main 0c1b9e8 + kernel 1.4.0, measured at 508709b, run in rotation with the other factory PRs, 2 rounds. Median (min-max):

	main	this PR
Average QPS (setup included)	6094 (6040-6147)	6021 (5969-6072)
Peak QPS (best second)	6513 (6503-6523)	6499 (6493-6505)
Query latency p50 (ms)	124 (123-124)	124 (124-124)
Client CPU per query (ms)	1.34 (1.33-1.35)	1.34 (1.34-1.35)

Average QPS change per round: -1.2%, -1.2%.

This pull request and its description were written by Isaac.

[peco-review-bot]

Verdict: 1 Low

Looks good, with one low concern. Skipping the factory on kernel connections is correct and consistent: the _kernel_owns_telemetry predicate matches the truthiness check in is_telemetry_enabled, the latency logger is already gated, and the tests cover the refcount and the order of kernel and Thrift connections. The concern is that host-keyed failure telemetry raised from exc.Error can still send kernel-connection errors through a co-resident Thrift client.