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
- 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
Assessment
This issue has not been assessed yet.