-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cli/demo,cdc: enable rangefeeds by default #83282
cli/demo,cdc: enable rangefeeds by default #83282
Conversation
fee96b7
to
1635252
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious: what's the benefit of enabling the user to disable the feature?
Reviewed 7 of 8 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @amruss)
Fixes cockroachdb#82719, which made it annoying to run changefeeds in cockroach demo as you need to enable rangefeeds twice, once at the system level. Now cockroach demo enables rangefeeds at startup. You can override this behavior for performance or to demo the process of enabling rangefeeds with the flag `--auto-enable-rangefeeds=false`. Release note (cli change): cockroach demo now enables rangefeeds by default. You can restore the old behavior with --auto-enable-rangefeeds=false.
1635252
to
b0d2e6d
Compare
I feel like I've seen (or done) a changefeed demo at least once where the user hits the "you need this cluster setting" error deliberately, talks through the implications, and then demonstrates setting it to true. Also if you're doing something performance-sensitive with a workload it could be annoying to have to turn it off via sql. |
Thanks for the reviews! |
bors r=[knz,stevendanna] |
Build succeeded: |
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from b0d2e6d to blathers/backport-release-22.1-83282: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 22.1.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
Fixes #82719,
which made it annoying to run changefeeds in cockroach demo
as you need to enable rangefeeds twice, once at the system
level. Now cockroach demo enables rangefeeds at startup.
You can override this behavior for performance or to demo
the process of enabling rangefeeds with the flag
--auto-enable-rangefeeds=false
.Release note (cli change): cockroach demo now enables rangefeeds by default. You can restore the old behavior with --auto-enable-rangefeeds=false.