From 72d6e758b41162d1a0727873ee80fa262eb64656 Mon Sep 17 00:00:00 2001
From: Islam AbdelRahman <tec@fb.com>
Date: Tue, 27 Oct 2015 21:04:00 -0700
Subject: [PATCH] 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
---
 util/file_reader_writer.cc      |  8 ++++---
 util/file_reader_writer_test.cc | 41 +++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/util/file_reader_writer.cc b/util/file_reader_writer.cc
index ff459262c..8b2e4cb2f 100644
--- a/util/file_reader_writer.cc
+++ b/util/file_reader_writer.cc
@@ -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() {
diff --git a/util/file_reader_writer_test.cc b/util/file_reader_writer_test.cc
index d1f0dcbec..370f52392 100644
--- a/util/file_reader_writer_test.cc
+++ b/util/file_reader_writer_test.cc
@@ -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) {