prometheus-community / pro-bing

A library for creating continuous probers
MIT License
309 stars 52 forks source link

Memory bloat when sending pings to something which does not respond #31

Open TheRushingWookie opened 1 year ago

TheRushingWookie commented 1 year ago

When looking through the code I noticed the awaitingSequences map can grow infinitely if ICMP requests are never responded to. This causes increased memory bloat over time and is something I'd like to get rid of.

I know its a fairly slow bloat and most users probably use pro-bing in short lived contexts, but I'd like to have many pingers (100s) with very fast ping intervals (100 ms) and will be long lived (months). So far I haven't run into any issues but I don't want to wait for issues to pop up related to this in the future.

The original idea I had was to use timeouts per packet like what https://github.com/prometheus-community/pro-bing/issues/19 wants. Once it times out, remove from awaitingSequences. This doesn't seem that good as we lose visibility into timed out duplicated ICMP responses while what we care about is the amount of resources a ping uses.

Instead I've looked into limiting the total number of in flight packets pro-bing tracks instead. This means we only lose visibility into duplicated and timed out packets when we're past a certain number of them.

I have a very early draft for this in https://github.com/prometheus-community/pro-bing/compare/main...TheRushingWookie:pro-bing:remove_bloat?expand=1

Its not clean or fully done, but it does limit the resources by using what is basically an evicting cache which generates the UUID/sequence number for pro-bing to send.

What do you guys think about this?