Skip to content

fix(storage): preserve appended data when retrying a stream - #18563

Open
Shubham-Padkonde wants to merge 2 commits into
googleapis:mainfrom
Shubham-Padkonde:fix/append-retry-buffer-offset
Open

Shubham-Padkonde wants to merge 2 commits into
googleapis:mainfrom
Shubham-Padkonde:fix/append-retry-buffer-offset

Conversation

@Shubham-Padkonde

@Shubham-Padkonde Shubham-Padkonde commented Oct 4, 2026 •

Copy link
Copy Markdown

Fixes #18475.

After the first append, persisted_size is an absolute object offset but the retry buffer contains only the current call's data. Seeking that absolute offset can move beyond the buffer and let the append return without resending its unpersisted bytes.

Record the buffer's starting object offset and use relative positions in both failure recovery and stream reopening. A stale progress response can precede the current append, so reopening still obtains the authoritative position before any bytes are resent. If that position lies outside the available buffer, raise ValueError instead of silently completing an unrecoverable upload.

Regression coverage exercises partial and complete persistence, stale progress, and offsets outside the available data. Nine cases fail on unchanged current upstream.

Current validation (November 3, merge commit 0917d4e8, Linux/Python 3.12):

  • Merged main ac9c597b, retaining upstream tracing around append operations and the same four-file PR scope.
  • All 2,245 Storage unit tests pass (python -m pytest tests/unit -q).
  • The 12 later-append cases pass after the resolution; using unchanged upstream implementations with those same tests produces nine failures and three passes.
  • The four PR files pass the package's Ruff import/format settings and Flake8. The diff against main passes whitespace checks.

The earlier Windows run had six unrelated encoding/path test failures; all async tests passed there. Windows was not rerun for this merge. No live GCS system tests were run, and the pre-existing writer module does not have 100% overall coverage.

@Shubham-Padkonde
Shubham-Padkonde requested a review from a team as a code owner October 4, 2026 11:56

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for buffer-relative offsets during the resume and recovery of append operations in the asynchronous appendable object writer. It tracks the buffer_start_offset in the write state, validates that the recovery offset falls within the current append buffer, and updates the user buffer seek position relative to this start offset. Corresponding unit tests have been added to verify correct recovery behavior and error handling for unrecoverable offsets. I have no feedback to provide as there are no review comments.

Retain buffer-relative retry recovery within the upstream tracing span and keep all append regression coverage.
@Shubham-Padkonde
Shubham-Padkonde requested a review from a team as a code owner November 3, 2026 16:41

This branch has not been deployed

No deployments
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.

google-cloud-storage: AsyncAppendableObjectWriter.append() silently drops data after a stream reconnect on non-first appends

1 participant