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

[19.03] Fix rate limiting for logger, increase refill rate #40628

Merged

Conversation

cpuguy83
Copy link
Member

@cpuguy83 cpuguy83 commented Mar 5, 2020

Backports #39360 to 19.03


Signed-off-by: Ethan Mosbaugh ethan@replicated.com

- What I did

fixes #38640

- How I did it

The rate limiter will get refilled at a rate of 1 token per second. My understanding of the code it is intended to get refilled at a rate of 10M / second.

// A Limiter controls how frequently events are allowed to happen.
// It implements a "token bucket" of size b, initially full and refilled
// at rate r tokens per second.
// Informally, in any large enough time interval, the Limiter limits the
// rate to r tokens per second, with a maximum burst size of b events.
// As a special case, if r == Inf (the infinite rate), b is ignored.
// See https://en.wikipedia.org/wiki/Token_bucket for more about token buckets.

- How to verify it

$ docker service create --name lotsologs --restart-condition on-failure -d debian:stretch-slim bash -c "for i in \$(seq 1 60000); do cat /dev/urandom | tr -dc 'a-zA-Z0-9 ' | fold -w 256 | head -n 1; done"
y0uhc0ulyxcqe2r5gehfzeiks

wait a while for the logs to fill up

$ docker service logs lotsologs > tmp # hangs
^C
$ ls -lh tmp
-rw-rw-r-- 1 ethan ethan 12M Jun 12 21:07 tmp

- Description for the changelog

Fixed an issue that caused requests for docker swarm service and task logs to hang indefinitely for logs with size greater than 10 MB.

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

Gizmo PNG

Signed-off-by: Ethan Mosbaugh <ethan@replicated.com>
(cherry picked from commit 50c6a5f)
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah thaJeztah modified the milestones: 19.03.8, 19.03.9 Mar 11, 2020
@thaJeztah thaJeztah merged commit d12b6d2 into moby:19.03 Apr 2, 2020
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.

4 participants