Merge #150592

150592: kv: clean up 25.1 gates r=RaduBerinde a=RaduBerinde

Epic: none
Release note: None

Co-authored-by: Radu Berinde <[email protected]>

Author: craig[bot]

pkg/clusterversion/cockroach_versions.go

@@ -185,26 +185,9 @@ const (
 	// V24_3 is CockroachDB v24.3. It's used for all v24.3.x patch releases.
 	V24_3
 
-	TODO_Delete_V25_1_Start
-
-	// TODO_Delete_V25_1_AddRangeForceFlushKey adds the RangeForceFlushKey, a replicated
-	// range-ID local key, which is written below raft.
-	TODO_Delete_V25_1_AddRangeForceFlushKey
-
-	// TODO_Delete_V25_1_BatchStreamRPC adds the BatchStream RPC, which allows for more
-	// efficient Batch unary RPCs.
-	TODO_Delete_V25_1_BatchStreamRPC
-
-	// TODO_Delete_V25_1_PreparedTransactionsTable adds the system.prepared_transactions
-	// table. The table is used to store information about prepared transaction
-	// that are part of the XA two-phase commit protocol.
-	TODO_Delete_V25_1_PreparedTransactionsTable
-
 	// V25_1 is CockroachDB v25.1. It's used for all v25.1.x patch releases.
 	V25_1
 
-	TODO_Delete_V25_2_Start
-
 	// V25_2 is CockroachDB v25.2. It's used for all v25.2.x patch releases.
 	V25_2
 
@@ -262,19 +245,7 @@ var versionTable = [numKeys]roachpb.Version{
 	V24_1: {Major: 24, Minor: 1, Internal: 0},
 	V24_2: {Major: 24, Minor: 2, Internal: 0},
 	V24_3: {Major: 24, Minor: 3, Internal: 0},
-
-	// v25.1 versions. Internal versions must be even.
-	TODO_Delete_V25_1_Start: {Major: 24, Minor: 3, Internal: 2},
-
-	TODO_Delete_V25_1_AddRangeForceFlushKey:     {Major: 24, Minor: 3, Internal: 8},
-	TODO_Delete_V25_1_BatchStreamRPC:            {Major: 24, Minor: 3, Internal: 10},
-	TODO_Delete_V25_1_PreparedTransactionsTable: {Major: 24, Minor: 3, Internal: 12},
-
 	V25_1: {Major: 25, Minor: 1, Internal: 0},
-
-	// v25.2 versions. Internal versions must be even.
-	TODO_Delete_V25_2_Start: {Major: 25, Minor: 1, Internal: 2},
-
 	V25_2: {Major: 25, Minor: 2, Internal: 0},
 
 	// v25.3 versions. Internal versions must be even.

pkg/kv/kvserver/batcheval/cmd_end_transaction.go

@@ -1505,9 +1505,7 @@ func splitTriggerHelper(
 	// replicas that already have the unsplit range, *and* these snapshots are
 	// rejected (which is very wasteful). See the long comment in
 	// split_delay_helper.go for more details.
-	if rec.ClusterSettings().Version.IsActive(ctx, clusterversion.TODO_Delete_V25_1_AddRangeForceFlushKey) {
-		pd.Replicated.DoTimelyApplicationToAllReplicas = true
-	}
+	pd.Replicated.DoTimelyApplicationToAllReplicas = true
 
 	pd.Local.Metrics = &result.Metrics{
 		SplitsWithEstimatedStats:     h.splitsWithEstimates,
@@ -1610,9 +1608,7 @@ func mergeTrigger(
 	// the merge distributed txn, when sending a kvpb.SubsumeRequest. But since
 	// we have force-flushed once during the merge txn anyway, we choose to
 	// complete the merge story and finish the merge on all replicas.
-	if rec.ClusterSettings().Version.IsActive(ctx, clusterversion.TODO_Delete_V25_1_AddRangeForceFlushKey) {
-		pd.Replicated.DoTimelyApplicationToAllReplicas = true
-	}
+	pd.Replicated.DoTimelyApplicationToAllReplicas = true
 
 	{
 		// If we have GC hints populated that means we are trying to perform

pkg/kv/kvserver/batcheval/cmd_migrate.go

@@ -9,7 +9,6 @@ import (
 	"context"
 	"time"
 
-	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/batcheval/result"
@@ -85,10 +84,7 @@ func Migrate(
 	// Set DoTimelyApplicationToAllReplicas so that migrates are applied on all
 	// replicas. This is done since MigrateRequests trigger a call to
 	// waitForApplication (see Replica.executeWriteBatch).
-	if cArgs.EvalCtx.ClusterSettings().Version.IsActive(ctx, clusterversion.TODO_Delete_V25_1_AddRangeForceFlushKey) ||
-		cArgs.EvalCtx.EvalKnobs().OverrideDoTimelyApplicationToAllReplicas {
-		pd.Replicated.DoTimelyApplicationToAllReplicas = true
-	}
+	pd.Replicated.DoTimelyApplicationToAllReplicas = true
 	return pd, nil
 }
 

pkg/kv/kvserver/batcheval/cmd_subsume.go

@@ -10,7 +10,6 @@ import (
 	"context"
 	"time"
 
-	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/batcheval/result"
@@ -245,9 +244,7 @@ func Subsume(
 	// Set DoTimelyApplicationToAllReplicas so that merges are applied on all
 	// replicas. This is needed since Replica.AdminMerge calls
 	// waitForApplication when sending a kvpb.SubsumeRequest.
-	if cArgs.EvalCtx.ClusterSettings().Version.IsActive(ctx, clusterversion.TODO_Delete_V25_1_AddRangeForceFlushKey) {
-		pd.Replicated.DoTimelyApplicationToAllReplicas = true
-	}
+	pd.Replicated.DoTimelyApplicationToAllReplicas = true
 	pd.Local.RepopulateSubsumeResponseLAI = args.PreserveUnreplicatedLocks
 	return pd, nil
 }

pkg/kv/kvserver/flow_control_integration_test.go

@@ -26,7 +26,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvflowcontrol"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvflowcontrol/kvflowinspectpb"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvflowcontrol/rac2"
-	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverbase"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/stateloader"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/server"
@@ -2849,15 +2848,6 @@ func TestFlowControlSendQueueRangeMigrate(t *testing.T) {
 					RaftReportUnreachableBypass: func(_ roachpb.ReplicaID) bool {
 						return true
 					},
-					EvalKnobs: kvserverbase.BatchEvalTestingKnobs{
-						// Because we are migrating from a version (currently) prior to the
-						// range force flush key version gate, we won't trigger the force
-						// flush via migrate until we're on the endV, which defeats the
-						// purpose of this test. We override the behavior here to allow the
-						// force flush to be triggered on the startV from a Migrate
-						// request.
-						OverrideDoTimelyApplicationToAllReplicas: true,
-					},
 					FlowControlTestingKnobs: &kvflowcontrol.TestingKnobs{
 						UseOnlyForScratchRanges: true,
 						OverrideTokenDeduction: func(tokens kvflowcontrol.Tokens) kvflowcontrol.Tokens {

pkg/kv/kvserver/kvserverbase/knobs.go

@@ -52,16 +52,6 @@ type BatchEvalTestingKnobs struct {
 	// useful in certain tests.
 	DisableTxnAutoGC bool
 
-	// OverrideDoTimelyApplicationToAllReplicas overrides the cluster version
-	// check for the timely replication directive which force flushes rac2 send
-	// queues to all replicas, if present for *Migrate* requests only. When set
-	// to true, the directive is always set, when set to false, the default
-	// behavior is used.
-	//
-	// NOTE: This currently only applies to Migrate requests and only ignores the
-	// cluster version.
-	OverrideDoTimelyApplicationToAllReplicas bool
-
 	// CommitTriggerError is called at commit triggers to simulate errors.
 	CommitTriggerError func() error
 

pkg/rpc/nodedialer/BUILD.bazel

@@ -10,7 +10,6 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//pkg/base",
-        "//pkg/clusterversion",
         "//pkg/kv/kvbase",
         "//pkg/kv/kvpb",
         "//pkg/roachpb",

pkg/rpc/nodedialer/nodedialer.go

@@ -12,7 +12,6 @@ import (
 	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/base"
-	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvbase"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
@@ -369,16 +368,7 @@ var batchStreamPoolingEnabled = settings.RegisterBoolSetting(
 )
 
 func shouldUseBatchStreamPoolClient(ctx context.Context, st *cluster.Settings) bool {
-	// NOTE: we use ActiveVersionOrEmpty(ctx).IsActive(...) instead of the more
-	// common IsActive(ctx, ...) to avoid a fatal error if an RPC is made before
-	// the cluster version is initialized.
-	if !st.Version.ActiveVersionOrEmpty(ctx).IsActive(clusterversion.TODO_Delete_V25_1_BatchStreamRPC) {
-		return false
-	}
-	if !batchStreamPoolingEnabled.Get(&st.SV) {
-		return false
-	}
-	return true
+	return batchStreamPoolingEnabled.Get(&st.SV)
 }
 
 // batchStreamPoolClient is a client that sends Batch RPCs using a pooled

pkg/sql/pg_catalog.go

@@ -16,7 +16,6 @@ import (
 	"time"
 	"unicode"
 
-	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/security/username"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
@@ -2459,11 +2458,6 @@ var pgCatalogPreparedXactsTable = virtualSchemaTable{
 https://www.postgresql.org/docs/9.6/view-pg-prepared-xacts.html`,
 	schema: vtable.PGCatalogPreparedXacts,
 	populate: func(ctx context.Context, p *planner, dbContext catalog.DatabaseDescriptor, addRow func(...tree.Datum) error) error {
-		if !p.IsActive(ctx, clusterversion.TODO_Delete_V25_1_PreparedTransactionsTable) {
-			// TODO(nvanbenschoten): Remove this logic when mixed-version support
-			// with v24.3 is no longer necessary.
-			return nil
-		}
 		rows, err := p.InternalSQLTxn().QueryBufferedEx(
 			ctx,
 			"select-prepared-transactions",

pkg/sql/two_phase_commit.go

@@ -8,7 +8,6 @@ package sql
 import (
 	"context"
 
-	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/kv"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
@@ -62,12 +61,6 @@ func (ex *connExecutor) execPrepareTransactionInOpenState(
 func (ex *connExecutor) execPrepareTransactionInOpenStateInternal(
 	ctx context.Context, s *tree.PrepareTransaction,
 ) error {
-	// TODO(nvanbenschoten): Remove this logic when mixed-version support with
-	// v24.3 is no longer necessary.
-	if !ex.planner.EvalContext().Settings.Version.IsActive(ctx, clusterversion.TODO_Delete_V25_1_PreparedTransactionsTable) {
-		return pgerror.Newf(pgcode.FeatureNotSupported, "PREPARE TRANSACTION unsupported in mixed-version cluster")
-	}
-
 	// TODO(nvanbenschoten): why are these needed here (and in the equivalent
 	// functions for commit and rollback)? Shouldn't they be handled by
 	// connExecutor.resetExtraTxnState?
@@ -213,12 +206,6 @@ func (p *planner) endPreparedTxnNode(globalID *tree.StrVal, commit bool) *endPre
 }
 
 func (f *endPreparedTxnNode) startExec(params runParams) error {
-	// TODO(nvanbenschoten): Remove this logic when mixed-version support with
-	// v24.3 is no longer necessary.
-	if !params.EvalContext().Settings.Version.IsActive(params.ctx, clusterversion.TODO_Delete_V25_1_PreparedTransactionsTable) {
-		return pgerror.Newf(pgcode.FeatureNotSupported, "%s unsupported in mixed-version cluster", f.stmtName())
-	}
-
 	if err := f.checkNoActiveTxn(params); err != nil {
 		return err
 	}