feat: add replicated host sync gate

[ku524]

Summary

• Add default-off reconcile.host.wait.replicas.sync rolling gate for replicated hosts.
• Wait for a recreated host to catch up to a bounded ClickHouse replication baseline before advancing to the next host.
• Use SYSTEM SYNC REPLICA ... LIGHTWEIGHT, replicated database sync, async-loader settling, and a stable health window.
• Do not require replication_queue to become empty.
• Clear stale caught-up markers on data-loss/missing-volume paths only when the gate is enabled.

Contribution checklist

• All commits in the PR are squashed.
• The PR targets the dedicated next-release branch 0.27.2, not master.
• The commit is signed off.

Operational scenario

This is primarily aimed at local or direct-attached storage recovery cases, such as NVMe-backed Local PVs. When a recreated ClickHouse pod starts on an empty or replaced local disk, Kubernetes readiness and the existing absolute_delay marker can become true before the replica has discovered all replicated objects and fetched the known parts from peers. The opt-in sync gate prevents the rolling reconcile from advancing to the next host until that recreated replica has passed ClickHouse-level async-load, object-discovery, sync, and health checks.

Why the existing caught-up check is not enough

The existing caught-up marker path is intentionally left unchanged when sync.enabled=false, but it is a weak proxy for recreated-host recovery because it only polls the local host's MAX(absolute_delay) from system.replicas before writing status.hostsWithReplicaCaughtUp. That metric is limited to replicated objects already loaded and visible on the local server.

That can miss the failure mode this PR targets. During startup/recreation, asynchronous database/table loading may not have exposed every replicated object on the local host yet, and a local delay metric cannot discover replicated DBs/tables that exist on peer replicas or issue a ClickHouse sync barrier for their known parts. This PR keeps the old behavior as the default, and adds an opt-in gate that first settles async loading, discovers replicated objects from peers, runs DB/table sync barriers, and only then writes the caught-up marker after a stable health window.

Workload and side-effect considerations

The legacy check is lighter: it only polls MAX(absolute_delay) on the local host. The new gate does more work when explicitly enabled, so it can extend rolling reconcile time and add ClickHouse/Keeper/replication load while a recreated host catches up.

The added work is intentionally scoped:

• Default-off: clusters keep the legacy lightweight behavior unless sync.enabled=true is configured.
• Rolling scope: the gate runs in the existing per-host rolling path, not as a cluster-wide concurrent sweep.
• Bounded object scope: it discovers replicated DBs/tables from peers and syncs those objects; it does not wait for the entire system.replication_queue to drain.
• Lightweight table sync: table sync uses SYSTEM SYNC REPLICA ... LIGHTWEIGHT instead of legacy full sync, so it waits for the relevant known part-acquisition work without blocking on unrelated merges, mutations, or new ingest after the sync baseline.
• Deadline controls: the whole gate uses a shared timeout; onTimeout=proceed can advance without writing the caught-up marker when operators prefer availability over blocking the rollout.

Operationally, enabling this gate trades faster rolling progress for a stronger recovery guarantee. That tradeoff is intended for local/direct-attached PV recovery cases where advancing to the next host before the recreated replica has rebuilt from peers is riskier than the extra catch-up work.

Related to #1704. This mitigates the “advance before recreated replica catches up” path, but does not close #1704 because the cross-operator-restart sequencing gap remains out of scope.

Safety

• Default-off: existing behavior is unchanged when sync.enabled=false.
• onTimeout=proceed advances without writing the caught-up marker.
• Parent context cancellation, query/connection errors, async-loader failures, and readonly/session-expired at deadline remain hard failures.
• Unsupported LIGHTWEIGHT versions fail explicitly.

Test plan

• bash ./dev/run_code_generator.sh
• bash ./dev/build_manifests.sh
• bash ./dev/generate_helm_chart.sh
• bash ./dev/go_build_all.sh
• bash ./dev/find_unformatted_sources.sh
• go test -count=1 ./pkg/apis/clickhouse.altinity.com/v1/... ./pkg/model/chi/schemer/... ./pkg/controller/chi/... ./pkg/controller/common/announcer
• python3 -m py_compile tests/e2e/test_operator.py
• yq eval-all 'true' tests/e2e/manifests/chopconf/test-079-sync-gate.yaml tests/e2e/manifests/chi/test-079-sync-gate-1.yaml tests/e2e/manifests/chi/test-079-sync-gate-2.yaml
• git diff --check

