Skip to content

Fewer github api calls for performance#27

Merged
skottler merged 3 commits into
jenkinsci:masterfrom
alexrothenberg:fewer-github-api-calls-for-performance
Nov 13, 2014
Merged

Fewer github api calls for performance#27
skottler merged 3 commits into
jenkinsci:masterfrom
alexrothenberg:fewer-github-api-calls-for-performance

Conversation

@alexrothenberg
Copy link
Copy Markdown
Contributor

On jenkins instances with a large number of jobs the previous implementation got very slow because it made a github api call per job & did not do a great job of caching. This pull reduces the chatter with the github api by asking which repos the user has access to and caching the results.

There are 2 big changes in this pull

The algorithm now builds and caches a list of "my repositories"

It makes these api calls to github:

  1. Get list of the non-organization repos user has access to /api/v3/user/repos
  2. Get list of organizations with repos user has access to /api/v3/user/orgs. For each organization
  • Get a list of repos in that org user has access to /api/v3/orgs/XXX/repos

Because of pagination it sometimes makes several calls to get the full list of repos.

Cache list of public repositories for read-only access.

For this it still makes an api call per repository but caches the results for all users.

On my jenkins instance with ~400 projects this has improved performance from unusable (minutes to load the home page) to pretty fast (~4 seconds as it builds the cache and faster after that).

alexrothenberg and others added 3 commits July 30, 2014 10:02
* For a user look up all their user repos & organization repos and cache results.
* When there are a large number of jobs/repositories, this is much more performant than asking each repository for its list of members
  which is what it was previously doing.
* There was also a bug with caching if the first person to ask about a private repository had no access (it incorrectly cached [] as the list of members)
@cloudbees-pull-request-builder
Copy link
Copy Markdown

plugins » github-oauth-plugin #55 SUCCESS
This pull request looks good

@alexrothenberg
Copy link
Copy Markdown
Contributor Author

Created jira issue for this: JENKINS-24069 (https://issues.jenkins-ci.org/browse/JENKINS-24069)

@suryagaddipati
Copy link
Copy Markdown
Member

@alexrothenberg Assuming this fix is for when you are using Github authorization . When does the cache expire? Looks like user would have access even if he is removed from the github org until there is a jenkins restart?

@alexrothenberg
Copy link
Copy Markdown
Contributor Author

Yes this is for when we are using github authorization and letting the repo permission determine the job permission. It is intended as a fix to PR #17 I submitted several months back.

I think the cache expires after an hour .expireAfterWrite(1,TimeUnit.HOURS) but admit I am guessing based on how the code reads and did not read the documentation to verify. Do you know, is that not how these caches work?

@alexrothenberg
Copy link
Copy Markdown
Contributor Author

I just looked it up (https://code.google.com/p/guava-libraries/wiki/CachesExplained#Timed_Eviction) and yes it should expire after 1 hour.

expireAfterWrite(long, TimeUnit) Expire entries after the specified duration has passed since the entry was created, or the most recent replacement of the value. This could be desirable if cached data grows stale after a certain amount of time.

@jenkinsadmin
Copy link
Copy Markdown
Member

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

@suryagaddipati
Copy link
Copy Markdown
Member

@alexrothenberg Got it. thanks for the link. That makes sense. I didn't look at the code, I just tested it functionally. I guess, 1 hr lag is not bad for most people. This PR works as intended based on my (limited) functional testing.

Ideal solution would be to listen on team_add events on webhook and modify the cache but I presume that would be out of scope for this plugin.

@luisbosque
Copy link
Copy Markdown

Ey guys, any new about this being merged? I'm experiencing the same problem too

@alexrothenberg
Copy link
Copy Markdown
Contributor Author

@skottler I saw this is assigned to you in jira JENKINS-24069. Anything I can do to help move this forward?

@zpatten
Copy link
Copy Markdown

zpatten commented Nov 13, 2014

👍 Having timeout issues as well due to this; please merge!

@sk-alvinwong
Copy link
Copy Markdown

Will anyone merge this?

skottler pushed a commit that referenced this pull request Nov 13, 2014
…-performance

Fewer github api calls for performance
@skottler skottler merged commit 0891245 into jenkinsci:master Nov 13, 2014
@skottler
Copy link
Copy Markdown
Member

Thanks, this will be included in the next release.

@alexrothenberg
Copy link
Copy Markdown
Contributor Author

Thanks @skottler for merging & maintaining this plugin!

@sean-zou
Copy link
Copy Markdown

Could we make a new release for this? We are having the issue right now.

@sunggun-yu
Copy link
Copy Markdown
Member

+1 still having issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants