Skip to content

Commit fe64979

Browse files
committed
changefeedccl: add test-only assertion for checkpoint fields invariant
This patch adds a test-only assertion that we won't ever see both checkpoint fields set on a changefeed job progress struct. It is test-only because a bug that existed on earlier versions of 25.2 could cause both checkpoint fields to be set and so the production code continues to discard the legacy checkpoint when that happens. Release note: None
1 parent 7546004 commit fe64979

File tree

3 files changed

+44
-9
lines changed

3 files changed

+44
-9
lines changed

pkg/ccl/changefeedccl/changefeed_dist.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,11 +277,15 @@ func startDistChangefeed(
277277
return errors.AssertionFailedf("both legacy and current checkpoint set on " +
278278
"changefeed job progress and legacy checkpoint has later timestamp")
279279
}
280-
// This should be an assertion failure but unfortunately due to a bug
280+
// This should always be an assertion failure but unfortunately due to a bug
281281
// that was included in earlier versions of 25.2 (#148620), we may fail
282282
// to clear the legacy checkpoint when we start writing the new one.
283283
// We instead discard the legacy checkpoint here and it will eventually be
284284
// cleared once the cluster is running a newer patch release with the fix.
285+
if buildutil.CrdbTestBuild {
286+
return errors.AssertionFailedf("both legacy and current checkpoint set on " +
287+
"changefeed job progress")
288+
}
285289
log.Warningf(ctx, "both legacy and current checkpoint set on changefeed job progress; "+
286290
"discarding legacy checkpoint")
287291
legacyCheckpoint = nil

pkg/ccl/changefeedccl/changefeed_test.go

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11874,13 +11874,9 @@ func TestChangefeedResumeWithBothLegacyAndCurrentCheckpoint(t *testing.T) {
1187411874
require.NoError(t, err)
1187511875

1187611876
sqlDB.Exec(t, `RESUME JOB $1`, jobFeed.JobID())
11877-
waitForJobState(sqlDB, t, jobFeed.JobID(), jobs.StateRunning)
11878-
11879-
// Wait for highwater to advance past the current time.
11880-
var tsStr string
11881-
sqlDB.QueryRow(t, `SELECT cluster_logical_timestamp()`).Scan(&tsStr)
11882-
ts := parseTimeToHLC(t, tsStr)
11883-
require.NoError(t, jobFeed.WaitForHighWaterMark(ts))
11877+
waitForJobState(sqlDB, t, jobFeed.JobID(), jobs.StateFailed)
11878+
require.ErrorContains(t, jobFeed.FetchTerminalJobErr(),
11879+
"both legacy and current checkpoint set on changefeed job progress")
1188411880
}
1188511881

1188611882
cdcTest(t, testFn, feedTestEnterpriseSinks)

pkg/cmd/roachtest/tests/mixed_version_cdc.go

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -672,5 +672,40 @@ func runCDCMixedVersionCheckpointing(ctx context.Context, t test.Test, c cluster
672672
mvt.InMixedVersion("wait and validate", tester.waitAndValidate)
673673
mvt.AfterUpgradeFinalized("wait and validate", tester.waitAndValidate)
674674

675-
mvt.Run()
675+
// Run the test.
676+
plan, err := mvt.RunE()
677+
if err != nil {
678+
isAffectedBy148620 := func(plan *mixedversion.TestPlan) bool {
679+
if plan == nil {
680+
return false
681+
}
682+
for _, v := range []string{"v25.2.0", "v25.2.1", "v25.2.2"} {
683+
version := clusterupgrade.MustParseVersion(v)
684+
for _, planVersion := range plan.Versions() {
685+
if planVersion.Equal(version) {
686+
return true
687+
}
688+
}
689+
}
690+
return false
691+
}
692+
693+
isExpectedErrorDueTo148620 := func(err error) bool {
694+
for _, s := range []string{
695+
"both legacy and current checkpoint set on change aggregator spec",
696+
"both legacy and current checkpoint set on changefeed job progress",
697+
} {
698+
if strings.Contains(err.Error(), s) {
699+
return true
700+
}
701+
}
702+
return false
703+
}
704+
705+
if isAffectedBy148620(plan) && isExpectedErrorDueTo148620(err) {
706+
t.Skipf("expected error due to #148620: %s", err)
707+
}
708+
709+
t.Fatal(err)
710+
}
676711
}

0 commit comments

Comments
 (0)