Skip to content

Conversation

@wenyihu6
Copy link
Contributor

This commit enables rangefeed when the --changefeed flag is specified with the
workload command.

Epic: https://cockroachlabs.atlassian.net/browse/CRDB-55203
Release note: none

@wenyihu6 wenyihu6 requested a review from a team as a code owner October 21, 2025 20:31
@wenyihu6 wenyihu6 requested review from golgeek, stevendanna and williamchoe3 and removed request for a team October 21, 2025 20:31
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Collaborator

@stevendanna stevendanna left a comment

Choose a reason for hiding this comment

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

Thanks!

Comment on lines 89 to 90
log.Dev.Infof(ctx, "cursor: %s", cursorStr)

Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems duplicative of some of the log lines below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

Comment on lines 57 to 59
if _, err := conn.Exec(ctx, "SET CLUSTER SETTING kv.rangefeed.enabled = true"); err != nil {
return err
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is going to fail if run against a secondary tenant.

Maybe before trying to set it, we could check if it is already set. The check should work in a secondary tenant and secondary tenants generally will have rangefeeds enabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TIL - is this because of

settings.SystemVisible,
or something else? Changed it so that it only tries to enable if not enabled already.

Comment on lines 135 to 136
if epoch, parseErr := strconv.ParseInt(cursorStr, 10, 64); parseErr == nil {
t := time.Unix(epoch, 0).UTC()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be better to use hlc.ParseHLC here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dropped this commit. Going to add it as part of #156276.

This commit enables rangefeed when the --changefeed flag is specified with the
workload command.
@wenyihu6
Copy link
Contributor Author

TFTR!

bors r+

@craig
Copy link
Contributor

craig bot commented Oct 27, 2025

@craig craig bot merged commit 75b649c into cockroachdb:master Oct 27, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants