From 7a23a2175bacc4bd30998e503544d066edc6147e Mon Sep 17 00:00:00 2001 From: Andrew Kryczka Date: Mon, 28 Sep 2020 15:11:54 -0700 Subject: [PATCH] enable Status check assertions for sst_file_reader_test (#7448) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7448 Reviewed By: jay-zhuang Differential Revision: D23973281 Pulled By: ajkr fbshipit-source-id: 90fe55f1db904f1a236f3f7994d0806b8d24282c --- Makefile | 1 + table/sst_file_writer.cc | 26 +++++++++++++++----------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index 093adb9f1..2e8497e28 100644 --- a/Makefile +++ b/Makefile @@ -613,6 +613,7 @@ ifdef ASSERT_STATUS_CHECKED options_test \ random_test \ range_del_aggregator_test \ + sst_file_reader_test \ range_tombstone_fragmenter_test \ repeatable_thread_test \ skiplist_test \ diff --git a/table/sst_file_writer.cc b/table/sst_file_writer.cc index 9d4ffd27d..ab1ee7c4e 100644 --- a/table/sst_file_writer.cc +++ b/table/sst_file_writer.cc @@ -99,9 +99,7 @@ struct SstFileWriter::Rep { file_info.largest_key.assign(user_key.data(), user_key.size()); file_info.file_size = builder->FileSize(); - InvalidatePageCache(false /* closing */); - - return Status::OK(); + return InvalidatePageCache(false /* closing */); } Status DeleteRange(const Slice& begin_key, const Slice& end_key) { @@ -135,15 +133,14 @@ struct SstFileWriter::Rep { file_info.num_range_del_entries++; file_info.file_size = builder->FileSize(); - InvalidatePageCache(false /* closing */); - - return Status::OK(); + return InvalidatePageCache(false /* closing */); } - void InvalidatePageCache(bool closing) { + Status InvalidatePageCache(bool closing) { + Status s = Status::OK(); if (invalidate_page_cache == false) { // Fadvise disabled - return; + return s; } uint64_t bytes_since_last_fadvise = builder->FileSize() - last_fadvise_size; @@ -151,11 +148,16 @@ struct SstFileWriter::Rep { TEST_SYNC_POINT_CALLBACK("SstFileWriter::Rep::InvalidatePageCache", &(bytes_since_last_fadvise)); // Tell the OS that we don't need this file in page cache - file_writer->InvalidateCache(0, 0); + s = file_writer->InvalidateCache(0, 0); + if (s.IsNotSupported()) { + // NotSupported is fine as it could be a file type that doesn't use page + // cache. + s = Status::OK(); + } last_fadvise_size = builder->FileSize(); } + return s; } - }; SstFileWriter::SstFileWriter(const EnvOptions& env_options, @@ -306,7 +308,9 @@ Status SstFileWriter::Finish(ExternalSstFileInfo* file_info) { if (s.ok()) { s = r->file_writer->Sync(r->ioptions.use_fsync); - r->InvalidatePageCache(true /* closing */); + if (s.ok()) { + s = r->InvalidatePageCache(true /* closing */); + } if (s.ok()) { s = r->file_writer->Close(); }