Merge #147930 #147942 #147946 #147989 #148002

147930: clusterversion: rm V25_1_MoveRaftTruncatedState r=RaduBerinde,arulajmani a=pav-kv

Part of #97613

147942: sql/export: extract EXPORT files from sql/importer r=yuzefovich a=yuzefovich

There is no point in keeping EXPORT-related files in the same package as IMPORT-related ones. This is beneficial also due to different owners. The only downside is that backports for EXPORT changes won't be clean for a couple of releases.

I copied using "large" pool for "heavy" test configs (i.e. duress).

Epic: None
Release note: None

147946: sql: remove miscellaneous unused functions and objects r=yuzefovich a=yuzefovich

These were exported, so our unused linter didn't catch them.

Epic: None
Release note: None

147989: crosscluster/logical: wipe status on clean pause r=jeffswenson a=msbutler

If the user pauses the LDR job, the status may be stale, like "catching up on 110 out of 230 ranges"

Epic: none

Release note: none

148002: Use make target to bump version in helm chart r=rail a=prafull01

Fixes: cockroachdb/helm-charts#525
Epic: none

Co-authored-by: Pavel Kalinnikov <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Michael Butler <[email protected]>
Co-authored-by: prafull01 <[email protected]>

Author: 5 people

.github/CODEOWNERS

@@ -77,7 +77,6 @@
 #!/pkg/sql/BUILD.bazel       @cockroachdb/sql-queries-noreview
 
 /pkg/sql/importer/           @cockroachdb/sql-queries-prs
-/pkg/sql/importer/export*    @cockroachdb/cdc-prs
 /pkg/ccl/importerccl/        @cockroachdb/sql-queries-prs
 
 /pkg/sql/appstatspb           @cockroachdb/obs-prs
@@ -261,6 +260,7 @@
 
 /pkg/ccl/jobsccl/            @cockroachdb/jobs-prs @cockroachdb/disaster-recovery
 /pkg/ccl/changefeedccl/      @cockroachdb/cdc-prs
+/pkg/sql/export/             @cockroachdb/cdc-prs
 
 /pkg/crosscluster/       @cockroachdb/disaster-recovery
 /pkg/backup/                 @cockroachdb/disaster-recovery

pkg/BUILD.bazel

