From d484275230995e62b3b24a64d357f66fd509db19 Mon Sep 17 00:00:00 2001 From: Levi Tamasi Date: Tue, 25 Oct 2022 17:51:20 -0700 Subject: [PATCH] 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 | 108 ++++++++++++++------------ db_stress_tool/db_stress_test_base.cc | 6 +- db_stress_tool/db_stress_test_base.h | 2 - 3 files changed, 59 insertions(+), 57 deletions(-) diff --git a/db_stress_tool/batched_ops_stress.cc b/db_stress_tool/batched_ops_stress.cc index e98a546d3..3f3446076 100644 --- a/db_stress_tool/batched_ops_stress.cc +++ b/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), ... - // ("9"+K, "9"+V) in DB atomically i.e in a single batch. + // Given a key K and value V, this puts ("0"+K, V+"0"), ("1"+K, V+"1"), ..., + // ("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; - const std::string v = prefix + value_suffix; + // Note: the digit in num is prepended to the key; however, it is appended + // 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& 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]; - char actual_prefix = (values[i])[0]; - if (actual_prefix != expected_prefix) { - fprintf(stderr, "error expected prefix = %c actual = %c\n", - expected_prefix, actual_prefix); + assert(!keys[i].empty()); + assert(!values[i].empty()); + + const char expected = keys[i].front(); + 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]; - char actual_prefix = (values[i])[0]; - if (actual_prefix != expected_prefix) { - fprintf(stderr, "error expected prefix = %c actual = %c\n", - expected_prefix, actual_prefix); + assert(!keys[i].empty()); + assert(!values[i].empty()); + + const char expected = keys[i][0]; + const char actual = values[i][values[i].size() - 1]; + + if (expected != actual) { + fprintf(stderr, "multiget error expected = %c actual = %c\n", + expected, actual); } - std::string str; - str.assign(values[i].data(), values[i].size()); - values[i].Reset(); - 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& 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 - const char expected_first = prefixes[i][0]; - const char actual_first = values[i][0]; + // make sure the last character of the value is the expected digit + assert(!prefixes[i].empty()); + assert(!values[i].empty()); + + const char expected = prefixes[i].front(); + const char actual = values[i].back(); - if (actual_first != expected_first) { - fprintf(stderr, "error expected first = %c actual = %c\n", - expected_first, actual_first); + if (expected != actual) { + fprintf(stderr, "prefix scan 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 // 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 {} diff --git a/db_stress_tool/db_stress_test_base.cc b/db_stress_tool/db_stress_test_base.cc index ea045cd7a..5f844edad 100644 --- a/db_stress_tool/db_stress_test_base.cc +++ b/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(value_base_slice), iter->value()); + const WideColumns expected_columns = + GenerateExpectedWideColumns(GetValueBase(iter->value()), iter->value()); if (iter->columns() != expected_columns) { fprintf(stderr, "Value and columns inconsistent for iterator: %s\n", DebugString(iter->value(), iter->columns(), expected_columns) diff --git a/db_stress_tool/db_stress_test_base.h b/db_stress_tool/db_stress_test_base.h index 4ec08dee8..81fbbe24b 100644 --- a/db_stress_tool/db_stress_test_base.h +++ b/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.