Skip to content

24.8.14 Backport of #79147 - Remove guard pages from fibers and alternative stack for signals in threads#2035

Open
mkmkme wants to merge 3 commits into
customizations/24.8.14from
backports/24.8/79147
Open

24.8.14 Backport of #79147 - Remove guard pages from fibers and alternative stack for signals in threads#2035
mkmkme wants to merge 3 commits into
customizations/24.8.14from
backports/24.8/79147

Conversation

@mkmkme

@mkmkme mkmkme commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Note for the reviewer

This PR originally changed two files and I had conflicts in both of them. One of them, FiberStack.cpp, was missing from the original 24.8 branch because it was added later.

Initially I landed the changes on FiberStack.h itself and introduced the missing memoryGuardInstall / memoryGuardRemove functions to Common/memory.h, creating Common/memory.cpp as well. This, however, broke the build for a missing jemalloc.h because FiberStack.h got an include for Common/memory.h:

In file included from /home/mk/git/altinity-clickhouse-24.8-79147/src/Core/ExternalTable.cpp:18:
In file included from /home/mk/git/altinity-clickhouse-24.8-79147/src/Core/ExternalTable.h:3:
In file included from /home/mk/git/altinity-clickhouse-24.8-79147/src/Client/Connection.h:11:
In file included from /home/mk/git/altinity-clickhouse-24.8-79147/src/IO/ReadBufferFromPocoSocketChunked.h:4:
In file included from /home/mk/git/altinity-clickhouse-24.8-79147/src/IO/ReadBufferFromPocoSocket.h:5:
In file included from /home/mk/git/altinity-clickhouse-24.8-79147/src/Common/AsyncTaskExecutor.h:5:
In file included from /home/mk/git/altinity-clickhouse-24.8-79147/src/Common/FiberStack.h:7:
/home/mk/git/altinity-clickhouse-24.8-79147/src/Common/memory.h:13:14: fatal error: 'jemalloc/jemalloc.h' file not found
   13 | #    include <jemalloc/jemalloc.h>
      |              ^~~~~~~~~~~~~~~~~~~~~
1 error generated.

Because of that, I decided to split FiberStack to a header and implementation file as it is in the upstream. When I did that, I got multiple compiler errors about missing fmt and formatReadable* functions because the files using those functions previously got those from transitive includes by FiberStack.h. After I moved most of inclusions to FiberStack.cpp, the functions became undefined.

Before adding the extra includes to Keeper.cpp, ClientBase.cpp, Connection.cpp and AsyncTaskExecutor.cpp I verified those includes existed in the upstream's versions of those files at the moment of the original PR.

Unfortunately all of this led to the situation where the PR got quite large in comparison to the original one.

Changelog category (leave one):

  • CI Fix or Improvement (changelog entry is not required)

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

