[WIP] Read and write from/to multiple ingesters #49
Conversation
| @@ -101,6 +103,11 @@ func main() { | |||
| flag.DurationVar(&cfg.flushPeriod, "ingester.flush-period", 1*time.Minute, "Period with which to attempt to flush chunks.") | |||
| flag.DurationVar(&cfg.maxChunkAge, "ingester.max-chunk-age", 10*time.Minute, "Maximum chunk age before flushing.") | |||
| flag.IntVar(&cfg.numTokens, "ingester.num-tokens", 128, "Number of tokens for each ingester.") | |||
| flag.IntVar(&cfg.distributorConfig.ReadReplicas, "distributor.read-replicas", 3, "The number of available ingesters to read from.") | |||
| flag.IntVar(&cfg.distributorConfig.WriteReplicas, "distributor.write-replicas", 3, "The number of available ingesters to write to.") | |||
There was a problem hiding this comment.
I think we just want number of replicas. I can't imagine a circumstance where we'd want these numbers to differ.
There was a problem hiding this comment.
Yeah, not 100% sure about this, but you're probably right and I can't see how differing numbers would make sense.
| Get(key uint32) (ring.IngesterDesc, error) | ||
| GetAll() []ring.IngesterDesc | ||
| Get(key uint32, n int, heartbeatTimeout time.Duration) ([]ring.IngesterDesc, error) | ||
| GetAll(heartbeatTimeout time.Duration) []ring.IngesterDesc |
There was a problem hiding this comment.
Another approach might be to exclude the heartbeatTimeout from the parameters and instead implement it as a decorator. Not sure it's a better approach, mind you.
There was a problem hiding this comment.
From discussion: consider making the heartbeat timeout a property of the ring instead of passing it in on each call. Then the ring could also report in its metrics Collect() method how many total vs. timed out ingesters there are.
| result = append(result, b[j]) | ||
| j++ | ||
| } else { | ||
| result = append(result, a[i]) |
There was a problem hiding this comment.
Is it possible for the value at b[i] to be different from a[i]?
There was a problem hiding this comment.
Nope, that's prevented in the ingesters, like in Prometheus itself. If two ingesters do end up having different values for the same timestamp for some broken reason, it's still a good thing to just filter that out here.
| @@ -101,24 +102,52 @@ func (r *Ring) loop() { | |||
| }) | |||
| } | |||
|
|
|||
| // Get returns a collector close to the hash in the circle. | |||
| func (r *Ring) Get(key uint32) (IngesterDesc, error) { | |||
| // Get returns up to n ingesters close to the hash in the circle. | |||
There was a problem hiding this comment.
I wonder if it's worth having a new type that implements Collector but has multiple ingesters as a backend?
There was a problem hiding this comment.
Like: Distributor -> Replicator -> Ingesters?
Hmm, could make sense, but one more type. Let's defer that decision for later, I would propose?
| ingesterQueries: prometheus.NewCounterVec(prometheus.CounterOpts{ | ||
| Namespace: "prometheus", | ||
| Name: "distributor_ingester_queries_total", | ||
| Help: "The total number queries sent to ingesters.", |
There was a problem hiding this comment.
Nit: "total number of queries"
fd875e2 Fix test wrt shellcheck 54ec2d9 Don't capitalise error messages 19d3b6e Merge pull request #49 from weaveworks/pin-shfmt fea98f6 Go get from the vendor dir 1d867b0 Try and vendor a specific version of shfmt 76619c2 Merge pull request #48 from weaveworks/revert-41-user-tokens 4f96c51 Revert "Add experimental support for user tokens" d00033f Merge pull request #41 from weaveworks/user-tokens 245ed26 Merge pull request #47 from weaveworks/46-shfmt c1d7815 Fix shfmt error cb39746 Don't overright lint_result with 0 when shellcheck succeeds 8ab80e8 Merge pull request #45 from weaveworks/lint 83d5bd1 getting integration/config and test shellcheck-compliant cff9ec3 Fix some shellcheck errors 7a843d6 run shellcheck as part of lint if it is installed 31552a0 removing spurious space from test 6ca7c5f Merge pull request #44 from weaveworks/shfmt 952356d Allow lint to lint itself b7ac59c Run shfmt on all shell files in this repo 5570b0e Add shfmt formatting of shell files in lint 0a67594 fix circle build by splatting gopath permissions b990f48 Merge pull request #42 from kinvolk/lorenzo/fix-git-diff 224a145 Check if SHA1 exists before calling `git diff` 1c3000d Add auto_apply config for wcloud 0ebf5c0 Fix wcloud -serivice 354e083 Fixing lint 586060b Add experimental support for user tokens git-subtree-dir: tools git-subtree-split: fd875e27c5379d443574bcf20f24a52a594871ca
@jml FYI
Not sure what to do about
LabelValuesForLabelName(). It is only used for the UI to retrieve all metric names for autocompletion. Currently I just try to read from all ingesters in there, and it's not clear whether it makes sense to introduce any failure thresholds in there (since we're not querying N ingesters for a hash, but all, independent of any hash).