iframe-proxy

groeneai · 2026-06-10T10:51:44Z

Description

In the distributed (urlCluster) read path, IStorageURLBase::read takes num_streams straight from the max_streams_for_files_processing_in_cluster_functions setting (user-controlled UInt64, default 0) with no upper bound. The sibling storages clamp this value to the actual work count before reserving (StorageFile to the file count, StorageObjectStorage to estimatedKeysCount(), the URL glob branch to the glob iterator size). The URL distributed branch did not, because its cluster iterator is a lazy callback with no upfront file count to clamp against, so a pathological value reached the pipeline unbounded:

near UINT64_MAX threw std::length_error from pipes.reserve(num_streams) in ReadFromURL::initializePipeline, tripping a LOGICAL_ERROR and aborting the server;
merely huge values (e.g. 1e8) passed reserve() but blew up memory, because the value is also stored in max_num_streams and drives pipe.resize(max_num_streams).

The setting is clamped at ingestion to 256 * getNumberOfCPUCoresToUse(), the same ceiling max_threads already gets (the "limit to something unreasonable" idiom in Context.cpp). Clamping here, before ReadFromURL is constructed, bounds both num_streams and max_num_streams, so pathological values produce a normal recoverable query error instead of aborting or OOMing the server. StorageURLSource construction is lazy, and realistic values stay well under the bound (the existing 04035 test uses 3697).

Found by the BuzzHouse fuzzer, which set the value at session level (so it was invisible in the query-level SETTINGS clause).

Changelog category (leave one):

Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Fix a server crash (std::length_error reported as a LOGICAL_ERROR) when reading from a *Cluster table function such as urlCluster with a very large max_streams_for_files_processing_in_cluster_functions setting. The number of streams is now bounded to a sane value.

groeneai · 2026-06-10T10:52:14Z

Pre-PR validation gate

#	Question	Answer
a	Deterministic repro?	Yes. `SELECT count() FROM urlCluster('test_shard_localhost', 'http://localhost:1/', 'TSV', 'a UInt8') SETTINGS max_streams_for_files_processing_in_cluster_functions = 18446744073709551615` aborts the server every time on master HEAD. The reserve happens before any network IO, so an unreachable URL is enough.
b	Root cause explained?	`IStorageURLBase::read` assigns `num_streams` directly from the user-controlled `max_streams_for_files_processing_in_cluster_functions` setting in the distributed branch, with no upper bound. `ReadFromURL::initializePipeline` then calls `pipes.reserve(num_streams)`; for values above `vector<Pipe>::max_size()` this throws `std::length_error`, reported as a `LOGICAL_ERROR` that aborts the server. For merely huge values (e.g. `1e8`) the reserve succeeds but the following loop hangs/OOMs constructing `num_streams` sources.
c	Fix matches root cause?	Yes. The fix clamps `num_streams` itself in the distributed branch (the source of the unbounded value), not the `reserve` call (the symptom). This also covers the loop-OOM case, which a `reserve`-only guard would not.
d	Test intent preserved / new tests added?	New regression test `04327_url_cluster_huge_num_streams_no_crash.sh` covers both the `UINT64_MAX` case (old reserve crash) and the `1e8` case (the loop hang). No existing test weakened; `04035` still uses `3697`, which stays well under the new bound.
e	Both directions demonstrated?	Yes. Without the fix, the repro query aborts the server with the exact `std::length_error -> vector<Pipe>::reserve` stack. With the fix, the server survives the `UINT64_MAX`, `1e8`, and `3697` cases and the test passes 5/5.
f	Fix is general, not a narrow patch?	Yes. Audited all three storages that read this setting into `num_streams`: `StorageFile` clamps to file count, `StorageObjectStorage` clamps to `estimatedKeysCount()`, and the URL glob branch clamps to glob size. `StorageURL`'s distributed branch was the only unclamped path. The fix addresses the source (unbounded `num_streams`), consistent with the sibling clamps.

Session id: cron:clickhouse-worker-slot-4:20260610-102000

