From 1f0ccd9a151eb4ac1c968b08f6009559ffde2bd8 Mon Sep 17 00:00:00 2001
From: Levi Tamasi <ltamasi@meta.com>
Date: Sun, 10 Nov 2024 19:21:35 -0800
Subject: [PATCH] Fix the handling of PrepareValue failures due to fault
 injection (#13131)

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

The earlier stress test code did not consider that `PrepareValue()` could fail because of read fault injection, leading to false positives. The patch shuffles the `PrepareValue()` calls around a bit in `TestIterate` / `TestIterateAgainstExpected` in order to prevent this by leveraging the existing code paths that intercept injected faults.

Reviewed By: cbi42

Differential Revision: D65731543

fbshipit-source-id: b21c6584ebaa2ff41cd4569098680b91ff7991d1
---
 db_stress_tool/db_stress_test_base.cc   | 41 ++++++++++----------
 db_stress_tool/no_batched_ops_stress.cc | 51 +++++++++++++++----------
 2 files changed, 50 insertions(+), 42 deletions(-)

diff --git a/db_stress_tool/db_stress_test_base.cc b/db_stress_tool/db_stress_test_base.cc
index bfa0c16f2d..039ef2f055 100644
--- a/db_stress_tool/db_stress_test_base.cc
+++ b/db_stress_tool/db_stress_test_base.cc
@@ -1748,6 +1748,15 @@ Status StressTest::TestIterateImpl(ThreadState* thread,
       op_logs += "S " + key.ToString(true) + " ";
     }
 
+    if (iter->Valid() && ro.allow_unprepared_value) {
+      op_logs += "*";
+
+      if (!iter->PrepareValue()) {
+        assert(!iter->Valid());
+        assert(!iter->status().ok());
+      }
+    }
+
     if (!iter->status().ok() && IsErrorInjectedAndRetryable(iter->status())) {
       return iter->status();
     } else if (!cmp_iter->status().ok() &&
@@ -1779,6 +1788,15 @@ Status StressTest::TestIterateImpl(ThreadState* thread,
 
       last_op = kLastOpNextOrPrev;
 
+      if (iter->Valid() && ro.allow_unprepared_value) {
+        op_logs += "*";
+
+        if (!iter->PrepareValue()) {
+          assert(!iter->Valid());
+          assert(!iter->status().ok());
+        }
+      }
+
       if (!iter->status().ok() && IsErrorInjectedAndRetryable(iter->status())) {
         return iter->status();
       } else if (!cmp_iter->status().ok() &&
@@ -2000,27 +2018,8 @@ void StressTest::VerifyIterator(
   }
 
   if (!*diverged && iter->Valid()) {
-    if (ro.allow_unprepared_value) {
-      // Save key in case PrepareValue fails and invalidates the iterator
-      const std::string prepare_value_key =
-          iter->key().ToString(/* hex */ true);
-
-      if (!iter->PrepareValue()) {
-        fprintf(
-            stderr,
-            "Iterator failed to prepare value for key %s %s under specified "
-            "iterator ReadOptions: %s (Empty string or missing field indicates "
-            "default option or value is used): %s\n",
-            prepare_value_key.c_str(), op_logs.c_str(),
-            read_opt_oss.str().c_str(), iter->status().ToString().c_str());
-        *diverged = true;
-      }
-    }
-
-    if (!*diverged && iter->Valid()) {
-      if (!verify_func(iter)) {
-        *diverged = true;
-      }
+    if (!verify_func(iter)) {
+      *diverged = true;
     }
   }
 
diff --git a/db_stress_tool/no_batched_ops_stress.cc b/db_stress_tool/no_batched_ops_stress.cc
index ae2d3bcbd5..cd14bac68a 100644
--- a/db_stress_tool/no_batched_ops_stress.cc
+++ b/db_stress_tool/no_batched_ops_stress.cc
@@ -2300,27 +2300,6 @@ class NonBatchedOpsStressTest : public StressTest {
       assert(iter);
       assert(iter->Valid());
 
-      if (ro.allow_unprepared_value) {
-        // Save key in case PrepareValue fails and invalidates the iterator
-        const std::string prepare_value_key =
-            iter->key().ToString(/* hex */ true);
-
-        if (!iter->PrepareValue()) {
-          shared->SetVerificationFailure();
-
-          fprintf(
-              stderr,
-              "Verification failed for key %s: failed to prepare value: %s\n",
-              prepare_value_key.c_str(), iter->status().ToString().c_str());
-          fprintf(stderr, "Column family: %s, op_logs: %s\n",
-                  cfh->GetName().c_str(), op_logs.c_str());
-
-          thread->stats.AddErrors(1);
-
-          return false;
-        }
-      }
-
       if (!VerifyWideColumns(iter->value(), iter->columns())) {
         shared->SetVerificationFailure();
 
@@ -2389,6 +2368,16 @@ class NonBatchedOpsStressTest : public StressTest {
     uint64_t curr = 0;
     while (true) {
       assert(last_key < ub);
+
+      if (iter->Valid() && ro.allow_unprepared_value) {
+        op_logs += "*";
+
+        if (!iter->PrepareValue()) {
+          assert(!iter->Valid());
+          assert(!iter->status().ok());
+        }
+      }
+
       if (!iter->Valid()) {
         if (!iter->status().ok()) {
           if (IsErrorInjectedAndRetryable(iter->status())) {
@@ -2451,6 +2440,16 @@ class NonBatchedOpsStressTest : public StressTest {
     last_key = ub;
     while (true) {
       assert(lb < last_key);
+
+      if (iter->Valid() && ro.allow_unprepared_value) {
+        op_logs += "*";
+
+        if (!iter->PrepareValue()) {
+          assert(!iter->Valid());
+          assert(!iter->status().ok());
+        }
+      }
+
       if (!iter->Valid()) {
         if (!iter->status().ok()) {
           if (IsErrorInjectedAndRetryable(iter->status())) {
@@ -2589,6 +2588,16 @@ class NonBatchedOpsStressTest : public StressTest {
     }
 
     for (int64_t i = 0; i < num_iter && iter->Valid(); ++i) {
+      if (ro.allow_unprepared_value) {
+        op_logs += "*";
+
+        if (!iter->PrepareValue()) {
+          assert(!iter->Valid());
+          assert(!iter->status().ok());
+          break;
+        }
+      }
+
       if (!check_columns()) {
         return Status::OK();
       }