⚠️ Internal work only. Community contributions are not being accepted for this issue.
Basekick Labs is implementing this internally. Please do not open PRs against it; they will be closed without review. Discussion and questions in the comments are welcome.## Summary
cleanupPartialWrite (internal/backup/backup.go:1243-1259) is the compensation a backup runs when a copy fails part-way. It only works on a backend that stages writes:
si, ok := backend.(storage.StagingInspector)
if !ok {
return
}LocalBackend stages, so the local backup destination is covered and the comment is right that a backend which never stages leaves no .part file. But it does not follow that nothing needs cleaning up: S3 and Azure do not stage, and a failed WriteReader to either can still leave a committed object (a complete small object whose surrounding operation then failed, or an aborted multipart upload holding storage the operator pays for). The function returns before considering that.
Called from three places, all of which pass a backend that may not stage: backup.go:911 (data file), internal/backup/cluster.go:404 (the manifest-files.json sidecar) and internal/backup/warehouse.go:343 (an Iceberg warehouse file). A fourth write, the SQLite metadata copy at backup.go:1322, has no compensation at all.
Why now
Not reachable today: the only backup destination is backup.local_path, always a LocalBackend, which always stages. It becomes reachable the moment a backup destination can be S3 or Azure, which is what named remote targets add (#1085).
Expected
For a destination that does not stage, compensate with a Delete of the destination key, and treat a failure to compensate the way the staging path does: log it at debug with the path, since the next run overwrites the key anyway and a failed cleanup must not fail the backup. Multipart uploads need the SDK's abort rather than a delete; worth confirming what WriteReader leaves behind on each backend before choosing.
Notes
cleanupPartialWrite(internal/backup/backup.go:1243-1259) is the compensation a backup runs when a copy fails part-way. It only works on a backend that stages writes:LocalBackendstages, so the local backup destination is covered and the comment is right that a backend which never stages leaves no.partfile. But it does not follow that nothing needs cleaning up: S3 and Azure do not stage, and a failedWriteReaderto either can still leave a committed object (a complete small object whose surrounding operation then failed, or an aborted multipart upload holding storage the operator pays for). The function returns before considering that.Called from three places, all of which pass a backend that may not stage:
backup.go:911(data file),internal/backup/cluster.go:404(themanifest-files.jsonsidecar) andinternal/backup/warehouse.go:343(an Iceberg warehouse file). A fourth write, the SQLite metadata copy atbackup.go:1322, has no compensation at all.Why now
Not reachable today: the only backup destination is
backup.local_path, always aLocalBackend, which always stages. It becomes reachable the moment a backup destination can be S3 or Azure, which is what named remote targets add (#1085).Expected
For a destination that does not stage, compensate with a
Deleteof the destination key, and treat a failure to compensate the way the staging path does: log it at debug with the path, since the next run overwrites the key anyway and a failed cleanup must not fail the backup. Multipart uploads need the SDK's abort rather than a delete; worth confirming whatWriteReaderleaves behind on each backend before choosing.Notes
.partsuffix and the reason the staging API is used instead of appending it to the key are Remaining many-to-one path mappings outside LocalBackend.validatePath #744.