Fix sending all series to all ingesters #3458
Conversation
Signed-off-by: Peter Stibrany <pstibrany@gmail.com>
Signed-off-by: Peter Stibrany <pstibrany@gmail.com>
|
|
||
| // we must not re-use buffers now until all DoBatch goroutines have finished, | ||
| // so set this flag false and pass cleanup() to DoBatch. | ||
| cleanupInDefer = false | ||
|
|
||
| err = ring.DoBatch(ctx, ring.WriteNoExtend, subRing, keys, func(ingester ring.InstanceDesc, indexes []int) error { | ||
| err := d.send(localCtx, ingester, req.Timeseries, req.Metadata, req.Source) | ||
| timeseries := make([]mimirpb.PreallocTimeseries, 0, len(indexes)) | ||
| var metadata []*mimirpb.MetricMetadata |
There was a problem hiding this comment.
why not
metadata := make([]*mimirpb.MetricMetadata, 0, len(indexes) - initialMetadataIndex)
There was a problem hiding this comment.
I don't think that's correct optimization, and could lead to panics.
Say series keys is [231, 342, 532, 984, 278, 1873, 784, 827], and metadataKeys is [478, 874, 983].
Then keys is [231, 342, 532, 984, 278, 1873, 784, 827, 478, 874, 983], and initialMetadataIndex=8.
If a given ingester should get values < 500, then indexes would be: [0, 1, 4, 8] (indexes into keys that are < 500).
We know that indexes below initialMetadataIndex refer to the keys, but we don't know how many there are. In this case, len(indexes) - initialMetadataIndex would even be negative number.
Given that most write requests push series, and not metadata, we optimize for that case: we preallocate slice for timeseries, but not metadata.
* Fix sending all series to all ingesters. * Add test. * CHANGELOG.md Signed-off-by: Peter Stibrany <pstibrany@gmail.com>
* Fix sending all series to all ingesters. * Add test. * CHANGELOG.md Signed-off-by: Peter Stibrany <pstibrany@gmail.com>
What this PR does
This PR fixes bug introduced in #3386: series and metadata should be properly sharded into ingesters. This PR also adds test to verify that each ingester only received series that "belong" to it based on ingester tokens.
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]