Skip to content
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

fix(storage): set flush and get_state to false on the last write in gRPC #9013

Merged
merged 3 commits into from
Nov 15, 2023

Conversation

BrennaEpp
Copy link
Contributor

Only receive if get_state was true (ie. remainingDataFitsInSingleReq && !lastWriteOfEntireObject) (or after CloseSend) to avoid blocking.

@BrennaEpp BrennaEpp requested review from a team as code owners November 15, 2023 21:24
@product-auto-label product-auto-label bot added size: s Pull request size is small. api: storage Issues related to the Cloud Storage API. labels Nov 15, 2023
storage/grpc_client.go Outdated Show resolved Hide resolved
}

// Confirm the persisted data if we have not finished uploading the object.
if !lastWriteOfEntireObject {
if resp.GetPersistedSize() != writeOffset {
// Retry if not all bytes were persisted.
writeOffset = resp.GetPersistedSize()
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it actually the case that we no longer need to revc() from the stream if we are ready to finalize the object on this msg?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do still receive (see L1775), but only until after CloseSend.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I see, makes sense.

@tritone
Copy link
Contributor

tritone commented Nov 15, 2023

Also, commit type could be fix for this PR.

@BrennaEpp BrennaEpp changed the title chore(storage): set flush and get_state to false on the last write in gRPC fix(storage): set flush and get_state to false on the last write in gRPC Nov 15, 2023
@BrennaEpp BrennaEpp merged commit c1e9fe5 into googleapis:main Nov 15, 2023
9 checks passed
@BrennaEpp BrennaEpp deleted the grpc-write-noflush branch November 15, 2023 23:53
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. size: s Pull request size is small.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants