Support sharding subqueries in instant query QF #4955
Conversation
Signed-off-by: Ben Ye <[email protected]>
yeya24
force-pushed
the
instant-query-matrix
branch
from
8476510 to
a893089
Compare
Signed-off-by: Ben Ye <[email protected]>
yeya24
force-pushed
the
instant-query-matrix
branch
from
a893089 to
41e1047
Compare
yeya24
changed the title
| // return a sub slice whose first element's is the smallest timestamp that is strictly | ||
| // bigger than the given minTs. Empty slice is returned if minTs is bigger than all the | ||
| // timestamps in samples. | ||
| func SliceSamples(samples []cortexpb.Sample, minTs int64) []cortexpb.Sample { |
There was a problem hiding this comment.
Would be lovely if we can add unit tests for this function. I understand this is a refactor, but since we export this function now, so it would be lovely to use unit test to establish contracts :)
There was a problem hiding this comment.
Cool idea. I added an unit test
| // SliceSamples assumes given samples are sorted by timestamp in ascending order and | ||
| // return a sub slice whose first element's is the smallest timestamp that is strictly | ||
| // bigger than the given minTs. Empty slice is returned if minTs is bigger than all the | ||
| // timestamps in samples. |
There was a problem hiding this comment.
nit: I think it's always a good idea to explicit called out what happens if caller violates assumption. Like "If the given samples is not sorted, the behaviour is undefined". Of course it's debatable that this is just stating the obvious; hence the nit :)
| @@ -324,6 +335,59 @@ func vectorMerge(resps []*PrometheusInstantQueryResponse) *Vector { | |||
| return result | |||
| } | |||
|
|
|||
| func matrixMerge(resps []*PrometheusInstantQueryResponse) *Matrix { | |||
There was a problem hiding this comment.
This function looks a lot like
Anyway we can reduce the duplication?
There was a problem hiding this comment.
No good way for now cause they are different types.
There was a problem hiding this comment.
We could expose an interface for sure... But this needs more refactoring. I prefer copying the function for now.
There was a problem hiding this comment.
From my brief look, I think we can break
into multiple functions one that takes in[]tripperware.SampleStream and the other that takes in resps []*PrometheusResponse.
Then here you may be able to use the new function that takes in []tripperware.SampleStream.
Yes some "mapping" code still need to be done, but at least the core logic of merging []tripperware.SampleStream becomes common.
There was a problem hiding this comment.
Updated. Could you please take another look?
Signed-off-by: Ben Ye <[email protected]>
yeya24
force-pushed
the
instant-query-matrix
branch
from
6c1f7fa to
6dda199
Compare
Signed-off-by: Ben Ye <[email protected]>
yeya24
force-pushed
the
instant-query-matrix
branch
from
54702fa to
4988400
Compare
Signed-off-by: Ben Ye <[email protected]>
Signed-off-by: Ben Ye <[email protected]>
Signed-off-by: Ben Ye <[email protected]>
yeya24
force-pushed
the
instant-query-matrix
branch
from
68dc314 to
e6383d5
Compare
Signed-off-by: Ben Ye <[email protected]>
* support matrix response merge in instant query QF Signed-off-by: Ben Ye <[email protected]> * update changelog Signed-off-by: Ben Ye <[email protected]> * remove utils folder Signed-off-by: Ben Ye <[email protected]> * update unit tests for slice samples Signed-off-by: Ben Ye <[email protected]> * fix import Signed-off-by: Ben Ye <[email protected]> * update pb Signed-off-by: Ben Ye <[email protected]> * extract common merge function Signed-off-by: Ben Ye <[email protected]> * update unit test Signed-off-by: Ben Ye <[email protected]> Signed-off-by: Ben Ye <[email protected]>
What this PR does:
thanos-io/thanos#5811
Thanos doesn't support query sharding for subqueries.
This PR updates the Thanos version to get it supported and also support Matrix response type in instant query so that we can merge the sharded sub query responses.
Which issue(s) this PR fixes:
Fixes #
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]