CNDB-10085: Add guardrail for the number of column filters per query after applying analyzers #1417
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…after applying analyzers
adelapena
force-pushed
the
CNDB-10085-main
branch
from
8bd292d
to
7a9af25
Compare
|
❌ Build ds-cassandra-pr-gate/PR-1417 rejected by Butler4 new test failure(s) in 2 builds Found 4 new test failures
Found 9 known test failures |
| @@ -145,6 +145,10 @@ public class GuardrailsConfig | |||
| public volatile Integer offset_rows_warn_threshold; | |||
| public volatile Integer offset_rows_failure_threshold; | |||
|
|
|||
| // Limit the number of column value filters per SELECT query (after applying analyzers) | |||
There was a problem hiding this comment.
Suggested change
| // Limit the number of column value filters per SELECT query (after applying analyzers) | |
| // Limit the number of column value filters per SELECT query (after applying analyzers, in case they are used) |
| operations, FAIL_THRESHOLD), | ||
| query); | ||
| } | ||
| } |
There was a problem hiding this comment.
nit: I think by default we leave a line at the end of every class?
|
|
||
| config().query_filters_warn_threshold = -1; | ||
| testValidationOfStrictlyPositiveProperty((c, v) -> c.query_filters_fail_threshold = v.intValue(), | ||
| "query_filters_fail_threshold"); |
There was a problem hiding this comment.
Do you want to add a tiny test for the other validation, too? I did not find one. For the unhappy path where we set warn bigger than fail.
validateWarnLowerThanFail
| assertFails("SELECT * FROM %s WHERE k1 = 0 AND k2 = 0 AND x = '1 2 3 4 5'", 5); | ||
| assertFails("SELECT * FROM %s WHERE k1 = 0 AND k2 = 0 AND x = '1 2 3 4 5 6'", 6); | ||
|
|
||
| // partial partition key restrictions don't count as filters |
There was a problem hiding this comment.
Suggested change
| // partial partition key restrictions don't count as filters | |
| // partial partition key restrictions do count as filters |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds a new
query_filtersguardrail to limit the number of column value filters per query after applying index analyzers.The guardrail is applied to the sum of all column filters, analyzed or not, indexed or not. Complete primary key restrictions are not considered column value filters so they don't count towards the configured limit.
This should be useful to limit the number of filtered columns in general, and to prevent the explosion of filtering operations produced by analyzers applied to large values.
Checklist before you submit for review
NoSpamLoggerfor log lines that may appear frequently in the logs