[Auditbeat] Memory leak in 7.5.2 #16879
Conversation
|
Pinging @elastic/siem (Team:SIEM) |
There was a problem hiding this comment.
Looking good, but there's some issues.
I'm also worried about process expiration. Expiring a socket or flow is not an issue because further traffic will just re-create them. The worst that can happen is to have a network flow split in two output events.
But with processes, once they expire they won't be recreated and we lose the ability to enrich flows with process information. So for example any process that doesn't perform network i/o for 30s will not be detected.
Given that PIDs, unlike socket pointers, are reused, if they're not expired the cache will max out at whatever the system limit is (/proc/sys/kernel/pid_max seems to default at 32768). So I think maybe it's safe to not expire processes, WDYT?
| } | ||
|
|
||
| func NewState(r mb.PushReporterV2, log helper.Logger, processTimeout, inactiveTimeout, closeTimeout, clockMaxDrift time.Duration) *State { | ||
| s := makeState(r, log, processTimeout, inactiveTimeout*2, inactiveTimeout, closeTimeout, clockMaxDrift) |
There was a problem hiding this comment.
This is setting the socket timeout to 2*inactiveTimeout, which is config.FlowInactiveTimeout, but at config.go says that InactiveTimeout is used for both sockets and processes.
| "github.com/elastic/beats/v7/x-pack/auditbeat/tracing" | ||
| ) | ||
|
|
||
| type Endpoint struct { |
There was a problem hiding this comment.
All public declarations should have a comment starting with the type/function/variable name:
// Endpoint blah blah.
We have a bot that reviews all this automatically, seems that it's been broken for some weeks now.
|
|
||
| func (e *UDPQueueRcvSkbCall) Flow() *common.Flow { | ||
| var remote *common.Endpoint | ||
| if valid := validIPv4Headers(e.IPHdr, e.UDPHdr, e.Packet[:]); valid { |
There was a problem hiding this comment.
It seems that during the refactor the logic in here was messed up. The second call to validIPv4Headers below should only happen if the first call returns false. It's a workaround for the case where e.IPHdr/e.UDPHdr are (the lowest 16bit) of pointers instead of indices inside e.Packet.
x-pack/auditbeat/module/system/socket/events/udpv6_queue_rcv_skb.go
Outdated
Show resolved
Hide resolved
|
Also, it needs an entry in CHANGELOG.next.asciidoc and a rebase to pass CI. |
adriansr
force-pushed
the
socket_timeouts
branch
from
457542a
to
e19f1f4
Compare
andrewstucki
force-pushed
the
socket_timeouts
branch
from
b1b65f2
to
48c52cd
Compare
|
rebased from #17500 |
|
Hi! We're labeling this issue as |
|
Hi! |
What does this PR do?
Why is it important?
Without the timeout-based cache eviction, we're leaking memory.
Checklist
Related issues
Original Issue at: https://discuss.elastic.co/t/auditbeat-memory-leak-in-7-5-2/218335