[coordinator] Influxdb write endpoint tag copy fix #2126
Conversation
Reusing Tags objects directly over the iteration did not work. Second best performing option was to reuse the Tag objects within array, but replace the Tag with the __name__ with new one. Fixes issue m3db#2125.
| func (ii *ingestIterator) Current() (models.Tags, ts.Datapoints, xtime.Unit, []byte) { | ||
| if ii.pointIndex < len(ii.points) && ii.nextFieldIndex > 0 && len(ii.fields) > (ii.nextFieldIndex-1) { | ||
| point := ii.points[ii.pointIndex] | ||
| field := ii.fields[ii.nextFieldIndex-1] | ||
| tags := ii.tags.SetName(field.name) | ||
| tags := copyTagsWithNewName(ii.tags, field.name) | ||
|
|
||
| return tags, []ts.Datapoint{ts.Datapoint{Timestamp: point.Time(), | ||
| Value: field.value}}, xtime.Nanosecond, nil |
There was a problem hiding this comment.
Note: I missed this before, it might be worth trying to choose the most granular precision to improve the M3TSZ compression ratio.
Otherwise there could be high variability in the delta-of-delta encoding when not required (i.e. a delta-of-delta encoding 4000000 nanoseconds takes many more bits than a delta-to-delta encoding of 4 milliseconds).
You could basically divide each time unit by the nanoseconds and see which one falls within exactly each unit then choose the highest occuring one that equally divides (i.e. try seconds first, then millisecond, then so on). If they exactly fall into a unit, then emit with that unit?
There was a problem hiding this comment.
I actually wondered about that, because we get pretty lousy M3TSZ compression ratio. That would explain it, I will address that too.
There was a problem hiding this comment.
Added it in commit, but it's only ~10% space savings in our test load (1/N scale of real thing). However, better than nothing.
Even with fully per-second data points, this seems to shave off ~10% in my test env; not as much as I had hoped.
Reusing Tags objects directly over the iteration did not work. Second
best performing option was to reuse the Tag objects within array, but
replace the Tag with the name with new one.
No existing test but fixed panics in our dev env. Not tested in our prod.
Fixes #2125.