Merge cockroachdb#99636 cockroachdb#99742 cockroachdb#99746

99636: roachtest: loq recovery use new min range logic r=erikgrinaker a=aliher1911

This commit makes test compliant with new enforcement of min range size. Test needs small range and it now sets env var that restricts size to value of 1 byte.

Release note: None

Fixes cockroachdb#99529
Fixes cockroachdb#99569
Fixes cockroachdb#99576
Fixes cockroachdb#99577


99742: kv: deflake and unskip TestSecondaryTenantFollowerReadsRouting r=arulajmani a=nvanbenschoten

Fixes cockroachdb#95338.

The test was failing to wait for configuration changes that added VOTER replicas to apply on all replicas, causing flakiness. This commit fixes that issue.

Before the fix, the test failed after 100 iterations on average on a gceworker. After the fix, the test has not failed in over 2000 iterations.

Release note: None

99746: sql: complete EXPLAIN (VEC, VERBOSE) in the bundle r=yuzefovich a=yuzefovich

This commit makes it so that `vec-v.txt` file in the stmt bundle is complete. In particular, previously we would not include the auxiliary stats-related operators since we hard-coded `recordingStats=false` when generating the operator chains, and this is now fixed. The impact is very minor so there is no release note, but I believe it's worth backporting still.

Informs: cockroachdb#99741

Epic: None

Release note: None

Co-authored-by: Oleg Afanasyev <oleg@cockroachlabs.com>
Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>

pkg/ccl/kvccl/kvfollowerreadsccl/followerreads_test.go

