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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Mar 17, 2022
Merged

fix(storage): return an error on short writes #8562

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions google/cloud/storage/internal/object_write_streambuf.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,6 @@ void ObjectWriteStreambuf::FlushRoundChunk(ConstBufferSequence buffers) {
// GCS upload returns an updated range header that sets the next expected
// byte. Check to make sure it remains consistent with the bytes stored in the
// buffer.
auto first_buffered_byte = upload_session_->next_expected_byte();
auto expected_next_byte = upload_session_->next_expected_byte() + actual_size;
last_response_ = upload_session_->UploadChunk(payload);

Expand All @@ -200,8 +199,10 @@ void ObjectWriteStreambuf::FlushRoundChunk(ConstBufferSequence buffers) {
// even if no "final chunk" is sent. The resumable upload classes know how
// to deal with this mess, so let's not duplicate that code here.
auto actual_next_byte = upload_session_->next_expected_byte();
if (actual_next_byte < expected_next_byte &&
actual_next_byte < first_buffered_byte) {
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 "
Copy link

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?

<< actual_next_byte << " which has already been uploaded.";
Expand Down
29 changes: 29 additions & 0 deletions google/cloud/storage/internal/object_write_streambuf_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,35 @@ TEST(ObjectWriteStreambufTest, NextExpectedByteDecreases) {
EXPECT_THAT(streambuf.last_status(), StatusIs(StatusCode::kAborted));
}

/// @test verify that the upload steam transitions to a bad state on a partial
/// write.
TEST(ObjectWriteStreambufTest, PartialUploadChunk) {
auto mock = absl::make_unique<testing::MockResumableUploadSession>();

auto const quantum = UploadChunkRequest::kChunkSizeQuantum;
std::string const payload = std::string(quantum * 4, '*');

auto next_byte = 2 * quantum - 1;
EXPECT_CALL(*mock, UploadChunk).WillOnce(InvokeWithoutArgs([&]() {
return make_status_or(ResumableUploadResponse{
"", ResumableUploadResponse::kInProgress, next_byte, {}, {}});
}));
EXPECT_CALL(*mock, next_expected_byte()).WillRepeatedly([&]() {
return next_byte;
});
EXPECT_CALL(*mock, done).WillRepeatedly(Return(false));
std::string id = "id";
EXPECT_CALL(*mock, session_id).WillRepeatedly(ReturnRef(id));

ObjectWriteStreambuf streambuf(
std::move(mock), quantum, CreateNullHashFunction(), HashValues{},
CreateNullHashValidator(), AutoFinalizeConfig::kEnabled);
std::ostream output(&streambuf);
output.write(payload.data(), payload.size());
EXPECT_FALSE(output.good());
EXPECT_THAT(streambuf.last_status(), StatusIs(StatusCode::kAborted));
}

/// @test Verify that a stream flushes when mixing operations that add one
/// character at a time and operations that add buffers.
TEST(ObjectWriteStreambufTest, MixPutcPutn) {
Expand Down