Removed chunks storage backend logic and storage clients #755
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
pracucci
commented
Jan 14, 2022
| @@ -44,7 +44,6 @@ type Flusher struct { | |||
|
|
|||
| cfg Config | |||
| ingesterConfig ingester.Config | |||
| chunkStore ingester.ChunkStore | |||
pracucci
commented
Jan 14, 2022
| @@ -537,8 +536,29 @@ func TestIngesterMetricLimitExceeded(t *testing.T) { | |||
|
|
|||
| // Construct a set of realistic-looking samples, all with slightly different label sets | |||
| func benchmarkData(nSeries int) (allLabels []labels.Labels, allSamples []mimirpb.Sample) { | |||
| // Real example from Kubernetes' embedded cAdvisor metrics, lightly obfuscated. | |||
| var benchmarkLabels = labels.Labels{ | |||
There was a problem hiding this comment.
Moved here from pkg/chunk.
pracucci
commented
Jan 14, 2022
| @@ -170,9 +164,6 @@ func (c *Config) RegisterFlags(f *flag.FlagSet, logger log.Logger) { | |||
| c.MemberlistKV.RegisterFlags(f) | |||
| c.ActivityTracker.RegisterFlags(f) | |||
| c.QueryScheduler.RegisterFlags(f) | |||
|
|
|||
| // These don't seem to have a home. | |||
| f.IntVar(&chunk_util.QueryParallelism, "querier.query-parallelism", 100, "Max subqueries run in parallel per higher-level query.") | |||
There was a problem hiding this comment.
Was chunks storage specific.
pracucci
commented
Jan 14, 2022
| @@ -158,7 +157,6 @@ func (l *Limits) RegisterFlags(f *flag.FlagSet) { | |||
| f.IntVar(&l.LabelNamesAndValuesResultsMaxSizeBytes, "querier.label-names-and-values-results-max-size-bytes", 400*1024*1024, "Maximum size in bytes of distinct label names and values. When querier receives response from ingester, it merges the response with responses from other ingesters. This maximum size limit is applied to the merged(distinct) results. If the limit is reached, an error is returned.") | |||
| f.BoolVar(&l.CardinalityAnalysisEnabled, "querier.cardinality-analysis-enabled", false, "Enables endpoints used for cardinality analysis.") | |||
| f.IntVar(&l.LabelValuesMaxCardinalityLabelNamesPerRequest, "querier.label-values-max-cardinality-label-names-per-request", 100, "Maximum number of label names allowed to be queried in a single /api/v1/cardinality/label_values API call.") | |||
| f.IntVar(&l.CardinalityLimit, "store.cardinality-limit", 1e5, "Cardinality limit for index queries. This limit is ignored when using blocks storage. 0 to disable.") | |||
There was a problem hiding this comment.
Was chunks storage specific.
56quarters
approved these changes
Jan 14, 2022
pracucci
commented
Jan 15, 2022
| }, | ||
| }, | ||
| { | ||
| name: "url-config", |
There was a problem hiding this comment.
I removed test cases that don't apply anymore to the new S3 client.
pracucci
commented
Jan 15, 2022
| @@ -61,9 +35,6 @@ type Chunk struct { | |||
| // missing, we default to DoubleDelta. | |||
| Encoding prom_chunk.Encoding `json:"encoding"` | |||
| Data prom_chunk.Chunk `json:"-"` | |||
|
|
|||
| // The encoded version of the chunk, held so we don't need to re-encode it | |||
| encoded []byte | |||
pracucci
commented
Jan 15, 2022
| return a.UserID == b.UserID && a.Fingerprint == b.Fingerprint && | ||
| a.From == b.From && a.Through == b.Through && a.Checksum == b.Checksum | ||
| } | ||
|
|
||
| // Samples returns all SamplePairs for the chunk. | ||
| func (c *Chunk) Samples(from, through model.Time) ([]model.SamplePair, error) { |
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
pracucci
force-pushed
the
remove-chunks-storage-store
branch
from
401fe61
to
ddfcba6
Compare
Signed-off-by: Marco Pracucci <marco@pracucci.com>
pstibrany
approved these changes
Jan 17, 2022
pkg/util/validation/limits.go
Outdated
| @@ -151,14 +148,13 @@ func (l *Limits) RegisterFlags(f *flag.FlagSet) { | |||
| f.IntVar(&l.MaxChunksPerQuery, "querier.max-fetched-chunks-per-query", 2e6, "Maximum number of chunks that can be fetched in a single query from ingesters and long-term storage. This limit is enforced in the querier, ruler and store-gateway. 0 to disable.") | |||
| f.IntVar(&l.MaxFetchedSeriesPerQuery, "querier.max-fetched-series-per-query", 0, "The maximum number of unique series for which a query can fetch samples from each ingesters and blocks storage. This limit is enforced in the querier only when running with blocks storage. 0 to disable") | |||
| f.IntVar(&l.MaxFetchedChunkBytesPerQuery, "querier.max-fetched-chunk-bytes-per-query", 0, "The maximum size of all chunks in bytes that a query can fetch from each ingester and storage. This limit is enforced in the querier and ruler only when running with blocks storage. 0 to disable.") | |||
| f.Var(&l.MaxQueryLength, "store.max-query-length", "Limit the query time range (end - start time). This limit is enforced in the query-frontend (on the received query), in the querier (on the query possibly split by the query-frontend) and in the chunks storage. 0 to disable.") | |||
| f.Var(&l.MaxQueryLength, "store.max-query-length", "Limit the query time range (end - start time). This limit is enforced in the query-frontend (on the received query) and in the querier (on the query possibly split by the query-frontend). 0 to disable.") | |||
There was a problem hiding this comment.
Right. I went through the limits CLI flags descriptions and update it in few other places too.
| @@ -21,7 +16,6 @@ require ( | |||
| github.com/go-openapi/strfmt v0.21.1 | |||
| github.com/go-openapi/swag v0.19.15 | |||
| github.com/go-redis/redis/v8 v8.11.4 | |||
| github.com/gocql/gocql v0.0.0-20200526081602-cd04bd7f22a7 | |||
There was a problem hiding this comment.
We can remove replace github.com/gocql/gocql too.
There was a problem hiding this comment.
I even looked at go.mod before opening the PR and I missed that 🤦
Signed-off-by: Marco Pracucci <marco@pracucci.com>
pracucci
force-pushed
the
remove-chunks-storage-store
branch
from
afffd39
to
3f9c531
Compare
3 tasks
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.
What this PR does:
In this PR I'm removing most of the chunks storage backend logic and storage clients. To keep PR easier to review, I haven't addressed the packages
pkg/chunk/encodingandpkg/chunk/cacheyet, as well as the 2nd storage engine support and the storage engine config (it's still configurable). These will be addressed in follow up PRs.Which issue(s) this PR fixes:
N/A
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]