Skip to content

Commit

Permalink
batcheval: include version in fixture directory
Browse files Browse the repository at this point in the history
This benchmark is sometimes failing in CI and I cannot reproduce
locally. The error is consistent with a leftover fixture from an older
version (which shouldn't happen because CI should be cleaning up the
repo).

This change adds the version to the fixture name so that this kind of
cross-version problem cannot occur. This is a good idea regardless of
the CI issue.

The logging around the fixture is also improved.

Informs #97061

Release note: None
Epic: none
  • Loading branch information
RaduBerinde committed Feb 14, 2023
1 parent 3b9d8cb commit 1ea9b89
Showing 1 changed file with 13 additions and 3 deletions.
16 changes: 13 additions & 3 deletions pkg/kv/kvserver/batcheval/cmd_refresh_range_bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"testing"

"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/batcheval"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
Expand Down Expand Up @@ -210,6 +211,9 @@ func setupMVCCPebble(b testing.TB, dir string, lBaseMaxBytes int64, readOnly boo
func setupData(
ctx context.Context, b *testing.B, emk engineMaker, opts benchDataOptions,
) (storage.Engine, string) {
// Include the current version in the fixture name, or we may inadvertently
// run against a left-over fixture that is no longer supported.
verStr := fmt.Sprintf("v%s", clusterversion.TestingBinaryVersion.String())
orderStr := "linear"
if opts.randomKeyOrder {
orderStr = "random"
Expand All @@ -218,22 +222,28 @@ func setupData(
if opts.readOnlyEngine {
readOnlyStr = "_readonly"
}
loc := fmt.Sprintf("refresh_range_bench_data_%s%s_%d_%d_%d",
orderStr, readOnlyStr, opts.numKeys, opts.valueBytes, opts.lBaseMaxBytes)
loc := fmt.Sprintf("refresh_range_bench_data_%s_%s%s_%d_%d_%d",
verStr, orderStr, readOnlyStr, opts.numKeys, opts.valueBytes, opts.lBaseMaxBytes)
exists := true
if _, err := os.Stat(loc); oserror.IsNotExist(err) {
exists = false
} else if err != nil {
b.Fatal(err)
}

absPath, err := filepath.Abs(loc)
if err != nil {
absPath = loc
}

if exists {
log.Infof(ctx, "using existing refresh range benchmark data: %s", absPath)
testutils.ReadAllFiles(filepath.Join(loc, "*"))
return emk(b, loc, opts.lBaseMaxBytes, opts.readOnlyEngine), loc
}

eng := emk(b, loc, opts.lBaseMaxBytes, false)
log.Infof(ctx, "creating refresh range benchmark data: %s", loc)
log.Infof(ctx, "creating refresh range benchmark data: %s", absPath)

// Generate the same data every time.
rng := rand.New(rand.NewSource(1449168817))
Expand Down

0 comments on commit 1ea9b89

Please sign in to comment.