@@ -482,6 +482,7 @@ ALL_TESTS = [
     "//pkg/sql/execinfrapb:execinfrapb_test",
     "//pkg/sql/execstats:execstats_disallowed_imports_test",
     "//pkg/sql/execstats:execstats_test",
+    "//pkg/sql/export:export_test",
     "//pkg/sql/exprutil:exprutil_test",
     "//pkg/sql/flowinfra:flowinfra_disallowed_imports_test",
     "//pkg/sql/flowinfra:flowinfra_test",
@@ -1988,6 +1989,8 @@ GO_TARGETS = [
     "//pkg/sql/execstats:execstats",
     "//pkg/sql/execstats:execstats_test",
     "//pkg/sql/execversion:execversion",
+    "//pkg/sql/export:export",
+    "//pkg/sql/export:export_test",
     "//pkg/sql/exprutil:evalexpr",
     "//pkg/sql/exprutil:exprutil",
     "//pkg/sql/exprutil:exprutil_test",

pkg/clusterversion/cockroach_versions.go

@@ -190,10 +190,6 @@ const (
 	// TODO_Delete_V25_1_AddJobsTables added new jobs tables.
 	TODO_Delete_V25_1_AddJobsTables
 
-	// TODO_Delete_V25_1_MoveRaftTruncatedState moves the RaftTruncatedState in eval result
-	// from ReplicaState to its own field.
-	TODO_Delete_V25_1_MoveRaftTruncatedState
-
 	// TODO_Delete_V25_1_AddRangeForceFlushKey adds the RangeForceFlushKey, a replicated
 	// range-ID local key, which is written below raft.
 	TODO_Delete_V25_1_AddRangeForceFlushKey
@@ -276,7 +272,6 @@ var versionTable = [numKeys]roachpb.Version{
 	TODO_Delete_V25_1_Start: {Major: 24, Minor: 3, Internal: 2},
 
 	TODO_Delete_V25_1_AddJobsTables:             {Major: 24, Minor: 3, Internal: 4},
-	TODO_Delete_V25_1_MoveRaftTruncatedState:    {Major: 24, Minor: 3, Internal: 6},
 	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},

pkg/cmd/release/update_helm.go

@@ -13,18 +13,14 @@ import (
 
 // updateHelm runs commands in the helm-charts repo in order to update the cockroachdb version.
 func updateHelm(workDir string, version string) error {
-	commands := []*exec.Cmd{
-		exec.Command("bazel", "build", "//build"),
-		// Helm charts use the version without the "v" prefix, but the bumper trims it if present.
-		exec.Command("sh", "-c", fmt.Sprintf("$(bazel info bazel-bin)/build/build_/build bump %s", version)),
-	}
-	for _, cmd := range commands {
-		cmd.Dir = workDir
-		out, err := cmd.CombinedOutput()
-		if err != nil {
-			return fmt.Errorf("failed running '%s' with message '%s': %w", cmd.String(), string(out), err)
-		}
-		log.Printf("ran '%s': %s\n", cmd.String(), string(out))
+	// Helm charts use the version without the "v" prefix, but the bumper trims it if present.
+	cmd := exec.Command("sh", "-c", fmt.Sprintf("make bump/%s", version))
+	cmd.Dir = workDir
+	out, err := cmd.CombinedOutput()
+	if err != nil {
+		return fmt.Errorf("failed running '%s' with message '%s': %w", cmd.String(), string(out), err)
 	}
+	log.Printf("ran '%s': %s\n", cmd.String(), string(out))
+
 	return nil
 }

pkg/crosscluster/logical/logical_replication_job.go

@@ -98,6 +98,7 @@ func (r *logicalReplicationResumer) handleResumeError(
 	ctx context.Context, execCtx sql.JobExecContext, err error,
 ) error {
 	if err == nil {
+		r.updateStatusMessage(ctx, "")
 		return nil
 	}
 	if jobs.IsPermanentJobError(err) {

pkg/kv/kvserver/batcheval/cmd_truncate_log.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"
@@ -125,16 +124,11 @@ func TruncateLog(
 	}
 	ms.SysBytes = -ms.SysBytes // simulate the deletion
 
-	tState := &kvserverpb.RaftTruncatedState{
+	var pd result.Result
+	pd.Replicated.SetRaftTruncatedState(&kvserverpb.RaftTruncatedState{
 		Index: args.Index - 1,
 		Term:  term,
-	}
-
-	var pd result.Result
-	pd.Replicated.SetRaftTruncatedState(tState,
-		cArgs.EvalCtx.ClusterSettings().Version.IsActive(
-			ctx, clusterversion.TODO_Delete_V25_1_MoveRaftTruncatedState),
-	)
+	})
 	pd.Replicated.RaftLogDelta = ms.SysBytes
 	pd.Replicated.RaftExpectedFirstIndex = firstIndex
 	return pd, nil

pkg/kv/kvserver/kvserverpb/proposer_kv.go

@@ -81,24 +81,17 @@ func (r *ReplicatedEvalResult) IsTrivial() bool {
 }
 
 // SetRaftTruncatedState puts the given RaftTruncatedState into this evaluation
-// result. Since v25.1, it uses r.RaftTruncatedState field, and falls back to
-// r.State.TruncatedState for backwards compatibility in mixed-version clusters.
-//
-// TODO(#97613): use r.RaftTruncatedState unconditionally when the ReplicaState
-// field is removed.
-func (r *ReplicatedEvalResult) SetRaftTruncatedState(ts *RaftTruncatedState, v25dot1 bool) {
-	if v25dot1 {
-		r.RaftTruncatedState = ts
-	} else if r.State != nil {
-		r.State.TruncatedState = ts
-	} else {
-		r.State = &ReplicaState{TruncatedState: ts}
-	}
+// result.
+func (r *ReplicatedEvalResult) SetRaftTruncatedState(ts *RaftTruncatedState) {
+	r.RaftTruncatedState = ts
 }
 
 // GetRaftTruncatedState returns the RaftTruncatedState present in this
 // evaluation result. Since v25.1, it can be present in one of the two places.
-// See SetRaftTruncatedState.
+//
+// TODO(#97613): use r.RaftTruncatedState unconditionally when the truncated
+// state field in ReplicaState is removed and can't be set. Currently,
+// historical proposals can still have it.
 func (r *ReplicatedEvalResult) GetRaftTruncatedState() *RaftTruncatedState {
 	if ts := r.RaftTruncatedState; ts != nil {
 		return ts

pkg/kv/kvserver/replica_application_state_machine_test.go

@@ -159,7 +159,7 @@ func TestReplicaStateMachineRaftLogTruncationStronglyCoupled(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
-	runTest := func(t *testing.T, accurate, v25dot1 bool) {
+	runTest := func(t *testing.T, accurate bool) {
 		tc := testContext{}
 		ctx := context.Background()
 		stopper := stop.NewStopper()
@@ -201,7 +201,7 @@ func TestReplicaStateMachineRaftLogTruncationStronglyCoupled(t *testing.T) {
 		}
 		evalResult.SetRaftTruncatedState(&kvserverpb.RaftTruncatedState{
 			Index: truncatedIndex + 1,
-		}, v25dot1)
+		})
 
 		ent := &raftlog.Entry{
 			Entry: raftpb.Entry{
@@ -258,17 +258,15 @@ func TestReplicaStateMachineRaftLogTruncationStronglyCoupled(t *testing.T) {
 	}
 
 	testutils.RunTrueAndFalse(t, "accurate first index", func(t *testing.T, accurate bool) {
-		testutils.RunTrueAndFalse(t, "v25.1", func(t *testing.T, v25dot1 bool) {
-			runTest(t, accurate, v25dot1)
-		})
+		runTest(t, accurate)
 	})
 }
 
 func TestReplicaStateMachineRaftLogTruncationLooselyCoupled(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
-	runTest := func(t *testing.T, accurate, v25dot1 bool) {
+	runTest := func(t *testing.T, accurate bool) {
 		tc := testContext{}
 		ctx := context.Background()
 		stopper := stop.NewStopper()
@@ -329,7 +327,7 @@ func TestReplicaStateMachineRaftLogTruncationLooselyCoupled(t *testing.T) {
 			}
 			evalResult.SetRaftTruncatedState(&kvserverpb.RaftTruncatedState{
 				Index: truncatedIndex + 1,
-			}, v25dot1)
+			})
 			ent := &raftlog.Entry{
 				Entry: raftpb.Entry{
 					Index: uint64(raftAppliedIndex + 1),
@@ -406,9 +404,7 @@ func TestReplicaStateMachineRaftLogTruncationLooselyCoupled(t *testing.T) {
 	}
 
 	testutils.RunTrueAndFalse(t, "accurate first index", func(t *testing.T, accurate bool) {
-		testutils.RunTrueAndFalse(t, "v25.1", func(t *testing.T, v25dot1 bool) {
-			runTest(t, accurate, v25dot1)
-		})
+		runTest(t, accurate)
 	})
 }
 

pkg/settings/registry.go

@@ -267,6 +267,9 @@ var retiredSettings = map[InternalKey]struct{}{
 	"changefeed.new_pubsub_sink_enabled":                     {},
 	"changefeed.new_pubsub_sink.enabled":                     {},
 	"logical_replication.consumer.use_implicit_txns.enabled": {},
+
+	// removed as of 25.3
+	"sql.metrics.max_stmt_fingerprints_per_explicit_txn": {},
 }
 
 // grandfatheredDefaultSettings is the list of "grandfathered" existing sql.defaults

pkg/sql/catalog/post_deserialization_changes.go

@@ -79,10 +79,6 @@ const (
 	// field was unset and that the MVCC timestamp value was assigned to it.
 	SetModTimeToMVCCTimestamp
 
-	// SetCreateAsOfTimeUsingModTime indicates that a table's CreateAsOfTime field
-	// was unset and the ModificationTime value was assigned to it.
-	SetCreateAsOfTimeUsingModTime
-
 	// SetSystemDatabaseDescriptorVersion indicates that the system database
 	// descriptor did not have its version set.
 	SetSystemDatabaseDescriptorVersion