fix(storage): Update offset on resumable upload retry #12086
Conversation
When resumable uploads retry, they may observe a flush offset which is past the start of the current send (in fact the whole send might have already completed). In that case, we avoided re-sending unnecessary data, but we didn't update the offset to account for the data not sent.
|
The test failure here is only on "earliest version" (go 1.23.6) not "latest version" (go 1.24.0): Which is... surprising :). Maybe just flaky comparison, but I don't think related to this PR. I can look later. |
This is somewhat surprising for the emulator but might just be a flake; I triggered a rerun. |
The issue is that we check specifically that the error is context.DeadlineExceeded, but this is a gRPC-layer DeadlineExceeded. I don't think that can be reliable: the context deadline is propagated to the server, so now there are two clocks, and the server might return a DeadlineExceeded error before the local context is actually cancelled. I'll send a PR to check for gRPC errors too. |
|
#12092 to fix the flake |
When resumable uploads retry, they may observe a flush offset which is past the start of the current send (in fact the whole send might have already completed). In that case, we avoided re-sending unnecessary data, but we didn't update the offset to account for the data not sent.
When resumable uploads retry, they may observe a flush offset which is past the start of the current send (in fact the whole send might have already completed). In that case, we avoided re-sending unnecessary data, but we didn't update the offset to account for the data not sent.