Repository navigation
Drop ClickHouse query condition cache on every replica after restore - #130
Conversation
A restored table keeps its UUID and part names, so ClickHouse 25.3+ can answer filtered queries from a query condition cache entry recorded before the restore. The traces migrator then sees otel_migrations as empty, re-runs V1 DDL and crash-loops. The service port-forward reaches only one pod, so the existing mark-cache drop never covered the other replicas. Both drops now run ON CLUSTER default, the cluster the chart always defines. Servers older than 25.3 have no query condition cache and reject the statement as a syntax error; that case alone is skipped. Verified against a two-replica 26.8 cluster (both replicas' caches emptied) and a clustered 24.12 server (statement skipped, restore continues). Co-Authored-By: Claude Code <noreply@anthropic.com>
![ai-collaboration-app[bot]](https://avatars.githubusercontent.com/u/10511829?s=60&v=4){kind=small}
There was a problem hiding this comment.
Narrow, well-tested fix. Native-protocol *proto.Exception matching, skip scoped to the query condition cache only, and the default cluster + ZooKeeper assumption holds for the current chart (ZooKeeper isn't scaled down during a ClickHouse restore).
One change requested inline. Also worth considering: finalizeRestore (line 141) only warns on post-restore SQL failure. That's pre-existing, but these statements now decide whether traces work after restore, so the warning should say so and give the manual command (SYSTEM DROP QUERY CONDITION CACHE ON CLUSTER default). Not fatal: that would leave workloads scaled down.
Nit: the description says the skip lets older-chart restores still finish. They'd finish anyway since SQL errors only warn; the skip avoids a misleading warning.
ON CLUSTER makes the mark cache drop fail when any replica is unreachable (distributed DDL timeout). Returning on that first error skipped the query condition cache drop even on healthy replicas, and because finalizeRestore only warns, the CLI reported success while the traces initializer crash-looped. Each statement is now attempted and the failures are joined. The warning now says what stale caches break and lists the statements to run by hand. A failure stays non-fatal so workloads are still scaled up. Co-Authored-By: Claude Code <noreply@anthropic.com>
|
Addressed in 64a1eb0:
|
There was a problem hiding this comment.
Thanks, 64a1eb0 addresses everything: both drops are always attempted, the errors are joined, the timeout case is tested, and the warning names the consequence and the manual fix.
Optional nit: the manual-fix list always includes SYSTEM DROP QUERY CONDITION CACHE, which would hit a syntax error on servers older than 25.3. Fine to leave as is.
Post-restore SQL now runs
SYSTEM DROP MARK CACHEandSYSTEM DROP QUERY CONDITION CACHEwithON CLUSTER default. Before this change the SQL reached only the pod behind the port-forward, and the query condition cache was never dropped. That stale cache made the traces migrator re-run V1 DDL after a restore.Both statements are always attempted, so a replica that is down cannot stop the query condition cache from being dropped on the others. If either fails, the warning lists the statements to run by hand. Failures stay non-fatal so that workloads are still scaled back up.
ClickHouse older than 25.3 has no query condition cache and rejects that statement as a syntax error. The CLI skips the statement in that case instead of logging a misleading warning.
Reviewer check: this assumes the chart's
defaultcluster and ZooKeeper are always present. That holds for the current and oldersuse-observabilitycharts.Validated with unit tests, lint, and live runs against a two-replica 26.8 cluster (caches emptied on both replicas) and a 24.12 server (statement skipped).
Fixes #129
🤖 Generated with Claude Code