Ensure that TimeFilter's adjustments to a QueryRange Range are valid #663
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
Query range is a bit of an odd beast. It requires a start, end, and a step. Promql's calculations assume that the datapoint times are start plus a multiple of step; if they aren't you are subject to LookbackDelta constraints. Specifically with TimeFilter -- where we are setting either an absolute or relative time barrier we were causing some issues as we were previously truncating the time -- instead of finding the first multiple of step within our timerange. To explain the issue; lets consider the following query: ``` Start: t10 End: t25 Step: 3 ``` If this were to run normally we'd get results with data at 10,13,16,19,22,25 If we at the same time have an absoluteTimeFilter @ t20 -- we'd (prior to this patch) get: 10, 13, 16, 19, 20, 23. For shorter-range queries (i.e. within a day) this is generally not an issue because the step isn't generally long enough to exceed the LookbackDelta default of 5m. But as you look at *longer* time ranges this problem is easier and easier to hit (in #659 I was able to reproduce easily with a ~3 day query -- where step was ~9m. Fixes #659
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.
Query range is a bit of an odd beast. It requires a start, end, and a step. Promql's calculations assume that the datapoint times are start plus a multiple of step; if they aren't you are subject to LookbackDelta constraints.
Specifically with TimeFilter -- where we are setting either an absolute or relative time barrier we were causing some issues as we were previously truncating the time -- instead of finding the first multiple of step within our timerange.
To explain the issue; lets consider the following query:
If this were to run normally we'd get results with data at 10,13,16,19,22,25
If we at the same time have an absoluteTimeFilter @ t20 -- we'd (prior to this patch) get: 10, 13, 16, 19, 20, 23.
For shorter-range queries (i.e. within a day) this is generally not an issue because the step isn't generally long enough to exceed the LookbackDelta default of 5m. But as you look at longer time ranges this problem is easier and easier to hit (in #659 I was able to reproduce easily with a ~3 day query -- where step was ~9m.
Fixes #659