perf(clickhouse): reduce ingestion overhead#3237
Conversation
There was a problem hiding this comment.
Code Review
This pull request increases the ClickHouse batch processor timeout and batch size in the OpenTelemetry collector configuration, refines the metric filtering regex, and introduces a ClickHouse migration to optimize storage codecs for time-series columns. The reviewer suggested simplifying the migration SQL by omitting the data types and default values in the MODIFY COLUMN statements, as ClickHouse allows modifying codecs directly without repeating these definitions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
341e07c to
8b6d535
Compare
- otel collector: flush clickhouse batches every 15s (was 5s) with a 100k max batch, and narrow the external metrics filter to the e2b.sandbox/team namespaces consumed downstream - codec migration: Delta for timestamps and monotonic cgroup counters, Gorilla for float metric values; ZSTD(3) for webhook bodies/headers - kill-switch flag to bypass server-side async_insert for batched app inserts (client batches are already large Native blocks) - drop the persisted all-zeros baseline row per sandbox start; the in-memory prev seed already provides the first delta baseline - bound QueryLatestMetrics to maxSandboxTTL (24h) instead of sweeping the full retention history for argMaxIf - idle conn pool cap from LD flag (default 8) to avoid handshake churn on bursty batcher flushes
8b6d535 to
b220eb2
Compare
Low-risk ClickHouse utilization and insert-path optimizations. Single squashed commit, 10 files.
Why
ClickHouse query log showed the top write load coming from
sandbox_host_stats(~482k inserts),sandbox_events(~364k), and the OTelmetrics_gauge/metrics_sumpath (~156k), plus avoidable read scans on hot paths. These are the low-hanging fixes; structural work (sandbox_eventsORDER BY swap, projection removal) is deferred to follow-up PRs.OTel collector (
iac/modules/job-otel-collector/configs/otel-collector.yaml)batch/clickhouse: flush every 15s (was 5s) withsend_batch_max_size: 100000— fewer, larger inserts intometrics_gauge/metrics_sum.filter/external_metrics: narrowede2b.*→^e2b\.(sandbox|team)\..*$. All 9 currently emittede2b.*metrics match, so this is a no-op today; newe2b.*sub-namespaces must be added intentionally. This filter is shared by the ClickHouse, GCP-telemetry, and otel-router pipelines.Schema migrations (codec-only
MODIFY COLUMN, metadata-only, no part rewrites)20260708220300:Delta+ZSTDfor timestamps and monotonic cgroup counters,Gorilla+ZSTDfor float metric values onteam_metrics_gauge_local,team_metrics_sum_local,sandbox_metrics_gauge_local,sandbox_host_stats_local.20260709120000:ZSTD(3)forwebhook_deliveries_localbodies/headers (highly compressible JSON, capped at 64KB by the writer).DEFAULTexpressions (per review), so they cannot drift from the original definitions.Write path
clickhouse-batcher-async-insert(bool, defaulttrue= current behavior): when flipped tofalse, batched host-stats/events inserts setasync_insert=0. The Go batchers already send large Native blocks, so the server-side async buffer only adds a 400ms–4s blocking wait per flush on top of client-side batching.hoststats_collector.go): deltas are computed against the in-memoryprevseed, so the row was write-only noise — one extra insert per sandbox start, and itsinterval_us=0was a divide-by-zero hazard for rate queries.NewDriverWithFeatureFlags; idle conn pool cap now comes from LD flagclickhouse-max-idle-conns(int, default 8; was hardcoded 3) to avoid native-protocol handshake churn on bursty batcher flushes.NewDriverkeeps its signature (belt imports it) and uses the fallback.Read path
QueryLatestMetrics(hot dashboard path) now bounded bymaxSandboxTTL(24h) instead of sweeping the table's full retention history forargMaxIf. No sandbox outlives this window, so latest values always fall inside it. Invariant:maxSandboxTTLmust stay ≥ the largesttiers.max_length_hours.Rollout notes (after merge, LD only — no deploys)
clickhouse-batcher-async-insert=false.clickhouse-batcher-max-batch-size/clickhouse-batcher-max-delay(existing flags) together with queue size.batcher.items.dropped,ProfileEvents_InsertQuery,InsertedRows, and async-insert metrics before/after.