fix: propagate TemplateQueryText/TemplatePlanText through MergeQueryInfo - #47
Open
Sanikadze wants to merge 1 commit into
Open
fix: propagate TemplateQueryText/TemplatePlanText through MergeQueryInfo#47Sanikadze wants to merge 1 commit into
Sanikadze wants to merge 1 commit into
Conversation
MergeQueryInfo copies QueryInfo fields one by one and silently dropped TemplateQueryText/TemplatePlanText, so template texts were lost whenever segment-side QueryInfo was merged on the master.
Sanikadze
force-pushed
the
fix/mergequeryinfo-template-fields
branch
from
July 9, 2026 20:37
967629a to
ee025d9
Compare
Contributor
|
Right now, we intentionally do not use templates in order to save database space (in ClickHouse) and memory footprint (in YAGPCC). Frankly speaking, no one knows how to use them correctly, but they need memory and CPU for processing since it's a long string. |
leborchuk
pushed a commit
that referenced
this pull request
Aug 17, 2026
# Summary Add ClickHouse sink by using `ArchiveWriter` interface. What the new version does, in one line: completed queries, sessions and per-segment metrics that today go to the JSONL file archive can additionally (or instead) be streamed into ClickHouse tables `yagpcc.sessions_part` / `yagpcc.statements_part` / `yagpcc.segments_part`, row-for-row consistent with the JSONL contract. Relates to / builds on #51. ## What this does - **Transport**: implements `internal/master.ArchiveWriter` (`StoreSessions` / `StoreQuery` / `StoreSegmensMetrics`) in `internal/master/clickhouse_writer.go`. No sink-owned buffers, writers or backpressure - queues, deadlines and drops belong to the batch processor from #51. Each `Store*` call is exactly one batched native insert (`PrepareBatch` - `Append` per row - `Send`, clickhouse-go/v2 columnar blocks + LZ4). - **DDL = production schema**: writes into the tables `yagpcc.sessions_part` / `yagpcc.statements_part` / `yagpcc.segments_part` (columns/types/codecs/ORDER BY/PARTITION BY/TTL/skip-indexes copied verbatim from the prod `ch-schema.sql`). The in-repo migration is the standalone `ReplacingMergeTree` variant (no `ON CLUSTER`/`Replicated`/`Distributed`); `--dump-schema --replicated` prints the clustered variant. Idempotency on redelivery is guaranteed by `ReplacingMergeTree` + the deterministic ORDER BY. - **Field mapping**: source of truth is the JSONL that the #51 file writer emits (`SessionDataWrite.ToJSON()`, `QueryStatWriteSerializable.ToJSON()`, and the segment `ToJSON`), so ClickHouse rows are consistent with the existing Kafka/Transfer path - same JSON line, same columns. Table-driven mapping in `internal/sink/clickhouse/mapping.go`; unmapped JSON leaves flow into `_rest`. CDC columns on direct insert: `_timestamp = now`, `_partition = 'direct'`, `_offset = 0`, `_idx = row index`. 64-bit identifiers (`sess_id`/`ccnt`/`tm_id`/`query_id`/`plan_id`) are decoded with `json.Number` (`UseNumber`) - values above 2^53 survive the JSON round-trip without float64 precision loss. `template_query_text` / `template_plan_text` are preserved (keeps the #47 / 5d2427d fix). - **Multi-target fan-out**: any combination of `file` and/or `clickhouse` targets can be enabled at once. A single enabled target wires straight to the source channels; multiple targets tee each stream onto per-target channels via a non-blocking `fanOut` and run an **independent** batch-processor pipeline each (own bounded queue / write timeout / drops), so a slow or unreachable ClickHouse never stalls or drops the file writer. `tm_id` is stamped once in `fanOut` before the tee, so all targets see identical rows and writers stay read-only. `writer_*` pipeline metrics (`writer_processed_messages_total`, `writer_dropped_messages_total`, …) gain a `target` label. - **Robustness**: ClickHouse connects lazily - an unreachable server at startup does not crash the process; connection errors surface per batch, the pipeline drops that batch (counted in `writer_dropped_messages_total{target="clickhouse"}`) and the file target keeps writing losslessly. `ArchiveWriter` gained `Close()` so the ClickHouse connection is released on shutdown. Config validation rejects a `clickhouse` target with `database` != `yagpcc`: the DDL and inserts are fully qualified with `yagpcc.*`, so any other configured database would mean silently writing nowhere. ## Config ```yaml writers: targets: - type: file enabled: true # ... existing file writer fields - type: clickhouse enabled: true addrs: ["clickhouse-01:9000", "clickhouse-02:9000"] database: yagpcc user: yagpcc # password via YAGPCC_CH_PASSWORD env tls: enabled: true ``` The `YAGPCC_CH_PASSWORD` env override is applied on the normal startup path as well as in the schema CLI, so the password never has to live in the YAML. ## Schema management ```bash yagpcc --dump-schema # standalone DDL to stdout (no connection required) yagpcc --dump-schema --replicated # clustered (ReplicatedReplacingMergeTree + ON CLUSTER + Distributed) yagpcc --dump-migration --from=N --to=M yagpcc --migrate-only # apply standalone migrations and exit yagpcc --verify-schema # check schema version and exit ``` `--migrate-only` connects through `database=default` rather than the configured database: on a fresh server the `yagpcc` database does not exist yet, and pinning the connection to it made bootstrap fail with `code: 81` before `CREATE DATABASE` could run. All DDL statements are fully qualified (`yagpcc.*`), so the session database does not matter; regular runtime inserts still connect to the configured database, and the `database == yagpcc` validation guard is unaffected. Docs: `docs/historical-stats-flow.md` gains a "ClickHouse target" section.
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
2 participants
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
MergeQueryInfocopiesQueryInfofields one by one and silently droppedTemplateQueryText/TemplatePlanText. The hooks-collector only sends the template texts on the START event; any later merge (DONE/END arriving with aQueryInfowithout templates) wiped them, so template texts were lost for every finished query.Fix
Two lines in
internal/storage/group.go: carry both fields through the merge the same way as the other string fields (max, i.e. non-empty wins).Tests
TestQuery42goldens ininternal/storage/merger_test.gohad encoded the buggy behaviour (emptyTemplatePlanTextafter merge even though the START event carried it). Updated the expected results to assert the template now survives the merge.