Drop unsynced data in TestFSWritableFile::Close() (#12528)

Summary:
Our `FileSystem` for simulating unsynced data loss should not sync during `Close()` because it masks bugs where we forgot to sync as long as we closed the file.

Pull Request resolved: #12528

Test Plan:
Peeled back #10560 fix and verified it is caught much faster now (few seconds vs. ???) with command like

```
$ TEST_TMPDIR=./ python3 tools/db_crashtest.py blackbox --disable_wal=0 --max_key=1000 --write_buffer_size=131072 --max_bytes_for_level_base=524288 --target_file_size_base=131072 --interval=3 --sync_fault_injection=1 --enable_blob_files=0 --manual_wal_flush_one_in=10 --sync_wal_one_in=0 --get_live_files_one_in=0 --get_sorted_wal_files_one_in=0 --backup_one_in=0 --checkpoint_one_in=0 --write_fault_one_in=0 --read_fault_one_in=0 --open_write_fault_one_in=0 --compact_range_one_in=0 --compact_files_one_in=0 --open_read_fault_one_in=0 --get_property_one_in=0 --writepercent=100 -readpercent=0 -prefixpercent=0 -delpercent=0 -delrangepercent=0 -iterpercent=0
```

Reviewed By: anand1976

Differential Revision: D56033250

Pulled By: ajkr

fbshipit-source-id: 6bbf480d79a06c46f08f6214010937f6654af5ca

db/db_wal_test.cc

@@ -814,6 +814,7 @@ TEST_F(DBWALTest, WALWithChecksumHandoff) {
     writeOpt.disableWAL = false;
     // Data is persisted in the WAL
     ASSERT_OK(dbfull()->Put(writeOpt, handles_[1], "zoo", "v3"));
+    ASSERT_OK(dbfull()->SyncWAL());
     // The hash does not match, write fails
     fault_fs->SetChecksumHandoffFuncType(ChecksumType::kxxHash);
     writeOpt.disableWAL = false;

db/error_handler_fs_test.cc

@@ -1238,6 +1238,7 @@ TEST_F(DBErrorHandlingFSTest, AutoRecoverFlushError) {
                    ERROR_HANDLER_AUTORESUME_RETRY_TOTAL_COUNT));
   ASSERT_EQ(0, options.statistics->getAndResetTickerCount(
                    ERROR_HANDLER_AUTORESUME_SUCCESS_COUNT));
+  ASSERT_OK(dbfull()->SyncWAL());
 
   Reopen(options);
   ASSERT_EQ("val", Get(Key(0)));

db/fault_injection_test.cc

@@ -578,8 +578,10 @@ TEST_P(FaultInjectionTest, NoDuplicateTrailingEntries) {
 
   fault_fs->DisableWriteErrorInjection();
 
-  // Closing the log writer will cause WritableFileWriter::Close() and flush
-  // remaining data from its buffer to underlying file.
+  // Flush remaining data from its buffer to underlying file.
+  ASSERT_OK(log_writer->file()->writable_file()->Sync(IOOptions(),
+                                                      nullptr /* dbg */));
+  // Closing the log writer will cause WritableFileWriter::Close()
   log_writer.reset();
 
   {

utilities/cache_dump_load_impl.h

@@ -192,8 +192,12 @@ class ToFileCacheDumpWriter : public CacheDumpWriter {
 
   // Reset the writer
   IOStatus Close() override {
+    IOStatus io_s;
+    if (file_writer_ != nullptr && !file_writer_->seen_error()) {
+      io_s = file_writer_->Sync(IOOptions(), false /* use_fsync */);
+    }
     file_writer_.reset();
-    return IOStatus::OK();
+    return io_s;
   }
 
  private:

utilities/fault_injection_fs.cc

@@ -255,15 +255,10 @@ IOStatus TestFSWritableFile::Close(const IOOptions& options,
   }
   writable_file_opened_ = false;
   IOStatus io_s;
-  if (!target_->use_direct_io()) {
-    io_s = target_->Append(state_.buffer_, options, dbg);
-  }
-  if (io_s.ok()) {
-    state_.buffer_.resize(0);
-    // Ignore sync errors
-    target_->Sync(options, dbg).PermitUncheckedError();
-    io_s = target_->Close(options, dbg);
-  }
+  // Drop buffered data that was never synced because close is not a syncing
+  // mechanism in POSIX file semantics.
+  state_.buffer_.resize(0);
+  io_s = target_->Close(options, dbg);
   if (io_s.ok()) {
     IOStatus in_s = fs_->InjectMetadataWriteError();
     if (!in_s.ok()) {

utilities/transactions/transaction_test.cc

@@ -334,6 +334,7 @@ TEST_P(TransactionTest, SwitchMemtableDuringPrepareAndCommit_WC) {
     ASSERT_EQ("value", value);
   }
 
+  ASSERT_OK(dbimpl->SyncWAL());
   delete db;
   db = nullptr;
   Status s;