add --disk-limit to download and config option general -> download_disk_limit - #1543
Merged
Conversation
#1458 The free-space checks (#878, #1268) only guarantee the download fits, so a download can still fill a data disk to 100% and take clickhouse-server down. `--disk-limit <pct>` (CLI) and `disk_limit` (API) refuse the download when (used + downloaded) / total of any local disk would exceed the limit; 0 or absent keeps the current behavior. The check reuses the #1268 estimate, which already honors --tables and --partitions, and extends it with a per-disk breakdown. Parts of a disk missing in system.disks are attributed to the least-used local disk the same way the download rebalances them; object-disk parts are not counted against local disks. `system.disks.total_space` is now read into `Disk.TotalSpace`; disks without it are skipped with a warning. Resume downgrades the error to a warning like the existing checks. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
#1458 `download_disk_limit` (DOWNLOAD_DISK_LIMIT) sets the disk usage limit once in config.yml, `--disk-limit` still overrides it per command and the API `disk_limit` parameter behaves the same way. A download refused by the disk-limit or free-space check used to leave table metadata and the resumable state behind; with the default `use_resumable_state: true` the next run of the same command resumed and the refusal degraded to a warning, so the guard was bypassed by simply retrying. Remove what the refused run wrote when it was not a resume itself. Manual.md and the "Common CLI Usage" section of ReadMe.md are regenerated by generate_manual.sh. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Use env.dropDatabase helper instead of a raw DROP DATABASE query for consistent teardown, and update the testflows CLI snapshot to include the new download_disk_limit default config field.
Coverage Report for CI Build 34348178468Coverage decreased (-0.04%) to 66.901%Details
Uncovered Changes
Coverage Regressions45 previously-covered lines in 6 files lost coverage.
Coverage Stats
💛 - Coveralls |
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
2 participants
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.
fix #1458