storage: MVCC benchmarks unreliable under Bazel

[erikgrinaker]

pkg/storage has a set of MVCC benchmarks in bench_pebble_test.go, e.g. BenchmarkMVCCGet_Pebble and BenchmarkMVCCScan_Pebble. These operate on a relatively large dataset, which is pre-created if it does not already exist, but otherwise reused if one is found. This is generated by setupMVCCData, under pkg/storage/mvcc_data_*/.

When running under Bazel, e.g.:

$ dev bench pkg/storage -f "BenchmarkMVCCGet"

I often get highly unreliable results that can vary greatly between each run, especially when switching between two commits (to compare before/after benchmarks). I suspect this is some combination of Bazel reusing a previously created dataset in a working directory dependant on the build commit hash or some such. It may also be related to having cancelled the dataset creation, such that one of the builds uses a partial dataset while the other uses a complete one. I haven't dug into this. I don't see this variability with make, which stores the dataset directly in the Git clone, thus reusing the same dataset each run.

CC @rickystewart

Epic CRDB-17171
Jira issue: CRDB-17163

[rickystewart]

@erikgrinaker Quick question, it seems that the dataset creation you're talking about should fall into the "setup" category of things that we exclude from the benchmark results, so my intuition is that there's some b.ResetTimer() (or equivalent) missing somewhere after this setup is performed. Does that sound right to you, or do we have a reason to believe that it's not "just" that simple?

[erikgrinaker]

Yeah, the setup is excluded from the benchmark time, it calls ResetTimer().

In any case, I think this issue started out in the wrong end -- the above is what I observed (and it's admittedly a poor writeup, I was in a hurry and just needed to scribble something down). I think what we want is for the dataset to be persisted in a common directory, not inside the Bazel build directory, such that it's properly reused by all benchmark runs regardless of the commit that's checked out. Ideally somewhere easily accessible and discoverable, since we'll need to nuke it when we make changes to the generated dataset. What would be the best way to do that with Bazel?

[cucaroach]

Possibly fixed by #86167

[jbowens]

I don't think this is fixed by #86167 because the mvcc data is not made available across runs. This definitely contributes to variability, but besides that it greatly increases benchmark time because of the setup is performed every time you run ./dev bench.

[cucaroach]

@jbowens can you expand on what you think may be causing this and how bazel is different than make in this area?

[jbowens]

@erikgrinaker's comment above summarizes it pretty well:

I think what we want is for the dataset to be persisted in a common directory, not inside the Bazel build directory, such that it's properly reused by all benchmark runs regardless of the commit that's checked out. Ideally somewhere easily accessible and discoverable, since we'll need to nuke it when we make changes to the generated dataset. What would be the best way to do that with Bazel?

Many of the MVCC benchmarks require an initial database state, which is populated by setupMVCCData and written to the filesystem. If the required initial database state already exists, the benchmark does not recreate the database state. This scheme avoids needing to reconstruct the entire database state for every new benchmark invocation, and it ensures that when comparing commits, both commits run on identical databases.

With bazel, this initial database state is written to the sandbox's build directory instead of the checked out repository. When the benchmark is complete, the build directory is removed. Every run of dev bench needs to recreate the database state.

[jbowens]

Is there some way to lift this test data out of the bazel sandbox, so that we can reuse it between dev bench invocations?

[RaduBerinde]

Proposal: I am pretty sure that even when run under bazel, tests have access to the filesystem (more specifically to the user home dir). We can create a test-fixtures library which creates or reuses fixtures inside os.UserCacheDir() (something like $HOME/.cache/crdb-test-fixtures/TestName/FixtureName). The library should make sure that we don't reuse partially created fixtures (e.g. if the test gets interrupted during fixture creation, or fixture creation itself hits an error).

[RaduBerinde]

One wrinkle is that inside the sandbox $HOME is not set. But we could make dev bench/test pass the .cache directory through a custom env var.

[ajwerner]

I feel like the bazel-friendly option here would be to pull the state generation out to some other binary, generate it as a part of the bazel build step, then prosper. In the case that you're not running it with bazel, the files will continue to exist between runs and the current lazy behavior is fine.

The code to generate that initial state isn't so magical -- it seems like it could trivially be moved to a subpackage or something like that.

[RaduBerinde]

There are a handful of other benchmarks with the same problem. I feel it would get tedious to have separate generation binaries for each one. And these can be quite big and slow to generate, we wouldn't want to generate them unless we are actually interested in running the relevant (sub)benchmark.

[RaduBerinde]

I am pretty sure that even when run under bazel, tests have access to the filesystem (more specifically to the user home dir).

I think this is incorrect.

Edit: but can be made so with --sandbox_writable_path.