apiserver: rate-limit logsink receives #7474

Merged
merged 1 commit into from Jun 19, 2017

Conversation

Projects
None yet
3 participants
Member

axw commented Jun 8, 2017

Description of change

Update the logsink handler so that it will
rate-limit receipt of log messages. Rate-limiting
defaults to a burst of 1000 messages, after
which we allow 1 message per millisecond. This
is configurable by modifying the controller
agent's agent.conf. If we find that users
need to configure it regularly, we can promote
the configuration to controller config.

QA steps

  1. juju bootstrap localhost
  2. juju model-config -m controller logging-config='=TRACE'
  3. juju ssh -m controller 0 "sudo tail -F /var/log/juju/logsink.log"

Observe that the logs come in quickly.

  1. juju ssh -m controller 0
  2. edit /var/lib/juju/agents/machine-0/agent.conf, adding LOGSINK_RATELIMIT_REFILL: 1s under values.
  3. sudo service jujud-machine-0 restart

Observe that the log messages come in quickly to begin with (up to 1000 messages), and then they are limited to 1 per second.

Documentation changes

Probably not until we know that people need to tweak the defaults, at which point we can add controller-config.

Bug reference

None.

mjs approved these changes Jun 14, 2017

This looks great. Thanks.

How did you decide on the default rate limiting values?

api/apiclient_test.go
+ LogSink: apiserver.LogSinkConfig{
+ RateLimitBurst: apiserver.DefaultLogSinkRateLimitBurst,
+ RateLimitRefill: apiserver.DefaultLogSinkRateLimitRefill,
+ },
@mjs

mjs Jun 14, 2017

Contributor

It might be nicer if LogSink was a pointer. Then if it's nil then the defaults could be used. This might be preferable to having to specify the defaults all over the place.

@axw

axw Jun 15, 2017

Member

I've done that in my other PR that makes the db logging parameters configurable. I'll rebase this once that has landed.

Member

axw commented Jun 15, 2017

How did you decide on the default rate limiting values?

Pretty arbitrarily; 1000 log messages/s per agent seems like quite a lot already to me. I tested this with canonical-kubernetes, and it didn't cause any backing up. I'll put it in the cards to do some scale testing before we release 2.2.1.

Member

axw commented Jun 19, 2017

$$merge$$

Contributor

jujubot commented Jun 19, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

Contributor

jujubot commented Jun 19, 2017

Build failed: Generating tarball failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/11153

apiserver: rate-limit logsink receives
Update the logsink handler so that it will
rate-limit receipt of log messages. Rate-limiting
defaults to a burst of 1000 messages, after
which we allow 1 message per millisecond. This
is configurable by modifying the controller
agent's agent.conf. If we find that users
need to configure it regularly, we can promote
the configuration to controller config.
Member

axw commented Jun 19, 2017

$$merge$$

Contributor

jujubot commented Jun 19, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit 9700197 into juju:2.2 Jun 19, 2017

1 check passed

github-check-merge-juju Ran tests against PR. Use !!.*!! to request another build. IE, !!build!!, !!retry!!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment