Skip to content

fix(gs): abort uploads when source reader fails - #1480

Open
UniversePeak wants to merge 2 commits into
benbjohnson:mainfrom
UniversePeak:fix/1263-gcs-abort-partial-upload
Open

fix(gs): abort uploads when source reader fails#1480
UniversePeak wants to merge 2 commits into
benbjohnson:mainfrom
UniversePeak:fix/1263-gcs-abort-partial-upload

Conversation

@UniversePeak

Copy link
Copy Markdown

Summary

Abort a GCS upload when the source reader returns an error instead of allowing a deferred Writer.Close() to finalize the partial object under its final LTX key.

Problem

gs.ReplicaClient.WriteLTXFile() previously deferred w.Close() immediately after creating the GCS writer. If io.Copy() read a partial LTX stream and then returned a source error, the deferred close still completed the GCS write. That could leave a partial object visible at the final LTX path even though WriteLTXFile() returned an error.

Issue #1263 identifies this path, including the compaction case where an io.Pipe forwards a source or compaction error after partial output. The issue discussion also confirms that the earlier PR covering this behavior is no longer present and that the issue remains uncovered.

Solution

  • Create a writer-scoped context with context.WithCancel.
  • Pass that context to NewWriter.
  • Cancel the writer context on io.Copy() failure and return without calling normal Close().
  • Keep normal Close() only on the successful copy path.
  • Add a fake-GCS regression test with a reader that fails after returning partial data, asserting that the final object does not exist.

Scope

In scope:

  • GCS WriteLTXFile() failure cleanup.
  • A focused regression test for partial source reads.

Not in scope:

  • Other replica backends.
  • LTX format, object naming, or compaction behavior.

Test Plan

  • go test ./gs -run 'TestReplicaClient_(OpenLTXFileReadsFullObject|WriteLTXFileAbortsOnSourceError)$' -count=1
  • go test ./gs -count=1
  • go test ./... -count=1 (the GCS package passes; unrelated Windows filesystem/path assumptions cause failures in file, cmd/litestream, and tests/integration on this host)

The focused regression test passes against the fake GCS server. The full test command reaches the changed package successfully but has environment-specific failures involving Windows path semantics, directory syncing, and file locks outside this change.

Fixes #1263

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GCS WriteLTXFile may finalize a partial object when the source reader fails

1 participant