Skip to content
This repository has been archived by the owner on Oct 3, 2023. It is now read-only.

Cache seen resources for each export request #179

Merged
merged 2 commits into from
Aug 28, 2019

Conversation

songy23
Copy link
Contributor

@songy23 songy23 commented Aug 28, 2019

To avoid transforming the resource proto over and over again. There should be a fairly small amount of unique resources so caching shouldn't be a big problem for memory usage.

stats.go Outdated
@@ -106,6 +108,7 @@ func newStatsExporter(o Options) (*statsExporter, error) {
createdViews: make(map[string]*metricpb.MetricDescriptor),
protoMetricDescriptors: make(map[string]*metricpb.MetricDescriptor),
metricDescriptors: make(map[string]*metricpb.MetricDescriptor),
seenResources: make(map[*resourcepb.Resource]*monitoredrespb.MonitoredResource),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid extra memory usage, can we cache this for every export request. Every export request presumably will have >20 timeseries so we don't get as much win as we would get with this change, but at least we do not OOM.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense to me, updated in 8800548.

@bogdandrutu
Copy link
Contributor

To avoid transforming the resource proto over and over again. There should be a fairly small amount of unique resources so caching shouldn't be a big problem for memory usage.

This may not be true for the agent/collector. May get up to millions of resource.

@songy23 songy23 changed the title Cache seen resources in stats exporter Cache seen resources for each export request Aug 28, 2019
@@ -299,7 +308,8 @@ func (se *statsExporter) uploadMetricsProto(payloads []*metricProtoPayload) erro

var allTimeSeries []*monitoringpb.TimeSeries
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we call into ExportMetricsProtoSync from this point? Maybe in a different PR.

@bogdandrutu bogdandrutu merged commit ffafe44 into census-ecosystem:master Aug 28, 2019
@songy23 songy23 deleted the cache-rsc branch August 28, 2019 16:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants