feat: add replicated host sync gate#2018
Conversation
0fbfea6 to
edaf1e7
Compare
Signed-off-by: ku524 <yeonjuyeong@gmail.com>
edaf1e7 to
2e28e6c
Compare
|
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 — #2018Scope: ~1.6k lines. Opt-in VerdictUseful feature for local-PV / empty-disk recovery; design is thoughtful (default-off, LIGHTWEIGHT barrier, health window, What works
Blocking / high
Medium
Low / nits
Suggested merge criteria
|
Summary
reconcile.host.wait.replicas.syncrolling gate for replicated hosts.SYSTEM SYNC REPLICA ... LIGHTWEIGHT, replicated database sync, async-loader settling, and a stable health window.replication_queueto become empty.Contribution checklist
0.27.2, notmaster.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_delaymarker 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'sMAX(absolute_delay)fromsystem.replicasbefore writingstatus.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:
sync.enabled=trueis configured.system.replication_queueto drain.SYSTEM SYNC REPLICA ... LIGHTWEIGHTinstead 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.onTimeout=proceedcan 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
sync.enabled=false.onTimeout=proceedadvances without writing the caught-up marker.LIGHTWEIGHTversions fail explicitly.Test plan
bash ./dev/run_code_generator.shbash ./dev/build_manifests.shbash ./dev/generate_helm_chart.shbash ./dev/go_build_all.shbash ./dev/find_unformatted_sources.shgo test -count=1 ./pkg/apis/clickhouse.altinity.com/v1/... ./pkg/model/chi/schemer/... ./pkg/controller/chi/... ./pkg/controller/common/announcerpython3 -m py_compile tests/e2e/test_operator.pyyq 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.yamlgit diff --checkNot run locally:
test_010079*andtest_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.system.asynchronous_loader.is_readybehavior on the target e2e image; this is covered by the new e2e path/CI.