Skip to content

Commit

Permalink
crash_test to do some verification for prefix extractor and iterator …
Browse files Browse the repository at this point in the history 
…bounds. (facebook#5846)

Summary:
For now, crash_test is not able to report any failure for the logic related to iterator upper, lower bounds or iterators, or reseek. These are features prone to errors. Improve db_stress in several ways:
(1) For each iterator run, reseek up to 3 times.
(2) For every iterator, create control iterator with upper or lower bound, with total order seek. Compare the results with the iterator.
(3) Make simple crash test to avoid prefix size to have more coverage.
(4) make prefix_size = 0 a valid size and -1 to indicate disabling prefix extractor.
Pull Request resolved: facebook#5846

Test Plan: Manually hack the code to create wrong results and see they are caught by the tool.

Differential Revision: D17631760

fbshipit-source-id: acd460a177bd2124a5ffd7fff490702dba63030b
  • Loading branch information
@siying @facebook-github-bot
siying authored and facebook-github-bot committed Sep 27, 2019
1 parent 1afd061 commit 00007a9
Show file tree
Hide file tree
Showing 2 changed files with 167 additions and 25 deletions.
5 changes: 3 additions & 2 deletions tools/db_crashtest.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,9 @@ def is_direct_io_supported(dbname):
"max_background_compactions": 1,
"max_bytes_for_level_base": 67108864,
"memtablerep": "skip_list",
"prefixpercent": 25,
"readpercent": 25,
"prefixpercent": 0,
"readpercent": 50,
"prefix_size" : -1,
"target_file_size_base": 16777216,
"target_file_size_multiplier": 1,
"test_batches_snapshots": 0,
Expand Down
187 changes: 164 additions & 23 deletions tools/db_stress.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -693,14 +693,16 @@ static enum RepFactory FLAGS_rep_factory;
DEFINE_string(memtablerep, "prefix_hash", "");

static bool ValidatePrefixSize(const char* flagname, int32_t value) {
if (value < 0 || value > 8) {
fprintf(stderr, "Invalid value for --%s: %d. 0 <= PrefixSize <= 8\n",
if (value < -1 || value > 8) {
fprintf(stderr, "Invalid value for --%s: %d. -1 <= PrefixSize <= 8\n",
flagname, value);
return false;
}
return true;
}
DEFINE_int32(prefix_size, 7, "Control the prefix size for HashSkipListRep");
DEFINE_int32(prefix_size, 7,
"Control the prefix size for HashSkipListRep. "
"-1 is disabled.");
static const bool FLAGS_prefix_size_dummy __attribute__((__unused__)) =
RegisterFlagValidator(&FLAGS_prefix_size, &ValidatePrefixSize);

Expand Down Expand Up @@ -2348,6 +2350,11 @@ class StressTest {
lock);
} else {
// OPERATION iterate
int num_seeks = static_cast<int>(
std::min(static_cast<uint64_t>(thread->rand.Uniform(4)),
FLAGS_ops_per_thread - i - 1));
rand_keys = GenerateNKeys(thread, num_seeks, i);
i += num_seeks - 1;
TestIterate(thread, read_opts, rand_column_families, rand_keys);
}
thread->stats.FinishedSingleOp();
Expand Down Expand Up @@ -2445,7 +2452,7 @@ class StressTest {
std::string lower_bound_str;
Slice lower_bound;
if (thread->rand.OneIn(16)) {
// in 1/16 chance, set a iterator lower bound
// in 1/16 chance, enable iterator lower bound
int64_t rand_lower_key = GenerateOneKey(thread, FLAGS_ops_per_thread);
lower_bound_str = Key(rand_lower_key);
lower_bound = Slice(lower_bound_str);
Expand All @@ -2457,28 +2464,160 @@ class StressTest {
auto cfh = column_families_[rand_column_families[0]];
std::unique_ptr<Iterator> iter(db_->NewIterator(readoptionscopy, cfh));

std::string key_str = Key(rand_keys[0]);
Slice key = key_str;
iter->Seek(key);
for (uint64_t i = 0; i < FLAGS_num_iterations && iter->Valid(); i++) {
if (thread->rand.OneIn(2)) {
iter->Next();
} else {
iter->Prev();
for (int64_t rkey : rand_keys) {
std::string key_str = Key(rkey);
Slice key = key_str;

if (readoptionscopy.iterate_upper_bound != nullptr &&
thread->rand.OneIn(2)) {
// 1/2 chance, change the upper bound.
// It is possible that it is changed without first use, but there is no
// problem with that.
int64_t rand_upper_key = GenerateOneKey(thread, FLAGS_ops_per_thread);
upper_bound_str = Key(rand_upper_key);
upper_bound = Slice(upper_bound_str);
}
}

if (s.ok()) {
thread->stats.AddIterations(1);
} else {
thread->stats.AddErrors(1);
// Set up an iterator and does the same without bounds and with total
// order seek and compare the results. This is to identify bugs related
// to bounds, prefix extractor or reseeking. Sometimes we are comparing
// iterators with the same set-up, and it doesn't hurt to check them
// to be equal.
ReadOptions cmp_ro;
cmp_ro.snapshot = snapshot;
cmp_ro.total_order_seek = true;
std::unique_ptr<Iterator> cmp_iter(
db_->NewIterator(readoptionscopy, cfh));
bool diverged = false;

iter->Seek(key);
cmp_iter->Seek(key);
VerifyIterator(thread, readoptionscopy, iter.get(), cmp_iter.get(), key,
&diverged);

for (uint64_t i = 0; i < FLAGS_num_iterations && iter->Valid(); i++) {
if (thread->rand.OneIn(2)) {
iter->Next();
if (!diverged) {
assert(cmp_iter->Valid());
cmp_iter->Next();
}
} else {
iter->Prev();
if (!diverged) {
assert(cmp_iter->Valid());
cmp_iter->Prev();
}
}
VerifyIterator(thread, readoptionscopy, iter.get(), cmp_iter.get(), key,
&diverged);
}

if (s.ok()) {
thread->stats.AddIterations(1);
} else {
thread->stats.AddErrors(1);
break;
}
}

db_->ReleaseSnapshot(snapshot);

return s;
}

// Compare the two iterator, iter and cmp_iter are in the same position,
// unless iter might be made invalidate or undefined because of
// upper or lower bounds, or prefix extractor.
// Will flag failure if the verification fails.
// diverged = true if the two iterator is already diverged.
// True if verification passed, false if not.
void VerifyIterator(ThreadState* thread, const ReadOptions& ro,
Iterator* iter, Iterator* cmp_iter, const Slice& seek_key,
bool* diverged) {
if (*diverged) {
return;
}

if (iter->Valid() && !cmp_iter->Valid()) {
fprintf(stderr,
"Control interator is invalid but iterator has value %s seek key "
"%s\n",
iter->key().ToString(true).c_str(),
seek_key.ToString(true).c_str());

*diverged = true;
} else if (cmp_iter->Valid()) {
// Iterator is not valid. It can be legimate if it has already been
// out of upper or lower bound, or filtered out by prefix iterator.
const Slice& total_order_key = cmp_iter->key();
const SliceTransform* pe = options_.prefix_extractor.get();
const Comparator* cmp = options_.comparator;

if (pe != nullptr) {
if (!pe->InDomain(seek_key)) {
// Prefix seek a non-in-domain key is undefined. Skip checking for
// this scenario.
*diverged = true;
return;
}

if (!pe->InDomain(total_order_key) ||
pe->Transform(total_order_key) != pe->Transform(seek_key)) {
// If the prefix is exhausted, the only thing needs to check
// is the iterator isn't return a position in prefix.
// Either way, checking can stop from here.
*diverged = true;
if (!iter->Valid() || !pe->InDomain(iter->key()) ||
pe->Transform(iter->key()) != pe->Transform(seek_key)) {
return;
}
fprintf(stderr,
"Iterator stays in prefix bug contol doesn't"
" seek key %s iterator key %s control iterator key %s\n",
seek_key.ToString(true).c_str(),
iter->key().ToString(true).c_str(),
cmp_iter->key().ToString(true).c_str());
}
}
// Check upper or lower bounds.
if (!*diverged) {
if (!iter->Valid() ||
(iter->key() != cmp_iter->key() &&
(ro.iterate_upper_bound == nullptr ||
cmp->Compare(total_order_key, *ro.iterate_upper_bound) < 0) &&
(ro.iterate_lower_bound == nullptr ||
cmp->Compare(total_order_key, *ro.iterate_lower_bound) > 0))) {
fprintf(stderr,
"Iterator diverged from control iterator which"
" has value %s seek key %s\n",
total_order_key.ToString(true).c_str(),
seek_key.ToString(true).c_str());
if (iter->Valid()) {
fprintf(stderr, "iterator has value %s\n",
iter->key().ToString(true).c_str());
} else {
fprintf(stderr, "iterator is not valid\n");
}
if (ro.iterate_upper_bound != nullptr) {
fprintf(stderr, "upper bound %s\n",
ro.iterate_upper_bound->ToString(true).c_str());
}
if (ro.iterate_lower_bound != nullptr) {
fprintf(stderr, "lower bound %s\n",
ro.iterate_lower_bound->ToString(true).c_str());
}
*diverged = true;
}
}
}
if (*diverged) {
thread->stats.AddErrors(1);
// Fail fast to preserve the DB state.
thread->shared->SetVerificationFailure();
}
}

#ifdef ROCKSDB_LITE
virtual Status TestBackupRestore(
ThreadState* /* thread */,
Expand Down Expand Up @@ -2802,8 +2941,10 @@ class StressTest {
options_.max_background_flushes = FLAGS_max_background_flushes;
options_.compaction_style =
static_cast<rocksdb::CompactionStyle>(FLAGS_compaction_style);
options_.prefix_extractor.reset(
NewFixedPrefixTransform(FLAGS_prefix_size));
if (FLAGS_prefix_size >= 0) {
options_.prefix_extractor.reset(
NewFixedPrefixTransform(FLAGS_prefix_size));
}
options_.max_open_files = FLAGS_open_files;
options_.statistics = dbstats;
options_.env = FLAGS_env;
Expand Down Expand Up @@ -3814,8 +3955,8 @@ class BatchedOpsStressTest : public StressTest {
fprintf(stderr, "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
// find more errors if any
// we continue after error rather than exiting so that we can
// find more errors if any
}
}

Expand Down Expand Up @@ -4425,7 +4566,7 @@ int main(int argc, char** argv) {
FLAGS_env->SetBackgroundThreads(FLAGS_max_background_compactions);
FLAGS_env->SetBackgroundThreads(FLAGS_num_bottom_pri_threads,
rocksdb::Env::Priority::BOTTOM);
if (FLAGS_prefixpercent > 0 && FLAGS_prefix_size <= 0) {
if (FLAGS_prefixpercent > 0 && FLAGS_prefix_size < 0) {
fprintf(stderr,
"Error: prefixpercent is non-zero while prefix_size is "
"not positive!\n");
Expand All @@ -4437,7 +4578,7 @@ int main(int argc, char** argv) {
"test_batches_snapshots test!\n");
exit(1);
}
if (FLAGS_memtable_prefix_bloom_size_ratio > 0.0 && FLAGS_prefix_size <= 0) {
if (FLAGS_memtable_prefix_bloom_size_ratio > 0.0 && FLAGS_prefix_size < 0) {
fprintf(stderr,
"Error: please specify positive prefix_size in order to use "
"memtable_prefix_bloom_size_ratio\n");
Expand Down

0 comments on commit 00007a9

Please sign in to comment.