Skip to content

Fix restore --schema of Replicated database on second replica - #1612

Merged
Slach merged 1 commit into
masterfrom
replicated_db_restore_1611
Oct 12, 2026
Merged

Slach merged 1 commit into
masterfrom
replicated_db_restore_1611

Conversation

@Slach

@Slach Slach commented Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #1611

Problem

standard restore workflow from Examples.md "How to back up / restore a sharded cluster" restore the schema sequentially on every replica with restore --schema --rm; only shard1/replica1 gets RESTORE_SCHEMA_ON_CLUSTER. On the other replicas of an ENGINE=Replicated database the local restore failed:

  1. local DROP DATABASE ... SYNC removes only the current replica, CREATE DATABASE adds it again, and the replica recovers from the database DDL log concurrently with the following DROP/CREATE TABLE → code: 529 Cannot enqueue query on this replica, because it has replication lag (exactly the issue) or Table ... already exists
  2. replaceUUIDMacroValue replaced {uuid} in the Replicated*MergeTree path with the literal backup table UUID (no ON CLUSTER on this replica), and checkReplicaAlreadyExistsAndChangeReplicationPath could rebind the path to default_replica_path → explicit engine arguments differ from server default_replica_path → code: 36 It's not allowed to specify explicit zookeeper_path and replica_name ... in Replicated database
  3. the retry loops then burn len(tables) attempts without any delay

Fix (pkg/backup/restore.go)

  • skip local DROP DATABASE for a Replicated database when restore_schema_on_cluster is empty; tables are still dropped in dropExistsTables and these DROP TABLE are replicated
  • SYSTEM SYNC DATABASE REPLICA for every existing Replicated database before and after dropExistsTables (ClickHouse 22.5+)
  • inside a Replicated database don't replace {uuid} and don't rebind the replication path: the table UUID is generated by the initiator and shared via the DDL log

Each non-first replica still re-creates the whole database schema (replicated to all replicas), the data restore runs after all schema steps, so the final state is correct for any replica order. No restore workflow changes required.

Tests

New testflows feature replicated database (test/testflows/clickhouse_backup/tests/replicated_database.py), two replicas of replicated_cluster, Replicated database with 20 ReplicatedMergeTree without arguments + MergeTree + MV + View + Distributed:

  • restore existing database / ... reverse order / restore dropped database — standard restore sequence: download everywhere, restore --schema --rm per replica (first with RESTORE_SCHEMA_ON_CLUSTER), restore --data on the first replica
  • restore replication lag — max_replication_lag_to_enqueue=1 reproduces the issue's 529 with few tables
  • examples restore sharded cluster — Examples.md workflow

All 5 failed before the fix (529 / 36 / already exists) and pass after it on 26.8, 24.3 and 22.8. Testflows default_replica_path changed to (/clickhouse/{cluster}/tables/{shard}/{uuid}); with {database}/{table} in default_replica_path ClickHouse expands them at CREATE, so restoring an argument-less ReplicatedMergeTree inside a Replicated database fails with code 36 regardless of this PR, that's a separate issue.

Local runs: go test ./pkg/backup/ ok, TestRestoreMapping* ok; TestS3 / TestSFTPAuthKey / TestTablePatterns and the full testflows suite are still running, will report.

🤖 Generated with Claude Code

ACM and Examples.md restore schema sequentially on every replica with
`restore --schema --rm`, only the first replica uses ON CLUSTER. On the
other replicas of an ENGINE=Replicated database the local restore failed
with code 529 (replication lag), code 36 (explicit zookeeper_path) or
"Table already exists":

- local DROP DATABASE removed only the current replica, CREATE DATABASE
  added it again and recovery from the database DDL log raced with the
  following DROP/CREATE TABLE; skip it, DROP TABLE is replicated anyway
- run SYSTEM SYNC DATABASE REPLICA before and after dropping tables
- don't replace {uuid} with the backup table UUID and don't rebind the
  replication path inside a Replicated database, the table UUID comes
  from the database DDL log and explicit arguments are rejected

Add testflows feature `replicated database` which emulates ACM restore
(both orders, dropped database, replication lag) and the Examples.md
workflow on two replicas; testflows default_replica_path is now the same
as ACM uses.

Fixes #1611

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Slach

Slach commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator Author

Local runs finished: integration TestS3, TestSFTPAuthKey, TestTablePatterns PASS; full testflows suite (RUN_PARALLEL=8, CH 26.8) 10/10 suites PASS, so the default_replica_path change in testflows configs doesn't affect other suites.

@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 38210489244

Coverage increased (+0.03%) to 69.809%

Details

  • Coverage increased (+0.03%) from the base build.
  • Patch coverage: 11 uncovered changes across 1 file (20 of 31 lines covered, 64.52%).
  • 24 coverage regressions across 5 files.

Uncovered Changes

File Changed Covered %
pkg/backup/restore.go 31 20 64.52%

Coverage Regressions

24 previously-covered lines in 5 files lost coverage.

File Lines Losing Coverage Coverage
pkg/storage/general.go 12 69.48%
pkg/clickhouse/clickhouse.go 5 80.91%
pkg/backup/freezes.go 3 72.95%
pkg/backup/backuper.go 2 77.54%
pkg/backup/download.go 2 78.09%

Coverage Stats

Coverage Status
Relevant Lines: 29015
Covered Lines: 20255
Line Coverage: 69.81%
Coverage Strength: 34653.09 hits per line

💛 - Coveralls

@Slach
Slach merged commit cad10db into master Oct 12, 2026
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

database with engine=Replicated restore error

2 participants