-
Notifications
You must be signed in to change notification settings - Fork 392
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
Conversation
ef9af8d
to
c02e46b
Compare
c02e46b
to
8a84a62
Compare
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.
Pull Request Overview
This PR increases the size limit for cached overlay base databases from 6GB to 15GB to enable Ruby overlay analysis for the github/github
repository, which produces an 11GB uncompressed overlay base database.
- Increased
OVERLAY_BASE_DATABASE_MAX_UPLOAD_SIZE_MB
from 6000 to 15000 MB - Updated comments to reflect the new size limit and its implications
- Added TODO comment noting the limit should be revisited before broader rollout
* compared to the default 10GB Actions Cache capacity, but enforcement of Actions | ||
* Cache 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.
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.
* compared to the default 10GB Actions Cache capacity, but enforcement of Actions | |
* Cache quotas is not immediate. | |
* compared to the default 10GB Actions Cache capacity. |
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.
This PR increases the size limit imposed on (uncompressed) overlay base databases for uploading to the Actions Cache from 6GB to 15GB. With an expected compression ratio of 2.5:1 the 15GB uncompressed limit translates into an effective 6GB limit.
Overlay analysis is currently limited to the github and dsp-testing orgs and controlled by feature flags (codeql_action_overlay_analysis_*). This size limit will be revisited before removing the restriction to the
github
anddsp-testing
org.Risk assessment
Low risk: Overlay analysis is under feature flags and restricted to the
github
anddsp-testing
orgs.