Fix WritableFileWriter::Append() return

Summary: It looks like WritableFileWriter::Append() was returning OK() even when there is an error

Test Plan: make check

Reviewers: sdong, yhchiang, anthony, rven, kradhakrishnan, igor

Reviewed By: igor

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D49569
This commit is contained in:
Islam AbdelRahman 2015-10-27 21:04:00 -07:00
parent d0a18c2840
commit 72d6e758b4
2 changed files with 46 additions and 3 deletions

View file

@ -102,7 +102,7 @@ Status WritableFileWriter::Append(const Slice& data) {
// We double the buffer here because
// Flush calls do not keep up with the incoming bytes
// This is the only place when buffer is changed with unbuffered I/O
if (buf_.Capacity() < (1 << 20)) {
if (buf_.Capacity() < c_OneMb) {
size_t desiredCapacity = buf_.Capacity() * 2;
desiredCapacity = std::min(desiredCapacity, c_OneMb);
buf_.AllocateNewBuffer(desiredCapacity);
@ -116,8 +116,10 @@ Status WritableFileWriter::Append(const Slice& data) {
}
TEST_KILL_RANDOM("WritableFileWriter::Append:1", rocksdb_kill_odds);
filesize_ += data.size();
return Status::OK();
if (s.ok()) {
filesize_ += data.size();
}
return s;
}
Status WritableFileWriter::Close() {

View file

@ -84,6 +84,47 @@ TEST_F(WritableFileWriterTest, RangeSync) {
}
writer->Close();
}
TEST_F(WritableFileWriterTest, AppendStatusReturn) {
class FakeWF : public WritableFile {
public:
explicit FakeWF() : use_os_buffer_(true), io_error_(false) {}
virtual bool UseOSBuffer() const override { return use_os_buffer_; }
Status Append(const Slice& data) override {
if (io_error_) {
return Status::IOError("Fake IO error");
}
return Status::OK();
}
Status PositionedAppend(const Slice& data, uint64_t) override {
if (io_error_) {
return Status::IOError("Fake IO error");
}
return Status::OK();
}
Status Close() override { return Status::OK(); }
Status Flush() override { return Status::OK(); }
Status Sync() override { return Status::OK(); }
void SetUseOSBuffer(bool val) { use_os_buffer_ = val; }
void SetIOError(bool val) { io_error_ = val; }
protected:
bool use_os_buffer_;
bool io_error_;
};
unique_ptr<FakeWF> wf(new FakeWF());
wf->SetUseOSBuffer(false);
unique_ptr<WritableFileWriter> writer(
new WritableFileWriter(std::move(wf), EnvOptions()));
ASSERT_OK(writer->Append(std::string(2 * kMb, 'a')));
// Next call to WritableFile::Append() should fail
dynamic_cast<FakeWF*>(writer->writable_file())->SetIOError(true);
ASSERT_NOK(writer->Append(std::string(2 * kMb, 'b')));
}
} // namespace rocksdb
int main(int argc, char** argv) {