Split a per-partition WriteRequest into multiple Kafka records if bigger than max allowed size #8077
Conversation
…ger than max allowed size Signed-off-by: Marco Pracucci <marco@pracucci.com>
There was a problem hiding this comment.
Gave this PR an early look, and it would work.
I wonder if it would have been easier to work at serialized-message level, splitting the message by fields with tag 1 (timeseries) and tag 3 (metadata) until they fill the size, while copying tag 2 (source) and 1000 (skip_label_name_validation) into each submessage.
| } | ||
|
|
||
| // We assume that different timeseries roughly have the same size (no huge outliers) | ||
| // so we preallocate the returned slice just adding 1 extra item (+2 because a +1 is to round up). |
There was a problem hiding this comment.
I could understand +1, but why +2 again?
There was a problem hiding this comment.
+1 to round up, and +1 for an extra item. The +1 round up doesn't guarantees us space for 1 extra item (it depends what was the reminder of the division), but it's guaranteed by the 2nd +2. Does this answer your question?
| return []*WriteRequest{partialReq} | ||
| } | ||
|
|
||
| // We assume that different timeseries roughly have the same size (no huge outliers) |
There was a problem hiding this comment.
Given that size of each timeseries is dominated by labels, I have doubts that this assumption holds.
There was a problem hiding this comment.
In practice we split into 16MB partial requests. At this scale, the size of labels shouldn't matter much.
pkg/mimirpb/custom.go
Outdated
| if partialReqSize+seriesSize > maxSize && !partialReq.IsEmpty() { | ||
| // The current partial request is full (or close to be full), so we create a new one. | ||
| partialReqs = append(partialReqs, partialReq) | ||
| partialReq = newPartialReq(estimatedTimeseriesPerPartialReq) |
There was a problem hiding this comment.
Don't we need to reset partialReqSize here?
There was a problem hiding this comment.
Damn yes. It was a bad bug. Fixed in 0379d8b
Signed-off-by: Marco Pracucci <marco@pracucci.com>
What this PR does
To be written...
Which issue(s) this PR fixes or relates to
N/A
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX].about-versioning.mdupdated with experimental features.