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

github: Detect secondary rate limit #11

Merged
merged 1 commit into from
Nov 17, 2021

Conversation

puerco
Copy link
Member

@puerco puerco commented Nov 17, 2021

This commit adds logic to the github call retrier to detect the GitHub
secondary rate limit. When detecting the limit, the github client
will wait for the set time (1 min) plus a few seconds more.

The idea behind this is that when doing calls in parallel we don't
want to set loose all workers to the API at the same time.

Permanent fix for kubernetes/release#2324
Fixes kubernetes/release#2225
Fixes kubernetes/release#2302

/kind bug failing-test flake
/priority important-soon
/assign @justaugustus @Verolop @palnabarun @saschagrunert
/cc @kubernetes/release-engineering
/milestone v1.23

Signed-off-by: Adolfo García Veytia (Puerco) adolfo.garcia@uservers.net

This commit adds logic to the git call retrier to detect the GitHub
secondary rate limit. When detecting this limit, the github client
will wait for the set time (1 min) plus a few seconds more.

The idea behind this is that when doing calls in parallel we don't
want to set loose all workers to the API at the same time.

Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Nov 17, 2021
@k8s-ci-robot
Copy link
Contributor

@puerco: GitHub didn't allow me to request PR reviews from the following users: kubernetes/release-engineering.

Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

This commit adds logic to the github call retrier to detect the GitHub
secondary rate limit. When detecting the limit, the github client
will wait for the set time (1 min) plus a few seconds more.

The idea behind this is that when doing calls in parallel we don't
want to set loose all workers to the API at the same time.

Permanent fix for kubernetes/release#2324
Fixes #2225
Fixes #2302

/kind bug failing-test flake
/priority important-soon
/assign @justaugustus @Verolop @palnabarun @saschagrunert
/cc @kubernetes/release-engineering
/milestone v1.23

Signed-off-by: Adolfo García Veytia (Puerco) adolfo.garcia@uservers.net

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. kind/flake Categorizes issue or PR as related to a flaky test. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/release Categorizes an issue or PR as relevant to SIG Release. approved Indicates a PR has been approved by an approver from all required OWNERS files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 17, 2021
Copy link
Member

@cpanato cpanato left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 17, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cpanato, puerco

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

The pull request process is described here

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 merged commit 871b1c3 into kubernetes-sigs:main Nov 17, 2021
Comment on lines +92 to +104
if strings.Contains(err.Error(), "secondary rate limit. Please wait") {
rtime, err := rand.Int(rand.Reader, big.NewInt(30))
if err != nil {
logrus.Error(err)
return false
}
waitDuration := time.Duration(rtime.Int64()*int64(time.Second)) + defaultGithubSleep
logrus.
WithField("err", err).
Infof("Hit the GitHub secondary rate limit on try %d, sleeping for %s", try, waitDuration)
sleeper(waitDuration)
return true
}
Copy link
Member

Choose a reason for hiding this comment

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

How about our idea of locking the gatherer threads if a rate limit occurs? Would that probably prevent the secondary rate limit?

Ref https://docs.github.com/en/rest/overview/resources-in-the-rest-api#secondary-rate-limits

edsantiago added a commit to edsantiago/libpod that referenced this pull request May 26, 2022
Log the error; and try using a newer release-notes which might
be able to handle github secondary rate limit:

   kubernetes-sigs/release-sdk#11

Note that new release-notes needs --markdown-links option

Signed-off-by: Ed Santiago <santiago@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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/bug Categorizes issue or PR as related to a bug. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. kind/flake Categorizes issue or PR as related to a flaky test. 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. sig/release Categorizes an issue or PR as relevant to SIG Release. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

integration tests hit github api rate limit Release notes API rate limiting throws 403
7 participants