[coordinator] Expand and add metrics + configurability of downsample and ingest worker pool #2797
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
ryanhall07
approved these changes
Oct 24, 2020
arnikola
reviewed
Oct 25, 2020
Comment on lines
+416
to
419
| if err == nil { | ||
| err = d.store.Write(ctx, writeQuery) | ||
| } | ||
| if err != nil { |
arnikola
reviewed
Oct 25, 2020
Comment on lines
+336
to
339
| if err == nil { | ||
| err = d.store.Write(ctx, writeQuery) | ||
| } | ||
| if err != nil { |
There was a problem hiding this comment.
Can't change to else unfortunately since error reporting needs to happen for both code paths, this is a way to reuse the same error reporting.
arnikola
reviewed
Oct 25, 2020
| @@ -85,6 +85,12 @@ var ( | |||
| // By default, raise errors instead of truncating results so | |||
| // users do not experience see unexpected results. | |||
| defaultRequireExhaustive = true | |||
|
|
|||
| defaultWriteWorkerPool = xconfig.WorkerPoolPolicy{ | |||
There was a problem hiding this comment.
nit; defaultWriteWorkerPoolPolicy
arnikola
reviewed
Oct 25, 2020
| @@ -177,6 +183,14 @@ type Configuration struct { | |||
| Debug config.DebugConfiguration `yaml:"debug"` | |||
| } | |||
|
|
|||
| // WriteWorkerPoolOrDefault returns the write worker pool config or default. | |||
| func (c Configuration) WriteWorkerPoolOrDefault() xconfig.WorkerPoolPolicy { | |||
There was a problem hiding this comment.
nit: may be worth renaming these to WriteWorkerPoolPolicyOrDefault and same for read? happy to punt though
soundvibe
added a commit
that referenced
this pull request
Oct 29, 2020
* master: Support dynamic namespace resolution for embedded coordinators (#2815) [dbnode] Add index regexp DFA compilation cache to avoid allocating DFAs for same expressions (#2814) [dbnode] Set default cache on retrieve to false, prepare testing with cache none (#2813) [tools] Output annotations as base64 (#2743) Add Reset Transformation (#2794) [large-tiles] Fix for a reverse index when querying downsampled namespace (#2808) [dbnode] Evict info files cache before index bootstrap in peers bootstrapper (#2802) [dbnode] Bump default filesystem persist rate limit (#2806) [coordinator] Add metrics and configurability of downsample and ingest writer worker pool (#2797) [dbnode] Fix concurrency granularity of seekerManager.UpdateOpenLease (#2790) [dbnode] Use correct import path for atomic library (#2801) Regenerate carbon automapping rules on namespaces changes (#2793) Enforce matching retention and index block size (#2783) [query] Add ClusterNamespacesWatcher and use to generate downsample automapper rules (#2782) [query][dbnode] Wire up etcd-backed Clusters implementation in coordinator (#2758) [query] Fix quantile() argument not being passed through (#2780) # Conflicts: # src/query/parser/promql/matchers.go
soundvibe
added a commit
that referenced
this pull request
Oct 29, 2020
* master: [query] Improve precision for variance and stddev of equal values (#2799) Support dynamic namespace resolution for embedded coordinators (#2815) [dbnode] Add index regexp DFA compilation cache to avoid allocating DFAs for same expressions (#2814) [dbnode] Set default cache on retrieve to false, prepare testing with cache none (#2813) [tools] Output annotations as base64 (#2743) Add Reset Transformation (#2794) [large-tiles] Fix for a reverse index when querying downsampled namespace (#2808) [dbnode] Evict info files cache before index bootstrap in peers bootstrapper (#2802) [dbnode] Bump default filesystem persist rate limit (#2806) [coordinator] Add metrics and configurability of downsample and ingest writer worker pool (#2797) [dbnode] Fix concurrency granularity of seekerManager.UpdateOpenLease (#2790) [dbnode] Use correct import path for atomic library (#2801) Regenerate carbon automapping rules on namespaces changes (#2793) Enforce matching retention and index block size (#2783) [query] Add ClusterNamespacesWatcher and use to generate downsample automapper rules (#2782) [query][dbnode] Wire up etcd-backed Clusters implementation in coordinator (#2758) [query] Fix quantile() argument not being passed through (#2780) [query] Add "median" aggregation to Graphite aggregate() function (#2774) [r2] Ensure KeepOriginal is propagated when adding/reviving rules (#2796) [dbnode] Update default db read limits (#2784)
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 / why we need it:
Raising the default writer worker pool and adding configurability and metrics for the worker pool.
This also removes
(*downsamplerAndWriter).writeWithOptionswhich has a large stack options struct passed to it that may be causing the stack to split often:Special notes for your reviewer:
Does this PR introduce a user-facing and/or backwards incompatible change?:
Does this PR require updating code package or user-facing documentation?: