Overlay: add database upload size limit - #3031
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
Pull Request Overview
This PR adds a size limit to prevent uploading oversized overlay-base databases to the Actions Cache. The change introduces a 6GB uncompressed size limit (approximately 2.4GB compressed) to respect the 10GB Actions Cache capacity constraints.
- Adds size validation before uploading overlay-base databases to cache
- Returns early with a warning if database size cannot be determined or exceeds the limit
- Uses a conservative 6GB uncompressed limit assuming 2.5:1 compression ratio
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/overlay-database-utils.ts | Adds size constants and validation logic to check database size before upload |
| lib/overlay-database-utils.js | Generated JavaScript equivalent of the TypeScript changes |
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
Sorry, something went wrong.
mbg
left a comment
There was a problem hiding this comment.
Thanks! I added some inline thoughts on the limit, but am happy to approve the PR.
You could probably also add a test for this that mocks various results of tryGetFolderBytes to check that the cache isn't uploaded when we get undefined or a result larger than the limit.
Sorry, something went wrong.
| * Assuming 2.5:1 compression ratio, the 6 GB limit on uncompressed data would | ||
| * translate to a limit of around 2.4 GB after compression. |
There was a problem hiding this comment.
2.4GB seems like a relatively large allowance, given that you may have multiple overlay databases, and the 10GB quota is shared between all caches and workflows in a repo.
In the context of Advanced Setup, I am not too worried about this, because users can opt-out of overlay databases if they find that the cache interferes too much with other caches they depend on. (It would probably be nice to also give those users an option to customise the limit, but I appreciate that's unlikely to be a priority right now.)
In the context of Default Setup, this is more of a problem, because users can't opt-out but they may still have other workflows in the same repo that require the cache quota.
That said, a limit is obviously better than no limit and I am happy to approve the PR for that reason.
In the long run, we probably want to start working on a coherent strategy for the use of the cache quota across all three consumers we have in the CodeQL Action now (TRAP caching, dependency caching, and overlay databases). I don't think we have an internal issue to track that, and I'll open one on our team's end if it doesn't exist.
Sorry, something went wrong.
This PR updates uploadOverlayBaseDatabaseToCache() to limit the maximum size of the overlay-base database that we upload to the Actions Cache.
Risk assessment
For internal use only. Please select the risk level of this change:
Merge / deployment checklist