Improve Status message for block checksum mismatches

Summary:
We've got some DBs where iterators return Status with message "Corruption: block checksum mismatch" all the time. That's not very informative. It would be much easier to investigate if the error message contained the file name - then we would know e.g. how old the corrupted file is, which would be very useful for finding the root cause. This PR adds file name, offset and other stuff to some block corruption-related status messages.

It doesn't improve all the error messages, just a few that were easy to improve. I'm mostly interested in "block checksum mismatch" and "Bad table magic number" since they're the only corruption errors that I've ever seen in the wild.
Closes https://github.com/facebook/rocksdb/pull/2507

Differential Revision: D5345702

Pulled By: al13n321

fbshipit-source-id: fc8023d43f1935ad927cef1b9c55481ab3cb1339

db/external_sst_file_ingestion_job.cc

@@ -286,7 +286,8 @@ Status ExternalSstFileIngestionJob::GetIngestedFileInfo(
   if (!status.ok()) {
     return status;
   }
-  sst_file_reader.reset(new RandomAccessFileReader(std::move(sst_file)));
+  sst_file_reader.reset(new RandomAccessFileReader(std::move(sst_file),
+                                                   external_file));
 
   status = cfd_->ioptions()->table_factory->NewTableReader(
       TableReaderOptions(*cfd_->ioptions(), env_options_,

db/table_cache.cc

@@ -108,7 +108,7 @@ Status TableCache::GetTableReader(
     }
     StopWatch sw(ioptions_.env, ioptions_.statistics, TABLE_OPEN_IO_MICROS);
     std::unique_ptr<RandomAccessFileReader> file_reader(
-        new RandomAccessFileReader(std::move(file), ioptions_.env,
+        new RandomAccessFileReader(std::move(file), fname, ioptions_.env,
                                    ioptions_.statistics, record_read_stats,
                                    file_read_hist, ioptions_.rate_limiter,
                                    for_compaction));

db/version_set.cc

@@ -598,15 +598,15 @@ Status Version::GetTableProperties(std::shared_ptr<const TableProperties>* tp,
   // 2. Table is not present in table cache, we'll read the table properties
   // directly from the properties block in the file.
   std::unique_ptr<RandomAccessFile> file;
+  std::string file_name;
   if (fname != nullptr) {
-    s = ioptions->env->NewRandomAccessFile(
+    file_name = *fname;
-        *fname, &file, vset_->env_options_);
   } else {
-    s = ioptions->env->NewRandomAccessFile(
+    file_name =
       TableFileName(vset_->db_options_->db_paths, file_meta->fd.GetNumber(),
-                      file_meta->fd.GetPathId()),
+                    file_meta->fd.GetPathId());
-        &file, vset_->env_options_);
   }
+  s = ioptions->env->NewRandomAccessFile(file_name, &file, vset_->env_options_);
   if (!s.ok()) {
     return s;
   }
@@ -615,7 +615,7 @@ Status Version::GetTableProperties(std::shared_ptr<const TableProperties>* tp,
   // By setting the magic number to kInvalidTableMagicNumber, we can by
   // pass the magic number check in the footer.
   std::unique_ptr<RandomAccessFileReader> file_reader(
-      new RandomAccessFileReader(std::move(file)));
+      new RandomAccessFileReader(std::move(file), file_name));
   s = ReadTableProperties(
       file_reader.get(), file_meta->fd.GetFileSize(),
       Footer::kInvalidTableMagicNumber /* table's magic number */, *ioptions, &raw_table_properties);

table/cuckoo_table_builder_test.cc

@@ -56,7 +56,7 @@ class CuckooBuilderTest : public testing::Test {
     // Assert Table Properties.
     TableProperties* props = nullptr;
     unique_ptr<RandomAccessFileReader> file_reader(
-        new RandomAccessFileReader(std::move(read_file)));
+        new RandomAccessFileReader(std::move(read_file), fname));
     ASSERT_OK(ReadTableProperties(file_reader.get(), read_file_size,
                                   kCuckooTableMagicNumber, ioptions,
                                   &props));

table/cuckoo_table_reader_test.cc

@@ -116,7 +116,7 @@ class CuckooReaderTest : public testing::Test {
     std::unique_ptr<RandomAccessFile> read_file;
     ASSERT_OK(env->NewRandomAccessFile(fname, &read_file, env_options));
     unique_ptr<RandomAccessFileReader> file_reader(
-        new RandomAccessFileReader(std::move(read_file)));
+        new RandomAccessFileReader(std::move(read_file), fname));
     const ImmutableCFOptions ioptions(options);
     CuckooTableReader reader(ioptions, std::move(file_reader), file_size, ucomp,
                              GetSliceHash);
@@ -144,7 +144,7 @@ class CuckooReaderTest : public testing::Test {
     std::unique_ptr<RandomAccessFile> read_file;
     ASSERT_OK(env->NewRandomAccessFile(fname, &read_file, env_options));
     unique_ptr<RandomAccessFileReader> file_reader(
-        new RandomAccessFileReader(std::move(read_file)));
+        new RandomAccessFileReader(std::move(read_file), fname));
     const ImmutableCFOptions ioptions(options);
     CuckooTableReader reader(ioptions, std::move(file_reader), file_size, ucomp,
                              GetSliceHash);
@@ -322,7 +322,7 @@ TEST_F(CuckooReaderTest, WhenKeyNotFound) {
   std::unique_ptr<RandomAccessFile> read_file;
   ASSERT_OK(env->NewRandomAccessFile(fname, &read_file, env_options));
   unique_ptr<RandomAccessFileReader> file_reader(
-      new RandomAccessFileReader(std::move(read_file)));
+      new RandomAccessFileReader(std::move(read_file), fname));
   const ImmutableCFOptions ioptions(options);
   CuckooTableReader reader(ioptions, std::move(file_reader), file_size, ucmp,
                            GetSliceHash);
@@ -428,7 +428,7 @@ void WriteFile(const std::vector<std::string>& keys,
   std::unique_ptr<RandomAccessFile> read_file;
   ASSERT_OK(env->NewRandomAccessFile(fname, &read_file, env_options));
   unique_ptr<RandomAccessFileReader> file_reader(
-      new RandomAccessFileReader(std::move(read_file)));
+      new RandomAccessFileReader(std::move(read_file), fname));
 
   const ImmutableCFOptions ioptions(options);
   CuckooTableReader reader(ioptions, std::move(file_reader), file_size,
@@ -460,7 +460,7 @@ void ReadKeys(uint64_t num, uint32_t batch_size) {
   std::unique_ptr<RandomAccessFile> read_file;
   ASSERT_OK(env->NewRandomAccessFile(fname, &read_file, env_options));
   unique_ptr<RandomAccessFileReader> file_reader(
-      new RandomAccessFileReader(std::move(read_file)));
+      new RandomAccessFileReader(std::move(read_file), fname));
 
   const ImmutableCFOptions ioptions(options);
   CuckooTableReader reader(ioptions, std::move(file_reader), file_size,

table/format.cc

@@ -221,7 +221,9 @@ std::string Footer::ToString() const {
 Status ReadFooterFromFile(RandomAccessFileReader* file, uint64_t file_size,
                           Footer* footer, uint64_t enforce_table_magic_number) {
   if (file_size < Footer::kMinEncodedLength) {
-    return Status::Corruption("file is too short to be an sstable");
+    return Status::Corruption(
+      "file is too short (" + ToString(file_size) + " bytes) to be an "
+      "sstable: " + file->file_name());
   }
 
   char footer_space[Footer::kMaxEncodedLength];
@@ -237,7 +239,9 @@ Status ReadFooterFromFile(RandomAccessFileReader* file, uint64_t file_size,
   // Check that we actually read the whole footer from the file. It may be
   // that size isn't correct.
   if (footer_input.size() < Footer::kMinEncodedLength) {
-    return Status::Corruption("file is too short to be an sstable");
+    return Status::Corruption(
+      "file is too short (" + ToString(file_size) + " bytes) to be an "
+      "sstable" + file->file_name());
   }
 
   s = footer->DecodeFrom(&footer_input);
@@ -246,7 +250,11 @@ Status ReadFooterFromFile(RandomAccessFileReader* file, uint64_t file_size,
   }
   if (enforce_table_magic_number != 0 &&
       enforce_table_magic_number != footer->table_magic_number()) {
-    return Status::Corruption("Bad table magic number");
+    return Status::Corruption(
+      "Bad table magic number: expected "
+      + ToString(enforce_table_magic_number) + ", found "
+      + ToString(footer->table_magic_number())
+      + " in " + file->file_name());
   }
   return Status::OK();
 }
@@ -275,7 +283,11 @@ Status ReadBlock(RandomAccessFileReader* file, const Footer& footer,
     return s;
   }
   if (contents->size() != n + kBlockTrailerSize) {
-    return Status::Corruption("truncated block read");
+    return Status::Corruption(
+      "truncated block read from " + file->file_name() + " offset "
+      + ToString(handle.offset()) + ", expected "
+      + ToString(n + kBlockTrailerSize) + " bytes, got "
+      + ToString(contents->size()));
   }
 
   // Check the crc of the type and the block contents
@@ -293,10 +305,17 @@ Status ReadBlock(RandomAccessFileReader* file, const Footer& footer,
         actual = XXH32(data, static_cast<int>(n) + 1, 0);
         break;
       default:
-        s = Status::Corruption("unknown checksum type");
+        s = Status::Corruption(
+          "unknown checksum type " + ToString(footer.checksum())
+          + " in " + file->file_name() + " offset "
+          + ToString(handle.offset()) + " size " + ToString(n));
     }
     if (s.ok() && actual != value) {
-      s = Status::Corruption("block checksum mismatch");
+      s = Status::Corruption(
+        "block checksum mismatch: expected " + ToString(actual)
+        + ", got " + ToString(value) + "  in " + file->file_name()
+        + " offset " + ToString(handle.offset())
+        + " size " + ToString(n));
     }
     if (!s.ok()) {
       return s;

table/table_reader_bench.cc

@@ -139,7 +139,7 @@ void TableReaderBenchmark(Options& opts, EnvOptions& env_options,
     uint64_t file_size;
     env->GetFileSize(file_name, &file_size);
     unique_ptr<RandomAccessFileReader> file_reader(
-        new RandomAccessFileReader(std::move(raf)));
+        new RandomAccessFileReader(std::move(raf), file_name));
     s = opts.table_factory->NewTableReader(
         TableReaderOptions(ioptions, env_options, ikc), std::move(file_reader),
         file_size, &table_reader);

tools/sst_dump_tool.cc

@@ -79,7 +79,7 @@ Status SstFileReader::GetTableReader(const std::string& file_path) {
     s = options_.env->GetFileSize(file_path, &file_size);
   }
 
-  file_.reset(new RandomAccessFileReader(std::move(file)));
+  file_.reset(new RandomAccessFileReader(std::move(file), file_path));
 
   if (s.ok()) {
     s = ReadFooterFromFile(file_.get(), file_size, &footer);
@@ -93,7 +93,7 @@ Status SstFileReader::GetTableReader(const std::string& file_path) {
         magic_number == kLegacyPlainTableMagicNumber) {
       soptions_.use_mmap_reads = true;
       options_.env->NewRandomAccessFile(file_path, &file, soptions_);
-      file_.reset(new RandomAccessFileReader(std::move(file)));
+      file_.reset(new RandomAccessFileReader(std::move(file), file_path));
     }
     options_.comparator = &internal_comparator_;
     // For old sst format, ReadTableProperties might fail but file can be read

util/file_reader_writer.h

@@ -59,6 +59,7 @@ class SequentialFileReader {
 class RandomAccessFileReader {
  private:
   std::unique_ptr<RandomAccessFile> file_;
+  std::string     file_name_;
   Env*            env_;
   Statistics*     stats_;
   uint32_t        hist_type_;
@@ -68,6 +69,7 @@ class RandomAccessFileReader {
 
  public:
   explicit RandomAccessFileReader(std::unique_ptr<RandomAccessFile>&& raf,
+                                  std::string _file_name,
                                   Env* env = nullptr,
                                   Statistics* stats = nullptr,
                                   uint32_t hist_type = 0,
@@ -75,6 +77,7 @@ class RandomAccessFileReader {
                                   RateLimiter* rate_limiter = nullptr,
                                   bool for_compaction = false)
       : file_(std::move(raf)),
+        file_name_(std::move(_file_name)),
         env_(env),
         stats_(stats),
         hist_type_(hist_type),
@@ -109,6 +112,8 @@ class RandomAccessFileReader {
 
   RandomAccessFile* file() { return file_.get(); }
 
+  std::string file_name() const { return file_name_; }
+
   bool use_direct_io() const { return file_->use_direct_io(); }
 };
 

util/testutil.cc

@@ -139,7 +139,8 @@ WritableFileWriter* GetWritableFileWriter(WritableFile* wf) {
 
 RandomAccessFileReader* GetRandomAccessFileReader(RandomAccessFile* raf) {
   unique_ptr<RandomAccessFile> file(raf);
-  return new RandomAccessFileReader(std::move(file));
+  return new RandomAccessFileReader(std::move(file),
+                                    "[test RandomAccessFileReader]");
 }
 
 SequentialFileReader* GetSequentialFileReader(SequentialFile* se) {

utilities/blob_db/blob_dump_tool.cc

@@ -50,7 +50,7 @@ Status BlobDumpTool::Run(const std::string& filename, DisplayType show_key,
   if (file_size == 0) {
     return Status::Corruption("File is empty.");
   }
-  reader_.reset(new RandomAccessFileReader(std::move(file)));
+  reader_.reset(new RandomAccessFileReader(std::move(file), filename));
   uint64_t offset = 0;
   uint64_t footer_offset = 0;
   s = DumpBlobLogHeader(&offset);

utilities/blob_db/blob_file.cc

@@ -212,7 +212,8 @@ std::shared_ptr<RandomAccessFileReader> BlobFile::GetOrOpenRandomAccessReader(
     return nullptr;
   }
 
-  ra_file_reader_ = std::make_shared<RandomAccessFileReader>(std::move(rfile));
+  ra_file_reader_ = std::make_shared<RandomAccessFileReader>(std::move(rfile),
+                                                             PathName());
   *fresh_open = true;
   return ra_file_reader_;
 }

utilities/column_aware_encoding_util.cc

@@ -49,7 +49,7 @@ void ColumnAwareEncodingReader::InitTableReader(const std::string& file_path) {
   options_.env->NewRandomAccessFile(file_path, &file, soptions_);
   options_.env->GetFileSize(file_path, &file_size);
 
-  file_.reset(new RandomAccessFileReader(std::move(file)));
+  file_.reset(new RandomAccessFileReader(std::move(file), file_path));
 
   options_.comparator = &internal_comparator_;
   options_.table_factory = std::make_shared<BlockBasedTableFactory>();

utilities/persistent_cache/block_cache_tier_file.cc

@@ -214,7 +214,7 @@ bool RandomAccessCacheFile::OpenImpl(const bool enable_direct_reads) {
           status.ToString().c_str());
     return false;
   }
-  freader_.reset(new RandomAccessFileReader(std::move(file), env_));
+  freader_.reset(new RandomAccessFileReader(std::move(file), Path(), env_));
 
   return true;
 }