iframe-proxy

mstetsyuk · 2026-06-11T21:55:58Z

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 into CHANGELOG.md):

Fix the stuck shutdown bug on server and local due to static thread pools keeping idle threads indefinitely if max_*_thread_pool_free_size > 0.

Join threads from the global thread pool before exiting from main in all binaries. As a result, all static object accesses by threads happen-before static object destruction. Currently, we only do it for the server and keeper binaries.

Version info

Merged into: 26.6.1.924

clickhouse-gh · 2026-06-11T21:56:35Z

Workflow [PR], commit [68c9eb9]

Summary: ✅

Performance Comparison: Performance dashboard
Keeper Stress Tests (PR): Grafana: Run details (this run), Grafana: 1vs1 Comparison, Grafana: Historical progression

AI Review

Summary

This PR adds explicit shutdown of static ThreadPool instances before shutting down GlobalThreadPool, which addresses the reproduced server and local shutdown hang. The approach is directionally right, but the implementation still does not satisfy the PR contract of covering all binaries that can create global-pool workers, so I would request changes.

Missing context / blind spots

⚠️ The Praktika report linked by the robot for commit 68c9eb9e currently exposes no test results, so I could not use CI as evidence that the new integration test passed.

Findings

⚠️ Majors

[src/IO/SharedThreadPools.cpp:155] The shutdown lifetime invariant is still only applied from a hand-picked subset of mainEntry functions. Registered tools such as mainEntryClickHouseClient, mainEntryClickHouseKeeperClient, mainEntryClickHouseZooKeeperDumpTree, mainEntryClickHouseZooKeeperRemoveByList, mainEntryClickHouseExtractFromConfig, and mainEntryClickHouseKeeperUtils can still create GlobalThreadPool workers through parallel output formatting or zkutil::ZooKeeper, then return to the dispatcher without an explicit GlobalThreadPool::shutdown. Those workers are then joined from the static GlobalThreadPool destructor, which leaves the same static-destruction ordering race this PR is fixing for server and local.
Suggested fix: install one top-level scope guard in programs/main.cpp and programs/keeper/keeper_main.cpp, or add equivalent guards to every registered mainEntry that can reach GlobalThreadPool.

Tests

⚠️ The new integration test covers server and local, but not the other registered binaries covered by the PR title and changelog. After moving the guard to the dispatchers or adding per-entry guards, add focused coverage for at least one formerly uncovered path that starts global-pool workers, for example a parallel-formatting client path or a Keeper/ZooKeeper utility path.

Final Verdict

Status: ⚠️ Request changes

Minimum required actions: cover the remaining registered entry points that can start GlobalThreadPool workers, and add focused evidence for one of those previously uncovered paths.

mstetsyuk · 2026-06-11T22:04:15Z

Joining threads before exiting from main is required to avoid such issues.

