vmware / go-ipfix

An ipfix library in Go
Other
40 stars 27 forks source link

Create copies of net.IP and net.HardwareAddr when decoding IE values #330

Closed antoninbas closed 1 year ago

antoninbas commented 1 year ago

When using the provided slice (provided by the collector) directly, the garbage collector will not be able to release the underlying array while references to the net objects exist. The underlying array is allocated when reading the message from the TCP connection and can be 100s of bytes in size. The collector will typically run aggregation and keep references to the data record and its IEs for some time. During that time, we cannot release the packet buffer, while new packets keep coming in. To avoid the issue, we create a new slice, with a new underlying array, and the packet buffer can be releases as soon as all IEs are instantiated.

codecov[bot] commented 1 year ago

Codecov Report

Merging #330 (7d8029e) into main (a9e36d6) will decrease coverage by 0.14%. The diff coverage is 40.00%.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/vmware/go-ipfix/pull/330/graphs/tree.svg?width=650&height=150&src=pr&token=448A94JZ59&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware)](https://app.codecov.io/gh/vmware/go-ipfix/pull/330?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware) ```diff @@ Coverage Diff @@ ## main #330 +/- ## ========================================== - Coverage 73.53% 73.39% -0.14% ========================================== Files 19 19 Lines 2800 2808 +8 ========================================== + Hits 2059 2061 +2 - Misses 574 578 +4 - Partials 167 169 +2 ``` | [Flag](https://app.codecov.io/gh/vmware/go-ipfix/pull/330/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware) | Coverage Δ | | |---|---|---| | [integration-tests](https://app.codecov.io/gh/vmware/go-ipfix/pull/330/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware) | `51.53% <ø> (ø)` | | | [unit-tests](https://app.codecov.io/gh/vmware/go-ipfix/pull/330/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware) | `72.47% <40.00%> (-0.14%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/vmware/go-ipfix/pull/330?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware) | Coverage Δ | | |---|---|---| | [pkg/entities/ie.go](https://app.codecov.io/gh/vmware/go-ipfix/pull/330?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware#diff-cGtnL2VudGl0aWVzL2llLmdv) | `51.85% <40.00%> (-0.55%)` | :arrow_down: |
antoninbas commented 1 year ago

Before the change:

=== RUN   TestTCPCollectingProcess_ReceiveDataRecordsMemoryUsage
I1004 13:18:40.576899   58679 tcp.go:39] Start TCP collecting process on 127.0.0.1:56807
    process_test.go:168: Data packet length: 33
I1004 13:18:40.713559   58679 process.go:121] stopping the collecting process
E1004 13:18:40.713533   58679 tcp.go:79] "Error when retrieving message length" err="read tcp 127.0.0.1:56807->127.0.0.1:56809: use of closed network connection"
    process_test.go:195: Live objects: 4432
    process_test.go:196: Bytes of allocated heap objects: 514984

After the change:

=== RUN   TestTCPCollectingProcess_ReceiveDataRecordsMemoryUsage
I1004 13:40:25.811470   62567 tcp.go:39] Start TCP collecting process on 127.0.0.1:56963
    process_test.go:169: Data packet length: 33
I1004 13:40:25.941870   62567 process.go:121] stopping the collecting process
    process_test.go:197: Live objects: 4427
    process_test.go:198: Bytes of allocated heap objects: 481208

The difference is roughly 33KB (1,000 packets x 33B per packet). In a real deployment with 100s connections per second and data records that can be as large as 1000B (think Pods with labels), the difference can be quite significant.