@@ -834,7 +834,6 @@ func TestFollowerReadsWithStaleDescriptor(t *testing.T) {
 // where it needs to be estimated using node localities.
 func TestSecondaryTenantFollowerReadsRouting(t *testing.T) {
 	defer leaktest.AfterTest(t)()
-	skip.WithIssue(t, 95338, "flaky test")
 	defer utilccl.TestingEnableEnterprise()()
 
 	skip.UnderStressRace(t, "times out")
@@ -973,6 +972,7 @@ func TestSecondaryTenantFollowerReadsRouting(t *testing.T) {
 
 		startKey := keys.MakeSQLCodec(serverutils.TestTenantID()).TenantPrefix()
 		tc.AddVotersOrFatal(t, startKey, tc.Target(1), tc.Target(2))
+		tc.WaitForVotersOrFatal(t, startKey, tc.Target(1), tc.Target(2))
 		desc := tc.LookupRangeOrFatal(t, startKey)
 		require.Equal(t, []roachpb.ReplicaDescriptor{
 			{NodeID: 1, StoreID: 1, ReplicaID: 1},

pkg/cmd/roachtest/tests/loss_of_quorum_recovery.go

@@ -167,7 +167,9 @@ func runRecoverLossOfQuorum(ctx context.Context, t test.Test, c cluster.Cluster,
 	workloadHistogramFile := "restored.json"
 
 	c.Put(ctx, t.Cockroach(), "./cockroach", c.All())
-	settings := install.MakeClusterSettings()
+	settings := install.MakeClusterSettings(install.EnvOption([]string{
+		"COCKROACH_MIN_RANGE_MAX_BYTES=1",
+	}))
 	c.Start(ctx, t.L(), option.DefaultStartOpts(), settings, nodes)
 
 	// Cleanup stale files generated during recovery. We do this for the case
@@ -383,7 +385,9 @@ func runHalfOnlineRecoverLossOfQuorum(
 	workloadHistogramFile := "restored.json"
 
 	c.Put(ctx, t.Cockroach(), "./cockroach", c.All())
-	settings := install.MakeClusterSettings()
+	settings := install.MakeClusterSettings(install.EnvOption([]string{
+		"COCKROACH_MIN_RANGE_MAX_BYTES=1",
+	}))
 	c.Start(ctx, t.L(), option.DefaultStartOpts(), settings, nodes)
 
 	// Cleanup stale files generated during recovery. We do this for the case

pkg/sql/colflow/explain_vec.go

@@ -42,6 +42,7 @@ func convertToVecTree(
 	flow *execinfrapb.FlowSpec,
 	localProcessors []execinfra.LocalProcessor,
 	isPlanLocal bool,
+	recordingStats bool,
 ) (opChains execopnode.OpChains, cleanup func(), err error) {
 	if !isPlanLocal && len(localProcessors) > 0 {
 		return nil, func() {}, errors.AssertionFailedf("unexpectedly non-empty LocalProcessors when plan is not local")
@@ -54,10 +55,11 @@ func convertToVecTree(
 	// execinfra.BatchReceiver, so we always pass in a fakeBatchReceiver to the
 	// creator.
 	creator := newVectorizedFlowCreator(
-		nil /* flowBase */, newNoopFlowCreatorHelper(), nil /* componentCreator */, false, false,
-		nil, &execinfra.RowChannel{}, &fakeBatchReceiver{}, flowCtx.Cfg.PodNodeDialer, execinfrapb.FlowID{}, colcontainer.DiskQueueCfg{},
-		flowCtx.Cfg.VecFDSemaphore, flowCtx.NewTypeResolver(flowCtx.Txn),
-		admission.WorkInfo{},
+		nil /* flowBase */, newNoopFlowCreatorHelper(), nil, /* componentCreator */
+		recordingStats, false /* isGatewayNode */, nil, /* waitGroup */
+		&execinfra.RowChannel{}, &fakeBatchReceiver{}, flowCtx.Cfg.PodNodeDialer,
+		execinfrapb.FlowID{}, colcontainer.DiskQueueCfg{}, flowCtx.Cfg.VecFDSemaphore,
+		flowCtx.NewTypeResolver(flowCtx.Txn), admission.WorkInfo{},
 	)
 	opChains, _, err = creator.setupFlow(ctx, flowCtx, flow.Processors, localProcessors, nil /*localVectorSources*/, fuseOpt)
 	cleanup = func() {
@@ -101,6 +103,7 @@ func ExplainVec(
 	gatewaySQLInstanceID base.SQLInstanceID,
 	verbose bool,
 	distributed bool,
+	recordingStats bool,
 ) ([]string, error) {
 	tp := treeprinter.NewWithStyle(treeprinter.CompactStyle)
 	root := tp.Child("│")
@@ -122,7 +125,7 @@ func ExplainVec(
 			sort.Slice(sortedFlows, func(i, j int) bool { return sortedFlows[i].sqlInstanceID < sortedFlows[j].sqlInstanceID })
 			for _, flow := range sortedFlows {
 				var cleanup func()
-				opChains, cleanup, err = convertToVecTree(ctx, flowCtx, flow.flow, localProcessors, !distributed)
+				opChains, cleanup, err = convertToVecTree(ctx, flowCtx, flow.flow, localProcessors, !distributed, recordingStats)
 				// We need to delay the cleanup until after the tree has been
 				// formatted.
 				defer cleanup()

pkg/sql/distsql_physical_planner.go

@@ -918,10 +918,13 @@ func (p *PlanningCtx) getDefaultSaveFlowsFunc(
 			flowCtx, cleanup := newFlowCtxForExplainPurposes(ctx, p, planner)
 			defer cleanup()
 			getExplain := func(verbose bool) []string {
+				// When we're collecting the bundle, we're always recording the
+				// stats.
+				const recordingStats = true
 				explain, err := colflow.ExplainVec(
 					ctx, flowCtx, flows, p.infra.LocalProcessors, opChains,
 					planner.extendedEvalCtx.DistSQLPlanner.gatewaySQLInstanceID,
-					verbose, planner.curPlan.flags.IsDistributed(),
+					verbose, planner.curPlan.flags.IsDistributed(), recordingStats,
 				)
 				if err != nil {
 					// In some edge cases (like when subqueries are present or

pkg/sql/explain_vec.go

@@ -76,9 +76,13 @@ func (n *explainVecNode) startExec(params runParams) error {
 	}
 	verbose := n.options.Flags[tree.ExplainFlagVerbose]
 	willDistribute := physPlan.Distribution.WillDistribute()
+	// When running EXPLAIN (VEC) we choose the option of "not recording stats"
+	// since we don't know whether the next invocation of the explained
+	// statement would result in the collection of execution stats or not.
+	const recordingStats = false
 	n.run.lines, err = colflow.ExplainVec(
 		params.ctx, flowCtx, flows, physPlan.LocalProcessors, nil, /* opChains */
-		distSQLPlanner.gatewaySQLInstanceID, verbose, willDistribute,
+		distSQLPlanner.gatewaySQLInstanceID, verbose, willDistribute, recordingStats,
 	)
 	if err != nil {
 		return err

pkg/testutils/testcluster/testcluster.go

@@ -852,6 +852,14 @@ func (tc *TestCluster) WaitForVoters(
 	return tc.waitForNewReplicas(startKey, true /* waitForVoter */, targets...)
 }
 
+// WaitForVotersOrFatal is the same as WaitForVoters but it will Fatal the test
+// on error.
+func (tc *TestCluster) WaitForVotersOrFatal(
+	t testing.TB, startKey roachpb.Key, targets ...roachpb.ReplicationTarget,
+) {
+	require.NoError(t, tc.WaitForVoters(startKey, targets...))
+}
+
 // waitForNewReplicas waits for each of the targets to have a fully initialized
 // replica of the range indicated by startKey.
 //