WIP: replicated host catch-up gate rework + design research

[sunsingerus]

DRAFT — do not merge. The code here has a confirmed broken path (see Blockers). This PR exists to preserve the rework and the design research so the work is not lost while it is paused.

Follows on from #2018 (originating issue: #1682).

What is in the code

Two blocking bugs in #2018, fixed:

1. SETTINGS was appended to SYSTEM SYNC REPLICA. SYSTEM statements have no SETTINGS production — this is a parse-time SYNTAX_ERROR (Code 62), so the gate could never have worked against a real server. The deadline now rides on opts.SetQueryTimeout instead.
2. The chopconf CRD had no sync sub-schema, so the whole block was silently pruned when set through a ClickHouseOperatorConfiguration CR — the gate read as disabled while the user's manifest said enabled. Both verified against a live apiserver.

Also in this branch:

• Enum values accepted case-insensitively (abort/Abort, proceed/Proceed), matching every other enum-valued option in this config.
• minimum: 1 on the integer fields.
• Config block renamed sync: → catchUp:, and the gate identifiers with it. The schemer helpers that issue SYSTEM SYNC REPLICA keep sync — there the word names the SQL. See "Naming" below; this rename is now considered a mistake.
• Catch-up wait moved ahead of the host ascend, so an excluded host stays deprioritised in remote_servers for the duration of the wait instead of receiving distributed queries while still behind.
• The caught-up marker is cleared unconditionally on storage loss, and is no longer written when a reconcile is cancelled mid-wait.
• The 100-year poll timeout replaced with a bounded per-pass wait plus a re-enqueue. This is the part that does not work — see Blockers.
• docs/chi-examples/99-clickhouseoperatorconfiguration-max.yaml: an exhaustive operator-config example, covering 86 of the 100 leaves the chopconf CRD declares.

Blockers — why this is a draft

1. The bounded-wait path is dead code. poller.Poll() returns an error on timeout (poller.go:109), not nil. waitHostHasNoReplicationDelay assumes nil and re-checks the predicate, so errReplicationCatchUpNotFinished is never reached and scheduleReplicationCatchUpRetry never fires. On expiry the host is left out of the Service with nothing re-including it. Fix: a poller.ErrTimeout sentinel plus errors.Is.
2. The re-enqueue would be a no-op even once (1) is fixed. The re-enqueued CR has no spec diff and its pod is Ready, so reconcileCR takes the "No reconcile work - abort reconcile" exit. Stuck-host recovery needed an explicit bypass for exactly this shape; the catch-up retry has none.
3. test_010079 does not test the gate. With sync.enabled: false the default wait.replicas.new: yes runs the legacy probe, which blocks the same way and writes the same marker — every assertion passes with the gate off. Needs a gate-exclusive discriminator (e.g. asserting SYSTEM SYNC REPLICA appears in system.query_log).

Design research

The config shape is wrong, and the rename made it worse

reconcile.host.wait.replicas already is the catch-up configuration — all/new choose which replicas wait, delay defines what "caught up" means. #2018 bolted a sub-block on top whose switch is called enabled, which reads as though it turns catch-up on and off. It does not: with it off, catch-up still runs via the legacy delay probe. And with it on but all: no, new: no, the gate is configured and silently never runs.

Renaming the block sync: → catchUp: fixed the vocabulary but worsened the contradiction, because catchUp names what both paths do. sync was the better block name — only that path syncs.

The better answer: make catch-up uniform, and delete the toggle

Rather than a user-facing choice between two paths, let the operator decide at runtime:

1. Is SELECT MAX(absolute_delay) FROM system.replicas meaningful on this host yet?
2. If not, prepare the ground — async-load barrier, peer discovery, SYSTEM SYNC REPLICA.
3. Run the delay check.

This is mostly resequencing; every building block already exists (HostAsyncLoadBarrier, PeerReplicatedObjectCount, HostSyncReplicatedObjects, the health window). The config then collapses, with nothing released moving:

