This repository was archived by the owner on Oct 13, 2023. It is now read-only.
[18.09] backport journald reading fixes (ENGCORE-941) #318
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As in other similar drivers (jsonlog, local), use a set
(i.e. `map[whatever]struct{}`), making the code simpler.
While at it, make sure we remove the reader from the set
after calling `ProducerGone()` on it.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit b2b169f)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Clean up a deferred function call in the journal reading logic. Signed-off-by: Nalin Dahyabhai <nalin@redhat.com> (cherry picked from commit 1ada3e8) Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Minor code simplification. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> (cherry picked from commit e8f6166) Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In case Tail=N parameter is requested, we need to show N lines. It does not make sense to walk backwards one by one if we can do it at once. Now, if Since=T is also provided, make sure we haven't jumped too far (before T), and if we did, move forward. The primary motivation for this was to make the code simpler. This also fixes a tiny bug in the "since" implementation. Before this commit: > $ docker logs -t --tail=6000 --since="2019-03-10T03:54:25.00" $ID | head > 2019-03-10T03:54:24.999821000Z 95981 After: > $ docker logs -t --tail=6000 --since="2019-03-10T03:54:25.00" $ID | head > 2019-03-10T03:54:25.000013000Z 95982 Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> (cherry picked from commit ff3cd16) Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In case the LogConsumer is gone, the code that sends the message can stuck forever. Wrap the code in select case, as all other loggers do. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> (cherry picked from commit 7903972) Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. The journald client library initializes inotify watch(es) during the first call to sd_journal_get_fd(), and it make sense to open it earlier in order to not lose any journal file rotation events. 2. It only makes sense to call this if we're going to use it later on -- so add a check for config.Follow. 3. Remove the redundant call to sd_journal_get_fd(). NOTE that any subsequent calls to sd_journal_get_fd() return the same file descriptor, so there's no real need to save it for later use in wait_for_data_cancelable(). Based on earlier patch by Nalin Dahyabhai <nalin@redhat.com>. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> (cherry picked from commit 981c016) Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
TL;DR: simplify the code, fix --follow hanging indefinitely
Do the following to simplify the followJournal() code:
1. Use Go-native select instead of C-native polling.
2. Use Watch{Producer,Consumer}Gone(), eliminating the need
to have journald.closed variable, and an extra goroutine.
3. Use sd_journal_wait(). In the words of its own man page:
> A synchronous alternative for using sd_journal_get_fd(),
> sd_journal_get_events(), sd_journal_get_timeout() and
> sd_journal_process() is sd_journal_wait().
Unfortunately, the logic is still not as simple as it
could be; the reason being, once the container has exited,
journald might still be writing some logs from its internal
buffers onto journal file(s), and there is no way to
figure out whether it's done so we are guaranteed to
read all of it back. This bug can be reproduced with
something like
> $ ID=$(docker run -d busybox seq 1 150000); docker logs --follow $ID
> ...
> 128123
> $
(The last expected output line should be `150000`).
To avoid exiting from followJournal() early, add the
following logic: once the container is gone, keep trying
to drain the journal until there's no new data for at
least `waitTimeout` time period.
Should fix docker/for-linux#575
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit f091feb)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
If we take a long time to process log messages, and during that time journal file rotation occurs, the journald client library will keep those rotated files open until sd_journal_process() is called. By periodically calling sd_journal_process() during the processing loop we shrink the window of time a client instance has open file descriptors for rotated (deleted) journal files. This code is modelled after that of journalctl [1]; the above explanation as well as the value of 1024 is taken from there. [v2: fix CErr() argument] [1] https://github.com/systemd/systemd/blob/dc16327c48d/src/journal/journalctl.c#L2676 Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> (cherry picked from commit b73fb8f) Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
From the first glance, `docker logs --tail 0` does not make sense, as it is supposed to produce no output, but `tail -n 0` from GNU coreutils is working like that, plus there is even a test case (`TestLogsTail` in integration-cli/docker_cli_logs_test.go). Now, something like `docker logs --follow --tail 0` makes total sense, so let's make it work. (NOTE if --tail is not used, config.Tail is set to -1) Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> (cherry picked from commit dd4bfe3) Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Use "in-place" variables for if statements to limit their scope to the respectful `if` block. 2. Report the error returned from sd_journal_* by using CErr(). 3. Use errors.New() instead of fmt.Errorf(). Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> (cherry picked from commit 20a0e58) Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a backport of
Clean 🍒-pick, no issues