Merge #155616 #155861

155616: rangefeed: add assertion that OnError is called on disconnected registration r=stevendanna a=stevendanna

Epic: none
Release note: None

155861: kvserver: deflake TestPromoteNonVoterInAddVoter r=arulajmani a=arulajmani

This test could previously fail because a lease transfer could race with a split. Without the split, the effects of the span config (that the test asserts on) will not apply. We sidestep this problem by removing the need of a split entirely -- by tuning some cluster settings to ensure that every table is on its own range to begin with.

H/t to `@pav-kv` for the analysis on this one.

Closes #155828 
Closes #155747 
Closes #155316 
Closes #154773 
Closes #134383

Release note: None

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

Author: 3 people

pkg/kv/kvserver/rangefeed/stream_manager.go

@@ -121,8 +121,8 @@ func (sm *StreamManager) OnError(streamID int64) {
 	func() {
 		sm.streams.Lock()
 		defer sm.streams.Unlock()
-		if _, ok := sm.streams.m[streamID]; ok {
-			// TODO(ssd): We should be able to assert we are disconnected here.
+		if d, ok := sm.streams.m[streamID]; ok {
+			assertTrue(d.IsDisconnected(), "OnError called on connected registration")
 			delete(sm.streams.m, streamID)
 			sm.metrics.ActiveMuxRangeFeed.Dec(1)
 		}

pkg/kv/kvserver/replicate_queue_test.go

@@ -41,6 +41,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/server/serverpb"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/spanconfig"
+	"github.com/cockroachdb/cockroach/pkg/spanconfig/spanconfigstore"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/bootstrap"
 	"github.com/cockroachdb/cockroach/pkg/testutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
@@ -2188,13 +2189,20 @@ func TestPromoteNonVoterInAddVoter(t *testing.T) {
 	defer testutils.StartExecTrace(t, scope.GetDirectory()).Finish(t)
 
 	ctx := context.Background()
+	st := cluster.MakeTestingClusterSettings()
+	// NB: Ensure that tables created by the test start off on their own range.
+	// This ensures that any span config changes made by the test don't have to
+	// first induce a split, which is a known source of flakiness.
+	spanconfigstore.StorageCoalesceAdjacentSetting.Override(ctx, &st.SV, false)
+	spanconfigstore.TenantCoalesceAdjacentSetting.Override(ctx, &st.SV, false)
 
 	// Create 7 stores: 3 in Region 1, 2 in Region 2, and 2 in Region 3.
 	const numNodes = 7
 	serverArgs := make(map[int]base.TestServerArgs)
 	regions := [numNodes]int{1, 1, 1, 2, 2, 3, 3}
 	for i := 0; i < numNodes; i++ {
 		serverArgs[i] = base.TestServerArgs{
+			Settings: st,
 			Locality: roachpb.Locality{
 				Tiers: []roachpb.Tier{
 					{

pkg/spanconfig/spanconfigstore/span_store.go

@@ -19,10 +19,10 @@ import (
 	"github.com/cockroachdb/errors"
 )
 
-// tenantCoalesceAdjacentSetting is a public cluster setting that
+// TenantCoalesceAdjacentSetting is a public cluster setting that
 // controls whether we coalesce adjacent ranges across all secondary
 // tenant keyspaces if they have the same span config.
-var tenantCoalesceAdjacentSetting = settings.RegisterBoolSetting(
+var TenantCoalesceAdjacentSetting = settings.RegisterBoolSetting(
 	settings.SystemOnly,
 	"spanconfig.tenant_coalesce_adjacent.enabled",
 	`collapse adjacent ranges with the same span configs across all secondary tenant keyspaces`,
@@ -157,7 +157,7 @@ func (s *spanConfigStore) computeSplitKey(
 				return roachpb.RKey(match.span.Key), nil
 			}
 		} else {
-			if !tenantCoalesceAdjacentSetting.Get(&s.settings.SV) {
+			if !TenantCoalesceAdjacentSetting.Get(&s.settings.SV) {
 				return roachpb.RKey(match.span.Key), nil
 			}
 		}