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

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

Merged
andrewhsu merged 2 commits into
docker-archive:17.06from
andrewhsu:fix-log-readers
Jul 12, 2017
Merged

[17.06] Fix log readers can block writes indefinitely#98
andrewhsu merged 2 commits into
docker-archive:17.06from
andrewhsu:fix-log-readers

Conversation

@andrewhsu

Copy link
Copy Markdown
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 <cpuguy83@gmail.com>
(cherry picked from commit e220918)

Conflicts:
components/engine/daemon/logger/jsonfilelog/read.go
Signed-off-by: Andrew Hsu <andrewhsu@docker.com>
@cpuguy83

cpuguy83 commented Jul 6, 2017

Copy link
Copy Markdown
Contributor

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

}

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

Copy link
Copy Markdown
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
Copy Markdown
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

… working

Signed-off-by: Andrew Hsu <andrewhsu@docker.com>

@cpuguy83 cpuguy83 left a comment

Copy link
Copy Markdown
Contributor

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