Memory leak

[SimSonic]

Hi! I'm logging a lot of stuff to Graylog using GELF UDP.

Application (Spring Boot) has an thread pools which are not fixed at size — at peak loads there are new threads born and die after timeout.

I use -XX:MaxDirectMemorySize=150M and cannot grow constantly due to set limits in Kubernetes.

I've looked at source code and read following issues:

• https://github.com/mp911de/logstash-gelf/issues/172
• https://github.com/mp911de/logstash-gelf/issues/169

What if store ByteBuffer's in ThreadLocals via soft or weak references?

Also, I think that operation mode able to leak native memory should not be default.

Thanks!

[mp911de]

It's a tradeoff between GC and throughput performance. These issues always work for exactly 50% of users so there's no right default. You can disable buffers through the config.

[SimSonic]

In my opinion it is disbalanced now.

Disabled buffers: just slower performance.

Enabled (default): better performance but garantee risk of native OOMs and completely failed service.

What about wrapping ByteBuffer's into weak references? It should be also memory leak but for a cents, not megabytes.

[mp911de]

GC is exactly what we want to avoid. If your workload pattern spins up constantly new threads, then disabling buffers is the way to go so you can stay within your memory limits. Using buffers comes at the cost of memory. There's nothing we can do about it.

Since this ticket isn't actionable, I'm closing this one.

[SimSonic]

Spent all day fixing production after release!

Previously moved library into separate spring boot starter with ApplicationContextInitializer where I'm setting these two system properties. Library's impl is so I cannot switch behaviour at runtime, only before first appender call (you use static boolean fields 😥). With own starter I have no control over command line parameters (-D...) of all microservices.

In todays case almost non-corellating functionality (updating Spring Cloud Vault properties) affected somehow logging. Appender was initialized before my starter and I late to set system properties even in context initializer:

@mp911de please, think into possibility to control behaviour in runtime, or change defaults to not be so dangerous.

[mp911de]

The loggers do need to rely on their own config mechanism while some components of the library share internal state. We cannot make any assumptions over the container in which it is running.

not be so dangerous

Any kind of not met assumptions imposes negative impact regardless of what we assume. Switching to non-pooled buffers increases CPU and GC pressure and slows applications down.

[SimSonic]

@mp911de You can change code to store parsed from system properties settings not in static final fields but in own containers (AtomicBoolean, e.g.) with publically available setter.

So this will not introduce container-awareness and give possibility to change setting in runtime.

I'm gonna make PR for you to show approach.

[mp911de]

Likely it would make sense to introduce a mutable configuration, given that there's quite some control from the application server-side. 8 years ago, pre-installed application servers didn't allow for pre-main activity but rather applications were dropped into the container so system properties kind-of made sense. I also see that we've reached a point in time where we need to revisit our configuration mechanism.

A volatile properties holder would be a good mechanism.