Skip to content
This repository has been archived by the owner on Aug 9, 2022. It is now read-only.

GitHub API rate limit exceeded #74

Open
ShotaKitazawa opened this issue Dec 26, 2021 · 2 comments · Fixed by #96 · May be fixed by #108
Open

GitHub API rate limit exceeded #74

ShotaKitazawa opened this issue Dec 26, 2021 · 2 comments · Fixed by #96 · May be fixed by #108
Assignees
Labels
bug Something isn't working high priority

Comments

@ShotaKitazawa
Copy link
Member

GitHub API の rate limit に引っかがってしまったので、以下あたりを見直す

  • busy wait でめちゃくちゃ GitHub API を叩いてしまっていないか
  • resync period の値を延ばせるか
2021-12-26T03:29:37.963Z        ERROR   controller.reviewapp    Reconciler error        {"reconciler group": "dreamkast.cloudnativedays.jp", "reconciler kind": "ReviewApp", "name": "dreamkast-dk-cloudnativedaysjp-dreamkast-1055", "namespace": "reviewapp-operator-system", "error": "GET https://api.github.com/users/showks-containerdaysjp: 403 API rate limit exceeded for user ID 44684172. [rate reset in 5m28s]: GET https://api.github.com/users/showks-containerdaysjp: 403 API rate limit exceeded for user ID 44684172. [rate reset in 5m28s]", "errorVerbose": "GET https://api.github.com/users/showks-containerdaysjp: 403 API rate limit exceeded for user ID 44684172. [rate reset in 5m28s]:\n    github.com/cloudnativedaysjp/reviewapp-operator/gateways.(*GitHub).WithCredential\n        /workspace/gateways/github.go:66\n  - GET https://api.github.com/users/showks-containerdaysjp: 403 API rate limit exceeded for user ID 44684172. [rate reset in 5m28s]"}
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2
        /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:227
@ShotaKitazawa
Copy link
Member Author

golang.org/x/sync/singleflight を使って、「n秒以内の同様の API リクエストは singleflight させる」ようにしたほうがいいかも

@ShotaKitazawa ShotaKitazawa changed the title GitHub API の rate limit GitHub API の rate limit に引っかがる Dec 26, 2021
@ShotaKitazawa ShotaKitazawa added bug Something isn't working high priority labels Dec 26, 2021
@ShotaKitazawa ShotaKitazawa mentioned this issue Apr 9, 2022
@ShotaKitazawa ShotaKitazawa mentioned this issue Apr 18, 2022
@ShotaKitazawa ShotaKitazawa reopened this Apr 18, 2022
@ShotaKitazawa
Copy link
Member Author

ShotaKitazawa commented Apr 18, 2022

In version v0.1.2, I implemented with below policy.

  • prepare ReviewApp object's Status.Sync.SyncedPullRequest to store PullRequest information
  • ReviewAppManager controller update to ReviewApp object's Status.Sync.SyncedPullRequest
    • I tried to decrease request to GitHub API by ReviewApp controller (hooked each ReviewApp objects).

However, above "updating by ReviewAppManager controller-side" implementation causes the action of updating all of ReviewApp object's status, which has below problems.

  • Because ReviewAppManager controller own ReviewApp, ReviewAppManager reconciler was hooked every patched ReviewApp object from ReviewAppManager controller. (It is busy loop!)
  • conflict in process of patching ReviewApp object's status between ReviewAppManager and ReviewApp controllers.
    • cf.
      1. ReviewAppManager controller get ReviewApp object.
      2. ReviewApp controller update ReviewApp object's status.
      3. ReviewAppManager controller update ReviewApp object's status.
        • Because ReviewAppManager controller calculate difference at "sequence 1" in controller-side, updation in "sequence 2" is lost.

approach

I will declare new CR: PullRequestCache.
ReviewAppManager and ReviewApp controller will refer to PullRequestChace object. This object's update don't cause to hook any Reconciler.

@ShotaKitazawa ShotaKitazawa changed the title GitHub API の rate limit に引っかがる GitHub API rate limit exceeded Apr 18, 2022
@ShotaKitazawa ShotaKitazawa self-assigned this Apr 18, 2022
@ShotaKitazawa ShotaKitazawa linked a pull request May 22, 2022 that will close this issue
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working high priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant