diff --git a/internal/controller/clickhouse/commands.go b/internal/controller/clickhouse/commands.go index 409602cee..2f65f2643 100644 --- a/internal/controller/clickhouse/commands.go +++ b/internal/controller/clickhouse/commands.go @@ -23,6 +23,10 @@ WHERE engine NOT IN ('Atomic', 'Lazy', 'SQLite', 'Ordinary', 'Memory') SETTINGS format_display_secrets_in_show_and_select=1` + // system.clusters also lists static clusters from remote_servers and Replicated databases with + // names the operator did not choose; their shard and replica names are empty or non-numeric. + // toInt32OrNull keeps the cast from throwing on those rows, and the NULL filter drops them, so + // only replicas that the operator itself numbered are considered for cleanup. listStaleDatabaseReplicasQuery = `SELECT database, shard_id, @@ -31,11 +35,11 @@ SETTINGS FROM ( SELECT cluster as database, - toInt32(database_shard_name) AS shard_id, - toInt32(database_replica_name) AS replica_id, + toInt32OrNull(database_shard_name) AS shard_id, + toInt32OrNull(database_replica_name) AS replica_id, is_active FROM clusterAllReplicas(default, system.clusters) - WHERE database_replica_name != '' AND (shard_id >= ? OR replica_id >= ?) + WHERE shard_id IS NOT NULL AND replica_id IS NOT NULL AND (shard_id >= ? OR replica_id >= ?) ) GROUP BY database, shard_id, replica_id SETTINGS skip_unavailable_shards=1` diff --git a/internal/controller/clickhouse/commands_test.go b/internal/controller/clickhouse/commands_test.go index b06fa0370..5475facf5 100644 --- a/internal/controller/clickhouse/commands_test.go +++ b/internal/controller/clickhouse/commands_test.go @@ -431,6 +431,22 @@ var _ = Describe("commander", Ordered, Label("integration"), func() { Expect(cmd.CleanupDatabaseReplicas(ctx, cmd.log, map[v1.ClickHouseReplicaID]struct{}{})).To(Succeed()) }) + It("should ignore replicated databases whose shard and replica names the operator did not choose", func(ctx context.Context) { + // system.clusters lists every Replicated database on the server, including ones created by + // users with their own shard and replica names, next to the static clusters from + // remote_servers whose names are empty. None of those parse as replica numbers. + id0 := v1.ClickHouseReplicaID{ShardID: 0, Index: 0} + conn, err := cmd.getConn(id0) + Expect(err).NotTo(HaveOccurred()) + Expect(conn.Exec(ctx, "CREATE DATABASE named ENGINE = Replicated('/clickhouse/databases/named', 'shard-a', 'replica-b')")).To(Succeed()) + + DeferCleanup(func(ctx context.Context) { + Expect(conn.Exec(ctx, "DROP DATABASE named SYNC")).To(Succeed()) + }) + + Expect(cmd.CleanupDatabaseReplicas(ctx, cmd.log, map[v1.ClickHouseReplicaID]struct{}{})).To(Succeed()) + }) + It("should do nothing if still active", func(ctx context.Context) { *cmd.cluster.Spec.Replicas = testReplicas - 1 err := cmd.CleanupDatabaseReplicas(ctx, cmd.log, map[v1.ClickHouseReplicaID]struct{}{})