Skip to content
This repository has been archived by the owner on Oct 13, 2023. It is now read-only.

[17.06] Fix log readers can block writes indefinitely #98

Merged
merged 2 commits into from
Jul 12, 2017
Merged

[17.06] Fix log readers can block writes indefinitely #98

merged 2 commits into from
Jul 12, 2017

Conversation

andrewhsu
Copy link
Contributor

Backport fix:

The only conflict was in the components/engine/daemon/logger/jsonfilelog/read.go file:

  if config.Tail != 0 {
<<<<<<< HEAD
    tailer := ioutils.MultiReadSeeker(append(files, latestFile)...)
=======
    tailer := multireader.MultiReadSeeker(append(files, latestChunk)...)
>>>>>>> e2209185ed... Fix log readers can block writes indefinitely
    tailFile(tailer, logWatcher, config.Tail, config.Since)
  }

@cpuguy83 Because moby/moby@2445e6b moved multireader. I decided to go with the ioutils line to resolve the conflict. Let me know if I should go the other way and and also cherry-pick the multireader move commit.

Before this patch, a log reader is able to block all log writes
indefinitely (and other operations) by simply opening the log stream and
not consuming all the messages.

The reason for this is we protect the read stream from corruption by
ensuring there are no new writes while the log stream is consumed (and
caught up with the live entries).

We can get around this issue because log files are append only, so we
can limit reads to only the section of the file that was written to when
the log stream was first requested.

Now logs are only blocked until all files are opened, rather than
streamed to the client.

Signed-off-by: Brian Goff <[email protected]>
(cherry picked from commit e220918)

Conflicts:
components/engine/daemon/logger/jsonfilelog/read.go
Signed-off-by: Andrew Hsu <[email protected]>
@cpuguy83
Copy link
Contributor

cpuguy83 commented Jul 6, 2017

22:08:13 [init] daemon/logger/jsonfilelog/read.go:80: latestChunk declared and not used

logWatcher.Err <- err
return
}

if config.Tail != 0 {
tailer := ioutils.MultiReadSeeker(append(files, latestFile)...)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to replace latestFile here with latestChunk

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done with an additional commit

Copy link
Contributor

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@andrewhsu andrewhsu merged commit 0aba544 into docker-archive:17.06 Jul 12, 2017
@andrewhsu andrewhsu deleted the fix-log-readers branch July 12, 2017 02:01
@andrewhsu andrewhsu modified the milestone: 17.06.1 Jul 12, 2017
docker-jenkins pushed a commit that referenced this pull request Apr 4, 2018
Add packaging code for Fedora 28
Upstream-commit: 9efc79d
Component: packaging
silvin-lubecki pushed a commit to silvin-lubecki/docker-ce that referenced this pull request Jan 30, 2020
Add packaging code for Fedora 28
Upstream-commit: 9efc79d
Component: packaging
silvin-lubecki pushed a commit to silvin-lubecki/docker-ce that referenced this pull request Jan 30, 2020
silvin-lubecki pushed a commit to silvin-lubecki/docker-ce that referenced this pull request Feb 3, 2020
[17.06] Fix log readers can block writes indefinitely
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants