Adjust value generation in batched ops stress tests (#10872)

Summary:
The patch adjusts the generation of values in batched ops stress tests so that the digits 0..9 are appended (instead of prepended) to the values written. This has the advantage of aligning the encoding of the "value base" into the value string across non-batched, batched, and CF consistency stress tests.

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

Test Plan: Tested using some black box stress test runs.

Reviewed By: riversand963

Differential Revision: D40692847

Pulled By: ltamasi

fbshipit-source-id: 26bf8adff2944cbe416665f09c3bab89d80416b3

db_stress_tool/batched_ops_stress.cc

@@ -18,8 +18,8 @@ class BatchedOpsStressTest : public StressTest {
 
   bool IsStateTracked() const override { return false; }
 
-  // Given a key K and value V, this puts ("0"+K, "0"+V), ("1"+K, "1"+V), ...
+  // Given a key K and value V, this puts ("0"+K, V+"0"), ("1"+K, V+"1"), ...,
-  // ("9"+K, "9"+V) in DB atomically i.e in a single batch.
+  // ("9"+K, V+"9") in DB atomically i.e in a single batch.
   // Also refer BatchedOpsStressTest::TestGet
   Status TestPut(ThreadState* thread, WriteOptions& write_opts,
                  const ReadOptions& /* read_opts */,
@@ -29,12 +29,12 @@ class BatchedOpsStressTest : public StressTest {
     assert(!rand_column_families.empty());
     assert(!rand_keys.empty());
 
-    const std::string key_suffix = Key(rand_keys[0]);
+    const std::string key_body = Key(rand_keys[0]);
 
     const uint32_t value_base =
         thread->rand.Next() % thread->shared->UNKNOWN_SENTINEL;
     const size_t sz = GenerateValue(value_base, value, sizeof(value));
-    const std::string value_suffix = Slice(value, sz).ToString();
+    const std::string value_body = Slice(value, sz).ToString();
 
     WriteBatch batch(0 /* reserved_bytes */, 0 /* max_bytes */,
                      FLAGS_batch_protection_bytes_per_key,
@@ -44,10 +44,14 @@ class BatchedOpsStressTest : public StressTest {
     assert(cfh);
 
     for (int i = 9; i >= 0; --i) {
-      const std::string prefix = std::to_string(i);
+      const std::string num = std::to_string(i);
 
-      const std::string k = prefix + key_suffix;
+      // Note: the digit in num is prepended to the key; however, it is appended
-      const std::string v = prefix + value_suffix;
+      // to the value because we want the "value base" to be encoded uniformly
+      // at the beginning of the value for all types of stress tests (e.g.
+      // batched, non-batched, CF consistency).
+      const std::string k = num + key_body;
+      const std::string v = value_body + num;
 
       if (FLAGS_use_merge) {
         batch.Merge(cfh, k, v);
@@ -72,7 +76,7 @@ class BatchedOpsStressTest : public StressTest {
     return s;
   }
 
-  // Given a key K, this deletes ("0"+K), ("1"+K),... ("9"+K)
+  // Given a key K, this deletes ("0"+K), ("1"+K), ..., ("9"+K)
   // in DB atomically i.e in a single batch. Also refer MultiGet.
   Status TestDelete(ThreadState* thread, WriteOptions& writeoptions,
                     const std::vector<int>& rand_column_families,
@@ -122,9 +126,9 @@ class BatchedOpsStressTest : public StressTest {
     std::terminate();
   }
 
-  // Given a key K, this gets values for "0"+K, "1"+K,..."9"+K
+  // Given a key K, this gets values for "0"+K, "1"+K, ..., "9"+K
   // in the same snapshot, and verifies that all the values are of the form
-  // "0"+V, "1"+V,..."9"+V.
+  // V+"0", V+"1", ..., V+"9".
   // ASSUMES that BatchedOpsStressTest::TestPut was used to put (K, V) into
   // the DB.
   Status TestGet(ThreadState* thread, const ReadOptions& readoptions,
@@ -156,13 +160,19 @@ class BatchedOpsStressTest : public StressTest {
       } else {
         values[i] = from_db;
 
-        char expected_prefix = (keys[i])[0];
+        assert(!keys[i].empty());
-        char actual_prefix = (values[i])[0];
+        assert(!values[i].empty());
-        if (actual_prefix != expected_prefix) {
+
-          fprintf(stderr, "error expected prefix = %c actual = %c\n",
+        const char expected = keys[i].front();
-                  expected_prefix, actual_prefix);
+        const char actual = values[i].back();
+
+        if (expected != actual) {
+          fprintf(stderr, "get error expected = %c actual = %c\n", expected,
+                  actual);
         }
-        (values[i])[0] = ' ';  // blank out the differing character
+
+        values[i].pop_back();  // get rid of the differing character
+
         thread->stats.AddGets(1, 1);
       }
     }
@@ -171,7 +181,7 @@ class BatchedOpsStressTest : public StressTest {
     // Now that we retrieved all values, check that they all match
     for (int i = 1; i < 10; i++) {
       if (values[i] != values[0]) {
-        fprintf(stderr, "error : inconsistent values for key %s: %s, %s\n",
+        fprintf(stderr, "get error: inconsistent values for key %s: %s, %s\n",
                 key.ToString(true).c_str(), StringToHex(values[0]).c_str(),
                 StringToHex(values[i]).c_str());
         // we continue after error rather than exiting so that we can
@@ -214,7 +224,7 @@ class BatchedOpsStressTest : public StressTest {
       for (size_t i = 0; i < num_prefixes; i++) {
         Status s = statuses[i];
         if (!s.ok() && !s.IsNotFound()) {
-          fprintf(stderr, "get error: %s\n", s.ToString().c_str());
+          fprintf(stderr, "multiget error: %s\n", s.ToString().c_str());
           thread->stats.AddErrors(1);
           ret_status[rand_key] = s;
           // we continue after error rather than exiting so that we can
@@ -223,17 +233,19 @@ class BatchedOpsStressTest : public StressTest {
           thread->stats.AddGets(1, 0);
           ret_status[rand_key] = s;
         } else {
-          char expected_prefix = (keys[i])[0];
+          assert(!keys[i].empty());
-          char actual_prefix = (values[i])[0];
+          assert(!values[i].empty());
-          if (actual_prefix != expected_prefix) {
+
-            fprintf(stderr, "error expected prefix = %c actual = %c\n",
+          const char expected = keys[i][0];
-                    expected_prefix, actual_prefix);
+          const char actual = values[i][values[i].size() - 1];
-          }
+
-          std::string str;
+          if (expected != actual) {
-          str.assign(values[i].data(), values[i].size());
+            fprintf(stderr, "multiget error expected = %c actual = %c\n",
-          values[i].Reset();
+                    expected, actual);
-          str[0] = ' ';  // blank out the differing character
+          }
-          values[i].PinSelf(str);
+
+          values[i].remove_suffix(1);  // get rid of the differing character
+
           thread->stats.AddGets(1, 1);
         }
       }
@@ -242,7 +254,8 @@ class BatchedOpsStressTest : public StressTest {
       // Now that we retrieved all values, check that they all match
       for (size_t i = 1; i < num_prefixes; i++) {
         if (values[i] != values[0]) {
-          fprintf(stderr, "error : inconsistent values for key %s: %s, %s\n",
+          fprintf(stderr,
+                  "multiget error: inconsistent values for key %s: %s, %s\n",
                   StringToHex(key_str[i]).c_str(),
                   StringToHex(values[0].ToString()).c_str(),
                   StringToHex(values[i].ToString()).c_str());
@@ -255,11 +268,11 @@ class BatchedOpsStressTest : public StressTest {
     return ret_status;
   }
 
-  // Given a key, this does prefix scans for "0"+P, "1"+P,..."9"+P
+  // Given a key, this does prefix scans for "0"+P, "1"+P, ..., "9"+P
   // in the same snapshot where P is the first FLAGS_prefix_size - 1 bytes
   // of the key. Each of these 10 scans returns a series of values;
   // each series should be the same length, and it is verified for each
-  // index i that all the i'th values are of the form "0"+V, "1"+V,..."9"+V.
+  // index i that all the i'th values are of the form V+"0", V+"1", ..., V+"9".
   // ASSUMES that MultiPut was used to put (K, V)
   Status TestPrefixScan(ThreadState* thread, const ReadOptions& readoptions,
                         const std::vector<int>& rand_column_families,
@@ -317,21 +330,24 @@ class BatchedOpsStressTest : public StressTest {
                iters[i]->key().starts_with(prefix_slices[i]));
         values[i] = iters[i]->value().ToString();
 
-        // make sure the first character of the value is the expected digit
+        // make sure the last character of the value is the expected digit
-        const char expected_first = prefixes[i][0];
+        assert(!prefixes[i].empty());
-        const char actual_first = values[i][0];
+        assert(!values[i].empty());
+
+        const char expected = prefixes[i].front();
+        const char actual = values[i].back();
 
-        if (actual_first != expected_first) {
+        if (expected != actual) {
-          fprintf(stderr, "error expected first = %c actual = %c\n",
+          fprintf(stderr, "prefix scan error expected = %c actual = %c\n",
-                  expected_first, actual_first);
+                  expected, actual);
         }
 
-        values[i][0] = ' ';  // blank out the differing character
+        values[i].pop_back();  // get rid of the differing character
 
         // make sure all values are equivalent
         if (values[i] != values[0]) {
           fprintf(stderr,
-                  "error : %" ROCKSDB_PRIszt
+                  "prefix scan error : %" ROCKSDB_PRIszt
                   ", inconsistent values for prefix %s: %s, %s\n",
                   i, prefix_slices[i].ToString(/* hex */ true).c_str(),
                   StringToHex(values[0]).c_str(),
@@ -341,16 +357,11 @@ class BatchedOpsStressTest : public StressTest {
         }
 
         // make sure value() and columns() are consistent
-        // note: in these tests, value base is stored after a single-digit
-        // prefix
-        Slice value_base_slice = iters[i]->value();
-        value_base_slice.remove_prefix(1);
-
         const WideColumns expected_columns = GenerateExpectedWideColumns(
-            GetValueBase(value_base_slice), iters[i]->value());
+            GetValueBase(iters[i]->value()), iters[i]->value());
         if (iters[i]->columns() != expected_columns) {
           fprintf(stderr,
-                  "error : %" ROCKSDB_PRIszt
+                  "prefix scan error : %" ROCKSDB_PRIszt
                   ", value and columns inconsistent for prefix %s: %s\n",
                   i, prefix_slices[i].ToString(/* hex */ true).c_str(),
                   DebugString(iters[i]->value(), iters[i]->columns(),
@@ -377,11 +388,6 @@ class BatchedOpsStressTest : public StressTest {
     return Status::OK();
   }
 
-  Slice GetValueBaseSlice(Slice slice) override {
-    slice.remove_prefix(1);
-    return slice;
-  }
-
   void VerifyDb(ThreadState* /* thread */) const override {}
 
   void ContinuouslyVerifyDb(ThreadState* /* thread */) const override {}

db_stress_tool/db_stress_test_base.cc

@@ -1487,10 +1487,8 @@ void StressTest::VerifyIterator(ThreadState* thread,
   }
 
   if (!*diverged && iter->Valid()) {
-    const Slice value_base_slice = GetValueBaseSlice(iter->value());
+    const WideColumns expected_columns =
-
+        GenerateExpectedWideColumns(GetValueBase(iter->value()), iter->value());
-    const WideColumns expected_columns = GenerateExpectedWideColumns(
-        GetValueBase(value_base_slice), iter->value());
     if (iter->columns() != expected_columns) {
       fprintf(stderr, "Value and columns inconsistent for iterator: %s\n",
               DebugString(iter->value(), iter->columns(), expected_columns)

db_stress_tool/db_stress_test_base.h

@@ -139,8 +139,6 @@ class StressTest {
     return column_families_[column_family_id];
   }
 
-  virtual Slice GetValueBaseSlice(Slice slice) { return slice; }
-
 #ifndef ROCKSDB_LITE
   // Generated a list of keys that close to boundaries of SST keys.
   // If there isn't any SST file in the DB, return empty list.