-
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
sql: add cluster setting to disable RU estimation #92968
Conversation
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.
Do we plan to remove this setting during 23.1 release cycle? If so, it might be better to open up this PR against 22.2 branch right away and only merge a small change to master to add the setting's name into retiredSettings
in settings/registry.go
.
Reviewed 9 of 9 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball)
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.
LGTM
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.
Do we plan to remove this setting during 23.1 release cycle? If so, it might be better to open up this PR against 22.2 branch right away and only merge a small change to master to add the setting's name into retiredSettings in settings/registry.go.
The main work for RU estimation still hasn't been backported, so it seems easiest to just merge this and then backport it all together, rather than do the work separately. Do you feel strongly about it?
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball)
This patch adds a cluster setting, `sql.tenant_ru_estimation.enabled`, which is used to determine whether tenants collect an RU estimate for queries run with `EXPLAIN ANALYZE`. This is an escape hatch so that the RU estimation logic can be more safely backported. Informs cockroachdb#74441 Release note: None
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.
Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @DrewKimball)
TFTRs! bors r+ |
Build succeeded: |
This patch adds a cluster setting,
sql.tenant_ru_estimation.enabled
, which is used to determine whether tenants collect an RU estimate for queries run withEXPLAIN ANALYZE
. This is an escape hatch so that the RU estimation logic can be more safely backported.Informs #74441
Release note: None