ClickHouse / ClickHouse/ClickHouse

BACKUP to S3: a crash after the .lock PUT permanently wedges the destination (BACKUP_ALREADY_EXISTS 'is being written already'), no in-server recovery

Open
#114,109 2 comments 0 reactions 1 assignee Claimed by @jkartseva View on GitHub
bug clickgap-analyzed comp-backup comp-object-storage culprit-pr-not-found
Dominant language
C++
Stars
49.9k
Forks
9k
Avg merge
21h 32m
Merged PRs (30d)
515

Description

### Describe what's wrong

If a `BACKUP ... TO S3(...)` is interrupted (server crash / power loss / OOM kill) after it has created its `.lock` object but before it finalizes, the destination is left **permanently unusable**: every subsequent `BACKUP` to the same destination fails with `Code: 598. DB::Exception: Backup S3('...') is being written already. (BACKUP_ALREADY_EXISTS)`, and nothing in the server ever reclaims the leftover `.lock`. The only recovery is to manually delete the `.lock` object from the bucket.

`BackupImpl::createLockFile` (`src/Backups/BackupImpl.cpp`) does an unconditional PUT of `/.lock` near the start of the backup, before the bulk of the entries and long before `.backup` is finalized. The only places that remove `.lock` are in-process: `removeLockFile` in the normal finalize, and `tryRemoveAllFiles` on an in-process failure. A crash runs neither, and there is no TTL, age, or liveness check on the lock anywhere in `src/Backups/` — so a `.lock` orphaned by a crash survives forever.

`BackupImpl::checkBackupDoesntExist` then rejects the next backup to that destination on the mere *existence* of the lock:

```cpp
if (writer->fileExists(file_name_to_check_existence)) // ".backup" — absent after a crash, so this passes
throw Exception(ErrorCodes::BACKUP_ALREADY_EXISTS, "Backup {} already exists", ...);

if (!params.is_internal_backup)
{
if (writer->fileExists(lock_file_name)) // ".lock" — the stale one from the crash
throw Exception(ErrorCodes::BACKUP_ALREADY_EXISTS, "Backup {} is being written already", ...);
}
```

Because `.backup` was never written, the first guard passes and the second throws on the stale `.lock`. The destination name is bricked until an operator cleans it up out of band.

This is distinct from #112407: that issue is about two *concurrent live* backups racing the check-then-create claim. This one is a *single* backup that crashes and never recovers its own destination — a crash-recovery gap, not a concurrency one.

### Does it reproduce on the most recent release?

Yes — reproduced on `26.8.1.1`; the code path above is present on current `master`.

### How to reproduce

`26.8.1.1`, single node, BACKUP to S3/MinIO. Kill the server after the `.lock` PUT reaches the object store and before `.backup` is finalized (e.g. an S3 proxy that forwards the `.lock` PUT and then holds while the server is `SIGKILL`ed):

```sql
CREATE TABLE t (id UInt64, v String) ENGINE = MergeTree ORDER BY id;
INSERT INTO t SELECT number, toString(number) FROM numbers(2000);

BACKUP TABLE t TO S3('http://.../bucket/dest/', 'key', 'secret');
-- SIGKILL the server while this is in flight, after /.lock has been PUT
-- (before /.backup exists).

-- restart the server, then retry the same destination:
BACKUP TABLE t TO S3('http://.../bucket/dest/', 'key', 'secret');
-- Code: 598. DB::Exception: Backup S3('.../bucket/dest/') is being written already. (BACKUP_ALREADY_EXISTS)
-- ... and so is every retry thereafter, until /.lock is deleted by hand.
```

The bucket after the crash holds `/.lock` (durable) and no `/.backup`.

Deterministic rig (fault proxy forwards the `.lock` PUT, then `SIGKILL`s the server; a clean BACKUP+RESTORE control and a pre-`.lock` crash control that succeeds on retry, isolating the stale lock as the cause): `5/5` wedge, controls green.

### Expected behavior

After a crash mid-backup, the destination remains usable: a subsequent `BACKUP` to the same destination either reclaims/overwrites the stale `.lock` left by the dead writer (there is no `.backup`, so no committed backup is at risk) or reports a clearly recoverable error rather than an indefinite `BACKUP_ALREADY_EXISTS`. A crashed backup should not permanently claim its destination with no in-server way to recover.

### Error message and/or stacktrace

```
Code: 598. DB::Exception: Backup S3('.../') is being written already. (BACKUP_ALREADY_EXISTS)
```

### Additional context

`backups.allow_concurrent_backups` defaults to `true`, so no non-default setting is involved. Related but distinct: #112407 (concurrent live racers on the same `.lock` claim-check). The lock's UUID-content comparison in `checkLockFile` is a *later* guard used while writing; the initial `checkBackupDoesntExist` gate rejects on existence alone, so a stale lock from any prior (dead) UUID blocks every future backup to that destination.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.