You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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?
The text was updated successfully, but these errors were encountered:
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
andBufferLimitBytes
. Both limits would be checked before deciding whether to return an error or enqueue the message. The defaults would be 8192 forBufferLimit
and 0 forBufferLimitBytes
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?
The text was updated successfully, but these errors were encountered: