Query store for series lookups #3461
Merged
Conversation
This file contains hidden or 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
gouthamve
force-pushed
the
query-store-meta
branch
3 times, most recently
from
e325246 to
6703601
Compare
gouthamve
changed the title
|
@pracucci and @pstibrany This is now ready for review. I am still querying the head even if the data is purely in object store because the abstractions are different and I can't think of a clean way to pass this flag down to Also, I know the flag description says labels APIs as well, but I'll add them in future PRs. |
pstibrany
approved these changes
Nov 5, 2020
pkg/querier/querier.go
Outdated
| @@ -84,6 +85,7 @@ func (cfg *Config) RegisterFlags(f *flag.FlagSet) { | |||
| f.BoolVar(&cfg.IngesterStreaming, "querier.ingester-streaming", true, "Use streaming RPCs to query ingester.") | |||
| f.IntVar(&cfg.MaxSamples, "querier.max-samples", 50e6, "Maximum number of samples a single query can load into memory.") | |||
| f.DurationVar(&cfg.QueryIngestersWithin, "querier.query-ingesters-within", 0, "Maximum lookback beyond which queries are not sent to ingester. 0 means all queries are sent to ingester.") | |||
| f.BoolVar(&cfg.QueryStoreForLabels, "querier.query-store-for-labels", false, "Query long-term store for series, label values and label names APIs. Works only with blocks engine.") | |||
There was a problem hiding this comment.
I'm wondering whether we need this flag at all, or we should explicitly call it out as temporary? Can we make it true for blocks, and false for chunks?
There was a problem hiding this comment.
I would add the -enabled suffix (and same for the YAML config option).
pracucci
approved these changes
Nov 6, 2020
There was a problem hiding this comment.
Good job @gouthamve ! I left few minor comments. A part from this, few more things:
- Could you mark it as experimental in
docs/configuration/v1-guarantees.md? - In the doc pages
docs/configuration/v1-guarantees.md,docs/api/_index.mdanddocs/guides/limitations.mdwe mention the/api/v1/seriesAPI can't query the long-term storage. Could you improve the documentation, clearly explaining it's not supported for the chunks storage but the blocks storage supports it when the flag is enabled?
pkg/querier/querier.go
Outdated
| @@ -84,6 +85,7 @@ func (cfg *Config) RegisterFlags(f *flag.FlagSet) { | |||
| f.BoolVar(&cfg.IngesterStreaming, "querier.ingester-streaming", true, "Use streaming RPCs to query ingester.") | |||
| f.IntVar(&cfg.MaxSamples, "querier.max-samples", 50e6, "Maximum number of samples a single query can load into memory.") | |||
| f.DurationVar(&cfg.QueryIngestersWithin, "querier.query-ingesters-within", 0, "Maximum lookback beyond which queries are not sent to ingester. 0 means all queries are sent to ingester.") | |||
| f.BoolVar(&cfg.QueryStoreForLabels, "querier.query-store-for-labels", false, "Query long-term store for series, label values and label names APIs. Works only with blocks engine.") | |||
There was a problem hiding this comment.
I would add the -enabled suffix (and same for the YAML config option).
Signed-off-by: Goutham Veeramachaneni <[email protected]>
gouthamve
force-pushed
the
query-store-meta
branch
2 times, most recently
from
c20a287 to
e74d58f
Compare
pracucci
approved these changes
Nov 9, 2020
There was a problem hiding this comment.
Thanks @gouthamve for addressing my feedback! Final comments and then we're good to go 🚀
Signed-off-by: Goutham Veeramachaneni <[email protected]>
gouthamve
force-pushed
the
query-store-meta
branch
from
e74d58f to
f301635
Compare
pracucci
approved these changes
Nov 9, 2020
jacksontj
added a commit
to jacksontj/promxy
that referenced
this pull request
Apr 2, 2021
In prometheus 2.24 dep upgrade prometheus changed how it signaled to queriers whether the request was for data or just the series. So with that upgrade we started *always* fetching all the data regardless of the query (in this case a series call would fetch all data for those series in the time range requested). As with before we're working around this by introspecting (cortex does the same -- cortexproject/cortex#3461). Fixes #415
jacksontj
added a commit
to jacksontj/promxy
that referenced
this pull request
Apr 2, 2021
In prometheus 2.24 dep upgrade prometheus changed how it signaled to queriers whether the request was for data or just the series. So with that upgrade we started *always* fetching all the data regardless of the query (in this case a series call would fetch all data for those series in the time range requested). As with before we're working around this by introspecting (cortex does the same -- cortexproject/cortex#3461). Fixes #415
htross
pushed a commit
to HubSpot/promxy
that referenced
this pull request
Jun 4, 2021
In prometheus 2.24 dep upgrade prometheus changed how it signaled to queriers whether the request was for data or just the series. So with that upgrade we started *always* fetching all the data regardless of the query (in this case a series call would fetch all data for those series in the time range requested). As with before we're working around this by introspecting (cortex does the same -- cortexproject/cortex#3461). Fixes jacksontj#415
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.
Signed-off-by: Goutham Veeramachaneni [email protected]
What this PR does:
Which issue(s) this PR fixes:
Fixes #
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]