Skip to content

Commit 60dbb03

Browse files
scooby87scooby87
committed
fix(backups): keep a ClickHouse archive an earlier attempt finished
A backup Pod that loses sight of its upload, by dying or by losing six polls in a row, is retried while the sidecar finishes that upload. The retry found it in the action log as a success, took the sidecar for idle and asked for create_remote with resume again. clickhouse-backup then rebuilds the local backup and uploads it once more over the archive already in the bucket, which is left broken if that second upload dies halfway. The archive name belongs to one BackupJob, so a success row of exactly this upload can only be an earlier attempt's, and the retry now takes it as its result. An error or cancel row still leads to a resumed upload, and a restore still runs again, since a restore is not named per RestoreJob. Assisted-by: LLM Signed-off-by: Alexey Artamonov <[email protected]>
1 parent 55e0573 commit 60dbb03

2 files changed

Lines changed: 47 additions & 6 deletions

File tree

‎hack/altinity-backup-strategy.bats‎

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -483,15 +483,49 @@ run_script() {
483483
grep -qx 'create_remote --resume clickhouse-myapp-20260304T050607Z' "$tmp/state/accepted"
484484
}
485485

486-
# The action log is never trimmed, so a finished row of the same archive may
487-
# be an earlier run's and is not taken for this attempt's result.
488-
@test "backup starts create_remote again when the log holds a finished one of the same archive" {
489-
for status in success error cancel; do
486+
# The archive is named per BackupJob, so a success row of exactly this
487+
# upload is an earlier attempt of the same BackupJob that finished while it
488+
# could not see it. Uploading again with resume would re-upload over the
489+
# finished archive and break it if that second upload dies halfway.
490+
@test "a retried backup takes the upload of its archive an earlier attempt finished as its result" {
491+
tmp=$(mktemp -d)
492+
make_stubs "$tmp/bin" "$tmp/state"
493+
echo '{"command":"create_remote --resume clickhouse-myapp-20260304T050607Z","status":"success","start":"2026-01-01 00:01:30"}' > "$tmp/state/prior"
494+
render_script "$tmp/s.sh" backup myapp clickhouse-myapp-20260304T050607Z
495+
run_script "$tmp"
496+
if grep -q '^POST ' "$tmp/state/requests"; then
497+
echo "FAIL: uploaded again over a finished archive: $(cat "$tmp/state/requests")"
498+
false
499+
fi
500+
}
501+
502+
@test "a retried backup resumes an upload of its archive that ended in error or was cancelled" {
503+
for status in error cancel; do
490504
tmp=$(mktemp -d)
491505
make_stubs "$tmp/bin" "$tmp/state"
492506
echo "{\"command\":\"create_remote --resume clickhouse-myapp-20260304T050607Z\",\"status\":\"$status\",\"start\":\"2026-01-01 00:01:30\"}" > "$tmp/state/prior"
493507
render_script "$tmp/s.sh" backup myapp clickhouse-myapp-20260304T050607Z
494508
run_script "$tmp"
495-
if [ "$(accepted "$tmp")" != 1 ]; then echo "FAIL: a $status row of an earlier upload must not stand in for this one"; false; fi
509+
if [ "$(accepted "$tmp")" != 1 ]; then echo "FAIL: a $status row must lead to a resumed upload"; false; fi
496510
done
497511
}
512+
513+
@test "a retried backup resumes when its latest upload failed after an earlier one succeeded" {
514+
tmp=$(mktemp -d)
515+
make_stubs "$tmp/bin" "$tmp/state"
516+
printf '%s\n' \
517+
'{"command":"create_remote --resume clickhouse-myapp-20260304T050607Z","status":"success","start":"2026-01-01 00:01:00"}' \
518+
'{"command":"create_remote --resume clickhouse-myapp-20260304T050607Z","status":"error","start":"2026-01-01 00:01:30"}' > "$tmp/state/prior"
519+
render_script "$tmp/s.sh" backup myapp clickhouse-myapp-20260304T050607Z
520+
run_script "$tmp"
521+
[ "$(accepted "$tmp")" = 1 ]
522+
}
523+
524+
@test "backup does not take another archive's finished upload as its own" {
525+
tmp=$(mktemp -d)
526+
make_stubs "$tmp/bin" "$tmp/state"
527+
echo '{"command":"create_remote --resume clickhouse-myapp-20260304T040000Z","status":"success","start":"2026-01-01 00:01:30"}' > "$tmp/state/prior"
528+
render_script "$tmp/s.sh" backup myapp clickhouse-myapp-20260304T050607Z
529+
run_script "$tmp"
530+
[ "$(accepted "$tmp")" = 1 ]
531+
}

‎packages/system/backupstrategy-controller/templates/strategy-altinity-default.yaml‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,11 +104,18 @@ spec:
104104
# so a local or partial remote backup an earlier attempt left is
105105
# resumed rather than refused as existing, whatever
106106
# use_resumable_state the sidecar runs with. The sidecar logs
107-
# the request as "create_remote --resume <name>".
107+
# the request as "create_remote --resume <name>". The name is
108+
# this BackupJob's alone, so an upload of it that last ended in
109+
# success is an earlier attempt's finished result: resuming
110+
# over it would upload the archive again and leave it broken if
111+
# that upload died halfway.
108112
cmd="create_remote --resume ${target}"
109113
wait_turn "${cmd}"
110114
if [ "${turn}" = "join" ]; then
111115
echo "${cmd} is already running; waiting for it"
116+
elif printf '%s\n' "${actions}" | jq -es --arg cmd "${cmd}" \
117+
'[.[] | select(.command == $cmd)] | sort_by(.start) | last | .status == "success"' >/dev/null; then
118+
echo "${cmd} already succeeded in an earlier attempt"
112119
else
113120
echo "${cmd}"
114121
curl -fsS ${auth} -X POST "${api}/backup/create_remote?name=${target}&resume=true" >/dev/null

0 commit comments

Comments
 (0)