Not run locally:

• test_010079* and test_010056* e2e runtime, because the local docker-compose runner fails on Apple Silicon nested minikube/runc and no safe local native cluster is currently available.
• Runtime confirmation of ClickHouse system.asynchronous_loader.is_ready behavior on the target e2e image; this is covered by the new e2e path/CI.

[alex-zaitsev]

Hi @ku524, thank you, it is really useful feature. We plan to include it into 0.28.x. Meanwhile, could you please check the AI review comments:

Review — #2018

Scope: ~1.6k lines. Opt-in reconcile.host.wait.replicas.sync gate before advancing a rolling reconcile after a recreated/rebuilt replica. Base: 0.27.2.

Verdict

Useful feature for local-PV / empty-disk recovery; design is thoughtful (default-off, LIGHTWEIGHT barrier, health window, onTimeout). Not merge-ready as-is: stale base, a few correctness risks, and incomplete runtime proof. Treat as request changes.

What works

• Clear problem statement: absolute_delay alone is too weak while async DB/table load is still in progress.
• Default-off preserves current behavior.
• Pipeline is coherent: async-load settle → peer object discovery → SYSTEM SYNC DATABASE REPLICA / WAIT LOADING PARTS / SYNC REPLICA … LIGHTWEIGHT → stable health window → hostsWithReplicaCaughtUp.
• Data-loss / missing-volume paths clear the marker and set forceReplicaCatchUp so even the first host can be gated.
• includeHost now fails closed when the gate is enabled and catch-up fails (important intentional behavior change).
• Docs + config plumbing look complete; unit tests cover config/normalize and some gate helpers.

Blocking / high

