Skip to content

Log when client rate limiter latency is very high at a lower log level#87740

Merged
k8s-ci-robot merged 1 commit intokubernetes:masterfrom
jennybuckley:rate-limit-log
Feb 2, 2020
Merged

Log when client rate limiter latency is very high at a lower log level#87740
k8s-ci-robot merged 1 commit intokubernetes:masterfrom
jennybuckley:rate-limit-log

Conversation

@jennybuckley
Copy link
Copy Markdown

@jennybuckley jennybuckley commented Feb 1, 2020

What type of PR is this?:
/kind feature
/priority important-soon
/sig api-machinery
/cc @lavalamp

What this PR does / why we need it:
Currently, high client side rate limiter latency logging only happens when at log level 3, it would be useful to also log this information at lower log levels, but we need to avoid spamming the logs. This PR logs a maximum of once per second whenever the rate limiter latency exceeds 1s.

Does this PR introduce a user-facing change?:

API request throttling (due to a high rate of requests) is now reported in client-go logs at log level 2.  The messages are of the form

Throttling request took 1.50705208s, request: GET:<URL>

The presence of these messages, may indicate to the administrator the need to tune the cluster accordingly.

@k8s-ci-robot k8s-ci-robot requested a review from lavalamp February 1, 2020 00:22
@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 1, 2020
@lavalamp
Copy link
Copy Markdown
Contributor

lavalamp commented Feb 1, 2020

Thanks!

/lgtm
/approve

In a followup, can you

  • sum the total latency across all invocations?
  • include that in the log messages?

Optionally, make a second rate limiter instance for the existing message and promote it to V(2) but not more than once every 10 seconds or something like that.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 1, 2020
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Feb 1, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jennybuckley, lavalamp

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 1, 2020
@fejta-bot
Copy link
Copy Markdown

fejta-bot commented Feb 1, 2020

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

7 similar comments
@fejta-bot
Copy link
Copy Markdown

fejta-bot commented Feb 1, 2020

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@fejta-bot
Copy link
Copy Markdown

fejta-bot commented Feb 1, 2020

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@fejta-bot
Copy link
Copy Markdown

fejta-bot commented Feb 1, 2020

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@fejta-bot
Copy link
Copy Markdown

fejta-bot commented Feb 1, 2020

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@fejta-bot
Copy link
Copy Markdown

fejta-bot commented Feb 1, 2020

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@fejta-bot
Copy link
Copy Markdown

fejta-bot commented Feb 2, 2020

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@fejta-bot
Copy link
Copy Markdown

fejta-bot commented Feb 2, 2020

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@k8s-ci-robot k8s-ci-robot merged commit 9de5763 into kubernetes:master Feb 2, 2020
@k8s-ci-robot k8s-ci-robot added this to the v1.18 milestone Feb 2, 2020
@jennybuckley
Copy link
Copy Markdown
Author

jennybuckley commented Feb 3, 2020

@lavalamp Sure, this one is at v(2) should I make the other instance at v(1) or v(0)?

@lavalamp
Copy link
Copy Markdown
Contributor

lavalamp commented Feb 3, 2020 via email

@jennybuckley jennybuckley mentioned this pull request Feb 13, 2020
Merged
@SataQiu SataQiu mentioned this pull request Apr 30, 2021
Closed
@liggitt liggitt mentioned this pull request May 10, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lavalamp lavalamp Awaiting requested review from lavalamp

Assignees

@lavalamp lavalamp

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

v1.18

Development

Successfully merging this pull request may close these issues.

4 participants

@jennybuckley @lavalamp @k8s-ci-robot @fejta-bot