Fix --table with --resume on incremental backups (b.dst corruption + state wipe)

[minguyen9988]

Problem

Using --table with --resume on incremental backups crashes or produces incorrect results due to two interacting bugs.

Bug 1: Recursive Download() corrupts parent's BackupDestination

When downloading an incremental backup, Download() recursively calls itself to download the RequiredBackup (base backup):

// pkg/backup/download.go
if !schemaOnly && !b.cfg.General.DownloadByPart && remoteBackup.RequiredBackup != "" {
    err := b.Download(remoteBackup.RequiredBackup, tablePattern, ...)
    // After this returns, b.dst has been overwritten and closed!
}

The recursive call runs on the same Backuper instance. Inside, it calls initDisksPathsAndBackupDestination() which overwrites b.dst with a new BackupDestination. On return, the child's defer b.dst.Close(ctx) closes this connection. The parent then tries to use b.dst for its own data downloads — but it's been closed.

Without --table, this is masked because the resume state says "all parts already processed" and b.dst is never actually called for data transfer. With --table, the state wipe (Bug 2) forces actual downloads, exposing the closed connection.

Bug 2: tablePattern in resume state params triggers unnecessary state wipe

The resume state includes tablePattern in its stored params:

b.resumableState = resumable.NewState(b.GetStateDir(), backupName, "download", map[string]interface{}{
    "tablePattern": tablePattern,  // <-- causes state wipe
    "partitions":   partitions,
    "schemaOnly":   schemaOnly,
})

NewState() calls cleanupStateIfParamsChange(), which compares stored params with current params. When the user changes from no --table (full download) to --table pnlcrypto_prod.lotted_position (partial resume), the params don't match, so ALL resume progress is wiped.

This forces actual data downloads (instead of validated skips), which exposes Bug 1.

Proposed Fix

Fix 1: Save/restore b.dst around recursive call

if !schemaOnly && !b.cfg.General.DownloadByPart && remoteBackup.RequiredBackup != "" {
    savedDst := b.dst
    err := b.Download(remoteBackup.RequiredBackup, tablePattern, ...)
    b.dst = savedDst  // restore parent's connection
    if err != nil && !errors.Is(err, ErrBackupIsAlreadyExists) {
        return errors.WithMessage(err, "download RequiredBackup")
    }
}

Fix 2: Remove tablePattern from resume state params

b.resumableState = resumable.NewState(b.GetStateDir(), backupName, "download", map[string]interface{}{
    // tablePattern intentionally excluded — resume state tracks individual
    // part paths which are independent of the table filter. Including
    // tablePattern would wipe ALL resume progress whenever the user
    // changes --table.
    "partitions": partitions,
    "schemaOnly": schemaOnly,
})

Resume state is part-level (keys are individual part remote paths). The table filter only controls which parts are queued for download — it doesn't affect the validity of previously-downloaded parts. A part downloaded as part of a full backup is equally valid when resuming with --table.

Expected behavior after fix

# Full download (interrupted)
./clickhouse-backup download my_backup --resume
# Downloads 1000/1200 tables before crash

# Resume just one table
./clickhouse-backup download my_backup --resume --table pnlcrypto_prod.lotted_position
# Only processes pnlcrypto_prod.lotted_position
# Does NOT wipe state for the other 1000 tables
# Uses the same b.dst connection throughout

[minguyen9988]

Reproduction Steps (against master branch, commit 1a5b06b)

Environment

• S3-compatible storage
• ClickHouse with multiple databases/tables
• At least one incremental backup (backup with RequiredBackup set in metadata)
• clickhouse-backup built from master branch

Reproducing Bug 1: b.dst corruption

# 1. Create a full backup and an incremental one
clickhouse-backup create full_backup
clickhouse-backup upload full_backup
clickhouse-backup create incr_backup --diff-from-remote full_backup
clickhouse-backup upload incr_backup

# 2. Download the incremental backup fully
clickhouse-backup download incr_backup --resume