groeneai · 2026-06-10T10:52:37Z

cc @Avogar @kssenii could you review this? It clamps num_streams in the urlCluster distributed read path, which previously took the user-controlled max_streams_for_files_processing_in_cluster_functions setting unbounded and aborted the server via std::length_error from pipes.reserve(). Avogar added the setting in #91323; the sibling StorageFile/StorageObjectStorage paths already clamp it, this brings StorageURL in line.

clickhouse-gh · 2026-06-10T11:03:43Z

Workflow [PR], commit [4d50938]

Summary: ❌

Performance Comparison: Performance dashboard

job_name	test_name	status	info	comment
Stateless tests (arm_asan_ubsan, azure, parallel)		FAIL
	03469_json_read_subcolumns_combined_2_compact_merge_tree	FAIL	cidb

AI Review

Summary

This PR clamps max_streams_for_files_processing_in_cluster_functions before the urlCluster, fileCluster, and object-storage cluster read steps capture max_num_streams, so both the pipes.reserve and pipe.resize allocation paths are bounded. I did not find a remaining code-level blocker, and the prior object-storage coverage thread is now addressed.

Missing context / blind spots

⚠️ The current PR workflow is still pending in GitHub checks; only the available Praktika report and check list were inspected. A completed PR workflow would close this CI evidence gap.

Final Verdict

Status: ✅ Approve

nikitamikhaylov · 2026-06-10T11:03:43Z

@groeneai link the issue or a URL to the failure in the CI. Write it down in your notes and always do so.

In the *Cluster table functions the max_streams_for_files_processing_in_cluster_functions setting (UInt64, user-controlled, default 0) is read straight into num_streams in IStorageURLBase::read. The distributed branch has no upfront file count to clamp against (unlike the glob branch, StorageFile and StorageObjectStorage, which clamp to the actual file/key count), so a pathological value reached the pipeline unbounded: - near UINT64_MAX threw std::length_error from pipes.reserve(num_streams), tripping a LOGICAL_ERROR and aborting the server (BuzzHouse set the setting at session level, so the fuzzer queries looked innocuous); - merely huge values (e.g. 1e8) passed reserve() but then blew up memory: the value is also stored in max_num_streams and drives pipe.resize(max_num_streams). Clamp the setting at ingestion to 256 * getNumberOfCPUCoresToUse(), the same ceiling max_threads gets (Context.cpp), so both num_streams and max_num_streams stay bounded. Clamping here (before ReadFromURL is constructed) is what fixes the resize path; an earlier attempt that only clamped the local num_streams in createIterator left max_num_streams unbounded and turned the crash into an OOM under sanitizers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

groeneai · 2026-06-10T13:07:49Z

Pre-PR validation gate (re-run after fix correction)

#	Question	Answer
a	Deterministic repro?	Yes. On master HEAD (26.6.1.1): `SELECT count() FROM urlCluster('test_shard_localhost','http://127.0.0.1:9/','TSV','a UInt8') SETTINGS max_streams_for_files_processing_in_cluster_functions = 18446744073709551615, max_threads = 1` → `Fatal: Logical error: std::length_error, e.what() = vector` at `StorageURL.cpp` (`pipes.reserve`) every time; server aborts.
b	Root cause explained?	The setting (user-controlled `UInt64`) is read straight into `num_streams` in `IStorageURLBase::read` with no upper bound. It is then stored in BOTH `num_streams` and `max_num_streams` of `ReadFromURL`. Near `UINT64_MAX` → `pipes.reserve(num_streams)` exceeds `vector<Pipe>::max_size()` → `std::length_error` → `LOGICAL_ERROR` → abort. Merely huge (`1e8`) → reserve succeeds but `pipe.resize(max_num_streams)` blows up memory. Sibling storages clamp to the actual file/key count; the URL distributed branch has no upfront count, so it stayed unbounded.
c	Fix matches root cause?	Yes. Clamps the setting at ingestion (`IStorageURLBase::read`, before `ReadFromURL` is constructed) to `256 * getNumberOfCPUCoresToUse()`, the same ceiling `max_threads` gets (`Context.cpp`). Bounding it here fixes BOTH the `reserve` crash and the `pipe.resize(max_num_streams)` OOM, because the ctor copies the clamped value into both members.
d	Test intent preserved / new tests added?	New regression test `04327_url_cluster_huge_num_streams_no_crash.sh` drives both pathological values (`UINT64_MAX` and `1e8`) through `urlCluster` and asserts the server is still alive afterward. Existing `04035` (uses the setting at `3697`) still passes — legitimate values are below the clamp and unaffected.
e	Both directions demonstrated?	Yes. NEGATIVE: on unfixed master HEAD the crashing query aborts the server (`std::length_error`, connection refused after) and `04327` fails. POSITIVE: with the fix, `04327` passes 30/30 (~2.4s each), server stays alive for both cases and returns recoverable query errors (`ALL_CONNECTION_TRIES_FAILED`).
f	Fix is general, not a narrow patch?	Yes. Audited the three storages that read this setting into `num_streams`: `StorageFile` clamps to `paths.size()`, `StorageObjectStorage` clamps to `estimatedKeysCount()` (`ReadFromObjectStorageStep.cpp`), URL glob branch clamps to `glob_iterator->size()` — only the URL distributed branch was unclamped. Fix is at the single root site (setting ingestion), not a symptom guard at the `reserve`/`resize` call; the earlier symptom-site clamp (in `createIterator`, on the local `num_streams` only) was removed because it left `max_num_streams` unbounded.

Session id: cron:clickhouse-worker-slot-0:20260610-121200

groeneai · 2026-06-10T19:20:27Z

CI status for the corrected fix on HEAD 13b9700: the OOM / Server died failures seen on the prior commit a12ca6f are gone (the earlier clamp left max_num_streams unbounded, causing a 1e8-port ResizeProcessor; the corrected fix clamps at the ingestion point). All stateless, stress, and flaky-check variants pass on this HEAD. The only red check is LLVM Coverage, the per-test profdata aggregation job, which failed at the job level with no per-test failures (its per-test amd_llvm_coverage stateless runs all passed). That is a coverage-aggregation infrastructure failure, not caused by this PR. Ready for re-review.

groeneai · 2026-06-10T19:37:29Z

CI fully finished on HEAD 13b97006cd0b4f3011a0d7ed9fc248cbb96caf40. The corrected fix (clamp num_streams at setting ingestion, bounding both num_streams and max_num_streams) resolved the OOM that the prior HEAD (a12ca6f) hit: all four flaky checks (amd_asan_ubsan / amd_debug / amd_msan / amd_tsan) — which run this PR's new test 04327_url_cluster_huge_num_streams_no_crash.sh — now PASS, and every amd_llvm_coverage stateless/integration/unit job passes.

One red check, not PR-caused:

LLVM Coverage (the coverage-aggregation job) — infra. Failed in ~1m34s while all 17 underlying coverage jobs (Stateless 1/3, 2/3, 3/3, AsyncInsert, ParallelReplicas, old-analyzer variants; Integration 1-5/5; Unit) passed. This is the profdata-merge aggregation step; a fast failure with every coverage test green is an aggregation/infra issue, not a code or test regression.

Finish Workflow green. Ready for review.

