-
Notifications
You must be signed in to change notification settings - Fork 266
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
api: reducing db calls in repo list endpoints with quota enabled (PROJQUAY-6895) #2770
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2770 +/- ##
=======================================
Coverage 70.93% 70.93%
=======================================
Files 436 436
Lines 40701 40715 +14
Branches 5315 5320 +5
=======================================
+ Hits 28870 28881 +11
- Misses 10148 10152 +4
+ Partials 1683 1682 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
if len(repo_ids) > 1000: | ||
logger.warning( | ||
"Fetching more than 1000 repository sizes at once, you may experience performance issues." | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this function used for, wouldn't we want a way to paginate this instead?
Are the repo_ids restricted to a single namespace or is this a full table lookup?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could add pagination but for this particular use case it uses what ever the page size is for listing repositories. The repo_ids will belong to a single namespace the majority of the time but it's possible that they belong to separate namespaces.
I could also replace this with individual requests for fetching each repository size but it looked like this gave better performance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
/cherry-pick redhat-3.9 |
@bcaton85: #2770 failed to apply on top of branch "redhat-3.9":
In response to this:
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. |
/cherry-pick redhat-3.11 |
@bcaton85: #2770 failed to apply on top of branch "redhat-3.11":
In response to this:
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. |
…JQUAY-6895) (quay#2770) Reducing the number of DB calls in the repo list endpoint with quota enabled by: - Adding the id to RepositoryBaseElement when the repositories are initially fetched, removing the need to fetch the repository ID's again - Fetching the repository sizes with a single DB call using the IN operator
Reducing the number of DB calls in the repo list endpoint with quota enabled by:
RepositoryBaseElement
when the repositories are initially fetched, removing the need to fetch the repository ID's againIN
operator