Skip to content

Commit

Permalink
check return status for Sync() and Append() calls to avoid corruption
Browse files Browse the repository at this point in the history
Summary:
Right now in `SyncClosedLogs`, `CopyFile`, and `AddRecord`, where `Sync` and `Append` are invoked in a loop, the error status are not checked. This could lead to potential corruption as later calls will overwrite the error status.
Closes facebook#3740

Differential Revision: D7678848

Pulled By: miasantreble

fbshipit-source-id: 4b0b412975989dfe80348f73217b9c4122a4bd77
  • Loading branch information
miasantreble authored and petermattis committed Nov 29, 2018
1 parent 7410b3e commit a66ad5a
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 5 deletions.
3 changes: 3 additions & 0 deletions db/db_impl_compaction_flush.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ Status DBImpl::SyncClosedLogs(JobContext* job_context) {
"[JOB %d] Syncing log #%" PRIu64, job_context->job_id,
log->get_log_number());
s = log->file()->Sync(immutable_db_options_.use_fsync);
if (!s.ok()) {
break;
}
}
if (s.ok()) {
s = directories_.GetWalDir()->Fsync();
Expand Down
8 changes: 5 additions & 3 deletions db/log_writer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,11 @@ Status Writer::AddRecord(const Slice& slice) {
// Fill the trailer (literal below relies on kHeaderSize and
// kRecyclableHeaderSize being <= 11)
assert(header_size <= 11);
dest_->Append(
Slice("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00",
static_cast<size_t>(leftover)));
s = dest_->Append(Slice("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00",
static_cast<size_t>(leftover)));
if (!s.ok()) {
break;
}
}
block_offset_ = 0;
}
Expand Down
3 changes: 1 addition & 2 deletions util/file_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,7 @@ Status CopyFile(Env* env, const std::string& source,
}
size -= slice.size();
}
dest_writer->Sync(use_fsync);
return Status::OK();
return dest_writer->Sync(use_fsync);
}

// Utility function to create a file with the provided contents
Expand Down

0 comments on commit a66ad5a

Please sign in to comment.