…s and the resize target The first commit clamped num_streams in IStorageURLBase::read, but the same max_streams_for_files_processing_in_cluster_functions value reaches two other *Cluster readers unbounded, and even for StorageURL it only bounded the source count, not the resize target: - ReadFromURL/ReadFromFile/ReadFromObjectStorageStep all capture the raw num_streams into max_num_streams in their constructor. initializePipeline clamps only the local source count (to the glob/file/key count), then with parallelize_output_from_storages enabled calls pipe.resize(max_num_streams). Pipe::resize builds a ResizeProcessor with OutputPorts(max_num_streams), so a huge setting still drives the same length_error/OOM allocation through fileCluster/s3Cluster/azureBlobStorageCluster, just at the resize step. Move the clamp into a shared helper clampClusterFunctionNumStreams() in prepareReadingFromFormat and call it at ingestion in all three read() methods, before the read step is constructed. Because num_streams is clamped before the constructor copies it into max_num_streams, both the source count and the resize target are now bounded everywhere the setting is consumed. The ceiling is the same 256 * getNumberOfCPUCoresToUse() that max_threads gets. Extend the regression test to cover fileCluster with parallelize_output_from_storages, which exercises the StorageFile resize path that the URL-only test did not reach. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

groeneai · 2026-06-10T20:50:34Z

Pre-PR validation gate

Re-posted for the expanded fix at HEAD 21740f05fe85 (sibling readers + resize target).

#	Question	Answer
a	Deterministic repro?	Yes. urlCluster: `SELECT count() FROM urlCluster('test_shard_localhost','http://127.0.0.1:9/','TSV','a UInt8') SETTINGS max_streams_for_files_processing_in_cluster_functions = 18446744073709551615` crashes every time. fileCluster (sibling reader, resize path): a single-file `fileCluster(...) SETTINGS max_streams_for_files_processing_in_cluster_functions = 100000000, parallelize_output_from_storages = 1` hangs allocating then OOMs every time.
b	Root cause explained?	The setting is read straight into `num_streams` in the three `*Cluster` `read()` methods (StorageURL, StorageFile, StorageObjectStorage). The read step's constructor copies that into `max_num_streams`. `initializePipeline` clamps only the local source count (glob/file/key count), then under `parallelize_output_from_storages` calls `pipe.resize(max_num_streams)`, which builds a `ResizeProcessor` with `OutputPorts(max_num_streams)`. So a pathological value either overflows `pipes.reserve(num_streams)` (near `UINT64_MAX` -> `std::length_error`) or blows up the resize allocation (huge values -> OOM), through the source count and/or the resize target.
c	Fix matches root cause?	Yes. The clamp is applied at ingestion in each `read()`, before the read step is constructed, so `num_streams` is bounded before the constructor copies it into `max_num_streams`. Both the source count and the resize target are therefore bounded for all three readers. No symptom-guard; the value is bounded at the single point where it enters the read path.
d	Test intent preserved / new tests added?	Test renamed `04327_url_cluster_huge_num_streams_no_crash` -> `04327_cluster_functions_huge_num_streams_no_crash` and extended with `fileCluster` + `parallelize_output_from_storages = 1` (a non-URL cluster function that exercises the `StorageFile` resize path the URL-only cases did not reach). Both `UINT64_MAX` and `1e8` cases kept.
e	Both directions demonstrated?	Yes, debug build. Without the fix (master `fc73f46`): urlCluster `UINT64_MAX` -> `Fatal: std::length_error, e.what() = vector` at `ReadFromURL::initializePipeline` (`pipes.reserve`), server dies; fileCluster `1e8` + `parallelize_output` -> `MEMORY_LIMIT_EXCEEDED: would use 61.87 GiB` from the unbounded `pipe.resize(max_num_streams)`. With the fix: all cases survive, regression test passes via `clickhouse-test`.
f	Fix is general, not a narrow patch?	Yes. This is the generalization. The same pattern existed in all three readers (`ReadFromURL`, `ReadFromFile`, `ReadFromObjectStorageStep`) and at both the source-count and resize-target steps; the fix routes all three ingestion clamps through one shared helper `clampClusterFunctionNumStreams()`, so `fileCluster`/`s3Cluster`/`azureBlobStorageCluster` are covered, not just `urlCluster`.

Session id: cron:clickhouse-worker-slot-9:20260610-201400

groeneai · 2026-06-10T20:50:40Z

@nikitamikhaylov noted, will always link the source going forward.

This was found by the BuzzHouse fuzzer, which set max_streams_for_files_processing_in_cluster_functions at session level. The crash:

