Skip to content

Commit

Permalink
roachtest: allow stats mismatches in clearrange roachtest
Browse files Browse the repository at this point in the history
Previously, the clearrange roachtest was the only place anywhere in
the CockroachDB codebase where we would assert on MVCC stats matching
between replicas. This would trip up and fail the clearrange roachtest
even in known cases of MVCC stats mismatches. This change removes the
code to assert on stats mismatches with consistency checks, but retains
the clearrange roachtest's use of aggressive consistency checks, so
mismatches in checksums computed on data in each replica will
continue to fatal the test.

Related to #93896.

Fixes #108726.

Epic: none

Release note: None
  • Loading branch information
itsbilal committed Aug 15, 2023
1 parent 8ebc6f1 commit 96a031e
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 23 deletions.
2 changes: 1 addition & 1 deletion pkg/cmd/roachtest/tests/clearrange.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func runClearRange(ctx context.Context, t test.Test, c cluster.Cluster, aggressi
// This slows down merges, so it might hide some races.
//
// NB: the below invocation was found to actually make it to the server at the time of writing.
settings.Env = append(settings.Env, []string{"COCKROACH_CONSISTENCY_AGGRESSIVE=true", "COCKROACH_ENFORCE_CONSISTENT_STATS=true"}...)
settings.Env = append(settings.Env, "COCKROACH_CONSISTENCY_AGGRESSIVE=true")
}

c.Start(ctx, t.L(), option.DefaultStartOpts(), settings)
Expand Down
22 changes: 0 additions & 22 deletions pkg/kv/kvserver/replica_consistency.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/storage"
"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
"github.com/cockroachdb/cockroach/pkg/storage/fs"
"github.com/cockroachdb/cockroach/pkg/util/envutil"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/protoutil"
Expand All @@ -43,22 +42,6 @@ import (
"github.com/cockroachdb/redact"
)

// fatalOnStatsMismatch, if true, turns stats mismatches into fatal errors. A
// stats mismatch is the event in which
// - the consistency checker finds that all replicas are consistent
// (i.e. byte-by-byte identical)
// - the (identical) stats tracked in them do not correspond to a recomputation
// via the data, i.e. the stats were incorrect
// - ContainsEstimates==false, i.e. the stats claimed they were correct.
//
// Before issuing the fatal error, the cluster bootstrap version is verified.
// Note that on clusters that originally got bootstrapped on older releases
// (definitely 19.1, and likely also more recent ones) we know of the existence
// of stats bugs, so it has to be expected to see the assertion fire there.
//
// This env var is intended solely for use in Cockroach Labs testing.
var fatalOnStatsMismatch = envutil.EnvOrDefaultBool("COCKROACH_ENFORCE_CONSISTENT_STATS", false)

// replicaChecksum contains progress on a replica checksum computation.
type replicaChecksum struct {
// started is closed when the checksum computation has started. If the start
Expand Down Expand Up @@ -219,11 +202,6 @@ func (r *Replica) checkConsistencyImpl(
if !haveDelta {
return resp, nil
}
if delta.ContainsEstimates <= 0 && fatalOnStatsMismatch {
// We just found out that the recomputation doesn't match the persisted stats,
// so ContainsEstimates should have been strictly positive.
log.Fatalf(ctx, "found a delta of %+v", redact.Safe(delta))
}

// We've found that there's something to correct; send an RecomputeStatsRequest. Note that this
// code runs only on the lease holder (at the time of initiating the computation), so this work
Expand Down

0 comments on commit 96a031e

Please sign in to comment.