From 2280b2612a1be080d5219762e0ded53a2c2c974d Mon Sep 17 00:00:00 2001 From: Levi Tamasi Date: Mon, 26 Sep 2022 15:33:36 -0700 Subject: [PATCH] Small cleanup in NonBatchedOpsStressTest::VerifyDb (#10740) Summary: The PR cleans up the logic in `NonBatchedOpsStressTest::VerifyDb` so that the verification method is picked using a single random number generation. It also eliminates some repeated key comparisons and makes some small code hygiene improvements. Pull Request resolved: https://github.com/facebook/rocksdb/pull/10740 Test Plan: Ran a simple blackbox crash test. Reviewed By: riversand963 Differential Revision: D39828646 Pulled By: ltamasi fbshipit-source-id: 60ee5a3bb1851278f62c7d83b0c93b902ed9702e --- db_stress_tool/no_batched_ops_stress.cc | 136 ++++++++++++++++-------- 1 file changed, 91 insertions(+), 45 deletions(-) diff --git a/db_stress_tool/no_batched_ops_stress.cc b/db_stress_tool/no_batched_ops_stress.cc index 7249fb322..241fb322b 100644 --- a/db_stress_tool/no_batched_ops_stress.cc +++ b/db_stress_tool/no_batched_ops_stress.cc @@ -30,6 +30,7 @@ class NonBatchedOpsStressTest : public StressTest { ts = ts_str; options.timestamp = &ts; } + auto shared = thread->shared; const int64_t max_key = shared->GetMaxKey(); const int64_t keys_per_thread = max_key / shared->GetNumThreads(); @@ -37,44 +38,72 @@ class NonBatchedOpsStressTest : public StressTest { int64_t end = start + keys_per_thread; uint64_t prefix_to_use = (FLAGS_prefix_size < 0) ? 1 : static_cast(FLAGS_prefix_size); + if (thread->tid == shared->GetNumThreads() - 1) { end = max_key; } + for (size_t cf = 0; cf < column_families_.size(); ++cf) { if (thread->shared->HasVerificationFailedYet()) { break; } - if (thread->rand.OneIn(4)) { - // 1/4 chance use iterator to verify this range - Slice prefix; - std::string seek_key = Key(start); + + enum class VerificationMethod { + kIterator, + kGet, + kMultiGet, + kGetMergeOperands, + // Add any new items above kNumberOfMethods + kNumberOfMethods + }; + + const int num_methods = + static_cast(VerificationMethod::kNumberOfMethods); + const VerificationMethod method = + static_cast(thread->rand.Uniform(num_methods)); + + if (method == VerificationMethod::kIterator) { std::unique_ptr iter( db_->NewIterator(options, column_families_[cf])); + + std::string seek_key = Key(start); iter->Seek(seek_key); - prefix = Slice(seek_key.data(), prefix_to_use); - for (auto i = start; i < end; i++) { + + Slice prefix(seek_key.data(), prefix_to_use); + + for (int64_t i = start; i < end; ++i) { if (thread->shared->HasVerificationFailedYet()) { break; } - std::string from_db; - std::string keystr = Key(i); - Slice k = keystr; - Slice pfx = Slice(keystr.data(), prefix_to_use); + + const std::string key = Key(i); + const Slice k(key); + const Slice pfx(key.data(), prefix_to_use); + // Reseek when the prefix changes if (prefix_to_use > 0 && prefix.compare(pfx) != 0) { iter->Seek(k); - seek_key = keystr; + seek_key = key; prefix = Slice(seek_key.data(), prefix_to_use); } + Status s = iter->status(); + Slice iter_key; + std::string from_db; + if (iter->Valid()) { - Slice iter_key = iter->key(); - if (iter->key().compare(k) > 0) { - s = Status::NotFound(Slice()); - } else if (iter->key().compare(k) == 0) { + iter_key = iter->key(); + + const int diff = iter_key.compare(k); + + if (diff > 0) { + s = Status::NotFound(); + } else if (diff == 0) { from_db = iter->value().ToString(); iter->Next(); - } else if (iter_key.compare(k) < 0) { + } else { + assert(diff < 0); + VerificationAbort(shared, "An out of range key was found", static_cast(cf), i); } @@ -83,80 +112,95 @@ class NonBatchedOpsStressTest : public StressTest { // move to the next item in the iterator s = Status::NotFound(); } + VerifyOrSyncValue(static_cast(cf), i, options, shared, from_db, - s, true); - if (from_db.length()) { + s, /* strict */ true); + + if (!from_db.empty()) { PrintKeyValue(static_cast(cf), static_cast(i), - from_db.data(), from_db.length()); + from_db.data(), from_db.size()); } } - } else if (thread->rand.OneIn(3)) { - // 1/4 chance use Get to verify this range - for (auto i = start; i < end; i++) { + } else if (method == VerificationMethod::kGet) { + for (int64_t i = start; i < end; ++i) { if (thread->shared->HasVerificationFailedYet()) { break; } + + const std::string key = Key(i); std::string from_db; - std::string keystr = Key(i); - Slice k = keystr; - Status s = db_->Get(options, column_families_[cf], k, &from_db); + + Status s = db_->Get(options, column_families_[cf], key, &from_db); + VerifyOrSyncValue(static_cast(cf), i, options, shared, from_db, - s, true); - if (from_db.length()) { + s, /* strict */ true); + + if (!from_db.empty()) { PrintKeyValue(static_cast(cf), static_cast(i), - from_db.data(), from_db.length()); + from_db.data(), from_db.size()); } } - } else if (thread->rand.OneIn(2)) { - // 1/4 chance use MultiGet to verify this range - for (auto i = start; i < end;) { + } else if (method == VerificationMethod::kMultiGet) { + for (int64_t i = start; i < end;) { if (thread->shared->HasVerificationFailedYet()) { break; } + // Keep the batch size to some reasonable value size_t batch_size = thread->rand.Uniform(128) + 1; batch_size = std::min(batch_size, end - i); + std::vector keystrs(batch_size); std::vector keys(batch_size); std::vector values(batch_size); std::vector statuses(batch_size); + for (size_t j = 0; j < batch_size; ++j) { keystrs[j] = Key(i + j); - keys[j] = Slice(keystrs[j].data(), keystrs[j].length()); + keys[j] = Slice(keystrs[j].data(), keystrs[j].size()); } + db_->MultiGet(options, column_families_[cf], batch_size, keys.data(), values.data(), statuses.data()); + for (size_t j = 0; j < batch_size; ++j) { - Status s = statuses[j]; - std::string from_db = values[j].ToString(); + const std::string from_db = values[j].ToString(); + VerifyOrSyncValue(static_cast(cf), i + j, options, shared, - from_db, s, true); - if (from_db.length()) { + from_db, statuses[j], /* strict */ true); + + if (!from_db.empty()) { PrintKeyValue(static_cast(cf), static_cast(i + j), - from_db.data(), from_db.length()); + from_db.data(), from_db.size()); } } i += batch_size; } } else { - // 1/4 chance use GetMergeOperand to verify this range + assert(method == VerificationMethod::kGetMergeOperands); + // Start off with small size that will be increased later if necessary std::vector values(4); + GetMergeOperandsOptions merge_operands_info; merge_operands_info.expected_max_number_of_operands = static_cast(values.size()); - for (auto i = start; i < end; i++) { + + for (int64_t i = start; i < end; ++i) { if (thread->shared->HasVerificationFailedYet()) { break; } + + const std::string key = Key(i); + const Slice k(key); std::string from_db; - std::string keystr = Key(i); - Slice k = keystr; int number_of_operands = 0; + Status s = db_->GetMergeOperands(options, column_families_[cf], k, values.data(), &merge_operands_info, &number_of_operands); + if (s.IsIncomplete()) { // Need to resize values as there are more than values.size() merge // operands on this key. Should only happen a few times when we @@ -173,11 +217,13 @@ class NonBatchedOpsStressTest : public StressTest { if (number_of_operands) { from_db = values[number_of_operands - 1].ToString(); } + VerifyOrSyncValue(static_cast(cf), i, options, shared, from_db, - s, true); - if (from_db.length()) { + s, /* strict */ true); + + if (!from_db.empty()) { PrintKeyValue(static_cast(cf), static_cast(i), - from_db.data(), from_db.length()); + from_db.data(), from_db.size()); } } } @@ -696,7 +742,7 @@ class NonBatchedOpsStressTest : public StressTest { std::string from_db; Status s = db_->Get(read_opts, cfh, k, &from_db); if (!VerifyOrSyncValue(rand_column_family, rand_key, read_opts, shared, - from_db, s, true)) { + from_db, s, /* strict */ true)) { return s; } }