-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
release-22.1: vendor: bump cloud.google.com/go/storage from v18.2.0 to v1.21.0 #85763
release-22.1: vendor: bump cloud.google.com/go/storage from v18.2.0 to v1.21.0 #85763
Conversation
…o v1.21.0 This commit bumps the `cloud.google.com/go/storage` vendor to include the ability to inject custom retry functions when reading and writing from the underlying SDK - https://pkg.go.dev/cloud.google.com/go/storage#Client.SetRetry. The motivation for this change is to combat the high rate of restores we are seeing fail due to an internal http2 stream error that is being surfaced by the SDK in our roachtests. As seen in cockroachdb#85024 we would like to wrap the default retry logic with our custom retry handling for this particular error. This is the recommended solution as per: googleapis/google-cloud-go#3735 googleapis/google-cloud-go#784 Note, the dependencies have been bumped to the version that we have been running on master since the 22.1 branch was cut. Release note (general change): bump cloud.google.com/go/storage from v18.2.0 to v1.21.0 to allow for injection of custom retry logic in the SDK
Thanks for opening a backport. Please check the backport criteria before merging:
If some of the basic criteria cannot be satisfied, ensure that the exceptional criteria are satisfied within.
Add a brief release justification to the body of your PR to justify this backport. Some other things to consider:
|
We have seen a support escalation with this error too - https://cockroachdb.zendesk.com/agent/tickets/13190 and discussed internally that we would like to consider a backport for #85024 given that its early days for 22.1. This change does bump:
as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've reviewed manually the protobuf and grpc changes and those two LGTM.
For protobufs, the only change that stood out to me was the recursion limit change: https://github.com/protocolbuffers/protobuf-go/releases/tag/v1.28.0 but the default limit of 10000 seems high enough to avoid any practical problems. For gRPC, the only change that jumped out to me was the status code change here https://github.com/grpc/grpc-go/releases/tag/v1.45.0 but I believe we fixed the only known issue with that here: aea1915 |
Thanks for the reviews! |
This commit bumps the
cloud.google.com/go/storage
vendor toinclude the ability to inject custom retry functions when reading
and writing from the underlying SDK -
https://pkg.go.dev/cloud.google.com/go/storage#Client.SetRetry.
The motivation for this change is to combat the high rate of
restores we are seeing fail due to an internal http2 stream error
that is being surfaced by the SDK in our roachtests. As seen in #85024
we would like to wrap the default retry logic with our custom retry
handling for this particular error. This is the recommended solution as per:
googleapis/google-cloud-go#3735
googleapis/google-cloud-go#784
Note, the dependencies have been bumped to the version that we have
been running on master since the 22.1 branch was cut.
Release note (general change): bump cloud.google.com/go/storage from
v18.2.0 to v1.21.0 to allow for injection of custom retry logic in the
SDK
Release justification: necessary dependency bump that allows us to backport a fix
to prevent restores from failing with an internal stream HTTP2 error as seen in
several roachtests. The fix that we wish to backport is #85024.