Make clickhouse-local temporary storage limit configurable#106689
Conversation
clickhouse-gh
Bot
added
pr-improvement
![clickhouse-gh[bot]](https://avatars.githubusercontent.com/in/928874?s=60&v=4){kind=small}
`clickhouse-local` hard-coded the temporary storage limit to `1 GiB`, which is limiting when spilling to disk, e.g. during an external merge sort with a low `max_bytes_before_external_sort`. Honor the existing `max_temporary_data_on_disk_size` server setting instead. Unlike the regular server, `clickhouse-local` stores temporary data in the system temporary directory by default, which can be small (and is sometimes backed by RAM), so the conservative `1 GiB` default is kept when the setting is not specified. It can now be raised, or lifted entirely with a value of `0`, via the configuration file. Closes: #101697 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The previous `1 GiB` default was still fairly limiting for external operations. Raise it to `1 TiB`, which keeps a safety net against runaway queries filling up the disk while being generous enough for typical local workloads. It remains configurable (including unlimited with `0`) via the `max_temporary_data_on_disk_size` server setting. Add the `TiB` constant and the `_TiB` literal to `base/base/unit.h`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
alexey-milovidov
force-pushed
the
fix-local-tmp-storage-limit-101697
branch
from
e0c9277 to
b157a6e
Compare
|
Recreated this branch off the current `master` so it contains only the two commits relevant to this PR (the `clickhouse-local` temporary-storage limit). It had accidentally been based on the still-open #106650 (ipnsort/driftsort), which polluted the diff, reviews, and CI. The two remaining review threads (on `contrib/ipnsort/ipnsort.h` and `base/base/sort.h`) belong to #106650 and no longer apply here; resolving them. |
|
The only failing check is This is a known issue: #106649. @groeneai already has a fix in progress for exactly this STID: #106025 (see also #106414). |
|
Confirmed. The Fix status: our #106025 (you approved it; currently held by KochetovNicolai's "temporarily block" pending his review) and the alternative #106414 in the mutations path. Either one closes this STID. This PR is unaffected and safe to merge independent of that fix. session: cron:clickhouse-worker-slot-3:20260612-001500 |
|
@groeneai, fix this failure: |
|
@alexey-milovidov The The test does Same limitation already handled for sibling |
The "Apply suggestion" commit removed `as a safety net against` from the end of the first comment line but left the trailing `runaway queries filling up the disk` on the next line, producing a broken sentence. Restore a connecting phrase so the comment reads correctly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
LLVM Coverage Report
Changed lines: Changed C/C++ lines covered by tests: 10/10 (100.00%) | Lost baseline coverage: none · Uncovered code |
|
@groeneai, two more tests have failed on Mac OS - fix them. |
robot-ch-test-poll4
added
the
pr-synced-to-cloud
3 participants
Closes: #101697
clickhouse-localhard-coded the temporary storage limit to1 GiB, which is limiting when spilling to disk, e.g. during an external merge sort with a lowmax_bytes_before_external_sort.This change makes
clickhouse-localhonor the existingmax_temporary_data_on_disk_sizeserver setting (the same one the regular server uses). It can be raised, or lifted entirely with a value of0, via the configuration file (-C config.xml). The default is raised from1 GiBto1 TiB, which keeps a safety net against runaway queries filling up the disk while being generous enough for typical local workloads.Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
The temporary storage size limit of
clickhouse-localis no longer hard-coded to 1 GiB. The default is raised to 1 TiB and can be configured with themax_temporary_data_on_disk_sizeserver setting.