1. Base branch is stale (0.27.2)
   Needs rebase onto current release line (0.27.3 / post-Protect last healthy shard replica during interrupted rolls (#1704) #2046). It will collide with shard-safety / wait-path work in worker-wait-exclude-include-restart.go and worker-status-helpers.go. Interaction with #2046 should be spelled out: this PR delays advancing after a host is back; Protect last healthy shard replica during interrupted rolls (#1704) #2046 avoids disrupting the last healthy peer. Complementary, not substitutes.

2. system.asynchronous_loader readiness query looks suspicious
   Pending jobs are counted as:
   PENDING OR is_executing OR is_ready OR is_blocked.
   If is_ready means “job finished / ready,” the barrier may never clear. The PR author notes this was not verified on a real image. Please confirm column semantics and fix before merge.

3. ClickHouse version floor for all new SQL
   Gate requires >= 23.4 for LIGHTWEIGHT (PR #48085; often cited as 23.4/23.5). Also uses SYSTEM SYNC DATABASE REPLICA and SYSTEM WAIT LOADING PARTS — confirm those exist at the same floor, or fail with a precise “unsupported statement” message. Silent partial support would be bad.

Medium

4. Double async-load barrier — HostAsyncLoadBarrier runs in runReplicaSyncGate and again inside HostSyncReplicatedObjects. Fine but noisy; consider one place.

5. Peer discovery via clusterAllReplicas — Correct idea; under heavy unavailability / wrong cluster name it can return empty and push caught-up early (replicatedObjects == 0). Ensure “no peers reachable” ≠ “nothing to sync.”

6. timeout: 0 ⇒ ~100-year deadline — Document loudly; easy foot-gun for stuck rolls.

7. Mode is effectively fixed — Config advertises mode: lightweight with no other modes / no full-sync fallback. Either drop mode until needed or reject unknown values explicitly (verify Normalize does).

8. E2E not run — test_010079 is the main proof. Needs CI/native run before merge, especially async-loader + LIGHTWEIGHT on the suite’s ClickHouse version.

Low / nits

• Unrelated log-format churn in worker-deleter / secret / service (Info("%s", name)) — prefer drop from this PR.
• healthWindowStep / onSoftTimeout are tiny; fine, but gate loop is dense — a short state comment would help reviewers.
• Relation to Shard unavailable during reconciliation #1704 is accurately caveated in the description; keep that so it isn’t mistaken for a full Shard unavailable during reconciliation #1704 fix.

Suggested merge criteria

1. Rebase onto current 0.27.3 (or successor) and resolve overlap with Protect last healthy shard replica during interrupted rolls (#1704) #2046.
2. Validate / fix asynchronous_loader pending predicate on a real CH version used in e2e.
3. Pin and document minimum CH version for every new SYSTEM statement.
4. Land a green test_010079 run.

[sunsingerus]

Thanks for this — the analysis in the description is accurate, and the "why the existing caught-up check is not enough" section matches what we found independently: the local MAX(absolute_delay) probe is a weak proxy, and the caught-up marker is the real problem.

We merged this locally onto 0.27.3, fixed what we found, and ran the full e2e suite. Two of the findings are blockers that would have made the feature non-functional, so they're worth writing up in detail. Everything below is measured, not inferred.

Blocker 1 — SETTINGS receive_timeout makes every SYSTEM statement a syntax error

sqlWithReceiveTimeout (pkg/model/chi/schemer/sql.go) appends SETTINGS receive_timeout=N to the SQL text, and its only call site is execHostWithDeadline, which executes all three of SYSTEM SYNC DATABASE REPLICA, SYSTEM WAIT LOADING PARTS and SYSTEM SYNC REPLICA ... LIGHTWEIGHT. The SYSTEM grammar has no SETTINGS production, so all three fail to parse.

Reproduced against a real Replicated database plus ReplicatedMergeTree on embedded Keeper, over the HTTP interface (the operator's own transport):

SYSTEM SYNC DATABASE REPLICA "repdb"                    -> 200 OK
SYSTEM SYNC DATABASE REPLICA "repdb" SETTINGS receive_timeout=5
  -> Code: 62. DB::Exception: Syntax error: failed at position 38 (SETTINGS):
     Expected one of: STRICT, ParallelWithClause, PARALLEL WITH, end of query. (SYNTAX_ERROR)

SYSTEM WAIT LOADING PARTS "repdb"."t"                   -> 200 OK
  ... + SETTINGS receive_timeout=5                      -> Code: 62 at position 39

SYSTEM SYNC REPLICA "repdb"."t" LIGHTWEIGHT             -> 200 OK
  ... + SETTINGS receive_timeout=5                      -> Code: 62 at position 45

Same on clickhouse/clickhouse-server:25.8 and on Altinity builds (also checked 23.3 / 23.8 / 24.3). The error is raised at parse time, before table resolution, so no cluster state avoids it. Effect: with sync.enabled: "true", the gate fails on the first replicated object on every host — it can only "succeed" vacuously on a cluster with no replicated databases or tables.

The unit test at sql_sync_test.go asserts strings.HasSuffix(sql, "SETTINGS receive_timeout=2") with no server round-trip, which is why CI stayed green.

Fix we applied: drop sqlWithReceiveTimeout entirely and carry the deadline out of band. opts.SetQueryTimeout(remaining) is already called immediately above the exec, and it survives into the connection layer, so the bound is preserved. (receive_timeout also works as an HTTP URL parameter if a server-side bound is specifically wanted.)

Blocker 2 — the chopconf CRD prunes the sync: block

The ClickHouseOperatorConfiguration CRD schema is hand-authored in deploy/builder/templates-install-bundle/...-crd-02-chopconf.yaml, not derived from the Go types, so build_manifests.sh does not pick up new config fields. It restricts reconcile.host.wait.replicas to all / new / delay with no x-kubernetes-preserve-unknown-fields, so the apiserver silently drops sync::

$ kubectl apply --validate=false -f tests/e2e/manifests/chopconf/test-079-sync-gate.yaml
$ kubectl get chopconf sync-gate -o jsonpath='{.spec}'
{"reconcile":{"host":{"wait":{"replicas":{}}}}}

and with validation on (which is what the e2e's apply_operator_config uses):

Error from server (BadRequest): ... strict decoding error:
unknown field "spec.reconcile.host.wait.replicas.sync"

So test_010079 could not have run green — consistent with your note that the e2e wasn't runnable locally. The ConfigMap and Helm paths work fine (this PR updates both); only the CR form — the one the new docs example shows — was affected.

Fix we applied: added the sync sub-schema to the CRD template and regenerated (build_manifests.sh + generate_helm_chart.sh). Verified afterwards that every key the e2e manifest sets (enabled, timeout, onTimeout, health.pollInterval, health.successThreshold) is present in the generated schema, and that the CR round-trips intact.

e2e: a TestFlows structural error

tests/e2e/test_operator.py, in test_010079:

with When("START REPLICATED SENDS"):
    clickhouse.query(chi, "SYSTEM START REPLICATED SENDS", host=source_host)

    with And("Live inserts continue after sync starts"):   # <-- TypeError

clickhouse.query() opens no TestFlows step, so the And is the first child of the When and has no sibling to inherit its subtype from:

TypeError: `And` step can't be used here as it has no sibling from which to inherit test subtype

It fires deterministically on entry, so the entire release half of the scenario — caught-up marker, ready label, third replica, row count — never executed. Changing that one And to When fixes it.

Other changes we made

• Version check fails open. The >= 23.4 gate reads host.Runtime.Version, which is resolved once during buildCR. A digest-pinned or non-numeric tag yields MinVersion 0.0.1, producing requires ClickHouse >= 23.4, got 0.0.1 on a server that may well support LIGHTWEIGHT. We now fall back to the full SYSTEM SYNC REPLICA when the version is unknown rather than hard-failing. (:latest was fine — it maps to 99.99.99.)
• Discovery scoped to the shard. clusterAllReplicas(cluster, system.tables) discovers cluster-wide, but WAIT LOADING PARTS / SYNC REPLICA then run host-locally, so a Replicated table living only on another shard aborted the reconcile. Replication is per-shard, so shard scope is the correct scope.
• Gate moved before ascendHostInClickHouseCluster. includeHost restored full remote_servers priority before catchReplicationLag, so a catching-up replica was already receiving distributed queries while the gate held. That's the short-count symptom the linked issues describe, so the gate has to run first to close it.
• Marker invalidation made unconditional. forceReplicaCatchUpAfterStorageLoss was gated on Sync.IsEnabled(). A stale caught-up marker on a host that just lost its storage is wrong regardless of whether the new gate is on — and since the marker is otherwise append-only for a host that still exists, a recreated replica would skip the catch-up wait entirely. We think that part is a bug fix rather than a feature and should not sit behind the flag.
• timeout default 0 → 900. At 0 the deadline was effectively infinite, so the readonly/session-expired hard-fail branch could never fire.
• Dropped mode — isValidReconcileHostWaitReplicasSyncMode accepts exactly one value.
• Added test_010079_2, a gate-OFF control on the same fixture. Without it test_010079 can't distinguish the gate holding the roll from the pre-existing replication-delay wait or plain reconcile slowness.

We also added two safety changes on our side, prompted by the marker invalidation now being live: doesHostHaveNoReplicationDelay no longer discards its query error (a failed query returned delay 0, which read as "no lag" and minted an unearned marker), and waitHostHasNoReplicationDelay is now bounded — it was Timeout: 100 years, and the poller reports a cancelled context as success, so a replica that never caught up could pin a reconcile thread indefinitely.

What the e2e actually showed

First full-suite run, with the gate enabled, the hold half passed on all five attempts before the TypeError aborted the scenario:

• status.hostsWithReplicaCaughtUp stayed [] while the replica lagged
• the third replica was not created while the gate waited
• the delayed replica stayed ready=no
• max(absolute_delay) measured 42-43s at probe time
• and the chopconf sync: block survived the apiserver round-trip, confirming the CRD fix against a live cluster

After the TypeError fix, the release half runs too — live inserts, caught-up marker, ready label, third replica created, row count propagated. That run is still completing, so I'll follow up if anything else turns up, but the gate's core behaviour now demonstrably works in both directions.

One thing you flagged as unverified in the description — system.asynchronous_loader.is_ready semantics on the target image — we checked empirically: the column comment is "The job is ready to be executed and waits for a worker", and the barrier drains the pending set to zero as expected. Your predicate is correct.

Happy to push the reworked branch or open a follow-up PR with these fixes if that's easier than applying them here — whichever you prefer.