Logical error: std::length_error, e.what() = vector
  std::vector<DB::Pipe>::reserve(unsigned long)
  DB::ReadFromURL::initializePipeline(...) at src/Storages/StorageURL.cpp:1322   // pipes.reserve(num_streams)

Earliest CI sighting: BuzzHouse (arm_asan_ubsan) on #102690 (2026-04-20), recurring on the BuzzHouse-source PR #101684 (latest 2026-06-10):
https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=101684&sha=03a746352a5f61856c2be31acaa081fa262e548c&name_0=PR&name_1=BuzzHouse%20%28arm_asan_ubsan%29

groeneai · 2026-06-10T20:50:41Z

Both review points are addressed in 21740f05fe85.

You were right on both counts: the original commit only bounded StorageURL's source count, leaving (1) StorageURL's own resize target (ReadFromURL::max_num_streams, captured from the pre-clamp num_streams) and (2) the sibling readers ReadFromFile / ReadFromObjectStorageStep unbounded, so fileCluster/s3Cluster/azureBlobStorageCluster could still drive pipe.resize(max_num_streams) into the same ResizeProcessor OutputPorts allocation.

Fix: the clamp now lives in a shared helper clampClusterFunctionNumStreams() in prepareReadingFromFormat, called at ingestion in all three read() methods before the read step is constructed. Since num_streams is clamped before the constructor copies it into max_num_streams, both the source count and the resize target are bounded for every reader. The ceiling is the same 256 * getNumberOfCPUCoresToUse() max_threads uses.

Verified that without the clamp, fileCluster with parallelize_output_from_storages = 1 and max_streams_for_files_processing_in_cluster_functions = 100000000 drives the resize allocation to MEMORY_LIMIT_EXCEEDED (61.87 GiB), and with the clamp it returns normally. The regression test is extended to cover a non-URL cluster function (fileCluster + parallelize_output_from_storages), which exercises the StorageFile resize path the URL-only cases did not reach.

groeneai · 2026-06-11T00:25:06Z

CI fully finished (Finish Workflow passed). Failures, both pre-existing/known, none PR-caused (this PR clamps num_streams in the urlCluster read path -- StorageURL):