clickhouse-gh · 2026-06-11T22:29:11Z

 {
-    static StaticThreadPool instance("FormatParsingThreadPool", CurrentMetrics::FormatParsingThreads, CurrentMetrics::FormatParsingThreadsActive, CurrentMetrics::FormatParsingThreadsScheduled);
-    return instance;
+#define SHUTDOWN_STATIC_THREAD_POOL(SUFFIX, NAME, METRIC) get##SUFFIX##ThreadPool().shutdown();


⚠️ This lifetime fix is still incomplete because shutdownAll is only called from a hand-picked subset of mainEntry functions. Several registered binaries can still create GlobalThreadPool workers and then return without GlobalThreadPool::shutdown: mainEntryClickHouseClient reaches ClientBase::initOutputFormat -> getOutputFormatParallelIfPossible -> ParallelFormattingOutputFormat/ThreadFromGlobalPool; mainEntryClickHouseKeeperClient, mainEntryClickHouseZooKeeperDumpTree, mainEntryClickHouseZooKeeperRemoveByList, and mainEntryClickHouseExtractFromConfig create zkutil::ZooKeeper, whose constructor starts send/receive ThreadFromGlobalPool threads; mainEntryClickHouseKeeperUtils can do the same through parallel output.

Those idle global-pool OS threads are then still joined from the static GlobalThreadPool destructor, so the static-destruction race this PR fixes for server/local remains in those modes. Please either install one top-level scope guard in programs/main.cpp and programs/keeper/keeper_main.cpp, or add equivalent guards to every mainEntry that can reach GlobalThreadPool.

azat

Looks good

Do you have a stacktrace as an example?

mstetsyuk · 2026-06-12T17:33:49Z

Do you have a stacktrace as an example?

Of getting stuck? I added a regression test that reproduces it.

azat · 2026-06-12T17:36:45Z

Of getting stuck? I added a regression test that reproduces it.

Yes, but I just wanted to look at the two stacktraces w/o running test manually

clickhouse-gh · 2026-06-12T18:40:27Z

LLVM Coverage Report

Metric	Baseline	Current	Δ
Lines	84.70%	84.70%	+0.00%
Functions	92.40%	92.40%	+0.00%
Branches	77.30%	77.30%	+0.00%

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

Full report · Diff report

mstetsyuk · 2026-06-15T14:53:50Z

@azat, all threads stack traces: https://pastila.clickhouse.com/?008e7a5e/642ef6f364286499e10b06644c222b91#lABD0RXIc2wNqXKpHuds+w==GCM

TLDR: jemalloc threads, main thread waiting on join, one thread stuck in the parent_pool.new_job_or_shutdown.wait call

clickhouse-gh · 2026-06-17T03:12:39Z

📊 Cloud Performance Report

✅ AI verdict: no_change — no significant changes across 38 queries analysed

This PR only changes process-shutdown ordering — it destroys the static thread pools before joining the global thread pool, plus a macro refactor of the pool getters. That code runs at teardown, not during query execution, so it cannot plausibly speed up any SELECT. The four flagged improvements (clickbench Q22/Q23, TPC-H Q7/Q20) all sit off the changed path and several show noisy measurements, so we read them as run-to-run variance rather than a real effect and downgraded them to not-sure. No actionable performance change is attributable to this PR.

clickbench

⚠️ 2 inconclusive

Flagged queries (2 of 43)

	Query	Verdict	Baseline med (ms)	PR med (ms)	Change	q-value	Hint
⚠️	22	not_sure	334	256	-23.4%	<0.0001	PR only reorders thread-pool teardown at process shutdown; cannot affect query execution. Source CV ~78% — noise.
⚠️	23	not_sure	297	202	-32.0%	<0.0001	Shutdown-ordering-only change; nothing here touches the SELECT hot path. -32% is off-path, not a PR effect.

_{q-value = BH-FDR adjusted p; smaller is stronger evidence. MIRAI flags a query when q < fdr_q (default 0.10) — the value the verdict is based on.}

tpch_adapted_1_official

⚠️ 2 inconclusive

Flagged queries (2 of 22)

_{q-value = BH-FDR adjusted p; smaller is stronger evidence. MIRAI flags a query when q < fdr_q (default 0.10) — the value the verdict is based on.}

Debug info

StressHouse run: 6db4406e-598d-4d0e-b962-7b964a9ad6d4
MIRAI run: 9106e8dc-fca4-4720-8165-22de1aad2370
PR check IDs:
- clickbench_299448_1781675416
- clickbench_299573_1781675452
- clickbench_299721_1781675676
- tpch_adapted_1_official_299741_1781675677
- tpch_adapted_1_official_300017_1781676167
- tpch_adapted_1_official_300104_1781676202

PR ClickHouse#107291 introduced StaticThreadPool::shutdownAll() and placed it before GlobalThreadPool::instance().shutdown() in Server::main's SCOPE_EXIT. Background threads (notably SystemLog flush) run on the global pool and can lazily call into static pools — e.g. via SystemLog::prepareTable -> StorageMergeTree ctor -> loadDataParts -> getActivePartsLoadingThreadPool().get(). Tearing down the static pools first races those callers, producing "The MergeTreePartsLoaderThreadPool is not initialized" and TSan races on the underlying unique_ptr. Swap the order so background threads drain first. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Join threads from global thread pool before exiting from main

b219eaf

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

mstetsyuk assigned azat Jun 11, 2026

clickhouse-gh Bot reviewed Jun 11, 2026

View reviewed changes

better

68c9eb9

mstetsyuk mentioned this pull request Jun 12, 2026

Reapply "Add UntrackedMemory asynchronous metric" #106386

Open

azat approved these changes Jun 12, 2026

View reviewed changes

mstetsyuk enabled auto-merge June 17, 2026 13:44

mstetsyuk added this pull request to the merge queue Jun 17, 2026

Merged via the queue into master with commit eae0a52 Jun 17, 2026
166 checks passed

mstetsyuk deleted the static-thread-pool-shutdownall branch June 17, 2026 14:44

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

jaymebrd mentioned this pull request Jun 24, 2026

Speed up server shutdown for databases with many tables #104969

Open

Sunbelt Computer Software

PL/B Language Development and Support

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix stuck shutdown in server and local and join all global pool threads in all binaries#107291

Fix stuck shutdown in server and local and join all global pool threads in all binaries#107291
mstetsyuk merged 2 commits into
masterfrom
static-thread-pool-shutdownall

mstetsyuk commented Jun 11, 2026 •

edited by robot-clickhouse

Loading

Uh oh!

clickhouse-gh Bot commented Jun 11, 2026 •

edited

Loading

Uh oh!

mstetsyuk commented Jun 11, 2026

Uh oh!

clickhouse-gh Bot Jun 11, 2026

Uh oh!

azat left a comment

Uh oh!

mstetsyuk commented Jun 12, 2026

Uh oh!

azat commented Jun 12, 2026

Uh oh!

clickhouse-gh Bot commented Jun 12, 2026

Uh oh!

mstetsyuk commented Jun 15, 2026

Uh oh!

clickhouse-gh Bot commented Jun 17, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Sunbelt Computer Software

PL/B Language Development and Support

Uh oh!

Conversation

mstetsyuk commented Jun 11, 2026 • edited by robot-clickhouse Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelog category (leave one):

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

Version info

Uh oh!

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

Uh oh!

AI Review

Summary

Missing context / blind spots

Findings

Tests

Final Verdict

Uh oh!

mstetsyuk commented Jun 11, 2026

Uh oh!

clickhouse-gh Bot Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

azat left a comment

Choose a reason for hiding this comment

Uh oh!

mstetsyuk commented Jun 12, 2026

Uh oh!

azat commented Jun 12, 2026

Uh oh!

clickhouse-gh Bot commented Jun 12, 2026

LLVM Coverage Report

Uh oh!

mstetsyuk commented Jun 15, 2026

Uh oh!

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

Uh oh!

clickbench

tpch_adapted_1_official

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mstetsyuk commented Jun 11, 2026 •

edited by robot-clickhouse

Loading

clickhouse-gh Bot commented Jun 11, 2026 •

edited

Loading

clickhouse-gh Bot commented Jun 17, 2026 •

edited

Loading