db_stress: exclude info logs from metadata read faults (#14616)

Summary:
- fix a false positive in `db_stress` PrefixScan fault accounting caused by info-log `FileExists()` probes going through metadata-read fault injection
- exclude `kInfoLogFile` from metadata-read injection and extend `FaultInjectionTestFS` filename parsing so LOG/LOG.old.* and `db_log_dir` info-log paths are recognized consistently
- add regression coverage showing info-log `FileExists()` bypasses metadata-read injection while manifest lookups still inject

## Testing
- make format-auto
- make -j48 fault_injection_fs_test db_stress
- timeout 60 ./fault_injection_fs_test
- timeout 60 ./fault_injection_fs_test --gtest_filter='*MetadataReadFaultExcludesInfoLogFiles*' --gtest_repeat=5

Pull Request resolved: https://github.com/facebook/rocksdb/pull/14616

Reviewed By: hx235

Differential Revision: D100838168

Pulled By: xingbowang

fbshipit-source-id: 9ef996908b393aea2a8221fcdda67ed609643dbe

db_stress_tool/db_stress_tool.cc

@@ -93,6 +93,10 @@ int db_stress_tool(int argc, char** argv) {
     FaultInjectionTestFS* fs =
         new FaultInjectionTestFS(raw_env->GetFileSystem());
     fault_fs_guard.reset(fs);
+    // Info logs are debugging artifacts, so exclude them from fault injection
+    // and keep error accounting focused on DB data and metadata.
+    fault_fs_guard->SetFileTypesExcludedFromFaultInjection(
+        {FileType::kInfoLogFile});
     // Set it to direct writable here to initially bypass any fault injection
     // during DB open This will correspondingly be overwritten in
     // StressTest::Open() for open fault injection and in RunStressTestImpl()

utilities/fault_injection_fs.cc

@@ -39,6 +39,35 @@ namespace ROCKSDB_NAMESPACE {
 
 const std::string kNewFileNoOverwrite;
 
+namespace {
+
+bool TryParseInfoLogFileName(const std::string& file_name, uint64_t* number,
+                             FileType* type) {
+  size_t prefix_len = std::string::npos;
+  if (file_name == "LOG" ||
+      (file_name.size() > 3 && file_name.compare(0, 7, "LOG.old") == 0)) {
+    prefix_len = sizeof("LOG") - 1;
+  } else {
+    size_t info_log_pos = file_name.rfind("_LOG");
+    if (info_log_pos != std::string::npos) {
+      size_t suffix_pos = info_log_pos + sizeof("_LOG") - 1;
+      if (suffix_pos == file_name.size() ||
+          (suffix_pos < file_name.size() &&
+           file_name.compare(suffix_pos, 4, ".old") == 0)) {
+        prefix_len = suffix_pos;
+      }
+    }
+  }
+  if (prefix_len == std::string::npos) {
+    return false;
+  }
+  Slice info_log_name_prefix(file_name.data(), prefix_len);
+  return ParseFileName(file_name, number, info_log_name_prefix, type) &&
+         *type == kInfoLogFile;
+}
+
+}  // namespace
+
 // Assume a filename, and not a directory name like "/foo/bar/"
 std::string TestFSGetDirName(const std::string filename) {
   size_t found = filename.find_last_of("/\\");
@@ -1422,7 +1451,8 @@ IOStatus FaultInjectionTestFS::MaybeInjectThreadLocalReadError(
   ErrorContext* ctx =
       static_cast<ErrorContext*>(injected_thread_local_read_error_.Get());
   if (ctx == nullptr || !ctx->enable_error_injection || !ctx->one_in ||
-      ShouldIOActivitiesExcludedFromFaultInjection(io_options.io_activity)) {
+      ShouldIOActivitiesExcludedFromFaultInjection(io_options.io_activity) ||
+      ShouldExcludeFromFaultInjection(file_name, FaultInjectionIOType::kRead)) {
     return IOStatus::OK();
   }
 
@@ -1501,9 +1531,13 @@ IOStatus FaultInjectionTestFS::MaybeInjectThreadLocalReadError(
 
 bool FaultInjectionTestFS::TryParseFileName(const std::string& file_name,
                                             uint64_t* number, FileType* type) {
-  std::size_t found = file_name.find_last_of('/');
-  std::string file = file_name.substr(found);
-  return ParseFileName(file, number, type);
+  std::size_t found = file_name.find_last_of("/\\");
+  std::string file =
+      found == std::string::npos ? file_name : file_name.substr(found + 1);
+  if (ParseFileName(file, number, type)) {
+    return true;
+  }
+  return TryParseInfoLogFileName(file, number, type);
 }
 
 IOStatus FaultInjectionTestFS::MaybeInjectThreadLocalError(
@@ -1520,8 +1554,7 @@ IOStatus FaultInjectionTestFS::MaybeInjectThreadLocalError(
   ErrorContext* ctx = GetErrorContextFromFaultInjectionIOType(type);
   if (ctx == nullptr || !ctx->enable_error_injection || !ctx->one_in ||
       ShouldIOActivitiesExcludedFromFaultInjection(io_options.io_activity) ||
-      (type == FaultInjectionIOType::kWrite &&
-       ShouldExcludeFromWriteFaultInjection(file_name))) {
+      ShouldExcludeFromFaultInjection(file_name, type)) {
     return IOStatus::OK();
   }
 

utilities/fault_injection_fs.h

@@ -821,6 +821,11 @@ class FaultInjectionTestFS : public FileSystemWrapper {
     file_types_excluded_from_write_fault_injection_ = types;
   }
 
+  void SetFileTypesExcludedFromFaultInjection(const std::set<FileType>& types) {
+    MutexLock l(&mutex_);
+    file_types_excluded_from_fault_injection_ = types;
+  }
+
   void EnableThreadLocalErrorInjection(FaultInjectionIOType type) {
     ErrorContext* ctx = GetErrorContextFromFaultInjectionIOType(type);
     if (ctx) {
@@ -931,6 +936,7 @@ class FaultInjectionTestFS : public FileSystemWrapper {
     }
   };
 
+  std::set<FileType> file_types_excluded_from_fault_injection_;
   std::set<FileType> file_types_excluded_from_write_fault_injection_;
   std::set<Env::IOActivity> io_activities_excluded_from_fault_injection;
   ThreadLocalPtr injected_thread_local_read_error_;
@@ -956,15 +962,27 @@ class FaultInjectionTestFS : public FileSystemWrapper {
       ErrorOperation op, Slice* slice, bool direct_io, char* scratch,
       bool need_count_increase, bool* fault_injected);
 
-  bool ShouldExcludeFromWriteFaultInjection(const std::string& file_name) {
+  bool ShouldExcludeFromFaultInjection(const std::string& file_name,
+                                       FaultInjectionIOType type) {
     MutexLock l(&mutex_);
     FileType file_type = kTempFile;
     uint64_t file_number = 0;
     if (!TryParseFileName(file_name, &file_number, &file_type)) {
       return false;
     }
-    return file_types_excluded_from_write_fault_injection_.find(file_type) !=
-           file_types_excluded_from_write_fault_injection_.end();
+    if (file_types_excluded_from_fault_injection_.count(file_type) > 0) {
+      return true;
+    }
+    switch (type) {
+      case FaultInjectionIOType::kWrite:
+        return file_types_excluded_from_write_fault_injection_.count(
+                   file_type) > 0;
+      case FaultInjectionIOType::kRead:
+      case FaultInjectionIOType::kMetadataRead:
+      case FaultInjectionIOType::kMetadataWrite:
+        return false;
+    }
+    return false;
   }
 
   // Extract number of type from file name. Return false if failing to fine

utilities/fault_injection_fs_test.cc

@@ -14,6 +14,22 @@
 namespace ROCKSDB_NAMESPACE {
 
 class InjectedErrorLogTest : public testing::Test {};
+class FaultInjectionTestFSTest : public testing::Test {};
+
+namespace {
+
+std::shared_ptr<FaultInjectionTestFS> NewFaultFsExcludingInfoLogs(
+    Env* env, FaultInjectionIOType type) {
+  auto fault_fs = std::make_shared<FaultInjectionTestFS>(env->GetFileSystem());
+  fault_fs->SetFileTypesExcludedFromFaultInjection({FileType::kInfoLogFile});
+  fault_fs->SetThreadLocalErrorContext(type, /*seed=*/0, /*one_in=*/1,
+                                       /*retryable=*/false,
+                                       /*has_data_loss=*/false);
+  fault_fs->EnableThreadLocalErrorInjection(type);
+  return fault_fs;
+}
+
+}  // namespace
 
 // Test basic Record and PrintAll functionality.
 TEST_F(InjectedErrorLogTest, BasicRecordAndPrint) {
@@ -85,6 +101,101 @@ TEST_F(InjectedErrorLogTest, HexHead) {
   ASSERT_EQ(result, "01 02 ...");
 }
 
+TEST_F(FaultInjectionTestFSTest, FaultInjectionExcludesInfoLogFiles) {
+  Env* env = Env::Default();
+  const std::string dbname =
+      test::PerThreadDBPath("fault_injection_fs_test_metadata_read");
+  const std::string log_dir = dbname + "_logs";
+  ASSERT_OK(env->CreateDirIfMissing(dbname));
+  ASSERT_OK(env->CreateDirIfMissing(log_dir));
+
+  const std::string current_info_log = InfoLogFileName(dbname, dbname, log_dir);
+  const std::string old_info_log =
+      OldInfoLogFileName(dbname, 123, dbname, log_dir);
+  const std::string manifest = DescriptorFileName(dbname, 1);
+  const std::string manifest_for_write = DescriptorFileName(dbname, 2);
+  const std::string manifest_for_delete = DescriptorFileName(dbname, 3);
+  ASSERT_OK(
+      WriteStringToFile(env, "old log", old_info_log, false /* should_sync */));
+  ASSERT_OK(
+      WriteStringToFile(env, "manifest", manifest, false /* should_sync */));
+  ASSERT_OK(WriteStringToFile(env, "manifest delete", manifest_for_delete,
+                              false /* should_sync */));
+
+  {
+    auto fault_fs =
+        NewFaultFsExcludingInfoLogs(env, FaultInjectionIOType::kMetadataRead);
+
+    ASSERT_OK(fault_fs->FileExists(old_info_log, IOOptions(), nullptr));
+    ASSERT_EQ(0, fault_fs->GetAndResetInjectedThreadLocalErrorCount(
+                     FaultInjectionIOType::kMetadataRead));
+
+    IOStatus s = fault_fs->FileExists(manifest, IOOptions(), nullptr);
+    ASSERT_NOK(s);
+    ASSERT_TRUE(s.IsIOError()) << s.ToString();
+    ASSERT_EQ(1, fault_fs->GetAndResetInjectedThreadLocalErrorCount(
+                     FaultInjectionIOType::kMetadataRead));
+  }
+
+  {
+    auto fault_fs =
+        NewFaultFsExcludingInfoLogs(env, FaultInjectionIOType::kRead);
+    std::unique_ptr<FSSequentialFile> seq_file;
+    ASSERT_OK(fault_fs->NewSequentialFile(old_info_log, FileOptions(),
+                                          &seq_file, nullptr /* dbg */));
+    char scratch[16];
+    Slice result;
+    ASSERT_OK(seq_file->Read(sizeof(scratch), IOOptions(), &result, scratch,
+                             nullptr /* dbg */));
+    ASSERT_EQ("old log", result.ToString());
+    ASSERT_EQ(0, fault_fs->GetAndResetInjectedThreadLocalErrorCount(
+                     FaultInjectionIOType::kRead));
+
+    std::unique_ptr<FSSequentialFile> manifest_seq_file;
+    IOStatus s = fault_fs->NewSequentialFile(
+        manifest, FileOptions(), &manifest_seq_file, nullptr /* dbg */);
+    ASSERT_NOK(s);
+    ASSERT_TRUE(s.IsIOError()) << s.ToString();
+    ASSERT_EQ(1, fault_fs->GetAndResetInjectedThreadLocalErrorCount(
+                     FaultInjectionIOType::kRead));
+  }
+
+  {
+    auto fault_fs =
+        NewFaultFsExcludingInfoLogs(env, FaultInjectionIOType::kWrite);
+    std::unique_ptr<FSWritableFile> info_log_writer;
+    ASSERT_OK(fault_fs->NewWritableFile(current_info_log, FileOptions(),
+                                        &info_log_writer, nullptr /* dbg */));
+    ASSERT_OK(
+        info_log_writer->Append("current log", IOOptions(), nullptr /* dbg */));
+    ASSERT_EQ(0, fault_fs->GetAndResetInjectedThreadLocalErrorCount(
+                     FaultInjectionIOType::kWrite));
+
+    std::unique_ptr<FSWritableFile> manifest_writer;
+    IOStatus s = fault_fs->NewWritableFile(manifest_for_write, FileOptions(),
+                                           &manifest_writer, nullptr /* dbg */);
+    ASSERT_NOK(s);
+    ASSERT_TRUE(s.IsIOError()) << s.ToString();
+    ASSERT_EQ(1, fault_fs->GetAndResetInjectedThreadLocalErrorCount(
+                     FaultInjectionIOType::kWrite));
+  }
+
+  {
+    auto fault_fs =
+        NewFaultFsExcludingInfoLogs(env, FaultInjectionIOType::kMetadataWrite);
+    ASSERT_OK(fault_fs->DeleteFile(old_info_log, IOOptions(), nullptr));
+    ASSERT_EQ(0, fault_fs->GetAndResetInjectedThreadLocalErrorCount(
+                     FaultInjectionIOType::kMetadataWrite));
+
+    IOStatus s =
+        fault_fs->DeleteFile(manifest_for_delete, IOOptions(), nullptr);
+    ASSERT_NOK(s);
+    ASSERT_TRUE(s.IsIOError()) << s.ToString();
+    ASSERT_EQ(1, fault_fs->GetAndResetInjectedThreadLocalErrorCount(
+                     FaultInjectionIOType::kMetadataWrite));
+  }
+}
+
 }  // namespace ROCKSDB_NAMESPACE
 
 int main(int argc, char** argv) {