-
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
release-22.2: sql/stats: don't use linear regression with NaN for stats forecasting #113799
release-22.2: sql/stats: don't use linear regression with NaN for stats forecasting #113799
Conversation
This patch adds a method to the quantile function, `invalid`, which checks for `NaN` and `+/-Inf` values within the function. These indicate a failure to derive a linear regression model for the observed histograms, and can lead to internal errors and panics if not caught. Stats forecasting now checks this method before attempting to use a quantile function to derive a histogram prediction. This patch also reverts 120132e, since `fixMalformed` is no longer expected to return an error. Fixes cockroachdb#113645 Fixes cockroachdb#113680 Release note (bug fix): Fixed a bug that could cause internal errors or panics while attempting to forecast statistics on a numeric column.
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 |
This patch adds panic-catching logic to `GetTableStats`, so that "safe" panics (out-of-bounds, assertion etc.) can be propagated as a returned error. This allows some code-paths to ignore the returned error, so that execution can proceed even there's an unexpected error in the stats code. This prevents situations where it becomes impossible to query a table because of a stats bug. Informs cockroachdb#113645 Release note: None
1087815
to
6c7e699
Compare
No, but this is a simple change that we are confident is correct.
If the change introduced a bug, it could make a table or tables effectively unavailable. However, as mentioned, we are confident that this is not the case.
Customers that run into the bug will find that a table is unavailable due to a NPE. This is rare, but has already happened in a few cases.
It would be best to backport to active releases, since table unavailability is fairly severe and the best workaround currently is to disable stats forecasting for the affected table. |
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 3 of 3 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @rafiss)
TFTRs! |
Backport 2/2 commits from #113712.
/cc @cockroachdb/release
This patch adds a method to the quantile function,
invalid
, which checks forNaN
values within the function. These indicate a failure to derive a linear regression model for the observed histograms, and can lead to internal errors and panics if not caught. Stats forecasting now checks this method before attempting to use a quantile function to derive a histogram prediction.Fixes #113645
Fixes #113680
Release note (bug fix): Fixed a bug that could cause internal errors or panics while attempting to forecast statistics on a numeric column.
Release justification: fix for internal error encountered by customer