Merge #64199

64199: kv: avoid panic on TestMergeQueue/non-collocated failure r=nvanbenschoten a=nvanbenschoten

Informs #63009.
Informs #64056.

In #63009/#64056, we saw that this test could flake with a nil pointer panic.
I don't know quite what's going on here, but when working on a patch for #62700,
I managed to hit this panic reliably by accidentally breaking all range merges.

After a bit of debugging, it became clear that we were always hitting a panic in
the `reset` stage of `TestMergeQueue/sticky-bit` because the previous subtest,
`TestMergeQueue/non-collocated`, was moving the RHS range to a different node,
failing to merge the two range, and failing itself. This soft failure was being
drowned out by the hard failure in the next subtest.

This commit replaces the crash with a failure that looks something like the
following when range merges are completely disabled:
```
--- FAIL: TestMergeQueue (0.34s)
    test_log_scope.go:73: test logs captured to: /var/folders/8k/436yf8s97cl_27vlh270yb8c0000gp/T/logTestMergeQueue627909827
    test_log_scope.go:74: use -show-logs to present logs inline
    --- FAIL: TestMergeQueue/both-empty (0.00s)
        client_merge_test.go:4183: ranges unexpectedly unmerged expected startKey /Table/Max, but got "\xfa\x00\x00"
    --- FAIL: TestMergeQueue/lhs-undersize (0.00s)
        client_merge_test.go:4192: ranges unexpectedly unmerged expected startKey /Table/Max, but got "\xfa\x00\x00"
    --- FAIL: TestMergeQueue/combined-threshold (0.00s)
        client_merge_test.go:4214: ranges unexpectedly unmerged expected startKey /Table/Max, but got "\xfa\x00\x00"
    --- FAIL: TestMergeQueue/non-collocated (0.03s)
        client_merge_test.go:4236: replica doesn't exist
    --- FAIL: TestMergeQueue/sticky-bit (0.00s)
        client_merge_test.go:4243: right-hand side range not found
    --- FAIL: TestMergeQueue/sticky-bit-expiration (0.00s)
        client_merge_test.go:4268: right-hand side range not found
```

I expect that under stress on master, we will see the `TestMergeQueue/non-collocated` subtest fail.

The fact that `TestMergeQueue/non-collocated` is the test failing means that we may want to have @aayushshah15 take over this investigation, since he's made changes in that area recently. What do you two think?

Co-authored-by: Nathan VanBenschoten <[email protected]>

pkg/kv/kvserver/client_merge_test.go

@@ -4140,9 +4140,18 @@ func TestMergeQueue(t *testing.T) {
 
 	// setThresholds simulates a zone config update that updates the ranges'
 	// minimum and maximum sizes.
-	setZones := func(zone zonepb.ZoneConfig) {
-		lhs().SetZoneConfig(&zone)
-		rhs().SetZoneConfig(&zone)
+	setZones := func(t *testing.T, zone zonepb.ZoneConfig) {
+		t.Helper()
+		if l := lhs(); l == nil {
+			t.Fatal("left-hand side range not found")
+		} else {
+			l.SetZoneConfig(&zone)
+		}
+		if r := rhs(); r == nil {
+			t.Fatal("right-hand side range not found")
+		} else {
+			r.SetZoneConfig(&zone)
+		}
 	}
 
 	reset := func(t *testing.T) {
@@ -4153,7 +4162,7 @@ func TestMergeQueue(t *testing.T) {
 				t.Fatal(err)
 			}
 		}
-		setZones(zoneConfig)
+		setZones(t, zoneConfig)
 		store.MustForceMergeScanAndProcess() // drain any merges that might already be queued
 		split(t, rhsStartKey.AsRawKey(), hlc.Timestamp{} /* expirationTime */)
 	}
@@ -4192,13 +4201,13 @@ func TestMergeQueue(t *testing.T) {
 		zone := protoutil.Clone(&zoneConfig).(*zonepb.ZoneConfig)
 		zone.RangeMinBytes = proto.Int64(rhs().GetMVCCStats().Total() + 1)
 		zone.RangeMaxBytes = proto.Int64(lhs().GetMVCCStats().Total() + rhs().GetMVCCStats().Total() - 1)
-		setZones(*zone)
+		setZones(t, *zone)
 		store.MustForceMergeScanAndProcess()
 		verifyUnmerged(t, store, lhsStartKey, rhsStartKey)
 
 		// Once the maximum size threshold is increased, the merge can occur.
 		zone.RangeMaxBytes = proto.Int64(*zone.RangeMaxBytes + 1)
-		setZones(*zone)
+		setZones(t, *zone)
 		l := lhs().RangeID
 		r := rhs().RangeID
 		log.Infof(ctx, "Left=%s, Right=%s", l, r)