Split labels/series API endpoints in query frontend #3276
Merged
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
Signed-off-by: Ben Ye <[email protected]>
Signed-off-by: Ben Ye <[email protected]>
Signed-off-by: Ben Ye <[email protected]>
yeya24
changed the title
yeya24
changed the title
Signed-off-by: Ben Ye <[email protected]>
Signed-off-by: Ben Ye <[email protected]>
yeya24
force-pushed
the
split-other-endpoints
branch
from
a8dc6f6 to
00c67cf
Compare
Signed-off-by: Ben Ye <[email protected]>
kakkoyun
reviewed
Oct 6, 2020
Signed-off-by: Ben Ye <[email protected]>
Signed-off-by: Ben Ye <[email protected]>
yeya24
force-pushed
the
split-other-endpoints
branch
from
2557f99 to
7d6c334
Compare
Signed-off-by: Ben Ye <[email protected]>
Signed-off-by: Ben Ye <[email protected]>
bwplotka
changed the title
Signed-off-by: Ben Ye <[email protected]>
Signed-off-by: Ben Ye <[email protected]>
Signed-off-by: Ben Ye <[email protected]>
Signed-off-by: Ben Ye <[email protected]>
yeya24
changed the title
kakkoyun
approved these changes
Oct 12, 2020
There was a problem hiding this comment.
@yeya24 Great work. It has taken some time to review the whole the PR. Overall it looks amazing. Thanks a lot for the contribution.
I have only added a small nit. Other than that we should test by integrating it with whole stack. It would be really nice to run it with quickstart script. WDYT?
Yes, that's what I want as well. Since this task is trivial, I will open an issue and let someone else contribute. Done: #3304 |
Signed-off-by: Ben Ye <[email protected]>
yeya24
force-pushed
the
split-other-endpoints
branch
from
5f5e987 to
35380a9
Compare
Signed-off-by: Ben Ye <[email protected]>
|
While I am working on the caching part, I find that all response format must be protobuf because they will be marshaled in the results cache middlware. I pushed another commit to switch the labels and series response to protobuf. |
bwplotka
reviewed
Oct 14, 2020
| } | ||
| } | ||
|
|
||
| func (c labelsCodec) MergeResponse(responses ...queryrange.Response) (queryrange.Response, error) { |
There was a problem hiding this comment.
Some commentary why queryrange.Response is used for labels would be nice ;p
| case *ThanosSeriesResponse: | ||
| seriesData := make([]labelpb.LabelSet, 0) | ||
|
|
||
| // seriesString is used in soring so we don't have to calculate the string of label sets again. |
OGKevin
pushed a commit
to OGKevin/thanos
that referenced
this pull request
Oct 15, 2020
* split metadata endpoints Signed-off-by: Ben Ye <[email protected]> * add metadata codec implementation Signed-off-by: Ben Ye <[email protected]> * cleanup go mod Signed-off-by: Ben Ye <[email protected]> * add back fix for thanos-io#3240 Signed-off-by: Ben Ye <[email protected]> * check set key exist Signed-off-by: Ben Ye <[email protected]> * update Signed-off-by: Ben Ye <[email protected]> * add default metadata range flag Signed-off-by: Ben Ye <[email protected]> * fix linting issues Signed-off-by: Ben Ye <[email protected]> * refactor flags and add test cases Signed-off-by: Ben Ye <[email protected]> * fix go lint issue Signed-off-by: Ben Ye <[email protected]> * add roundtrip tripperware tests for labels and series requests Signed-off-by: Ben Ye <[email protected]> * fix all unit tests and e2e tests Signed-off-by: Ben Ye <[email protected]> * fix lint issues Signed-off-by: Ben Ye <[email protected]> * add nolint unparam Signed-off-by: Ben Ye <[email protected]> * update flags and changelog Signed-off-by: Ben Ye <[email protected]> * switch response format to protobuf Signed-off-by: Ben Ye <[email protected]>
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.
Changes
This pr adds splitting and retry middleware support for all metadata API endpoints (label_name, label_value, series). This is the first step of #3230. Will add caching support in another pr since this pr is already very large.
Verification
Tests updated.