-
Notifications
You must be signed in to change notification settings - Fork 382
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): return an error on short writes #8562
Conversation
Very rarely the retry policy is exhausted during an upload. If this happens when the last request was a query to find the status of the upload the `UploadChunk()` succeeds with a short write. We need to determine if that is a good idea (it might be). In the interim, we need to signal to the application that the stream is no longer usable. They can use `.Suspend()` to reset the stream (maybe in another process) if that can be useful.
/fyi: @Jseph |
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
Codecov Report
@@ Coverage Diff @@
## main #8562 +/- ##
=======================================
Coverage 93.90% 93.90%
=======================================
Files 1459 1459
Lines 126428 126447 +19
=======================================
+ Hits 118720 118744 +24
+ Misses 7708 7703 -5
Continue to review full report at Codecov.
|
if (actual_next_byte < expected_next_byte) { | ||
// TODO(#8559) - if actual_next_byte < first_buffered_byte and there is | ||
// enough space in the buffer we could save the bytes for a future | ||
// write. | ||
std::ostringstream error_message; | ||
error_message << "Could not continue upload stream. GCS requested byte " |
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.
Does this error message need to be updated?
If actual next byte is less than expected next byte, that means some bytes have not been committed and therefore might not be uploaded yet?
Very rarely the retry policy is exhausted during an upload. If this
happens when the last request was a query to find the status of the
upload the
UploadChunk()
succeeds with a short write.We need to determine if that is a good idea (it might be). In the
interim, we need to signal to the application that the stream is no
longer usable. They can use
.Suspend()
to reset the stream (maybe inanother process) if that can be useful.
Part of the work for #8559
This change is