-
Notifications
You must be signed in to change notification settings - Fork 4k
changefeedccl: prep logging channel migration of changefeed events #151807
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
Conversation
ceffa74
to
1aec79f
Compare
1aec79f
to
ce204e5
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.
Nice! I'm curious whether the cluster setting log.channel_compatibility_mode.enabled
will be used to gate more channels than just the new CHANGEFEED channel?
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @aa-joshi, @Abhinav1299, @arjunmahishi, and @dhartunian)
pkg/util/log/eventpb/changefeed_events.proto
line 15 at r1 (raw file):
// TODO (#151948): Update channel from TELEMETRY to CHANGEFEED once the // `log.channel_compatibility_mode_enabled` cluster setting is set to false // cluster setting is set to false by default.
minor nit: repeated phrase
pkg/ccl/changefeedccl/changefeed_test.go
line 10791 at r1 (raw file):
closeFeed(t, coreFeed) failLogs := waitForLogs(t, &s.Server.ClusterSettings().SV) //failLogs := waitçorLogs(t, beforeCoreSinkClose)
nit: leftover debugging comment?
In v26.1, changefeed events will be moved from the TELEMETRY logging channel to the CHANGEFEED logging channel. This commit gates this migration under the cluster setting: `log.channel_compatibility_mode.enabled` and will log these events to the CHANGEFEED channel if this setting is set to false. Users can set this setting to false in their clusters to validate, test, and identify potential downstream impacts to their logging setups and pipelines. Epic: CRDB-53410 Part of: CRDB-53412 Release note (ops change): changefeed events will be moved to the CHANGEFEED channel in 26.1. In order to test the impact of these changes, users can set the setting: `log.channel_compatibility_mode.enabled` to false. Note that this will cause these logs to start logging in the CHANGEFEED channel so this shouldn't be tested in a production environment.
ce204e5
to
267e0fe
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.
Yup, there are some logs #151949 and #151827 also include events that are being moved to different logging channels. Do you have any concerns about it?
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @aa-joshi, @Abhinav1299, @arjunmahishi, and @dhartunian)
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.
No, I was just curious because of the generic naming. Thanks!
@rharding6373 reviewed 5 of 8 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @aa-joshi, @Abhinav1299, @arjunmahishi, and @dhartunian)
TFTR! bors r+ |
In v26.1, changefeed events will be moved from the TELEMETRY logging
channel to the CHANGEFEED logging channel.
This commit gates this migration under the cluster setting:
log.channel_compatibility_mode.enabled
and will log these eventsto the CHANGEFEED channel if this setting is set to false. Users can
set this setting to false in their clusters to validate, test, and
identify potential downstream impacts to their logging setups and
pipelines.
Epic: CRDB-53410
Part of: CRDB-53412
Release note (ops change): changefeed events will be moved to the
CHANGEFEED channel in 26.1. In order to test the impact of these
changes, users can set the setting:
log.channel_compatibility_mode.enabled
to false. Note that thiswill cause these logs to start logging in the CHANGEFEED channel so
this shouldn't be tested in a production environment.
This is a stacked PR, only the last commit should be reviewed