From 96a031e8a82c6b862c1aefda297e91788a794f40 Mon Sep 17 00:00:00 2001 From: Bilal Akhtar Date: Tue, 15 Aug 2023 13:29:56 -0400 Subject: [PATCH] roachtest: allow stats mismatches in clearrange roachtest 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 --- pkg/cmd/roachtest/tests/clearrange.go | 2 +- pkg/kv/kvserver/replica_consistency.go | 22 ---------------------- 2 files changed, 1 insertion(+), 23 deletions(-) diff --git a/pkg/cmd/roachtest/tests/clearrange.go b/pkg/cmd/roachtest/tests/clearrange.go index 1824d896837b..47b558e8f109 100644 --- a/pkg/cmd/roachtest/tests/clearrange.go +++ b/pkg/cmd/roachtest/tests/clearrange.go @@ -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) diff --git a/pkg/kv/kvserver/replica_consistency.go b/pkg/kv/kvserver/replica_consistency.go index bfd43c61cf0e..7baa6fa51187 100644 --- a/pkg/kv/kvserver/replica_consistency.go +++ b/pkg/kv/kvserver/replica_consistency.go @@ -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" @@ -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 @@ -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