From 575f1e0e769cc8e22624968e596c69779c4ac348 Mon Sep 17 00:00:00 2001 From: ruslanen Date: Fri, 19 Jun 2026 16:17:45 +0300 Subject: [PATCH 1/2] fix(create): batch system.mutations lookup to avoid O(N^2) on many tables GetInProgressMutations was called once per table during `create`. Each query against system.mutations enumerates every table on the server, so the per-table WHERE database/table filter does not bound the work: cost is O(total tables) per call * N calls = O(N^2). On installations with many tables this query family dominates `create` wall-clock (observed ~240ms/call across tens of thousands of tables). Fetch the whole in-progress mutation set once per backup via a new GetInProgressMutationsBatch (single system.mutations scan, same WHERE is_done=0 filter) and look it up per table from an in-memory map keyed by "database.table". Behavior is unchanged (same per-table Mutations in TableMetadata); only the query count changes (N -> 1). Measured on a 55k-table / 93.6GiB local backup: create ~35min -> ~230s. --- pkg/backup/create.go | 21 +++++++++++++++------ pkg/clickhouse/clickhouse.go | 25 +++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/pkg/backup/create.go b/pkg/backup/create.go index 4d80369d..6829e935 100644 --- a/pkg/backup/create.go +++ b/pkg/backup/create.go @@ -331,6 +331,18 @@ func (b *Backuper) createBackupLocal(ctx context.Context, backupName, diffFromRe log.Debug().Msgf("CheckSystemPartsColumnsForTables passed for %d tables", len(tablesToCheck)) } + // Fetch in-progress mutations ONCE for the whole backup. system.mutations scans all tables on + // every query, so the previous per-table GetInProgressMutations call was O(N^2) and dominated + // create time on installations with many tables. We now do a single scan and look up per table. + var allInProgressMutations map[string][]metadata.MutationMetadata + if b.cfg.ClickHouse.BackupMutations && !schemaOnly && !rbacOnly && !configsOnly && !namedCollectionsOnly { + var allInProgressMutationsErr error + allInProgressMutations, allInProgressMutationsErr = b.ch.GetInProgressMutationsBatch(ctx) + if allInProgressMutationsErr != nil { + return errors.Wrap(allInProgressMutationsErr, "b.ch.GetInProgressMutationsBatch") + } + } + var backupDataSize, backupObjectDiskSize, backupMetadataSize uint64 var metaMutex sync.Mutex createBackupWorkingGroup, createCtx := errgroup.WithContext(ctx) @@ -372,12 +384,9 @@ func (b *Backuper) createBackupLocal(ctx context.Context, backupName, diffFromRe logger.Debug().Msg("get in progress mutations list") inProgressMutations := make([]metadata.MutationMetadata, 0) if b.cfg.ClickHouse.BackupMutations && !schemaOnly && !rbacOnly && !configsOnly && !namedCollectionsOnly { - var inProgressMutationsErr error - inProgressMutations, inProgressMutationsErr = b.ch.GetInProgressMutations(createCtx, table.Database, table.Name) - if inProgressMutationsErr != nil { - logger.Error().Msgf("b.ch.GetInProgressMutations error: %v", inProgressMutationsErr) - return errors.Wrap(inProgressMutationsErr, "b.ch.GetInProgressMutations") - } + // looked up from the single GetInProgressMutationsBatch query above — avoids the + // O(N^2) per-table system.mutations scan. + inProgressMutations = allInProgressMutations[table.Database+"."+table.Name] } logger.Debug().Msg("create metadata") if schemaOnly || doBackupData { diff --git a/pkg/clickhouse/clickhouse.go b/pkg/clickhouse/clickhouse.go index e615a081..72122176 100644 --- a/pkg/clickhouse/clickhouse.go +++ b/pkg/clickhouse/clickhouse.go @@ -1363,6 +1363,31 @@ func (ch *ClickHouse) GetInProgressMutations(ctx context.Context, database strin return inProgressMutations, nil } +// GetInProgressMutationsBatch returns all in-progress mutations across the whole server in a +// SINGLE query, keyed by "database.table". system.mutations is an expensive virtual table — every +// query against it enumerates all tables on the server — so calling GetInProgressMutations once per +// table is O(N^2) and dominates `create` wall-clock on installations with many tables (observed: +// ~240ms/call * tens of thousands of tables). Fetching the whole in-progress set once per backup +// turns that into a single O(N) scan; per-table lookup is then an in-memory map access. +func (ch *ClickHouse) GetInProgressMutationsBatch(ctx context.Context) (map[string][]metadata.MutationMetadata, error) { + var rows []struct { + Database string `ch:"database"` + Table string `ch:"table"` + MutationId string `ch:"mutation_id"` + Command string `ch:"command"` + } + query := "SELECT database, table, mutation_id, command FROM system.mutations WHERE is_done=0" + if err := ch.SelectContext(ctx, &rows, query); err != nil { + return nil, errors.Wrap(err, "can't get in progress mutations") + } + result := make(map[string][]metadata.MutationMetadata, len(rows)) + for _, r := range rows { + key := r.Database + "." + r.Table + result[key] = append(result[key], metadata.MutationMetadata{MutationId: r.MutationId, Command: r.Command}) + } + return result, nil +} + func (ch *ClickHouse) ApplyMacros(ctx context.Context, s string) (string, error) { if !strings.Contains(s, "{") { return s, nil From eab04d52c35366fc6337e3840444a4fe34d93ca1 Mon Sep 17 00:00:00 2001 From: ruslanen Date: Fri, 19 Jun 2026 16:17:45 +0300 Subject: [PATCH 2/2] test(create): unit-test batch mutation grouping; extract pure helper Extract groupMutationsByTable (pure, no I/O) from GetInProgressMutationsBatch and add unit tests: two tables (one with two mutations) bucket to the correct database.table with no cross-table leakage, plus an empty-input case. --- pkg/clickhouse/clickhouse.go | 32 +++++++++++++++++++------------ pkg/clickhouse/clickhouse_test.go | 27 ++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 12 deletions(-) diff --git a/pkg/clickhouse/clickhouse.go b/pkg/clickhouse/clickhouse.go index 72122176..c6756ff8 100644 --- a/pkg/clickhouse/clickhouse.go +++ b/pkg/clickhouse/clickhouse.go @@ -1369,23 +1369,31 @@ func (ch *ClickHouse) GetInProgressMutations(ctx context.Context, database strin // table is O(N^2) and dominates `create` wall-clock on installations with many tables (observed: // ~240ms/call * tens of thousands of tables). Fetching the whole in-progress set once per backup // turns that into a single O(N) scan; per-table lookup is then an in-memory map access. -func (ch *ClickHouse) GetInProgressMutationsBatch(ctx context.Context) (map[string][]metadata.MutationMetadata, error) { - var rows []struct { - Database string `ch:"database"` - Table string `ch:"table"` - MutationId string `ch:"mutation_id"` - Command string `ch:"command"` - } - query := "SELECT database, table, mutation_id, command FROM system.mutations WHERE is_done=0" - if err := ch.SelectContext(ctx, &rows, query); err != nil { - return nil, errors.Wrap(err, "can't get in progress mutations") - } +type inProgressMutationRow struct { + Database string `ch:"database"` + Table string `ch:"table"` + MutationId string `ch:"mutation_id"` + Command string `ch:"command"` +} + +// groupMutationsByTable buckets flat mutation rows into per-table lists keyed by +// "database.table". Pure (no I/O) so it is unit-testable without a ClickHouse server. +func groupMutationsByTable(rows []inProgressMutationRow) map[string][]metadata.MutationMetadata { result := make(map[string][]metadata.MutationMetadata, len(rows)) for _, r := range rows { key := r.Database + "." + r.Table result[key] = append(result[key], metadata.MutationMetadata{MutationId: r.MutationId, Command: r.Command}) } - return result, nil + return result +} + +func (ch *ClickHouse) GetInProgressMutationsBatch(ctx context.Context) (map[string][]metadata.MutationMetadata, error) { + var rows []inProgressMutationRow + query := "SELECT database, table, mutation_id, command FROM system.mutations WHERE is_done=0" + if err := ch.SelectContext(ctx, &rows, query); err != nil { + return nil, errors.Wrap(err, "can't get in progress mutations") + } + return groupMutationsByTable(rows), nil } func (ch *ClickHouse) ApplyMacros(ctx context.Context, s string) (string, error) { diff --git a/pkg/clickhouse/clickhouse_test.go b/pkg/clickhouse/clickhouse_test.go index abcbc954..2b46c111 100644 --- a/pkg/clickhouse/clickhouse_test.go +++ b/pkg/clickhouse/clickhouse_test.go @@ -3,6 +3,7 @@ package clickhouse import ( "testing" + "github.com/Altinity/clickhouse-backup/v2/pkg/metadata" "github.com/go-faster/errors" "github.com/stretchr/testify/assert" ) @@ -198,3 +199,29 @@ func TestEnrichQueryWithOnCluster(t *testing.T) { }) } } + +// TestGroupMutationsByTable covers the per-backup batch mutation lookup that replaced the +// per-table system.mutations query (O(N^2) fix). Verifies that mutations from a single +// server-wide scan are bucketed to the correct database.table and never leak across tables. +func TestGroupMutationsByTable(t *testing.T) { + rows := []inProgressMutationRow{ + {Database: "db1", Table: "t1", MutationId: "0000000001", Command: "MODIFY COLUMN a UInt64"}, + {Database: "db1", Table: "t1", MutationId: "0000000002", Command: "DROP COLUMN b"}, + {Database: "db1", Table: "t2", MutationId: "0000000003", Command: "MODIFY COLUMN c String"}, + } + + got := groupMutationsByTable(rows) + + assert.Len(t, got, 2, "two distinct tables expected") + assert.Equal(t, []metadata.MutationMetadata{ + {MutationId: "0000000001", Command: "MODIFY COLUMN a UInt64"}, + {MutationId: "0000000002", Command: "DROP COLUMN b"}, + }, got["db1.t1"], "t1 must keep both of its mutations, in order") + assert.Equal(t, []metadata.MutationMetadata{ + {MutationId: "0000000003", Command: "MODIFY COLUMN c String"}, + }, got["db1.t2"], "t2 must get only its own mutation (no cross-table leak)") +} + +func TestGroupMutationsByTableEmpty(t *testing.T) { + assert.Empty(t, groupMutationsByTable(nil)) +}