-
Notifications
You must be signed in to change notification settings - Fork 18.6k
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
log without '\n' cause the docker-daemon's memory overload #18057
Comments
ping @LK4D4 |
@cloudfly What you think should happen if buffer reached? |
Interesting problem, can't really log part of a message, and should not be logging huge messages, but should degrade gracefully when such occurs. To even do a bail-out would be kind of tricky with several lines of lower-level code and possibly a loop. |
Or can run the logger in a separate process in the container's cgroup so that at least the process in the container can't destroy the host when |
@LK4D4 Flush data into log driver, when we get the |
i think @cloudfly 's suggestion on flushing if buffer is reached makes sense to me |
@cloudfly @crosbymichael As separate line? Do you think that log-driver design about separate messages wrong overall? |
bla |
oh...yes , a little complicated. another unreliable idea. change the type Message struct {
ContainerID string
Line io.Reader // []byte => io.Reader
Source string
Timestamp time.Time
} docker create a this solution leave the problem to log-driver. log-driver should not buffer the data to wait for |
+1
and
Is there any progress on fixing it? |
How big are those one line messages most of the time? At most 64 KB? Maybe we could buffer up to a certain amount and dump that to something such as LogBytes([]byte) in the logging drivers (as was suggested above). Once we go into this logging mode, we keep logging like this until we encounter the end of the line. When we're done with that, we look for the end of the line and log a regular EOL terminated message. I think this might be one of the last memory usage issues left to fix if it's not the very last one. What do you think? |
#13564 (for #13333) seems caused this issue. Basically +1 for Not only the memory usage but also CPU is heavily affected by massive logs: #21181 The cgroup-based approach should be also able to handle the CPU load issue. Plus as a side-effect, it would be easy for third parties to implement their own logging plugins. |
infinite loop of printing causes this, i think wether or not there are
...... @cloudfly - I don't think "\n" is really a valid workaround ? ...... |
We see this issue as well. |
@nalind PTAL |
Two things seem to be happening - in the logger copySrc() goroutine, the bufio ReadBytes() call just keeps waiting for a newline, buffering up whatever it's already read, until it finds one. If one doesn't come, we consume lots of memory. I think we could either switch to using ReadLine(), which lets us read and know if we only got part of a line, or do that logic ourselves since ReadLine() doesn't give us a "there wasn't a newline" indication correctly at the end of data. We also seem to be growing our memory usage when we read back those logs, in different places in the json-file (in decodeLogLine()) and journald (in GoBytes()) reader implementations, but after a point the garbage collector keeps the usage from getting higher. How you'd limit the disk usage of logs varies depending on the log driver. The journald daemon should be heeding its settings from journald.conf(5), and the json-file logger offers a "max-size" option to limit the size of a container's log file, with its "max-file" option used to turn on log rotation. The json-file settings are per-container, however, so you have to manage them a little more carefully. Adding a global setting might make some sense there. |
@nalind there is a global (daemon level) Also (slightly related), perhaps you have thoughts on #22450. We're unsure what the "right" approach is there |
@thaJeztah I was thinking more along the lines of having something that would limit the cumulative usage of json-file logs in all containers, as an alternative to having to do the math externally. The design for that could get complicated, though. |
@nalind ah, right; I think overall the JSON driver is not really designed for that. Overall not the recommended driver for anything in production / high volume logging |
Not able to remove containers, issue seems to be same as 20600 [not completely]. Docker version - Server: docker info Logs - docker rm 64af6442b25c docker ps -a |
I think we can close this, now that #22982 was merged; this will be in docker 1.13 |
This change updates how we handle long lines of output from the container. The previous logic used a bufio reader to read entire lines of output from the container through an intermediate BytesPipe, and that allowed the container to cause dockerd to consume an unconstrained amount of memory as it attempted to collect a whole line of output, by outputting data without newlines. To avoid that, we replace the bufio reader with our own buffering scheme that handles log lines up to 16k in length, breaking up anything longer than that into multiple chunks. If we can dispense with noting this detail properly at the end of output, we can switch from using ReadBytes() to using ReadLine() instead. We add a field ("Partial") to the log message structure to flag when we pass data to the log driver that did not end with a newline. The Line member of Message structures that we pass to log drivers is now a slice into data which can be overwritten between calls to the log driver's Log() method, so drivers which batch up Messages before processing them need to take additional care: we add a function (logger.CopyMessage()) that can be used to create a deep copy of a Message structure, and modify the awslogs driver to use it. We update the jsonfile log driver to append a "\n" to the data that it logs to disk only when the Partial flag is false (it previously did so unconditionally), to make its "logs" output correctly reproduce the data as we received it. Likewise, we modify the journald log driver to add a data field with value CONTAINER_PARTIAL_MESSAGE=true to entries when the Partial flag is true, and update its "logs" reader to refrain from appending a "\n" to the data that it retrieves if it does not see this field/value pair (it also previously did this unconditionally). fix http://code.huawei.com/docker/docker/issues/324 upsteam issue: moby#18057 fix DTS2017062307255 cherry-pick from: moby#22982 conflicts: daemon/logger/copier.go daemon/logger/journald/read.go daemon/logger/logger.go Signed-off-by: Nalin Dahyabhai <nalin@redhat.com> (github: nalind) Signed-off-by: Lei Jitang <leijitang@huawei.com>
Description of problem:
code here: https://github.com/docker/docker/blob/master/daemon/logger/copier.go#L47
if a container always print logs without
\n
, then docker will buffer the logs until it find the\n
this makes docker-daemon use up all the memory.
I think the condition should be: find a
\n
or the buffer reached64k
(or smaller).How reproducible:
Steps to Reproduce:
top
command to check the memory usage of docker, (wait a minute)docker stop xxxx
stop the container, memory usage will increased very high fastly during this step.the debug log is:
Actual Results: None
Expected Results: None
Additional info: None
The text was updated successfully, but these errors were encountered: