From 0534393fc889e6dc44eb69d89fd2385082b4bddb Mon Sep 17 00:00:00 2001
From: Peter Dillinger <peterd@fb.com>
Date: Tue, 19 Oct 2021 11:50:50 -0700
Subject: [PATCH] Fix stress/crash test handling of SST unique IDs (#9054)

Summary:
Was not handling the case of OnTableFileCreated invoked for
table file NOT created.

Also improved error reporting and caught a missing status check.

Also strengthened the db_stress listener to require file_size > 0 when
status.ok(). We would be violating the API contract if status is OK and
we didn't create a valid SST file.

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

Test Plan: make blackbox_crash_test for a while

Reviewed By: zhichao-cao

Differential Revision: D31765200

Pulled By: pdillinger

fbshipit-source-id: 7c527f5531bc239a5efd7a7b018545d480f926e2
---
 db_stress_tool/db_stress_listener.cc |  9 +++++++--
 db_stress_tool/db_stress_listener.h  | 17 +++++++++--------
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/db_stress_tool/db_stress_listener.cc b/db_stress_tool/db_stress_listener.cc
index b6fd5071c..9968f5465 100644
--- a/db_stress_tool/db_stress_listener.cc
+++ b/db_stress_tool/db_stress_listener.cc
@@ -123,10 +123,15 @@ void UniqueIdVerifier::Verify(const std::string& id) {
 }
 
 void DbStressListener::VerifyTableFileUniqueId(
-    const TableProperties& new_file_properties) {
+    const TableProperties& new_file_properties, const std::string& file_path) {
   // Verify unique ID
   std::string id;
-  GetUniqueIdFromTableProperties(new_file_properties, &id);
+  Status s = GetUniqueIdFromTableProperties(new_file_properties, &id);
+  if (!s.ok()) {
+    fprintf(stderr, "Error getting SST unique id for %s: %s\n",
+            file_path.c_str(), s.ToString().c_str());
+    assert(false);
+  }
   unique_ids_.Verify(id);
 }
 
diff --git a/db_stress_tool/db_stress_listener.h b/db_stress_tool/db_stress_listener.h
index 931e78afb..8ba7eda65 100644
--- a/db_stress_tool/db_stress_listener.h
+++ b/db_stress_tool/db_stress_listener.h
@@ -101,19 +101,17 @@ class DbStressListener : public EventListener {
   void OnTableFileCreated(const TableFileCreationInfo& info) override {
     assert(info.db_name == db_name_);
     assert(IsValidColumnFamilyName(info.cf_name));
-    if (info.file_size) {
-      VerifyFilePath(info.file_path);
-    }
     assert(info.job_id > 0 || FLAGS_compact_files_one_in > 0);
-    if (info.status.ok() && info.file_size > 0) {
+    if (info.status.ok()) {
+      assert(info.file_size > 0);
+      VerifyFilePath(info.file_path);
       assert(info.table_properties.data_size > 0 ||
              info.table_properties.num_range_deletions > 0);
       assert(info.table_properties.raw_key_size > 0);
       assert(info.table_properties.num_entries > 0);
+      VerifyTableFileUniqueId(info.table_properties, info.file_path);
     }
     --num_pending_file_creations_;
-
-    VerifyTableFileUniqueId(info.table_properties);
   }
 
   void OnMemTableSealed(const MemTableInfo& /*info*/) override {
@@ -130,7 +128,7 @@ class DbStressListener : public EventListener {
     RandomSleep();
     // Here we assume that each generated external file is ingested
     // exactly once (or thrown away in case of crash)
-    VerifyTableFileUniqueId(info.table_properties);
+    VerifyTableFileUniqueId(info.table_properties, info.internal_file_path);
   }
 
   void OnBackgroundError(BackgroundErrorReason /* reason */,
@@ -248,7 +246,10 @@ class DbStressListener : public EventListener {
 #endif  // !NDEBUG
   }
 
-  void VerifyTableFileUniqueId(const TableProperties& new_file_properties);
+  // Unique id is verified using the TableProperties. file_path is only used
+  // for reporting.
+  void VerifyTableFileUniqueId(const TableProperties& new_file_properties,
+                               const std::string& file_path);
 
   void RandomSleep() {
     std::this_thread::sleep_for(