Skip to content

changefeedccl: block db-level changefeed creation for non-empty tableset#154163

Draft
log-head wants to merge 2 commits intocockroachdb:masterfrom
log-head:log-head/db-level-changefeed-empty-watcher-running
Draft

changefeedccl: block db-level changefeed creation for non-empty tableset#154163
log-head wants to merge 2 commits intocockroachdb:masterfrom
log-head:log-head/db-level-changefeed-empty-watcher-running

Conversation

@log-head
Copy link
Contributor

@log-head log-head commented Sep 25, 2025

On startup, the changefeed should not be started until there exist tables in the database to watch. If no tables exist at the time of startup, then a tableset watcher is created and polled; the changefeed will be started from where there is first a target table.

Fixes: #148833
Epic: CRDB-1421

Release note: none

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@log-head
Copy link
Contributor Author

This works if there's sufficient time between creating the db-level changefeed and going from 0->1 tables in the database. Otherwise, through planning the changefeed, the table descriptors are being fetched at times where they don't exist. I'm still fiddling with a follow-up commit to fix the timestamp where table descriptors are queried.

@log-head log-head requested review from a team, KeithCh and asg0451 and removed request for a team and KeithCh September 25, 2025 15:57
}
var watcher *tableset.Watcher
watcherChan := make(chan []tableset.TableDiff)
if targets.NumUniqueTables() == 0 {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be a more explicit way to tell whether this is a db-level changefeed.

schemaTSOverride = earliestTimestamp
fmt.Printf("schemaTSOverride: %s\n", schemaTSOverride)
for _, diff := range diffs {
if diff.AsOf.LessEq(earliestTimestamp) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add comment that this is changes only at this timestamp, so add then drop should not work.

if watcher == nil {
continue
}
unchanged, diffs, err := watcher.PopUnchangedUpTo(ctx, execCfg.Clock.Now())
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the time we're getting from t.C, rather than Clock.Now()

On startup, the changefeed should not be started until there exist
tables in the database to watch. If no tables exist at the time of
startup, then a tableset watcher is created and polled; the changefeed
will be started from where there is first a target table.

Fixes: cockroachdb#147371
Epic: CRDB-1421

Release note: none
… creation

When a changefeed is planned and executed, there are several places
where target table descriptors are fetched. With a db-level changefeed,
the set of tables can change during changefeed startup. Now, the
timestamp is set to the initial high water for retrieving table
descriptors.  Fixes: cockroachdb#154549 Epic: CRDB-1421

Release note: None
@log-head log-head removed the request for review from asg0451 October 24, 2025 20:09
@log-head log-head force-pushed the log-head/db-level-changefeed-empty-watcher-running branch from fdc8d53 to b1e8e86 Compare October 24, 2025 20:14
@github-actions
Copy link

Potential Bug(s) Detected

The three-stage Claude Code analysis has identified potential bug(s) in this PR that may warrant investigation.

Next Steps:
Please review the detailed findings in the workflow run.

After you review the findings, please tag the issue as follows:

  • If the detected issue is real or was helpful in any way, please tag the issue with O-AI-Review-Real-Issue-Found
  • If the detected issue was not helpful in any way, please tag the issue with O-AI-Review-Not-Helpful

@github-actions github-actions bot added the o-AI-Review-Potential-Issue-Detected AI reviewer found potential issue. Never assign manually—auto-applied by GH action only. label Oct 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

o-AI-Review-Potential-Issue-Detected AI reviewer found potential issue. Never assign manually—auto-applied by GH action only.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

changefeedccl: db-level feeds: empty feed hibernation

2 participants