replicas:
  all: "no"            # which replicas must catch up
  new: "yes"
  delay: 10            # what counts as caught up
  pollInterval: 10
  successThreshold: 6  # only for hosts that needed preparation
  timeout: 900         # absorbs the hardcoded replicationCatchUpPassTimeout - same value
  onTimeout: "retry"   # retry (today's shipped behaviour) | proceed | abort

Why the second path is needed at all: system.replicas only has a row for a replicated table once that table is attached and loaded. On a replica recreated with an empty disk, MAX(absolute_delay) over an empty set returns 0 — indistinguishable from "caught up". The operator can therefore declare an empty replica healthy and take down a good one. The deep path asks a peer what the shard should hold, which is the only step that makes the measurement trustworthy.

Known hard parts, before anyone implements this:

• The readiness test cannot be proven sound. HostSyncReplicatedObjects runs the async barrier twice, because syncing a Replicated database materialises new tables that then load asynchronously — so a barrier pass before the DB sync provably does not prove completeness. Preparation must therefore be cheap and safe enough to run when in doubt, not perfectly targeted.
• Unknown ClickHouse version (which is what you get when a host was down at reconcile start — the recovery case) silently selects full SYSTEM SYNC REPLICA instead of LIGHTWEIGHT; one busy table can consume the entire budget.
• SYSTEM SYNC REPLICA needs a table-level privilege. ACCESS_DENIED currently propagates as a hard error, and includeHostIntoService runs only on success — so the host loses its ready mark and drops out of the Service pass after pass.
• SYSTEM WAIT LOADING PARTS is 23.1+ and is issued unguarded; system.asynchronous_loader is 23.12+.
• There are no catch-up metrics, and status.hostsWithReplicaCaughtUp is byte-identical whichever path wrote it — a misfiring auto-detector would be invisible. Dissenting view worth recording: keep the toggle as a third auto|always|never mode rather than removing the choice.

Suggested order: fix the three blockers → harden preparation (version guards, per-statement bounds, soft-fail on permission/peer errors) → then collapse the paths.

Separate defects found along the way

Not caused by this branch; each needs its own fix.

Severity	Defect
HIGH	doesHostHaveNoReplicationDelay discards its query error, so an unreachable host reads as delay 0 → "caught up" → the marker is persisted and the wait is skipped for that host forever. Bites under the shipped default all: no, new: yes.
MEDIUM	The chopconf CRD hook items omit events and failurePolicy, which the CHI and CHIT CRDs both declare. An operator-wide hook set through a CR is accepted, stored, and never fires; failurePolicy: Ignore silently becomes Fail. Same shape as the sync prune bug. Schema-only fix.
MEDIUM	reconcile.host.wait.replicas.{all,new,delay} are declared in the CHI CRD at three positions with descriptions promising per-CHI behaviour, but every reader uses the global chop.Config(). Three inert advertised fields.
LOW	template.chk.{path,policy} ship in config.yaml and 10 manifests, but OperatorConfigTemplate has only a CHI field — no Go field exists and the CRD prunes them. Dead knob, plus an unread config/chk/templates.d/.
LOW	logger.vmodule and logger.log_backtrace_at are parsed, normalised and in the CRD, but SetupLog() never applies them — the stubs are commented out.
LOW	The deprecated flat chScheme key migrates into ClickHouse.Access.Password instead of Scheme. Unconditional, ~4 years old; low impact only because the key is undocumented.
LOW	reconcile.coordination.keeper.onKeeperResourceUpdate is commented out in config.yaml despite being live and read by the CHK watcher.

Verification

go build clean · gofmt clean · go vet clean · unit tests pass in the affected packages · generators idempotent with no VERBOSITY drift · the regenerated chopconf CRD accepted by a live apiserver, with catchUp round-tripping unpruned and the old sync key correctly rejected.

Not run: the e2e suite, since the last green run predates the bounded-wait rework.