Merge #142788

142788: roachtest: deflake `gossip/restart-node-one` r=herkolategan a=srosenberg

When running in _local_ mode, e.g., as
part of the acceptance suite during PR
check, the rate of flakiness of
`gossip/restart-node-one` has notably
increased. In majority of cases, it fails
during post-test validations, namely
the replica consistency check.

It's surmised that a recent switch to
"leader leases", coupled with other
implementation details, may have resulted
in the uptick (of flakes). The other details
are the fact that n1 is stripped of all replicas,
owing to the specifics of the test. And, the
fact that the replica consistency check always
preferred n1. Consequently, n1 may be temporarily
unavailable upon restart, causing a timeout.

This change forces `WaitForReplication` upon
restart of n1. Further, it shuffles node health
statuses so that every node in the cluster is
equally likely to be chosen for the replica
consistency check.

Epic: none
Release note: None

Co-authored-by: Stan Rosenberg <[email protected]>

Author: craig[bot]

pkg/cmd/roachtest/roachtestutil/validation_check.go

@@ -44,7 +44,7 @@ func CheckReplicaDivergenceOnDB(ctx context.Context, l *logger.Logger, db *gosql
 	// Speed up consistency checks. The test is done, so let's go full throttle.
 	_, err := db.ExecContext(ctx, "SET CLUSTER SETTING server.consistency_check.max_rate = '1GB'")
 	if err != nil {
-		return err
+		return errors.Wrap(err, "unable to set 'server.consistency_check.max_rate'")
 	}
 
 	// NB: we set a statement_timeout since context cancellation won't work here.

pkg/cmd/roachtest/test_runner.go

@@ -1524,6 +1524,11 @@ func (r *testRunner) postTestAssertions(
 		}
 
 		validationNode := 0
+		// Shuffle node statuses so that we don't always pick the same node for validation checks.
+		prng.Shuffle(len(statuses), func(i, j int) {
+			statuses[i], statuses[j] = statuses[j], statuses[i]
+		})
+
 		for _, s := range statuses {
 			if s.Err != nil {
 				t.L().Printf("n%d: %s error=%s", s.Node, s.URL, s.Err)

pkg/cmd/roachtest/tests/gossip.go

@@ -538,7 +538,9 @@ SELECT count(replicas)
 	// Stop our special snowflake process which won't be recognized by the test
 	// harness, and start it again on the regular.
 	c.Stop(ctx, t.L(), option.DefaultStopOpts(), c.Node(1))
-	c.Start(ctx, t.L(), option.DefaultStartOpts(), install.MakeClusterSettings(), c.Node(1))
+	// N.B. Since n1 was initially stripped of all the replicas, we must wait for full replication. Otherwise, the
+	// replica consistency checks may time out.
+	c.Start(ctx, t.L(), option.NewStartOpts(option.WaitForReplication()), install.MakeClusterSettings(), c.Node(1))
 }
 
 func runCheckLocalityIPAddress(ctx context.Context, t test.Test, c cluster.Cluster) {