Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a separate BufferLimitBytes option to limit memory usage #128

Open
Kern-- opened this issue Jun 10, 2024 · 0 comments
Open

Add a separate BufferLimitBytes option to limit memory usage #128

Kern-- opened this issue Jun 10, 2024 · 0 comments

Comments

@Kern--
Copy link

Kern-- commented Jun 10, 2024

In #56 in 2018, the BufferLimit option had a semantic change. It used to mean the number of bytes that could be buffered, but after the change it means the number of messages that can be buffered (which is effectively the number of log lines that can be buffered). That PR also lowered the default limit from 8 MiB to 8192 log lines.

Docker and containerd (via shim-loggers-for-containerd) use fluent-logger-golang, but override the default value to what was originally 1MiB. After picking up the change, these runtimes went from allowing 1 MiB to 1M+ log lines which can be easily reach into the hundreds of MiB or even GiB in real use-cases. We've seen this cause problems where instead of back pressure or lost logs, we see OOM killed containers leading to wasted reasources on overprovisioned setups.

This was discussed before in #88, but I think it should be revisited.

Unfortunately, changing this transparently is probably not feasible. The change makes the limit looser which is less likely to cause impact. Trying to make it tighter could cause workloads that are currently fine to start experiencing issues.

So instead, I propose a new BufferLimitBytes option that allows a user to specify the maximum number of bytes that can be buffered. When queuing messages, the current bytes buffered would be incremented and after sending the message it would be decremented.

Users would be able to set either or both of BufferLimit and BufferLimitBytes. Both limits would be checked before deciding whether to return an error or enqueue the message. The defaults would be 8192 for BufferLimit and 0 for BufferLimitBytes meaning "no limit". This way, users can control both how many pending logs are outstanding and the total memory usage.

Docker and shim-loggers-for-containerd would be on the hook for how to expose this to end users, but that can be handled in those respective projects.

What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant