Use max_insert_threads for plain INSERTs without materialized views

[CheSema]

ConcurrencyControl has an issue (#102947) where CC slots are allocated eagerly at query start, and spawnThreads has a race where it spawns threads faster than they can register as idle, resulting in many unnecessary threads being created.

Until lazy CC slot allocation is implemented, we have no choice other than to set strict limits on the thread count for insert queries.

buildInsertPipeline was requesting max_threads CC slots for all INSERTs regardless of pipeline width. For plain INSERTs without MVs the pipeline is 1-wide, so this wasted CC capacity and spawned unnecessary threads.

Now we use max_insert_threads when no MVs are attached, and max_threads only when MVs are involved (their inner SELECTs benefit from full parallelism).

Mitigates #102947

Changelog category (leave one):

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

Changelog entry

Plain INSERTs without materialized views no longer request excessive ConcurrencyControl slots and threads (max_threads instead of max_insert_threads), preventing CC slot starvation and thread count blowup on clusters with high INSERT throughput.

Documentation entry for user-facing changes

• Documentation is written (mandatory for new features)

[clickhouse-gh]

Workflow [PR], commit [c4f5e7e]

Summary: ❌

job_name	test_name	status	info	comment
Upgrade check (amd_release)		FAIL
	Error message in clickhouse-server.log (see upgrade_error_messages.txt)	FAIL	cidb

---

AI Review

Summary

This PR changes InterpreterInsertQuery::buildInsertPipeline to use max_insert_threads for plain INSERT without dependent materialized views, and max_threads only when views are involved. The core C++ change is small and aligned with the reported concurrency-slot/thread over-allocation problem, but there are two merge-blocking review issues in metadata/test quality that should be addressed before approval.

Findings

⚠️ Majors

• [tests/queries/0_stateless/04102_insert_threads_eagerly_spawned.sh:2,37-45,67-76] The test is over-constrained by no-replicated-database and no-async-insert, which narrows CI coverage. Also, system.query_log lookups do not pin is_initial_query = 1 and do not select the latest single row, making the assertion more fragile in mixed-profile runs.
  • Suggested fix: remove those restrictive tags and make the query deterministic in-script (--async_insert=0 --wait_for_async_insert=1, plus is_initial_query = 1 and ORDER BY event_time_microseconds DESC LIMIT 1 in both checks).

💡 Nits

• [PR description: Changelog entry] The changelog text inverts the settings names and describes the opposite behavior (max_threads vs max_insert_threads) from the code.
  • Suggested fix: update the entry to state that plain INSERT without MVs now uses max_insert_threads, while inserts with MVs continue to use max_threads.

ClickHouse Rules

Item	Status	Notes
Deletion logging	➖
Serialization versioning	➖
Core-area scrutiny	✅
No test removal	✅
Experimental gate	➖
No magic constants	✅
Backward compatibility	✅
SettingsChangesHistory.cpp	➖
PR metadata quality	❌	Changelog entry currently contradicts the implemented behavior
Safe rollout	✅
Compilation time	✅
No large/binary files	✅

Final Verdict

• Status: ⚠️ Request changes
• Minimum required actions:
  1. Fix Changelog entry wording to match the actual setting switch (max_insert_threads for plain INSERT, max_threads when MVs are involved).
  2. Make the new stateless test deterministic without profile-narrowing tags (or otherwise preserve equivalent CI profile coverage).

[clickhouse-gh]

LLVM Coverage Report

Metric	Baseline	Current	Δ
Lines	84.00%	84.00%	+0.00%
Functions	91.10%	91.10%	+0.00%
Branches	76.50%	76.50%	+0.00%

Changed lines: 100.00% (4/4) · Uncovered code

Full report · Diff report