[query][coordinator] Add time range limit with M3-Limit-Max-Range to restrict time range of a query #3538
Conversation
… config to restrict time range of a query
robskillington
changed the title
Codecov Report
@@ Coverage Diff @@
## master #3538 +/- ##
========================================
- Coverage 55.9% 55.9% -0.1%
========================================
Files 549 549
Lines 61439 61439
========================================
- Hits 34365 34351 -14
- Misses 23971 23979 +8
- Partials 3103 3109 +6
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
robskillington
changed the title
| @@ -91,6 +91,10 @@ const ( | |||
| // the number of docs returned by each storage node. | |||
| LimitMaxDocsHeader = M3HeaderPrefix + "Limit-Max-Docs" | |||
|
|
|||
| // LimitMaxRangeHeader is the M3 limit range header that limits | |||
| // the time range returned by each storage node. | |||
| LimitMaxRangeHeader = M3HeaderPrefix + "Limit-Max-Range" | |||
There was a problem hiding this comment.
nit: if this is aggregate query specific, should scope the header name to indicate this
There was a problem hiding this comment.
It's applicable to both 👍
There was a problem hiding this comment.
(Updated integration tests to match)
| @@ -279,7 +279,7 @@ func divideSeriesLists(ctx *common.Context, dividendSeriesList, divisorSeriesLis | |||
|
|
|||
| // aggregate takes a list of series and returns a new series containing the | |||
| // value aggregated across the series at each datapoint using the specified function. | |||
| // This function can be used with aggregation functionsL average (or avg), avg_zero, | |||
| // This function can be used with aggregation functions average (or avg), avg_zero, | |||
There was a problem hiding this comment.
Nice, how did you even find this heh
There was a problem hiding this comment.
I think I had touched this file by accident and linter had gone off, then I reverted but the fix stayed. Good old linter hah.
This reverts commit 920cb9c.
| fetchOptions.RequireExhaustive, | ||
| fetchRangeLimit.String(), | ||
| fetchRange.String()) | ||
| err := xerrors.NewInvalidParamsError(consolidators.NewLimitError(msg)) |
There was a problem hiding this comment.
Nice, should this emit a metric too to match the other limit exceeded errors?
What this PR does / why we need it:
Useful when combined with require exhaustive false to enforce certain use cases where a time range limit must be enforced but the time windows can be arbitrarily still modified by the caller.
Special notes for your reviewer:
Does this PR introduce a user-facing and/or backwards incompatible change?:
Does this PR require updating code package or user-facing documentation?: