Skip to content

fix(storage): fix race condition during retries in gRPC writer#14649

Merged
krishnamd-jkp merged 2 commits into
googleapis:mainfrom
krishnamd-jkp:grpc_write_race
Jun 1, 2026
Merged

fix(storage): fix race condition during retries in gRPC writer#14649
krishnamd-jkp merged 2 commits into
googleapis:mainfrom
krishnamd-jkp:grpc_write_race

Conversation

@krishnamd-jkp

Copy link
Copy Markdown
Contributor

No description provided.

@krishnamd-jkp krishnamd-jkp requested review from a team as code owners May 26, 2026 14:11
@product-auto-label product-auto-label Bot added the api: storage Issues related to the Cloud Storage API. label May 26, 2026

@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 new retry conformance tests and updates the gRPC writer implementation. However, several blocks of code in storage/grpc_writer.go related to retry and drain logic have been left commented out and should either be uncommented or removed. Additionally, the new test cases in storage/retry_conformance_test.go can be made more readable by replacing bitwise shift operators with explicit multipliers and the existing MiB constant.

Comment thread storage/grpc_writer.go Outdated
Comment thread storage/grpc_writer.go Outdated
Comment thread storage/grpc_writer.go Outdated
Comment thread storage/grpc_writer.go Outdated
Comment thread storage/retry_conformance_test.go Outdated
Comment thread storage/retry_conformance_test.go Outdated
Comment thread storage/retry_conformance_test.go Outdated
@krishnamd-jkp krishnamd-jkp force-pushed the grpc_write_race branch 2 times, most recently from 2cf6f2f to 55dc6da Compare May 26, 2026 14:28
@krishnamd-jkp

Copy link
Copy Markdown
Contributor Author

/gemini review

@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 updates the gRPC writer implementation to handle retries more robustly by tracking the initial offset of write commands and adjusting the payload buffer based on bytes already persisted by the server. It also drains initial completions in the write loop and updates the conformance tests with new retry scenarios and test cases to exercise different chunk-writing patterns. I have no feedback to provide as there are no review comments.

Comment thread storage/grpc_writer.go
Comment thread storage/grpc_writer.go Outdated
Comment thread storage/grpc_writer.go
@krishnamd-jkp krishnamd-jkp merged commit c781a75 into googleapis:main Jun 1, 2026
17 checks passed
krishnamd-jkp added a commit that referenced this pull request Jun 1, 2026
PR created by the Librarian CLI to initialize a release. Merging this PR
will auto trigger a release.

Librarian Version: v0.15.0
Language Image:
us-central1-docker.pkg.dev/cloud-sdk-librarian-prod/images-prod/librarian-go@sha256:19bb93e8f1f916c61b597db2bad65dc432f79baaabb210499d7d0e4ad1dffe29
<details><summary>storage: v1.61.5</summary>

##
[v1.61.5](storage/v1.61.4...storage/v1.61.5)
(2026-06-01)

### Bug Fixes

* fix race condition during retries in gRPC writer (#14649)
([e87213b](e87213b7))

</details>
krishnamd-jkp added a commit that referenced this pull request Jun 4, 2026
PR created by the Librarian CLI to initialize a release. Merging this PR
will auto trigger a release.

Librarian Version: v0.16.0
Language Image:
us-central1-docker.pkg.dev/cloud-sdk-librarian-prod/images-prod/librarian-go@sha256:b04b076f5eedbb5546bd6fc1404969dd3698c8b19c0f34ae815a84ae735a606a
<details><summary>storage: v1.62.3</summary>

##
[v1.62.3](storage/v1.62.2...storage/v1.62.3)
(2026-06-03)

### Bug Fixes

* fix race condition during retries in gRPC writer (#14649)
([04b6c63](04b6c635))

* add server closed idle connection to retriable errors (#14594)
([20b37d6](20b37d65))

</details>
krishnamd-jkp added a commit that referenced this pull request Jun 26, 2026
🤖 I have created a release *beep* *boop*
---


##
[1.63.0](storage/v1.62.2...storage/v1.63.0)
(2026-06-25)


### Features

* **go:** Add full object checksum for negative offsets &gt; size
([#20026](#20026))
([a04d980](a04d980))
* **storage:** Add client feature tracking in HTTP client
([#14691](#14691))
([319cc4c](319cc4c))
* **storage:** App Centric Observability
([#14685](#14685))
([c3273bb](c3273bb))
* **storage:** Read checksums in gRPC partial reads
([#14586](#14586))
([d29f68a](d29f68a))
* **storage:** Support deleteSourceObjects option in object compose
([#14704](#14704))
([0d2d680](0d2d680))
* Update API sources and regenerate
([#14701](#14701))
([a9b7921](a9b7921))


### Bug Fixes

* **storage:** Add server closed idle connection to retriable errors
([#14594](#14594))
([a6bd392](a6bd392))
* **storage:** Fix race condition during retries in gRPC writer
([#14649](#14649))
([c781a75](c781a75))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Krishnam Dhanush <krishnamd@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: storage Issues related to the Cloud Storage API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants