limit memory used by FLP #376
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #376 +/- ##
==========================================
+ Coverage 61.71% 61.91% +0.19%
==========================================
Files 91 91
Lines 5835 5873 +38
==========================================
+ Hits 3601 3636 +35
- Misses 2014 2016 +2
- Partials 220 221 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| if (ct.config.MaxConnectionsTracked > 0) && (ct.config.MaxConnectionsTracked <= ct.connStore.mom.Len()) { | ||
| log.Warningf("too many connections; skipping flow log %v: ", fl) | ||
| ct.metrics.inputRecords.WithLabelValues("discarded").Inc() | ||
| continue |
There was a problem hiding this comment.
This continue will skip also over the if ct.shouldOutputFlowLogs which I think should be executed:
flowlogs-pipeline/pkg/pipeline/extract/conntrack/conntrack.go
Lines 99 to 105 in 8753a3f
One option would be to bring that "if" before if !exists. Maybe you could find a better solution.
There was a problem hiding this comment.
updated code to address suggestion.
| @@ -67,6 +67,11 @@ func (ct *conntrackImpl) Extract(flowLogs []config.GenericMap) []config.GenericM | |||
| } | |||
| conn, exists := ct.connStore.getConnection(computedHash.hashTotal) | |||
| if !exists { | |||
| if (ct.config.MaxConnectionsTracked > 0) && (ct.config.MaxConnectionsTracked <= ct.connStore.mom.Len()) { | |||
| log.Warningf("too many connections; skipping flow log %v: ", fl) | |||
| ct.metrics.inputRecords.WithLabelValues("discarded").Inc() | |||
There was a problem hiding this comment.
+1 for tracking the discarded flowlogs
| @@ -23,6 +23,6 @@ type AggregateOperation string | |||
| type AggregateDefinition struct { | |||
| Name string `yaml:"name,omitempty" json:"name,omitempty" doc:"description of aggregation result"` | |||
| GroupByKeys AggregateBy `yaml:"groupByKeys,omitempty" json:"groupByKeys,omitempty" doc:"list of fields on which to aggregate"` | |||
| OperationType AggregateOperation `yaml:"operationType,omitempty" json:"operationType,omitempty" doc:"sum, min, max, avg or raw_values"` | |||
| OperationType AggregateOperation `yaml:"operationType,omitempty" json:"operationType,omitempty" doc:"sum, min, max, count, avg or raw_values"` | |||
The memory usage of FLP seems to grow linearly with the number of concurrent connections being handled at the same time.
For encode_prom, state is saved (both in FLP and in the prometheus Go client) for each metric/labels combination that is reported. This results in memory growth that is linear with respect to the number of connections currently identified as active. In a configuration with limited memory resources it is desirable to instruct FLP to limit the amount of memory usage.
The tradeoff would be to not report on new connections once the memory limit is hit.
The conntrack stage memory usage also grows with the number of connections and should have the possibility of limiting the number of connections tracked.