Stateless tests (amd_asan_ubsan, distributed plan, parallel, 1/2) -- "Server died" / Segfault, the known db-disk / distributed-plan chronic crash class (issue Flaky test: 03710_parallel_alter_comment_rename_selects #104692, PR Fix crash moving an empty part to a plain_rewritable disk #107040).
Stateless tests (amd_asan_ubsan, flaky check) -- the PR-only flaky-check job (informational by design).
No Fast/Bugfix/Style failure.

nikitamikhaylov · 2026-06-11T11:16:15Z

@groeneai Flaky test check failure is a HUGE problem. This means you've added a bad test that will cause problems later. ALWAYS investigate this. Write it down to your memory.

The clamp introduced in this PR bounds num_streams to 256 * number-of-cores. On a high-core CI runner that is tens of thousands of stream objects per query, so a single run takes ~137s on the debug flaky-check builder (max 170s observed in CI). The flaky check fails any test whose run time exceeds TEST_MAX_RUN_TIME_IN_SECONDS (180s) unless it carries the long tag, and the test was running right at that edge. The long tag both exempts the test from the 180s flaky-check time limit and reduces its repeat count to long_test_runs_ratio (10%), so it no longer risks spurious flaky-check failures while keeping full regression coverage of both crash paths (length_error from pipes.reserve and the unbounded pipe.resize(max_num_streams) OOM) across urlCluster, fileCluster and the object-storage cluster readers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

groeneai · 2026-06-11T14:06:22Z

@nikitamikhaylov understood, and you are right to push on this. I investigated the flaky-check failure rather than dismissing it, and I also corrected my earlier comment on this PR that wrongly called it "informational by design". Findings and fix below.

What the flaky check actually reported (HEAD 21740f05fe85)

Stateless tests (amd_asan_ubsan, flaky check) verdict was Server died (1 failure). The server never came up: job.log shows it started then every select 1 health check returned exit 210 (Connection refused), and clickhouse-server.err.log shows the cause was host memory pressure (the system-log shipper hit User memory limit exceeded: would use 29.86 GiB, maximum: 29.80 GiB), not a crash from my change. The 03546_json_input_output_map_as_array TIMEOUT and the other scattered FAILs in that job are victims of the dead server, not my test. This overlaps the known 2026-06-09/06-10 asan host-memory outage.
The job log confirms the flaky check selected only my changed test: [flaky-check] Changed/new tests (1): 04327_cluster_functions_huge_num_streams_no_crash. On the three jobs where the server stayed up, my test ran 50x each and passed every time (amd_debug, amd_msan, amd_tsan: 150/150 OK). CIDB shows it was never non-OK on this HEAD.

The real, latent test problem I found and fixed
Even though the test never failed, it is a genuine liability and would have caused problems, exactly as you said. The clamp this PR adds bounds num_streams to 256 * number-of-cores. On a high-core CI runner that is tens of thousands of stream objects per query, so a single run averaged 137s on the debug flaky-check builder (max 170s observed). The flaky check FAILs any test that exceeds TEST_MAX_RUN_TIME_IN_SECONDS (180s) unless it carries the long tag, so the test was running right at the edge of a spurious TOO_LONG failure.

Fix in 4b176803cbd2: tag the test long. That exempts it from the 180s flaky-check limit and reduces its repeat count to 10% (long_test_runs_ratio), while keeping full regression coverage of both crash paths (the length_error from pipes.reserve and the unbounded pipe.resize(max_num_streams) OOM) across urlCluster, fileCluster and the object-storage cluster reader.

Verification (local)

Unfixed master + real urlCluster(..., max_streams_for_files_processing_in_cluster_functions = UINT64_MAX) reproduces the original Logical error: std::length_error vector and crashes the server.
Fixed binary: 15/15 OK debug, and under ASAN+UBSAN in --flaky-check mode the long tag reduces 30 requested runs to 3, all OK at 43-54s each (well under 180s).

I have written the rule to memory: a flaky-check failure is always investigated, never treated as informational.

groeneai · 2026-06-11T14:06:41Z

Pre-PR validation gate

For the long-tag fixup at HEAD 4b176803cbd2 (the flaky-check test-quality fix). The clamp fix itself was validated in the earlier gate at HEAD 21740f05fe85.

#	Question	Answer
a	Deterministic repro?	Yes. The test runs ~137s/run (max 170s) on the CI debug flaky-check builder (CIDB: avg 137105ms over 50 runs). The flaky check FAILs any run over `TEST_MAX_RUN_TIME_IN_SECONDS` (180s, `tests/clickhouse-test:136`) unless the test is tagged `long` (`tests/clickhouse-test:3200-3204`). The cost is deterministic: the clamp realizes `256 * getNumberOfCPUCoresToUse()` source objects per query.
b	Root cause explained?	The clamp this PR adds bounds `num_streams` to `256 * cores`. On a high-core runner that is tens of thousands of stream objects per query; the test issues 4 such queries, so a single run sits just under the 180s flaky-check time limit and would tip over it on a busier/larger builder, producing a spurious `TOO_LONG` flaky-check failure.
c	Fix matches root cause?	Yes. The `long` tag is the exact mechanism `clickhouse-test` provides for legitimately heavy tests: it exempts the test from the 180s flaky-check limit (line 3204) and reduces its repeat count to `long_test_runs_ratio` (10%, default 0.1). No production code changed.
d	Test intent preserved / new tests added?	Preserved fully. Only the `Tags` line changed; all four queries and both crash paths (the `length_error` from `pipes.reserve` near UINT64_MAX, and the unbounded `pipe.resize(max_num_streams)` OOM at 1e8) remain, across `urlCluster`, `fileCluster` and the object-storage cluster reader. No assertions weakened, no cases removed.
e	Both directions demonstrated?	Yes. Unfixed master + real `urlCluster(..., max_streams_for_files_processing_in_cluster_functions = UINT64_MAX)` reproduces `Logical error: std::length_error vector` and crashes the server. Fixed binary: 15/15 OK debug; under ASAN+UBSAN in `--flaky-check` mode the `long` tag reduces 30 requested runs to 3, all OK at 43-54s each (under 180s).
f	Fix is general, not a narrow patch?	N/A in the code-bug sense (this is a test-quality tag, no source change). The clamp code fix it accompanies was already verified general at HEAD `21740f05` across all three cluster readers and both num_streams and the resize target.

Session id: cron:clickhouse-worker-slot-6:20260611-124500

The bot review noted that the PR clamps StorageObjectStorage::read too, but the regression test only exercised the ReadFromURL and ReadFromFile paths, so a future regression that left the object-storage resize target unbounded would still pass. Add an s3Cluster case: write one TSV object via s3(), then read it back via s3Cluster with max_streams_for_files_processing_in_cluster_functions set to UINT64_MAX and 1e8 and parallelize_output_from_storages = 1, ending with the existing liveness probe. Verified both directions on a debug build with local Minio: with the StorageObjectStorage clamp reverted, the s3Cluster query reaches ReadFromObjectStorageStep::initializePipeline on the secondary with max_num_streams = 100000000 and pipe.resize(max_num_streams) drives RSS past 40 GiB into MEMORY_LIMIT_EXCEEDED; with the clamp in place the same query returns and the server stays alive. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

groeneai · 2026-06-11T15:25:49Z

Pre-PR validation gate (HEAD `4d50938` — object-storage cluster test coverage)

#	Question	Answer
a	Deterministic repro?	Yes. On a debug build with local Minio, reverting only `StorageObjectStorage::read`'s clamp and running `SELECT count() FROM s3Cluster('test_cluster_two_shards_localhost', '<minio>/o.tsv', 'TSV', 'a UInt8') SETTINGS max_streams_for_files_processing_in_cluster_functions = 100000000, parallelize_output_from_storages = 1, max_threads = 1` deterministically OOMs (RSS > 40 GiB, `MEMORY_LIMIT_EXCEEDED`).
b	Root cause explained?	`s3Cluster` dispatches the read to the secondary, which runs `StorageObjectStorage::read` with `distributed_processing=true`. `ReadFromObjectStorageStep` captures `max_num_streams` from `num_streams`; `initializePipeline` clamps the local source count to `estimatedKeysCount()` but, under `parallelize_output_from_storages`, calls `pipe.resize(max_num_streams)`. Unbounded, `max_num_streams = 1e8` makes `ResizeProcessor` allocate `OutputPorts(1e8)`. Instrumented confirmation: the step logs `max_num_streams=100000000` on the secondary before the resize.
c	Fix matches root cause?	This change is test-only. The source fix (already in `21740f0`) routes all three readers' ingestion clamps through `clampClusterFunctionNumStreams` before the read step copies `num_streams` into `max_num_streams`, bounding both the source count and the resize target. This commit only extends coverage to the object-storage path.
d	Test intent preserved / new tests added?	Preserved + extended. The `urlCluster` and `fileCluster` cases are unchanged; a new `s3Cluster` case covers `ReadFromObjectStorageStep` (both the `UINT64_MAX` reserve path and the `1e8` resize path), so a future regression that left the object-storage resize target unbounded is now caught.
e	Both directions demonstrated?	Yes. NOFIX (object-storage clamp reverted, instrumented binary): the `s3Cluster` case OOMs (40–54 GiB, `MEMORY_LIMIT_EXCEEDED`). FIXED (clean HEAD): the full test passes via `clickhouse-test` — `[ OK ] 7.89 sec` — and the server stays alive.
f	Fix is general, not a narrow patch?	Yes. The single shared `clampClusterFunctionNumStreams` helper already covers all three sibling cluster readers (`StorageURL`/`StorageFile`/`StorageObjectStorage`); the test now exercises all three (`urlCluster`, `fileCluster`, `s3Cluster`), so the regression guard matches the breadth of the fix.

Session id: cron:clickhouse-worker-slot-8:20260611-143300

clickhouse-gh · 2026-06-11T18:39:38Z

LLVM Coverage Report

Metric	Baseline	Current	Δ
Lines	84.60%	84.60%	+0.00%
Functions	92.30%	92.30%	+0.00%
Branches	77.20%	77.20%	+0.00%

Changed lines: Changed C/C++ lines covered by tests: 11/15 (73.33%) | Lost baseline coverage (was covered on master, now uncovered in this PR): 3 line(s) · Uncovered code

Full report · Diff report

groeneai · 2026-06-11T21:24:38Z

nikitamikhaylov added the can be tested Allows running workflows for external contributors label Jun 10, 2026

clickhouse-gh Bot added the pr-bugfix Pull request with bugfix, not backported by default label Jun 10, 2026

clickhouse-gh Bot reviewed Jun 10, 2026

View reviewed changes

Comment thread src/Storages/StorageURL.cpp Outdated

Avogar self-assigned this Jun 10, 2026

groeneai force-pushed the groeneai/fix-url-cluster-num-streams-length-error branch from a12ca6f to 13b9700 Compare June 10, 2026 13:07

clickhouse-gh Bot reviewed Jun 10, 2026

View reviewed changes

Comment thread src/Storages/StorageURL.cpp Outdated

groeneai mentioned this pull request Jun 10, 2026

Fix join reorder dropping outer-join rows in OUTER JOIN + comma queries #101684

Merged

1 task

clickhouse-gh Bot reviewed Jun 11, 2026

View reviewed changes

Comment thread tests/queries/0_stateless/04327_cluster_functions_huge_num_streams_no_crash.sh

Avogar approved these changes Jun 16, 2026

View reviewed changes

Avogar added this pull request to the merge queue Jun 16, 2026

Merged via the queue into ClickHouse:master with commit 85ade9f Jun 16, 2026
164 of 166 checks passed

robot-ch-test-poll4 added the pr-synced-to-cloud The PR is synced to the cloud repo label Jun 16, 2026

Sunbelt Computer Software

PL/B Language Development and Support

Uh oh!

Conversation

groeneai commented Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changelog category (leave one):

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Uh oh!

groeneai commented Jun 10, 2026

Pre-PR validation gate

Uh oh!

groeneai commented Jun 10, 2026

Uh oh!

clickhouse-gh Bot commented Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

AI Review

Summary

Missing context / blind spots

Final Verdict

Uh oh!

nikitamikhaylov commented Jun 10, 2026

Uh oh!

Uh oh!

groeneai commented Jun 10, 2026

Pre-PR validation gate (re-run after fix correction)

Uh oh!

Uh oh!

groeneai commented Jun 10, 2026

Uh oh!

groeneai commented Jun 10, 2026

Uh oh!

groeneai commented Jun 10, 2026

Pre-PR validation gate

Uh oh!

groeneai commented Jun 10, 2026

Uh oh!

groeneai commented Jun 10, 2026

Uh oh!

groeneai commented Jun 11, 2026

Uh oh!

nikitamikhaylov commented Jun 11, 2026

Uh oh!

groeneai commented Jun 11, 2026

Uh oh!

groeneai commented Jun 11, 2026

Pre-PR validation gate

Uh oh!

Uh oh!

groeneai commented Jun 11, 2026

Pre-PR validation gate (HEAD 4d50938 — object-storage cluster test coverage)

Uh oh!

clickhouse-gh Bot commented Jun 11, 2026

LLVM Coverage Report

Uh oh!

groeneai commented Jun 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

groeneai commented Jun 10, 2026 •

edited

Loading

clickhouse-gh Bot commented Jun 10, 2026 •

edited

Loading

Pre-PR validation gate (HEAD `4d50938` — object-storage cluster test coverage)