sneak/vaultik
1
0
Fork 0
Code Issues 38 Pull Requests 5 Actions Packages Projects Releases Wiki Activity

Interrupted blob upload leaves committed chunk rows; a retry silently produces an unrestorable snapshot #148

New Issue
Open
opened 2026-09-21 22:02:05 +02:00 by clawbot · 1 comment
clawbot commented 2026-09-21 22:02:05 +02:00
Collaborator
Copy Link
Copy Source

An interrupted blob upload leaves the local index able to deduplicate
against data that was never stored remotely, so a later backup run
completes successfully (exit 0) yet produces a snapshot that cannot be
restored. This is silent data loss and was found while writing the
fault-injection tests for
#72.

Root cause

During a scan, blob.Packer.finalizeCurrentBlob →
commitBlobToDatabase → insertBlobRecords
(internal/blob/packer.go) writes the chunks, blob_chunks, and
blobs (finished, uploaded_ts NULL) rows for a blob before the
blob is handed to Scanner.handleBlobReady
(internal/snapshot/scanner.go) for upload. If that upload fails — a
dropped connection mid-blob, or kill -9 — the scan aborts, but those
rows survive in the local index.

On the next run the same index is reused. Scanner.loadKnownChunks
loads every chunk from the chunks table regardless of whether its
blob was ever uploaded, so chunkExists returns true for the orphaned
chunks and they are never re-packed or re-uploaded. The new snapshot's
files reference those chunks, but no uploaded blob covers them. The
backup reports success.

Impact

Restoring the resulting snapshot fails at plan build with
chunk missing from blob map: <hash>; a verify --deep would report
the referenced blob missing from storage. The user is told nothing is
wrong at backup time.

Reproduction

internal/vaultik/fault_injection_test.go contains
TestBackupRetryAfterInterruptedUploadIsRestorable, skipped and
pointing at this issue. It fails a blob upload through the
faultstore seam, retries the backup on the same index with a working
backend, and asserts the retried snapshot restores. It currently fails
with the error above.

Definition of done

  1. Decide and implement one of:

    • (a) Persist a blob's chunks / blob_chunks / blobs rows
      only once its upload has succeeded, so an interrupted upload
      leaves no chunk rows to dedup against; or
    • (b) At the start of a run, detect blobs with finished_ts set
      and uploaded_ts NULL (or whose object is absent at the
      destination) and either re-upload them or drop their chunk rows so
      the data is re-chunked. This is also the "next run detects and
      repairs the state" behavior called for by scenario 2 of
      #72, so a fix here should
      cover an interrupted metadata export too (database uploaded, blobs
      present, but the run died before the manifest).

    State which was chosen and why in the commit message.

  2. A backup that is interrupted during upload and then re-run produces
    a snapshot that restores byte-for-byte, or fails loudly at backup
    time rather than silently emitting an unrestorable snapshot.

  3. Remove the t.Skip from
    TestBackupRetryAfterInterruptedUploadIsRestorable and have it pass.

  4. make check green.

model: claude-opus-4-8

An interrupted blob upload leaves the local index able to deduplicate against data that was never stored remotely, so a later backup run completes successfully (exit 0) yet produces a snapshot that cannot be restored. This is silent data loss and was found while writing the fault-injection tests for https://git.eeqj.de/sneak/vaultik/issues/72. ## Root cause During a scan, `blob.Packer.finalizeCurrentBlob` → `commitBlobToDatabase` → `insertBlobRecords` (`internal/blob/packer.go`) writes the `chunks`, `blob_chunks`, and `blobs` (finished, `uploaded_ts` NULL) rows for a blob **before** the blob is handed to `Scanner.handleBlobReady` (`internal/snapshot/scanner.go`) for upload. If that upload fails — a dropped connection mid-blob, or `kill -9` — the scan aborts, but those rows survive in the local index. On the next run the same index is reused. `Scanner.loadKnownChunks` loads every chunk from the `chunks` table regardless of whether its blob was ever uploaded, so `chunkExists` returns true for the orphaned chunks and they are never re-packed or re-uploaded. The new snapshot's files reference those chunks, but no uploaded blob covers them. The backup reports success. ## Impact Restoring the resulting snapshot fails at plan build with `chunk missing from blob map: <hash>`; a `verify --deep` would report the referenced blob missing from storage. The user is told nothing is wrong at backup time. ## Reproduction `internal/vaultik/fault_injection_test.go` contains `TestBackupRetryAfterInterruptedUploadIsRestorable`, skipped and pointing at this issue. It fails a blob upload through the `faultstore` seam, retries the backup on the same index with a working backend, and asserts the retried snapshot restores. It currently fails with the error above. ## Definition of done 1. Decide and implement one of: - **(a)** Persist a blob's `chunks` / `blob_chunks` / `blobs` rows only once its upload has succeeded, so an interrupted upload leaves no chunk rows to dedup against; or - **(b)** At the start of a run, detect blobs with `finished_ts` set and `uploaded_ts` NULL (or whose object is absent at the destination) and either re-upload them or drop their chunk rows so the data is re-chunked. This is also the "next run detects and repairs the state" behavior called for by scenario 2 of https://git.eeqj.de/sneak/vaultik/issues/72, so a fix here should cover an interrupted metadata export too (database uploaded, blobs present, but the run died before the manifest). State which was chosen and why in the commit message. 2. A backup that is interrupted during upload and then re-run produces a snapshot that restores byte-for-byte, or fails loudly at backup time rather than silently emitting an unrestorable snapshot. 3. Remove the `t.Skip` from `TestBackupRetryAfterInterruptedUploadIsRestorable` and have it pass. 4. `make check` green. model: claude-opus-4-8
clawbot commented 2026-09-22 01:25:48 +02:00
Author
Collaborator
Copy Link
Copy Source

Fixed in #175 (base next).

Chosen: issue option b. Deduplication now trusts a chunk only when a blob holding it has uploaded_ts set, and every scan drops blob rows with uploaded_ts IS NULL (their blob_chunks cascade) plus any chunks left unreferenced, so an interrupted run's data is re-chunked and re-uploaded and the index self-repairs. The storage-less path marks its blobs uploaded so the invariant holds uniformly.

The skipped test named here lives in the unmerged #72 branch, so the PR adds its own reproduction: TestBackupRetryAfterInterruptedUploadIsRestorable fails a blob upload, retries on the same index with a working backend, then restores and byte-compares. It fails without the change at plan build (chunk missing from blob map) and passes with it.

model: claude-opus-4-8

Fixed in https://git.eeqj.de/sneak/vaultik/pulls/175 (base `next`). Chosen: issue option b. Deduplication now trusts a chunk only when a blob holding it has `uploaded_ts` set, and every scan drops blob rows with `uploaded_ts IS NULL` (their `blob_chunks` cascade) plus any chunks left unreferenced, so an interrupted run's data is re-chunked and re-uploaded and the index self-repairs. The storage-less path marks its blobs uploaded so the invariant holds uniformly. The skipped test named here lives in the unmerged #72 branch, so the PR adds its own reproduction: `TestBackupRetryAfterInterruptedUploadIsRestorable` fails a blob upload, retries on the same index with a working backend, then restores and byte-compares. It fails without the change at plan build (`chunk missing from blob map`) and passes with it. model: claude-opus-4-8
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
merge-ready
All checks pass, reviewed, rebased, ready for merge
 needs-checks
make check does not pass cleanly
 needs-rebase
PR has merge conflicts or is behind main
 needs-review
Code review not yet done
 needs-rework
Code review found issues to fix
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
clawbot sneak (Jeffrey Paul)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/vaultik#148