test: producer-to-collector routing on pg_stat_ch.block_format#117
Open
JoshDreamland wants to merge 8 commits into
Open
test: producer-to-collector routing on pg_stat_ch.block_format#117JoshDreamland wants to merge 8 commits into
JoshDreamland wants to merge 8 commits into
Conversation
…to-end Introduce a new exporter that builds Arrow IPC RecordBatches through the typed StatsExporter column-factory interface (StatLC/StatHC/StatTimestamp) instead of the open-coded ArrowBatchBuilder used by arrow_batch.cc. Composition over inheritance: the new exporter holds an OTelExporter for gRPC transport (SendArrowBatch) but doesn't extend it, so the per-row LogRecord state machine in OTelExporter — which is unused on this path post-PR-#72 — stays out of scope. Wire shape targets events_raw (the unified schema authored in PR #99), not the legacy query_logs_arrow: * query_id, parent_query_id: Int64 (no sprintf decimal-string encoding) * pid: Int32 * err_elevel: UInt8 * buffer counters (shared/local/temp_blks_*, *_blk_*_time_us, wal_*, cpu_*_time_us): Int64 * parallel_workers_planned/launched: Int16 * jit_*: Int32 * LC strings (db_*, err_sqlstate, app, server_role, region, cell, service_version, read_replica_type) -> DictionaryUtf8 * HC strings (query_text, err_message, client_addr, instance_ubid, server_ubid, host_id, pod_name) -> plain utf8 * ts: arrow::timestamp(MICRO, "UTC") matching DateTime64(6, 'UTC') Column<T> wrappers are nested private types inside OTelArrowExporter (not at namespace scope) so they can inherit from the protected Column<T> base — same convention OTelExporter and ClickHouseExporter use for their own column types. Columns the caller doesn't explicitly populate are synthesized in BeginRow by the exporter itself, so stats_exporter.cc's column-emission loop stays unchanged: * parent_query_id (hardcoded 0 until PR #95 lands and PschEvent carries the field — events_raw requires the column on every insert, no DEFAULT) * 8 envelope columns from pg_stat_ch.extra_attributes (instance_ubid, server_ubid, server_role, region, cell, host_id, pod_name) plus read_replica_type (default 'none' if extra_attributes didn't supply) * service_version pinned to the compile-time PG_STAT_CH_VERSION macro This commit only adds the exporter file (no dispatcher wiring yet) — the next commit adds the GUC and routes batches through it when on. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New bool GUC pg_stat_ch.use_unified_arrow_exporter (default off, PGC_SIGHUP)
opts a producer instance into the OTelArrowExporter added in the previous
commit. When on (together with use_otel and otel_arrow_passthrough), the
bgworker constructs an OTelArrowExporter at init time and PschExportBatch
goes through ExportEventStats (the typed column interface) instead of
calling ExportEventsAsArrow (the legacy bypass that uses arrow_batch.cc
directly).
When off — the default — behavior is preserved bit-for-bit:
* arrow_batch.cc / ExportEventsAsArrow remain reachable on the
arrow_passthrough path
* The OTelExporter column-emission path (off-arrow OTel logs) remains
reachable when arrow_passthrough is off
* The CH-native ClickHouseExporter remains reachable when use_otel is off
The sprintf-decimal-string ID encoding in arrow_batch.cc lives on for
the legacy path; the new exporter neither calls into it nor perpetuates
it. After the GUC has been on in prod long enough to retire the legacy
query_logs_arrow table, the arrow_batch.cc + ExportEventsAsArrow* + the
otel_arrow_passthrough GUC itself can be deleted in a single follow-on.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…_raw
Closes the test gap that's existed since the Arrow path went live: no
existing test proves that pg_stat_ch's Arrow IPC output can actually be
ingested by ClickHouse against the unified events_raw schema. t/026
asserts on the IPC schema shape via pyarrow but never pushes the bytes
into CH; t/010 etc. exercise the CH-native Block path, not Arrow.
The new test wires the full producer-to-CH chain locally, bypassing
the OTel collector + receiver service entirely:
1. Spin up a node with use_unified_arrow_exporter=on +
debug_arrow_dump_dir set, an OTel endpoint that doesn't resolve so
gRPC send fails — MaybeDumpArrowBatch fires BEFORE send so IPC
files land on disk regardless.
2. Run a deliberately-shaped workload (SELECT, CREATE, INSERT,
SELECT count, DROP — five distinct statements).
3. Force pg_stat_ch_flush(), wait for IPC files in $dump_dir.
4. TRUNCATE pg_stat_ch.events_raw, then for each IPC file:
curl -X POST --data-binary @$f \
'http://localhost:18123/?query=INSERT INTO pg_stat_ch.events_raw FORMAT ArrowStream'
A type mismatch on the wire (e.g. if the producer regressed to
writing query_id as String) would surface here as a 4xx with a
clear error rather than silently corrupting data.
5. SELECT count() FROM events_raw, assert >= 5 rows.
6. Pull system.columns and assert each id/counter column has the
declared type from PR #99's schema (no silent string-typed regressions).
7. Pinpoint the marker SELECT row and assert db_name/db_operation/
query_text values match what we sent.
8. Assert envelope columns (instance_ubid, server_role, region, cell,
read_replica_type) carry the values from pg_stat_ch.extra_attributes.
9. Assert parent_query_id is 0 across all rows (synthesized by the
exporter until PR #95 lands).
Skips cleanly when Docker / the test CH container / the events_raw
schema aren't available — same patterns as t/010, t/013, t/021.
The "no OTel collector required" property makes this test purely a
producer⇄CH wire-format check. The clickgres-platform Go receiver is
not exercised here, since for verifying that the bytes match the
schema, a curl invocation is the simplest possible expression of "POST
this Arrow IPC body to CH" — the receiver's only added value over
curl in prod is OTel-collector pipeline integration, which we don't
care about for wire-format correctness.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three real findings worth addressing, plus a TODO for one that's worth
flagging but bigger than this PR:
src/export/otel_arrow_exporter.cc:
* NumExported() now returns row_count_ instead of delegating to the
inner OTelExporter. The inner exporter's exported_count_ accumulates
across batches because we never call inner_->BeginBatch() (its
per-record LogRecord state machine is unused on this path), and
PschExportBatch fetch_add's NumExported() into shared stats once per
successful commit — so the cumulative inner count would cause
quadratic over-reporting. (Cursor Bugbot, medium.)
* ExtraAttrs class comment said "last write wins on duplicate keys"
but Get() does a linear scan from the front and returns the first
match. Comment now reflects what the code actually does. Duplicate
keys in pg_stat_ch.extra_attributes are a user error in practice;
no behavior change. (Copilot.)
* Added a TODO(memory-budget) at CommitBatch documenting the missing
mid-batch flush against psch_otel_max_block_bytes. The legacy
ExportEventsAsArrowInternal flushes when the builder exceeds 3 MiB;
this exporter ships the whole batch in one IPC. Acceptable while
the GUC defaults off, but the budget check has to land before the
default flips. Plumbing it through requires either invalidating
caller-held column shared_ptrs at the flush boundary or threading
a per-row size hook through the StatsExporter interface. (Cursor
Bugbot, high — disagreeing on severity for the shadow-rollout
window but acknowledging the underlying gap.)
* Comment note on MaybeDumpArrowBatch acknowledging the intentional
duplication with stats_exporter.cc — both copies die when the
legacy path retires, so extracting a shared helper now would be a
header just to delete it later. (Copilot, push-back.)
t/036_unified_arrow_e2e.pl:
* curl now uses --fail-with-body so HTTP 4xx/5xx from CH surfaces as
a non-zero exit at the assertion site. The downstream SELECT count()
assertion would catch a real ingestion failure too, but the sharper
error at the source is a strict improvement. (Copilot.)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The bgworker constructs the exporter implementation once at init time
based on this GUC, but PschExportBatch was re-reading it every cycle to
choose between the legacy ExportEventsAsArrow bypass and the new
unified ExportEventStats path. Under PGC_SIGHUP, the two reads can
disagree:
- Init=on, runtime=off: dispatcher takes the legacy bypass and calls
SendArrowBatch on an OTelArrowExporter, which doesn't override it,
so every batch silently drops.
- Init=off, runtime=on: dispatcher runs the unified path through a
plain OTelExporter, which ships OTLP log records instead of Arrow
IPC. events_raw never sees the data.
Flag is a producer-shape feature toggle, not an operational tunable;
flipping it should be a deliberate restart.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…_block_bytes OTelArrowExporter accumulated rows into a single Arrow IPC up to psch_batch_max (default 200000) with no byte ceiling, so a queue backlog could produce a payload exceeding gRPC's 4 MiB wire cap or otelcol's HTTP body cap. Closes the gap relative to ExportEventsAsArrowInternal which flushes mid-batch when the builder's estimated bytes cross psch_otel_max_block_bytes. Column wrappers each take a pointer to bytes_estimate_ and bump it per Append (sizeof for fixed-width, value bytes + 4 for var-length offsets). BeginRow samples bytes_estimate_ at the row boundary; if it has crossed max_block_bytes_ and at least one row is already accumulated, Flush() runs, ships the chunk, and resets per-flush state. Arrow ArrayBuilder::Finish leaves builders empty and reusable, so subsequent chunks share the same slot vector + column shared_ptrs without any re-registration. NumExported now reports exported_in_batch_ (cumulative across all chunks in the active batch) instead of the per-chunk row_count_, otherwise mid-batch flushes would erase the dispatcher's view of work done. A sticky batch_failed_ flag poisons CommitBatch after any mid-batch Flush failure so partial batches never count as a success. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The central OTel collector's routingconnector matches on the pg_stat_ch.block_format OTLP attribute to fan batches between the legacy receiver (writing query_logs_arrow) and a new receiver configured for events_raw. Producer-side migration is therefore a single-value swap rather than an endpoint reconfiguration — keeps the extension dumb about deployment topology and the routing decision in the central collector where it belongs. The change widens StatsExporter::SendArrowBatch with an explicit string_view block_format argument (no default — Google Style forbids default args on virtuals because they dispatch on static type, not dynamic). Threaded through PopulateResource and the per-record attribute setter in OTelExporter::SendArrowBatch. Two callers: - ExportEventsAsArrowInternal (legacy ArrowBatchBuilder path) passes "arrow_ipc" — preserves the existing wire-format marker so the legacy receiver continues to be the routing target. - OTelArrowExporter::Flush (unified path) passes "arrow_events_raw" — the distinct value the routingconnector will key on to dispatch to the events_raw receiver. Coordinated with collector values in clickgres-platform/services/datagres-otelcol (separate PR). No behavior change for the legacy path. No-op for any deployment whose collector doesn't have the routingconnector configured (the attribute is just additional metadata the receiver can ignore). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes the producer-side contract gap: the unified Arrow exporter emits pg_stat_ch.block_format=arrow_events_raw on its OTLP envelope, while the legacy ArrowBatchBuilder path emits arrow_ipc. The central OTel collector's routingconnector is supposed to fan batches between the legacy query_logs_arrow target and the new events_raw target based on that marker. This test pins the producer half of the contract. New TAP test t/037_arrow_routing.pl exercises both arms: - arm 1 (unified=off, arrow_passthrough=on): legacy.jsonl grows, events_raw.jsonl and default.jsonl don't. - arm 2 (unified=on): events_raw.jsonl grows with arrow_events_raw in the routed payload, legacy.jsonl and default.jsonl don't. Stock otelcol-contrib image (matches existing t/024 setup, version 0.120.0). Routingconnector keyed on the resource attribute (where pg_stat_ch.block_format lives via PopulateResource), two file exporters writing JSONL to a bind-mounted dir the test reads from. No CH receiver — the events_raw column-set contract isn't validated here; that's covered by t/036 (producer Arrow IPC -> direct CH ingest) and by a separate receiver-faithful test in clickgres-platform. New helpers in t/psch.pm (psch_routing_collector_available, psch_start_routing_collector, psch_stop_routing_collector) live on a separate compose profile + container/ports so the new test can coexist with t/024's collector if both fire in the same suite. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the producer-side half of the Arrow exporter migration contract: pin
pg_stat_ch.block_formatemission and the routing-connector match-shape via a generic TAP test.The unified Arrow exporter (PR #116's commit 4) emits
pg_stat_ch.block_format=arrow_events_rawon its OTLP envelope; the legacyArrowBatchBuilderpath emitsarrow_ipc. In prod, the centraldatagres-otelcoldeployment'sroutingconnectoris supposed to fan batches between the legacyquery_logs_arrowreceiver target and a newevents_rawreceiver target based on that marker. This test pins the producer side of that contract: it doesn't run a real receiver (no CH, nodatagres-arrow-exporter) — it runs stockotelcol-contribwith twofileexporters and asserts which one received which arm's batches.What it catches (and what it doesn't)
Catches:
pg_stat_ch.block_formatemission (e.g. unified path silently falling back toarrow_ipc).Does NOT catch:
events_rawcolumn-set contract or the receiver's wire-format expectations — those live behinddatagres-arrow-exporterinclickgres-platformand are covered by a separate receiver-faithful test there (in flight).t/036_unified_arrow_e2e.plcontinues to handle the producer Arrow IPC -> direct CH ingest path.Mechanics
docker/docker-compose.arrow-route.yml(separate from the existingdocker-compose.otel.ymlsot/024andt/037can coexist; different ports, different container name).docker/otel-routing/collector-config.yaml— stockotel/opentelemetry-collector-contrib:0.120.0,routingconnectorkeyed on the resource attribute, threefileexporters (legacy / events_raw / default fall-through) writing JSONL to a bind-mounted dir.t/psch.pmhelpers:psch_routing_collector_available/psch_start_routing_collector/psch_stop_routing_collector.t/037_arrow_routing.plruns two arms (legacy + unified) and asserts that the corresponding file grew while the other files didn't. The unified arm also greps the routed JSONL tail forarrow_events_rawto pin the marker value content.Stacking
Based on
unified_arrow_exporter(PR #116). Hard-depends on commits 2 (the GUC) and 4 (the marker swap). Will retarget tomainafter #116 merges.Test plan
unified_arrow_exporter.psch_*_otelcol).🤖 Generated with Claude Code