# 3. Now try to resume with --table to download just one table
clickhouse-backup download incr_backup --resume --table mydb.mytable
# This crashes or hangs because b.dst is closed

Root cause: recursive Download() closes parent's connection

pkg/backup/download.go lines 141-144 on master:

if !schemaOnly && !b.cfg.General.DownloadByPart && remoteBackup.RequiredBackup != "" {
    err := b.Download(remoteBackup.RequiredBackup, tablePattern, ...)
    // After return: b.dst has been OVERWRITTEN and CLOSED by the child!
}

The call stack:

1. Parent Download("incr_backup") → initDisksPathsAndBackupDestination() sets b.dst = connection A
2. Parent calls b.Download("full_backup") (recursive, same Backuper instance)
3. Child Download("full_backup") → initDisksPathsAndBackupDestination() overwrites b.dst = connection B
4. Child returns → defer b.dst.Close(ctx) closes connection B
5. But b.dst is now connection B (which is closed), not connection A
6. Parent tries to download data using b.dst → uses closed connection → crash/error

Proof via code reading

The defer that closes b.dst is at line 114-116:

defer func() {
    if err := b.dst.Close(ctx); err != nil {
        log.Warn().Msgf("can't close BackupDestination error: %v", err)
    }
}()

Both parent and child create this defer. The child's defer runs first (on child return) and closes whatever b.dst points to at that moment. Since the child overwrote b.dst during its initDisksPathsAndBackupDestination() call, it closes the child's connection. But b.dst is now the child's (closed) connection — the parent's original connection is leaked, and the parent is left with a closed b.dst.

Why this is masked without --table

Without --table, the resume state from step 2 has all parts marked "processed". The parent's data download loop skips every part (via IsAlreadyProcessed returning true), so b.dst is never actually used for data transfer after the recursive call. The bug exists but is latent.

Reproducing Bug 2: state wipe on --table change

# 1. Download a backup fully (all tables)
clickhouse-backup download mybigbackup --resume
# State file records all parts as "processed" with params: {"tablePattern":"", ...}

# 2. Resume with --table to download one specific table
clickhouse-backup download mybigbackup --resume --table mydb.onetable
# cleanupStateIfParamsChange() detects {"tablePattern":""} != {"tablePattern":"mydb.onetable"}
# ALL resume state is WIPED — every part must be re-checked

Root cause in code

pkg/backup/download.go lines 167-172 on master:

b.resumableState = resumable.NewState(b.GetStateDir(), backupName, "download", map[string]interface{}{
    "tablePattern": tablePattern,  // <-- included in params
    "partitions":   partitions,
    "schemaOnly":   schemaOnly,
})

pkg/resumable/state.go lines 90-97:

func (s *State) cleanupStateIfParamsChange(params map[string]interface{}) {
    needCleanup := false
    if s.params != nil && params != nil && !common.CompareMaps(s.params, params) {
        needCleanup = true  // tablePattern changed → WIPE EVERYTHING
    }
    // ...
    if needCleanup {
        log.Info().Msgf("parameters changed old=%#v new=%#v, %s cleanup begin", ...)
        // Deletes all keys from bbolt bucket
    }
}

The state wipe forces all parts to be re-downloaded (since their "processed" state was deleted), which triggers Bug 1 (b.dst is closed after recursive RequiredBackup download).

Combined reproduction

# Full sequence that triggers both bugs together:
clickhouse-backup create full_bk && clickhouse-backup upload full_bk
clickhouse-backup create incr_bk --diff-from-remote full_bk && clickhouse-backup upload incr_bk

# Download fully
clickhouse-backup download incr_bk --resume
# Works fine — full_bk downloaded recursively, incr_bk data downloaded

# Now resume just one table
clickhouse-backup download incr_bk --resume --table mydb.onetable
# 1. State wipe: tablePattern changed "" → "mydb.onetable" → all state deleted
# 2. Recursive Download("full_bk") overwrites and closes b.dst
# 3. Parent tries to download mydb.onetable data with closed b.dst
# 4. CRASH or silent failure