Determine the overlay minimum disk space requirement from feature flags - #4080
Merged
Conversation
Contributor
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
This PR makes overlay-analysis disk requirements configurable through feature flags, with a new 14 GB baseline and lower thresholds from 8–13 GB.
Changes:
- Adds six minimum-disk feature flags and removes the previous v2 resource-check flag.
- Selects the lowest enabled threshold and adds successful-check debug logging.
- Expands overlay enablement tests for default, lowered, and combined thresholds.
Show a summary per file
| File | Description |
|---|---|
src/feature-flags.ts |
Defines and configures the new disk-threshold flags. |
src/config-utils.ts |
Computes and applies the effective disk threshold. |
src/config-utils.test.ts |
Tests default and feature-controlled disk limits. |
lib/entry-points.js |
Generated JavaScript artifact; excluded from review by policy. |
Review details
Files excluded by content exclusion policy (1)
- lib/entry-points.js
- Files reviewed: 3/4 changed files
- Comments generated: 2
- Review effort level: Balanced
Add six feature flags, overlay_analysis_min_disk_8_gb through overlay_analysis_min_disk_13_gb, which will be used to control the amount of available disk space that overlay analysis requires. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Overlay analysis required 20 GB of available disk space, lowered to 14 GB when overlay_analysis_resource_checks_v2 was enabled. That gave us a single step to roll out, and any further reduction needed another flag and another release. Determine the threshold from the new overlay_analysis_min_disk_N_gb flags instead, taking the lowest one that is enabled so that a lower limit can be rolled out to a subset of repositories without first disabling the flag above it. When none are enabled, the 14 GB limit now applies unconditionally, replacing the 20 GB default. Thresholds remain in decimal MB, matching the bytes-per-MB convention the disk check already used, so the effective byte values are unchanged from the previous 14 GB path. Also log the available and required space at debug level when the check passes, so that run logs show which threshold took effect. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The flag no longer has any effect now that its 14 GB threshold is the unconditional default, so remove it. Setting CODEQL_ACTION_OVERLAY_ANALYSIS_RESOURCE_CHECKS_V2 no longer does anything. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
henrymercer
force-pushed
the
henrymercer/studious-giggle
branch
from
August 6, 2026 15:38
49e5f70 to
99caaa8
Compare
Say "at or above" in the debug message logged when the check passes, since the comparison accepts exactly the minimum. Check each feature flag against the threshold its name declares, rather than only exercising a few of them, so that a mistake in one of the mappings cannot go unnoticed. Both sides of the boundary are needed to pin a threshold down: a mapping to a lower value would still pass the case at the limit, and one to a higher value would still fail the case below it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The comparison accepts exactly the minimum, so say "at or above", to match the wording of the disk space check. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Review details
Files excluded by content exclusion policy (1)
- lib/entry-points.js
Suppressed comments (1)
src/config-utils.test.ts:1307
- These tests do not pin the new 14 GB default: 15 GB succeeds and 10 GB fails for any threshold between them, so an accidental 11–15 GB baseline could pass. Please cover exactly 14 GB and immediately below 14 GB, as is done for each flag threshold below.
numAvailableBytes: 15_000_000_000,
- Files reviewed: 3/4 changed files
- Comments generated: 0 new
- Review effort level: Balanced
mbg
previously approved these changes
Aug 17, 2026
mbg
left a comment
mbg
left a comment
Member
There was a problem hiding this comment.
As discussed elsewhere, let's move ahead with this for now. The changes look fine I think, but the branch will need to be updated to account for the changes I cherry-picked into #4083 that has already been merged/released. Let me know if you'd like me to take over updating this branch, since I am responsible for the conflicts.
Comment on lines
+125
to
+136
| /** | ||
| * Controls whether lower disk space requirements are used for overlay hardware checks. | ||
| * Has no effect if `OverlayAnalysisSkipResourceChecks` is enabled. | ||
| * Feature flags that lower the amount of available disk space that the overlay hardware check | ||
| * requires. The lowest threshold that is enabled takes effect; if none are enabled, the default | ||
| * threshold applies. These flags have no effect if `OverlayAnalysisSkipResourceChecks` is | ||
| * enabled. | ||
| */ | ||
| OverlayAnalysisResourceChecksV2 = "overlay_analysis_resource_checks_v2", | ||
| OverlayAnalysisMinDisk8Gb = "overlay_analysis_min_disk_8_gb", | ||
| OverlayAnalysisMinDisk9Gb = "overlay_analysis_min_disk_9_gb", | ||
| OverlayAnalysisMinDisk10Gb = "overlay_analysis_min_disk_10_gb", | ||
| OverlayAnalysisMinDisk11Gb = "overlay_analysis_min_disk_11_gb", | ||
| OverlayAnalysisMinDisk12Gb = "overlay_analysis_min_disk_12_gb", | ||
| OverlayAnalysisMinDisk13Gb = "overlay_analysis_min_disk_13_gb", | ||
| OverlayAnalysisPython = "overlay_analysis_python", |
Member
There was a problem hiding this comment.
Minor, but maybe worth doing since you need to rebase these changes anyway / resolve the merge conflicts: it might be good to have a docs comment for each FF individually, particularly to explain the intended behaviour of each FF more clearly. E.g. something along the lines of "if all FFs are off, the minimum is determined by the OVERLAY_MINIMUM_AVAILABLE_DISK_SPACE_MB constant in config-utils.ts - otherwise, the FFs decrease the minimum to the value indicated by their name. The lowest value out of all enabled FFs is picked."
Contributor
Author
There was a problem hiding this comment.
Good idea, done.
| * threshold applies. These flags have no effect if `OverlayAnalysisSkipResourceChecks` is | ||
| * enabled. | ||
| */ | ||
| OverlayAnalysisResourceChecksV2 = "overlay_analysis_resource_checks_v2", |
Member
There was a problem hiding this comment.
I cherry-picked the change to remove OverlayAnalysisResourceChecksV2 in #4083 which was merged and has shipped. This PR will need to be updated based on that.
| const OVERLAY_MINIMUM_AVAILABLE_DISK_SPACE_V2_BYTES = | ||
| OVERLAY_MINIMUM_AVAILABLE_DISK_SPACE_V2_MB * 1_000_000; | ||
| const OVERLAY_MINIMUM_DISK_SPACE_FEATURES: ReadonlyArray< | ||
| [FeatureWithoutCLI, number] |
Member
There was a problem hiding this comment.
Minor: No strong feelings, but why a tuple instead of an object?
Contributor
Author
There was a problem hiding this comment.
No particular reason — changed.
Resolve the overlap with the separately shipped promotion of the overlay resource checks while preserving the feature-flagged minimum disk thresholds. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Document each minimum disk feature flag individually and replace the tuple list with an explicit feature-to-threshold mapping. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
mbg
approved these changes
Aug 18, 2026
mbg
left a comment
mbg
left a comment
Member
There was a problem hiding this comment.
LGTM as-is, with just one minor suggestion.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overlay analysis skips runners that don't have enough free disk space. Until now that threshold was a single hard-coded value with one flag-gated alternative: 20 GB normally, or 14 GB when
overlay_analysis_resource_checks_v2was enabled. That gave us exactly one step to roll out, and reaching a lower limit meant another flag and another release.This replaces that with a set of flags that each name a threshold, so we can tune the requirement without shipping code:
overlay_analysis_min_disk_8_gboverlay_analysis_min_disk_9_gboverlay_analysis_min_disk_10_gboverlay_analysis_min_disk_11_gboverlay_analysis_min_disk_12_gboverlay_analysis_min_disk_13_gbWhen several are enabled the lowest threshold wins, so we can roll a lower limit out to a subset of repositories without having to disable the flag above it first. When none are enabled the default of 14 GB applies.
overlay_analysis_resource_checks_v2is removed and its 14 GB value becomes the unconditional baseline, so the 20 GB limit is gone. Note that this part is not itself behind a flag — anyone previously on the 20 GB path moves to 14 GB when this ships. On GHES that only affects users who have explicitly opted into overlay analysis viaCODEQL_ACTION_OVERLAY_ANALYSIS, sinceoverlay_analysisdefaults to off there. Anyone settingCODEQL_ACTION_OVERLAY_ANALYSIS_RESOURCE_CHECKS_V2will find it no longer has any effect.Thresholds stay in decimal MB (
N * 1000 MB), matching the existing bytes-per-MB convention in the disk check, so the effective byte values are unchanged from the previous 14 GB path. The check also now logs the available and required space at debug level when it passes, so it's possible to tell from a run log which flag took effect.Risk assessment
For internal use only. Please select the risk level of this change:
Which use cases does this change impact?
Workflow types:
dynamicworkflows (Default Setup, Code Quality, ...).Products:
analysis-kinds: code-scanning.Environments:
github.comand/or GitHub Enterprise Cloud with Data Residency.How did/will you validate this change?
.test.tsfiles).If something goes wrong after this change is released, what are the mitigation and rollback strategies?
overlay_analysisfeature flag, theCODEQL_OVERLAY_DATABASE_MODEenvironment variable, or the disable-overlay repository property.How will you know if something goes wrong after this change is released?
Are there any special considerations for merging or releasing this change?
Merge / deployment checklist