Implement ChannelMode and sampling rate for extended aggregation #187
Conversation
hush-hush
force-pushed
the
maxime/add-channelmode-aggregator
branch
2 times, most recently
from
8c443d1
to
f2fb331
Compare
| } | ||
|
|
||
| func (bc *bufferedMetricContexts) sample(name string, value float64, tags []string, rate float64) error { | ||
| if !shouldSample(rate, bc.random, &bc.randomLock) { |
There was a problem hiding this comment.
I know there's no easy way around this, I'm a little concerned about all this locking here. We have one lock per metric type which is still a pretty coarse granularity. Can't really think of a good workaround.
There was a problem hiding this comment.
That's true. Also it's the same overhead as the default configuration right now. When using the default setting (ie mutext mode), we have the same lock and random. So the overhead should be the same with and without extended aggregation.
| @@ -11,59 +11,9 @@ type ( | |||
| countsMap map[string]*countMetric | |||
| gaugesMap map[string]*gaugeMetric | |||
| setsMap map[string]*setMetric | |||
| bufferedMetricMap map[string]*histogramMetric | |||
| bufferedMetricMap map[string]*bufferedMetric | |||
There was a problem hiding this comment.
So, I'm wondering if we should consider sync.Map? It's not totally clear to me, but we might get better performance. On paper a sync.Map might offer better performance when used as follows:
(1) when the entry for a given key is only ever written once but read many times, as in caches that only grow,
(2) when multiple goroutines read, write, and overwrite entries for disjoint sets of keys.
I think we have a bit of (1) in that we create the context once and then sample over and over, but it might not fit the description fully. We don't have super high concurrency for (2), and the sets of keys would not be disjoint, so I'm not sure that apples. In any case, might be worth giving it thought to decide if we should keep the current implementation.
There was a problem hiding this comment.
That's a really good point. I'll do that in a different PR !
When using extended aggregation we still want to respect sampling rate for histograms, distribution and timing as it will have direct consequences on the Agent (number of point to aggregate). For apps sending a high number of metrics the aggregator implements ChannelMode to avoid lock contention when generating random numbers.
hush-hush
force-pushed
the
maxime/add-channelmode-aggregator
branch
from
e3cb1a4
to
358b394
Compare
When using extended aggregation we still want to respect sampling rate for histograms, distribution and timing as it will have direct consequences on the Agent (number of metrics to aggregate).
For app sending a very high number of metrics the aggregator implements ChannelMode to avoid lock contention when generating
andom numbers.