Merge pull request #154478 from aerfrei/remove-per-table-pts-25.4

release-25.4: changefeedccl: force-disable per-table PTS

Author: aerfrei

pkg/ccl/changefeedccl/changefeed_dist.go

@@ -461,7 +461,8 @@ func makePlan(
 		var progressConfig *execinfrapb.ChangefeedProgressConfig
 		if execCtx.ExecCfg().Settings.Version.IsActive(ctx, clusterversion.V25_4) {
 			perTableTrackingEnabled := changefeedbase.TrackPerTableProgress.Get(sv)
-			perTableProtectedTimestampsEnabled := changefeedbase.PerTableProtectedTimestamps.Get(sv)
+			// In 25.4 we are hard disabling per table protected timestamps.
+			perTableProtectedTimestampsEnabled := false
 			progressConfig = &execinfrapb.ChangefeedProgressConfig{
 				PerTableTracking: perTableTrackingEnabled,
 				// If the per table pts flag was turned on between changefeed creation and now,

pkg/ccl/changefeedccl/changefeed_stmt.go

@@ -340,7 +340,8 @@ func changefeedPlanHook(
 
 			// We do not yet have the progress config here, so we need to check the settings directly.
 			perTableTrackingEnabled := changefeedbase.TrackPerTableProgress.Get(&p.ExecCfg().Settings.SV)
-			perTableProtectedTimestampsEnabled := changefeedbase.PerTableProtectedTimestamps.Get(&p.ExecCfg().Settings.SV)
+			// In 25.4 we are hard disabling per table protected timestamps.
+			perTableProtectedTimestampsEnabled := false
 			usingPerTablePTS := perTableTrackingEnabled && perTableProtectedTimestampsEnabled
 			if usingPerTablePTS {
 				protectedTimestampRecords := make(map[descpb.ID]uuid.UUID)

pkg/ccl/changefeedccl/changefeed_test.go

@@ -12065,7 +12065,9 @@ WITH resolved='10ms', min_checkpoint_frequency='10ms', no_initial_scan`
 			require.NoError(t, err)
 			return ts
 		}
-		perTablePTSEnabled := changefeedbase.PerTableProtectedTimestamps.Get(&s.Server.ClusterSettings().SV)
+
+		// In 25.4 we are hard disabling per table protected timestamps.
+		perTablePTSEnabled := false
 		perTableProgressEnabled := changefeedbase.TrackPerTableProgress.Get(&s.Server.ClusterSettings().SV)
 		getPTS := func() hlc.Timestamp {
 			if perTablePTSEnabled && perTableProgressEnabled {

pkg/ccl/changefeedccl/changefeedbase/settings.go

@@ -215,15 +215,6 @@ var ProtectTimestampLag = settings.RegisterDurationSetting(
 	10*time.Minute,
 	settings.PositiveDuration)
 
-// PerTableProtectedTimestamps enables per-table protected timestamp records
-// instead of a single record for all tables in a changefeed.
-var PerTableProtectedTimestamps = settings.RegisterBoolSetting(
-	settings.ApplicationLevel,
-	"changefeed.protect_timestamp.per_table.enabled",
-	"if true, creates separate protected timestamp records for each table in a changefeed; "+
-		"if false, uses a single protected timestamp record for all tables",
-	metamorphic.ConstantWithTestBool("changefeed.protect_timestamp.per_table.enabled", false))
-
 // MaxProtectedTimestampAge controls the frequency of protected timestamp record updates
 var MaxProtectedTimestampAge = settings.RegisterDurationSetting(
 	settings.ApplicationLevel,

pkg/ccl/changefeedccl/protected_timestamps_test.go

@@ -43,6 +43,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
 	"github.com/cockroachdb/cockroach/pkg/testutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
+	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
 	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
@@ -404,9 +405,8 @@ func TestChangefeedAlterPTS(t *testing.T) {
 
 		_, _ = expectResolvedTimestamp(t, f2)
 
-		perTablePTSEnabled :=
-			changefeedbase.PerTableProtectedTimestamps.Get(&s.Server.ClusterSettings().SV) &&
-				changefeedbase.TrackPerTableProgress.Get(&s.Server.ClusterSettings().SV)
+		// In 25.4 we are hard disabling per table protected timestamps.
+		perTablePTSEnabled := false
 
 		if perTablePTSEnabled {
 			eFeed, ok := f2.(cdctest.EnterpriseTestFeed)
@@ -751,8 +751,6 @@ func TestChangefeedMigratesProtectedTimestampTargets(t *testing.T) {
 			context.Background(), &s.Server.ClusterSettings().SV, ptsInterval)
 		changefeedbase.ProtectTimestampLag.Override(
 			context.Background(), &s.Server.ClusterSettings().SV, ptsInterval)
-		changefeedbase.PerTableProtectedTimestamps.Override(
-			context.Background(), &s.Server.ClusterSettings().SV, false)
 
 		sqlDB := sqlutils.MakeSQLRunner(s.DB)
 		sysDB := sqlutils.MakeSQLRunner(s.SystemServer.SQLConn(t))
@@ -870,12 +868,6 @@ func TestChangefeedMigratesProtectedTimestamps(t *testing.T) {
 		changefeedbase.ProtectTimestampLag.Override(
 			context.Background(), &s.Server.ClusterSettings().SV, ptsInterval)
 
-		// Since old style PTS records should not be created when per-table PTS records are enabled,
-		// we disable them for this test. Per-table PTS breaks assumptions about where we can find
-		// the PTS record uuids.
-		changefeedbase.PerTableProtectedTimestamps.Override(
-			context.Background(), &s.Server.ClusterSettings().SV, false)
-
 		sqlDB := sqlutils.MakeSQLRunner(s.DB)
 		sysDB := sqlutils.MakeSQLRunner(s.SystemServer.SQLConn(t))
 
@@ -977,10 +969,6 @@ func TestChangefeedProtectedTimestampUpdateForMultipleTables(t *testing.T) {
 		changefeedbase.ProtectTimestampLag.Override(
 			context.Background(), &s.Server.ClusterSettings().SV, 10*time.Hour)
 
-		// Ensure we use legacy single protected timestamp behavior for this test
-		changefeedbase.PerTableProtectedTimestamps.Override(
-			context.Background(), &s.Server.ClusterSettings().SV, false)
-
 		sqlDB.Exec(t, `CREATE TABLE foo (id INT)`)
 		sqlDB.Exec(t, `CREATE TABLE bar (id INT)`)
 		registry := s.Server.JobRegistry().(*jobs.Registry)
@@ -1087,12 +1075,14 @@ func TestChangefeedPerTableProtectedTimestampProgression(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
+	skip.WithIssue(t, 93793, "unreleased feature as of 25.4")
+
 	testFn := func(t *testing.T, s TestServer, f cdctest.TestFeedFactory) {
 		sqlDB := sqlutils.MakeSQLRunner(s.DB)
 
-		// Enable per-table protected timestamps and progress tracking
-		changefeedbase.PerTableProtectedTimestamps.Override(
-			context.Background(), &s.Server.ClusterSettings().SV, true)
+		// It's not possible to enable per-table protected timestamps
+		// since it's disabled in 25.4. This is where we will enable the setting
+		// for 26.1.
 		changefeedbase.TrackPerTableProgress.Override(
 			context.Background(), &s.Server.ClusterSettings().SV, true)
 

pkg/cmd/roachtest/tests/cdc.go

@@ -1837,10 +1837,6 @@ func runCDCMultiTablePTSBenchmark(
 		numRanges = params.numRanges
 	}
 
-	if _, err := db.Exec("SET CLUSTER SETTING changefeed.protect_timestamp.per_table.enabled = $1", params.perTablePTS); err != nil {
-		t.Fatalf("failed to set per-table protected timestamps: %v", err)
-	}
-
 	initCmd := fmt.Sprintf("./cockroach workload init bank --rows=%d --ranges=%d --tables=%d {pgurl%s}",
 		params.numRows, numRanges, params.numTables, ct.crdbNodes.RandNode())
 	if err := c.RunE(ctx, option.WithNodes(ct.workloadNode), initCmd); err != nil {
@@ -3106,7 +3102,7 @@ func registerCDC(r registry.Registry) {
 		CompatibleClouds: registry.AllExceptIBM,
 		Run:              runMessageTooLarge,
 	})
-	for _, perTablePTS := range []bool{false, true} {
+	for _, perTablePTS := range []bool{false} {
 		for _, config := range []struct {
 			numTables    int
 			numRanges    int