-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
release-24.1: changefeedccl: fix panics when min_checkpoint_frequency is 0 #125459
Conversation
Previously, we introduced the cluster setting changefeed.aggregator.flush_jitter to apply some jitter to the aggregator flush frequency. This was done to reduce the load on the coordinator by spreading out aggregator flush times. However, that PR did not account for the case where min_checkpoint_frequency is set to zero, which could cause panics due to rand.Int63(0). This patch fixes the issue by adding a check for when the frequency is zero, ensuring that jitter is not applied in such cases. Fixes: #125312 Release note (bug fix): Fixed a bug in v24.1, v23.2, and v23.1 where using changefeed.aggregator.flush_jitter with min_checkpoint_frequency set to zero could cause panics.
4b404af
to
16ef09b
Compare
Thanks for opening a backport. Please check the backport criteria before merging:
If your backport adds new functionality, please ensure that the following additional criteria are satisfied:
Also, please add a brief release justification to the body of your PR to justify this |
@stevendanna Do you mind taking a look at this when you have a moment? Hoping to backport the fix and close the support issue. |
The last push was to resolve merge conflicts. (Couldn't squash and didn't want to open a new custom backport) |
Backport 1/1 commits from #125317 on behalf of @wenyihu6.
/cc @cockroachdb/release
Previously, we introduced the cluster setting changefeed.aggregator.flush_jitter
to apply some jitter to the aggregator flush frequency. This was done to reduce
the load on the coordinator by spreading out aggregator flush times. However,
that PR did not account for the case where min_checkpoint_frequency is set to
zero, which could cause panics due to rand.Int63(0). This patch fixes the issue
by adding a check for when the frequency is zero, ensuring that jitter is not
applied in such cases.
Fixes: #125312
Release note (bug fix): Fixed a bug in v24.1, v23.2, and v23.1 where using
changefeed.aggregator.flush_jitter with min_checkpoint_frequency set to zero
could cause panics.
Release justification: (low risk bug fix) Fixed a bug in v24.1, v23.2, and v23.1 where using
changefeed.aggregator.flush_jitter with min_checkpoint_frequency set to zero
could cause panics.