Commit Graph

11 Commits

Author SHA1 Message Date
Aaron Lehmann cbd4e4d48a Merge pull request #24514 from cpuguy83/fix_log_follow_rotate
Fix issues with tailing rotated jsonlog file
2016-08-26 09:47:40 -07:00
Brian Goff 84e60a7e10 Fix issues with tailing rotated jsonlog file
Fixes a race where the log reader would get events for both an actual
rotation as we from fsnotify (`fsnotify.Rename`).
This issue becomes extremely apparent when rotations are fast, for
example:

```
$ docker run -d --name test --log-opt max-size=1 --log-opt max-file=2
busybox sh -c 'while true; do echo hello; usleep 100000; done'
```

With this change the log reader for jsonlogs can handle rotations that
happen as above.

Instead of listening for both fs events AND rotation events
simultaneously, potentially meaning we see 2 rotations for only a single
rotation due to channel buffering, only listen for fs events (like
`Rename`) and then wait to be notified about rotation by the logger.
This makes sure that we don't see 2 rotations for 1, and that we don't
start trying to read until the logger is actually ready for us to.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
2016-08-15 13:57:28 -04:00
allencloud 0ead624473 add defer file.Close to avoid potential fd leak
Signed-off-by: allencloud <allen.sun@daocloud.io>
2016-08-10 08:36:09 +08:00
Stefan J. Wernli 54f11b84d2 Fixing file handle leak for "docker logs"
If "docker logs" was used on an offline container, the logger is leaked, leaving it up to the finalizer to close the file handle, which could block removal of the container.  Further, the json file logger could leak an open handle if the logs are read without follow due to an early return without a close.  This change addresses both cases.

Signed-off-by: Stefan J. Wernli <swernli@microsoft.com>
2016-06-21 18:40:30 -07:00
Brian Goff bd9d14a07b Add support for reading logs extra attrs
The jsonlog logger currently allows specifying envs and labels that
should be propagated to the log message, however there has been no way
to read that back.

This adds a new API option to enable inserting these attrs back to the
log reader.

With timestamps, this looks like so:
```
92016-04-08T15:28:09.835913720Z foo=bar,hello=world hello
```

The extra attrs are comma separated before the log message but after
timestamps.

Without timestaps it looks like so:
```
foo=bar,hello=world hello
```

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
2016-05-06 20:42:20 -04:00
Brian Goff 1ae9dcf97d Fixes some issues with jsonfile write/read
This cleans up some of the use of the filepoller which makes reading
significantly more robust and gives fewer changes to fallback to the
polling based watcher.
In a lot of cases, if the file was being rotated while we were adding it
to the watcher, it would return an error that the file doesn't exist and
would fallback.
In some cases this fallback could be triggered multiple times even if we
were already on the fallback/poll-based watcher.

It also fixes an open file leak caused by not closing files properly on
rotate, as well as not closing files that were read via the `tail`
function until after the log reader is completed.

Prior to the above changes, it was relatively simple to cause the log
reader to error out by having quick rotations, for example:
```
$ docker run --name test --log-opt max-size=10b --log-opt max-files=10
-d busybox sh -c 'while true; do usleep 500000; echo hello; done'
$ docker logs -f test
```
After these changes I can run this forever without error.

Another fix removes 2 `os.Stat` calls when rotating files. The stat
calls are not needed since we are just calling `os.Rename` anyway, which
will in turn also just produce the same error that `Stat` would.
These `Stat` calls were also quite expensive.
Removing these stat calls also seemed to resolve an issue causing slow
memory growth on the daemon.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
2016-03-21 11:05:58 -04:00
Brian Goff 91fdfdd537 Revert "use pubsub instead of filenotify to follow json logs"
This reverts commit b1594c59f5.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
2016-02-23 21:43:52 -05:00
Shijiang Wei b1594c59f5 use pubsub instead of filenotify to follow json logs
inotify event is trigged immediately there's data written to disk.
But at the time that the inotify event is received, the json line might
not fully saved to disk. If the json decoder tries to decode in such
case, an io.UnexpectedEOF will be trigged.
We used to retry for several times to mitigate the io.UnexpectedEOF error.
But there are still flaky tests caused by the partial log entries.

The daemon knows exactly when there are new log entries emitted. We can
use the pubsub package to notify all the log readers instead of inotify.

Signed-off-by: Shijiang Wei <mountkin@gmail.com>

try to fix broken test. will squash once tests pass

Signed-off-by: Shijiang Wei <mountkin@gmail.com>
2016-02-15 19:25:16 +08:00
Justas Brazauskas 927b334ebf Fix typos found across repository
Signed-off-by: Justas Brazauskas <brazauskasjustas@gmail.com>
2015-12-13 18:04:12 +02:00
daehyeok mun 086c0b4a66 Extract rotate file writer from json log driver
Signed-off-by: Daehyeok Mun <daehyeok@gmail.com>
2015-11-14 16:15:44 -07:00
Brian Goff c136a33c5b Fallback to file polling for jsonlog reader on err
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
2015-11-05 10:05:05 -05:00