Skip to content
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

[20.10 backport] daemon/logger: read the length header correctly #43165

Merged
merged 4 commits into from Jan 27, 2022

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Jan 20, 2022

fixes #42125

Backport of:


Before this change, if Decode() couldn't read a log record fully,
the subsequent invocation of Decode() would read the record's non-header part
as a header and cause a huge heap allocation.

This change prevents such a case by having the intermediate buffer in
the decoder struct.

Fixes #42125.

- What I did

Changing decoder struct to fix #42125.

- How I did it

Changing decoder struct to keep read-but-unused bytes.

- How to verify it

Run docker logs -f $(docker run --rm -d --log-driver local alpine cat /dev/urandom) > /dev/null. It kills dockerd after a few minutes before the change.

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

Havam

(from https://www.zooborns.com/zooborns/2021/05/two-very-different-babies-emerge-at-woodland-park-zoo.html)

kzys added 4 commits Jan 20, 2022
Before this change, if Decode() couldn't read a log record fully,
the subsequent invocation of Decode() would read the record's non-header part
as a header and cause a huge heap allocation.

This change prevents such a case by having the intermediate buffer in
the decoder struct.

Fixes moby#42125.

Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
(cherry picked from commit 48d387a)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
(cherry picked from commit f2e458e)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
followLogs() is getting really long (170+ lines) and complex.
The function has multiple inner functions that mutate its variables.

To refactor the function, this change introduces follow{} struct.
The inner functions are now defined as ordinal methods, which are
accessible from tests.

Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
(cherry picked from commit 7a10f5a)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
(cherry picked from commit c91e09b)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

thaJeztah commented Jan 26, 2022

@cpuguy83 @samuelkarp ptal

Copy link
Member

@cpuguy83 cpuguy83 left a comment

LGTM

@cpuguy83 cpuguy83 merged commit dd61d8d into moby:20.10 Jan 27, 2022
2 checks passed
@thaJeztah thaJeztah deleted the 20.10_backport_logs_fixes branch Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants