Merge #148707 #148750

148707: kvserver: introduce TestStoreRangeSplitRaftSnapshotAfterRHSRebalanced  r=arulajmani a=arulajmani

This patch adds a new test,
TestStoreRangeSplitRaftSnapshotAfterRHSRebalanced, to show the hazard
described in #73462 is legit. In particular, when a replica learns about
a split through a snapshot after the post-split RHS has been rebalanced
away, we leak the RHS's on-disk data.

References #73462

Release note: None

148750: roachtest: fix failover disk stall r=golgeek a=DarrylWong

This fixes the failover disk staller to be compatible with the FI library. Specifically, it does not attempt to recover in cleanup, as this is already handled in the FI framework.

Additionally, it temporarily disables concurrent disk stall failures from being injected as this is an invariant of the FI framework. Instead, they should be injected in one shot.

Fixes: #148739
Fixes: #148742
Fixes: #148741
Fixes: #148740
Fixes: #148738
Fixes: #148737
Release note: none


Co-authored-by: Arul Ajmani <[email protected]>
Co-authored-by: DarrylWong <[email protected]>

Author: 3 people

pkg/cmd/roachtest/tests/failover.go

@@ -1594,18 +1594,24 @@ type diskStallFailer struct {
 	staller       diskStaller
 }
 
-func (f *diskStallFailer) Mode() failureMode           { return failureModeDiskStall }
-func (f *diskStallFailer) String() string              { return string(f.Mode()) }
-func (f *diskStallFailer) CanUseLocal() bool           { return false } // needs dmsetup
-func (f *diskStallFailer) CanUseChaos() bool           { return true }
-func (f *diskStallFailer) CanRunWith(failureMode) bool { return true }
+func (f *diskStallFailer) Mode() failureMode { return failureModeDiskStall }
+func (f *diskStallFailer) String() string    { return string(f.Mode()) }
+func (f *diskStallFailer) CanUseLocal() bool { return false } // needs dmsetup
+func (f *diskStallFailer) CanUseChaos() bool { return true }
+
+// CanRunWith returns false for other disk stalls, as the FI library it uses
+// does not allow concurrent failure modes to be injected without recovering
+// from them first.
+// TODO(darryl): This is a temporary workaround to reduce test failure noise.
+// We should fix this by merging concurrent disk stall failures and injecting
+// them in one shot.
+func (f *diskStallFailer) CanRunWith(other failureMode) bool { return other != failureModeDiskStall }
 
 func (f *diskStallFailer) Setup(ctx context.Context) {
 	f.staller.Setup(ctx)
 }
 
 func (f *diskStallFailer) Cleanup(ctx context.Context) {
-	f.staller.Unstall(ctx, f.c.All())
 	// We have to stop the cluster before cleaning up the staller.
 	f.m.ExpectDeaths(int32(f.c.Spec().NodeCount))
 	f.c.Stop(ctx, f.t.L(), option.DefaultStopOpts(), f.c.All())

pkg/kv/kvserver/BUILD.bazel

@@ -576,6 +576,7 @@ go_test(
         "@com_github_stretchr_testify//require",
         "@org_golang_google_grpc//:grpc",
         "@org_golang_google_grpc//metadata",
+        "@org_golang_x_exp//maps",
         "@org_golang_x_sync//errgroup",
         "@org_golang_x_sync//syncmap",
     ],