-
Notifications
You must be signed in to change notification settings - Fork 403
Overlay: Increase size limit for cached overlay base database #3098
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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 like the addition of a comment acknowledging that this is a large limit, but I don't think that justifying it based on the speed of the enforcement makes sense.
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 do think the speed of enforcement makes quite a big difference in practice with a limit that is over 50% of the Actions Cache quota. If each compressed overlay base database was +5GB and existing cache entries were immediately evicted from the cache to make room before a new cache entry could be uploaded, then the cache wouldn't be able to fit two overlay base databases and other concurrent CodeQL action runs on PRs would effectively fall back to normal analysis while a new overlay base databases was being uploaded. That will not happen because enforcement of quotas is not immediate.
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.
Note that the cache quota is not specific to CodeQL and is shared among all Actions caches for a repo. It may be that the speed of enforcement makes a difference for the overlay databases if those are the only caches for a repo and they are this large, but it would still hurt customers if we rapidly take up most of their quota and the rate of cache churn resulting from overlay caches competing with customer caches within the standard 10GB quota would plausibly also evict overlay databases before they can be useful.