facebook / facebook/rocksdb

db_bench should not reuse RNG seeds or hardwire them

Open
#9,640 0 comments 0 reactions 1 assignee Claimed by @mdcallag View on GitHub
enhancement
Dominant language
C++
Stars
32.1k
Forks
6.9k
Avg merge
32m
Merged PRs (30d)
1

Description

This took a while to figure out. I suspect there are two things that should be fixed (overwrite_gen, RandomGenerator) , but plenty of opportunity to introduce problems in the future.

db_bench should not reuse RNG seeds or hardwire them, unless that is desired (even better, commented). My assumption is that in most cases, the following should be true to avoid things (threads in one run, threads across runs) from generating the same sequence of keys unless the person running db_bench wants that to be done:

1. The same seeds should not be used across runs of db_bench
2. The same seeds should not be used across threads within one run of db_bench

The reason for avoiding seed reuse is [explained here](http://smalldatum.blogspot.com/2022/02/rocksdb-externals-avoiding-problems.html) and see https://github.com/facebook/rocksdb/issues/9632

I have no idea whether this is a bug:
- reservoir_id_gen [uses FLAG_seed](https://github.com/facebook/rocksdb/blob/ddb7620a617633f371b090a71254ac5d24f7eb2a/tools/db_bench_tool.cc#L4699) so the same seed is used across threads. I have no idea whether this is a problem.

Possible bugs:
- overwrite_gen [uses FLAGS_seed](https://github.com/facebook/rocksdb/blob/ddb7620a617633f371b090a71254ac5d24f7eb2a/tools/db_bench_tool.cc#L4689) in DoWrite
- RandomGenerator uses a hardwired seed (=301) in the Random constructor [for rnd](https://github.com/facebook/rocksdb/blob/ddb7620a617633f371b090a71254ac5d24f7eb2a/tools/db_bench_tool.cc#L1741). In this case each thread that uses RandomGenerator then generates the same character sequence in data_ and that sequence is a circular buffer consumed N bytes at a time (for some benchmarks N varies, for others it is fixed). So the threads can be generating the same bytes for values around the same point in time.
- Continuing with RandomGenerator, [NormalDistribution::gen_](https://github.com/facebook/rocksdb/blob/ddb7620a617633f371b090a71254ac5d24f7eb2a/tools/db_bench_tool.cc#L1690) and [UniformDistribution::gen_](https://github.com/facebook/rocksdb/blob/ddb7620a617633f371b090a71254ac5d24f7eb2a/tools/db_bench_tool.cc#L1710) use the default seed for std::mt19937, AFAIK that [is hardwired](https://www.cplusplus.com/reference/random/mt19937/) to 5489 (default_seed==5489).

Probably not a bug:
- rnd_disposable_entry [uses FLAGS_seed](https://github.com/facebook/rocksdb/blob/ddb7620a617633f371b090a71254ac5d24f7eb2a/tools/db_bench_tool.cc#L4736) as the seed for Random so the same seed is used across threads.

NormalDistribution and UniformDistribution are used by [RandomGenerator](https://github.com/facebook/rocksdb/blob/ddb7620a617633f371b090a71254ac5d24f7eb2a/tools/db_bench_tool.cc#L1722) and uses of RandomGenerator where seed reuse/hardwired should be avoided are in [DoWrite](https://github.com/facebook/rocksdb/blob/ddb7620a617633f371b090a71254ac5d24f7eb2a/tools/db_bench_tool.cc#L4659), [MixGraph](https://github.com/facebook/rocksdb/blob/ddb7620a617633f371b090a71254ac5d24f7eb2a/tools/db_bench_tool.cc#L6040), [BGWriter](https://github.com/facebook/rocksdb/blob/ddb7620a617633f371b090a71254ac5d24f7eb2a/tools/db_bench_tool.cc#L6445), [RandomWithVerify](https://github.com/facebook/rocksdb/blob/ddb7620a617633f371b090a71254ac5d24f7eb2a/tools/db_bench_tool.cc#L6693), [ReadRandomWriteRandom](https://github.com/facebook/rocksdb/blob/ddb7620a617633f371b090a71254ac5d24f7eb2a/tools/db_bench_tool.cc#L6764), [UpdateRandom](https://github.com/facebook/rocksdb/blob/ddb7620a617633f371b090a71254ac5d24f7eb2a/tools/db_bench_tool.cc#L6839), [XORUpdateRandom](https://github.com/facebook/rocksdb/blob/ddb7620a617633f371b090a71254ac5d24f7eb2a/tools/db_bench_tool.cc#L6906), [AppendRandom](https://github.com/facebook/rocksdb/blob/ddb7620a617633f371b090a71254ac5d24f7eb2a/tools/db_bench_tool.cc#L6972), [MergeRandom](https://github.com/facebook/rocksdb/blob/ddb7620a617633f371b090a71254ac5d24f7eb2a/tools/db_bench_tool.cc#L7049), [ReadRandomMergeRandom](https://github.com/facebook/rocksdb/blob/ddb7620a617633f371b090a71254ac5d24f7eb2a/tools/db_bench_tool.cc#L7095), [RandomReplaceKeys](https://github.com/facebook/rocksdb/blob/ddb7620a617633f371b090a71254ac5d24f7eb2a/tools/db_bench_tool.cc#L7421), and [TimeSeriesWrite](https://github.com/facebook/rocksdb/blob/ddb7620a617633f371b090a71254ac5d24f7eb2a/tools/db_bench_tool.cc#L4659).

Not a bug because --threads is hardwired to 1 when this is called:
- RandomShuffle [uses FLAGS_seed](https://github.com/facebook/rocksdb/blob/ddb7620a617633f371b090a71254ac5d24f7eb2a/tools/db_bench_tool.cc#L4575)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.