Remove guard pages for threads and async_socket_for_remote/use_hedge_requests. Change the allocation method in FiberStack from mmap to aligned_alloc. Since this splits VMAs and under heavy load vm.max_map_count can be reached (ClickHouse#79147 by @CheSema)

Documentation entry for user-facing changes

...

CI/CD Options

Exclude tests:

  • Fast test
  • Integration Tests
  • Stateless tests
  • Stateful tests
  • Performance tests
  • Aarch64 tests
  • All with ASAN
  • All with TSAN
  • All with MSAN
  • All with UBSAN
  • All with Coverage
  • All Regression
  • Disable CI Cache

Regression jobs to run:

  • Fast suites (mostly <1h)
  • Aggregate Functions (2h)
  • Alter (1.5h)
  • Benchmark (30m)
  • ClickHouse Keeper (1h)
  • Iceberg (2h)
  • LDAP (1h)
  • OAuth (5m)
  • Parquet (1.5h)
  • RBAC (1.5h)
  • SSL Server (1h)
  • S3 (2h)
  • S3 Export (2h)
  • Swarms (30m)
  • Tiered Storage (2h)

…ack-protection

stack allocation in Fiber and alternative stack for signals
@altinity-robot

altinity-robot commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

This is an automated comment for commit 26c354c with description of existing statuses. It's updated for the latest CI running

❌ Click here to open a full report in a separate page

Check nameDescriptionStatus
Integration testsThe integration tests report. In parenthesis the package type is given, and in square brackets are the optional part/total tests❌ failure
Successful checks
Check nameDescriptionStatus
AST fuzzerRuns randomly generated queries to catch program errors. The build type is optionally given in parenthesis. If it fails, ask a maintainer for help✅ success
BuildsThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Compatibility checkChecks that clickhouse binary runs on distributions with old libc versions. If it fails, ask a maintainer for help✅ success
Docker keeper imageThe check to build and optionally push the mentioned image to docker hub✅ success
Docker server imageThe check to build and optionally push the mentioned image to docker hub✅ success
Grype Scan altinityinfra/clickhouse-keeper:2035-24.8.14.10546.altinitytestThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Grype Scan altinityinfra/clickhouse-server:2035-24.8.14.10546.altinitytest-alpineThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Grype Scan altinityinfra/clickhouse-server:2035-24.8.14.10546.altinitytestThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Install packagesChecks that the built packages are installable in a clear environment✅ success
Ready for releaseThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release aes_encryptionThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release aggregate_functions_1There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release aggregate_functions_2There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release aggregate_functions_3There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release alter_attach_1There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release alter_attach_2There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release alter_moveThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release alter_replaceThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release atomic_insertThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release base_58There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release benchmark_aws_s3There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release benchmark_gcsThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release benchmark_minioThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release clickhouse_keeper_failoverThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release clickhouse_keeper_no_ssl_1There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release clickhouse_keeper_no_ssl_2There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release clickhouse_keeper_ssl_1There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release clickhouse_keeper_ssl_2There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release data_typesThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release datetime64_extended_rangeThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release disk_level_encryptionThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release dnsThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release enginesThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release exampleThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release extended_precision_data_typesThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release functionsThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release iceberg_1There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release iceberg_2There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release jwt_authenticationThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release kafkaThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release kerberosThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release key_valueThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release ldap_authenticationThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release ldap_role_mappingThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release lightweight_deleteThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release memoryThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release parquetThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release parquet_aws_s3There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release parquet_minioThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release part_moves_between_shardsThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release rbac_1There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release rbac_2There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release rbac_3There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release s3_aws_s3_1There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release s3_aws_s3_2There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release s3_azure_1There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release s3_azure_2There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release s3_export_partThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release s3_export_partitionThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release s3_gcs_1There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release s3_gcs_2There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release s3_minio_1There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release s3_minio_2There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release s3_minio_3There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release selectsThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release session_timezoneThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release settingsThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release ssl_server_1There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release ssl_server_2There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release ssl_server_3There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release swarmsThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release tiered_storage_localThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release tiered_storage_minioThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release tiered_storage_s3amazonThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release tiered_storage_s3gcsThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release versionThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Regression release window_functionsThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Sign releaseThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
Stateful testsRuns stateful functional tests for ClickHouse binaries built in various configurations -- release, debug, with sanitizers, etc✅ success
Stateless testsRuns stateless functional tests for ClickHouse binaries built in various configurations -- release, debug, with sanitizers, etc✅ success
Stress testRuns stateless functional tests concurrently from several clients to detect concurrency-related errors✅ success

@ilejn

ilejn commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Honestly I am a bit shocked because I expected something like this https://github.com/Altinity/ClickHouse/tree/backports/24.8.14/79147_tiny
Kind of a sketch, requires minor fixes, but it is compilable.

This is a compact change, it keeps ::munmap and everything that is natural for 24.8

Anyway, the PR looks good to me.

@mkmkme

mkmkme commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator Author

Honestly I am a bit shocked because I expected something like this https://github.com/Altinity/ClickHouse/tree/backports/24.8.14/79147_tiny

This is a compact change, it keeps ::munmap and everything that is natural for 24.8

Anyway, the PR looks good to me.

I was considering that, but wanted to keep it as close to upstream as possible.
We can take your approach instead since you've done it anyway :)

@ilejn

ilejn commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

I was considering that, but wanted to keep it as close to upstream as possible. We can take your approach instead since you've done it anyway :)

We already have this upstream PR in 25.3 #917
Since this backport is in line with 917, lets go further.

@CarlosFelipeOR

CarlosFelipeOR commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

❌ QA: Build fails — needs a fix before further verification

Verdict: Not approvable in its current state. This PR does not build on either architecture, so no functional or regression testing can run yet.

Build failure

Both BuilderDebRelease (amd64) and BuilderDebAarch64 (arm64) fail identically; everything downstream (stateless, integration, regression, etc.) is SKIPPED as a result.

src/Common/AsyncTaskExecutor.h:126:1: error: unknown type name 'String'; did you mean 'std::string'?
  126 | String getSocketTimeoutExceededMessageByTimeoutType(AsyncEventTimeoutType type, Poco::Timespan timeout, const String & socket_description);

Logs: release · aarch64

Root cause

AsyncTaskExecutor.h uses String (line 126) but obtained it only transitively through FiberStack.h. This PR empties FiberStack.h (moving its includes into the new FiberStack.cpp), so AsyncTaskExecutor.h no longer sees the String alias. The <fmt/format.h> include added to AsyncTaskExecutor.cpp covers the fmt::format usage but not String — and since the declaration lives in the header, the fix belongs in the header.

Suggested fix

// src/Common/AsyncTaskExecutor.h
#include <base/types.h>   // provides the `String` alias (using String = std::string)

This keeps the header self-contained and fixes every consumer in the ExternalTable.h → Connection.h → … → AsyncTaskExecutor.h chain, not just the .cpp.

Backport review (otherwise looks good)

Differential vs upstream ClickHouse#79147: the core logic of FiberStack.cpp and ThreadStatus.cpp matches the upstream final state exactly. The larger diff is justified — it reconstructs prerequisites the 24.8 branch lacked (splitting FiberStack into .h/.cpp, backporting the memoryGuardInstall/memoryGuardRemove infra into memory.cpp).


Root-cause diagnosis and the suggested fix were produced with AI-assisted analysis (static; confirmed by inspecting the header includes, not yet by a local rebuild).

@mkmkme

mkmkme commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator Author

@CarlosFelipeOR,

Thanks! I've addressed this in 26c354c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants