Set allocator_may_return_null=1 in the default sanitizer options so a single oversized allocation returns null (a recoverable CANNOT_ALLOCATE_MEMORY / std::bad_alloc) instead of the sanitizer runtime calling NORETURN ReportAllocationSizeTooBig and aborting the server. aarch64 MSan caps a single allocation at 8 GiB, below our configured max_allocation_size_mb=32768, so only arm_msan hit the abort as "Server died". The flag governs only the OOM/oversized paths and does not weaken use-of-uninitialized / data-race / UB detection. Once llvm/llvm-project#206649 lands in our toolchain this option may be removable.
Enable SYSTEM ALLOCATE MEMORY by default in clickhouse-local (a harmless diagnostic command) so the regression test uses clickhouse-local rather than a standalone server. The test issues an oversized SYSTEM ALLOCATE MEMORY, asserts a following SELECT still runs, and captures the recovery in the reference. ASan and MSan print a benign "<Sanitizer> failed to allocate <N> bytes" warning while TSan and non-sanitizer builds recover silently, so the test routes the sanitizer log to its own file (keeping it out of the runner fatal log) and normalizes the warning to its build-invariant substring with grep -o, falling back to the same token on silent builds, so one reference matches every build. clickhouse-test is left unchanged, so nothing is globally ignored.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
When `clickhouse-local` acts as a server (`SYSTEM START LISTEN TCP/HTTP`),
connections served over HTTP used the interactive terminal default output
format (`PrettyCompact`) instead of `TabSeparated`, which is what a real
`clickhouse-server` uses. This happened because `applyCmdOptions` set the
interactive default on the shared global context, which is also used by the
embedded HTTP/TCP listeners.
As a result, a client connecting over HTTP (for example `clickhouse-connect`)
received a `PrettyCompact`-formatted response to the version query performed
during the connection handshake and failed to parse it.
The interactive default is only meant for rendering query results in the
terminal and is applied separately via `ClientBase::default_output_format`, so
the global (server) default can safely fall back to `TabSeparated`. An explicit
`--format`/`--output-format` is still honored.
Add a `.expect` test that starts `clickhouse-local` interactively, enables the
HTTP listener, and verifies the served default format is `TabSeparated`.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Threads defer (de)allocations in a thread-local untracked_memory and flush to the
global MemoryTracker only past max_untracked_memory, so it undercounts by up to
nthreads * max_untracked_memory — tens of GiB with thousands of threads, enough to
overshoot max_server_memory_usage and OOM.
Bound it per-CPU instead. Each CPU keeps two counters, allocated (>= 0) and freed
(<= 0); a thread folds its untracked_memory into the counter for its sign, batched by
per_cpu_untracked_memory_thread_buffer (32 KiB), and must flush once a counter leaves
[-max_per_cpu_untracked_memory, max_per_cpu_untracked_memory]. The deferred total is
then bounded by ncpu * max_per_cpu_untracked_memory regardless of thread count, parked
threads included. The two sides are independent so a deferred free cannot admit a
deferred allocation (which the free would then strand untracked on its own flush).
max_per_cpu_untracked_memory (default 8 MiB, 0 disables) is a new server setting;
max_untracked_memory stays per-thread. Linux only (needs sched_getcpu).
v2: reimplemented — v1 bounded via sum % SLICE on one global counter, which cannot
bound the per-CPU deferred bytes; replaced with the PerCPUMemory class.
v3: two counters instead of one signed net — a signed net let a deferred free cancel
a deferred allocation, stranding it untracked when the free flushed.
v4: restore the per-CPU contribution unconditionally on a failed flush; a gated
re-reserve could drop still-live bytes if another thread took the budget meanwhile.
v5: release the per-CPU contribution only in the caller, not in reserve().
v6: Rename PerCPUMemory::reserve to sync (it sets an absolute value, not a reservation)
v7: Rename net_per_cpu member to slots
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Resolve include-region conflicts in programs/local/LocalServer.cpp and
programs/server/Server.cpp: master added <Functions/pointInPolygon.h>
and this branch added <Functions/UserDefined/UserDefinedSQLFunctionFactory.h>
at the same location; keep both (both are used in each file).
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Branch was 1157 commits behind master and the only CI red was the chronic
master-wide `Hung check failed, possible deadlock found` flake (13-70/day on
master itself; this PR is a compile-time no-op under `SANITIZER` so it cannot
affect the `arm_asan_ubsan` stress run). Re-merge to refresh CI on the current
master and to re-run the AI review against the already-updated PR description.
The merge conflicted only in `src/Common/MemoryWorker.cpp`: master evolved the
`memory_worker_dynamic_hard_limit` apply block to skip adjustments smaller than
`min_change_ratio` (1%) of the current limit, still naming the local
`current_hard_limit`. The branch had renamed that local to `installed_hard_limit`
to avoid shadowing the `current_hard_limit` the speculative-reserve block reads
earlier in `updateResidentMemoryThread` (`-Wshadow`/`-Werror`). Kept master's new
threshold logic verbatim and renamed its local to `installed_hard_limit`; the
`LOG_TRACE` already references `installed_hard_limit`. No behavior change.
Verified the resolved `MemoryWorker.cpp` compiles cleanly (`-fsyntax-only`,
`-Werror -Wall -Wextra`) against the merged tree.
Third slice of the UNIQUE KEY stack. Lands the per-part delete-bitmap
storage layer on top of PR-2's DeleteBitmap + DeleteBitmapFileOps
primitives, aligned with the CSN-based transaction model documented
in design/txn/transaction_consistency.md.
Why a separate bitmap store. UNIQUE KEY needs to mark logically-deleted
rows without rewriting the part. Each commit publishes one cumulative
bitmap per touched part, tagged by a monotonic per-partition version.
Readers pin a snapshot version and consult the bitmap with
`max version ≤ snapshot` for each part — that's the snapshot-isolation
primitive for UK reads, INSERT-time probe, and merge-time row
resolution. This PR delivers the storage and read/write API for that
bitmap; the transaction-control plane (version allocation, partition
mutex, query snapshot, manifest, recovery) lands in later PRs.
What's in this PR:
- MergeTreeBitmapStore — one per table. Atomic install at a version
(strictly monotonic; throws LOGICAL_ERROR otherwise), snapshot-pinned
read returning (shared_ptr<const DeleteBitmap>, chosen_version),
(V_b, V_next)-rule GC, and dropPart for parts leaving the active
set. Keeps an in-memory version index per part (rebuilt from disk on
first access) so the hot read path is map lookup + binary search,
not a syscall.
- DeleteBitmapCache — process-wide cache for deserialized bitmaps,
keyed by stable part identity + version. Configurable via
unique_key_bitmap_cache_* settings, wired through Context startup/
reload, observable via DeleteBitmapCache* metrics and
UniqueKeyBitmap* events.
- IMergeTreeDataPart — cache-identity accessor for callers building
cache keys; delete_bitmap_<csn>.rbm enumeration in
getFileNamesWithoutChecksums so BACKUP and replication-fetch carry
bitmap sidecars.
Why the API is shaped this way:
- Bitmaps are immutable by version. readBitmap returns
shared_ptr<const DeleteBitmap> so a writer building a new version
must copy, never mutate the cached older snapshot.
- The store is concrete (no IUniqueKeyBitmapStore interface). Future
SharedMergeTree variant runs a fundamentally different protocol
(Keeper-coordinated S3 with (bn, uuid) refs); a shared base wouldn't
fit at this layer.
- Integration with active reads/writes lands in later slices (PR 4
read filter, PR 5 DELETE, PR 13 INSERT/UPSERT commit, PR 14a merge
reconcile).
Tests: 39 / 39 PASS in DeleteBitmap*:*BitmapFileOps*:*MergeTreeBitmapStore*
filter:
- 20 DeleteBitmapTest (primitive)
- 1 DeleteBitmapCacheTest (cache wrapper)
- 11 DeleteBitmapFileOpsTest (file I/O)
- 7 MergeTreeBitmapStoreTest: read empty / max-≤-snapshot pick / cache
reuse / GC removal / GC committed-filter / dropPart / corrupt-file
surface.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
`LocalServer::startServers` started each listener's accept thread as soon
as its socket was bound (`createServer(..., start_server=true, ...)`).
With multiple `listen_host` values, an earlier listener could therefore
accept a connection and begin executing a query while a later bind in the
same call was still pending. If that later bind then failed, the
rollback stopped accepting new connections but, by the documented
lifetime rule, deliberately left the already-busy adapter in `servers` —
so a rejected `SYSTEM START LISTEN` could leave an external session
running. Under the default unauthenticated local users setup that is a
real concern.
Split the start into two phases. Phase 1 binds every requested
`listen_host` x protocol socket with `start_server=false`: the socket is
bound and listening at the kernel level, but no accept thread runs and no
connection is ever handed to a handler. Only after the per-protocol
verification confirms the whole requested set is bound does phase 2 start
the accept threads. The "Listening for ..." log line, previously emitted
by `createServer`, moves to phase 2.
This closes the accept window: on a bind or verification failure none of
the appended adapters has started accepting, so the rollback simply
closes their bound sockets (any connection queued in the kernel backlog
is reset, never served). The existing stop-then-erase rollback shape is
kept as the correct general rule for the rare case where `start` itself
throws in phase 2 after some accept threads are already live.
`TCPServer` (and `HTTPServer`, which derives from it) reports
`isOpen() == true` from construction, so a bound-but-unstarted adapter is
correctly counted by the per-protocol verification and its socket is
properly closed by `stop()` during rollback.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
A follow-up to validating the command-line `--tcp_port` / `--http_port`
options: a `tcp_port` / `http_port` value coming from a loaded config file
(rather than the command line) still reached the listener path unchecked.
`createServer` casts the configured port to `UInt16`, so an out-of-range
config value such as `<tcp_port>70000</tcp_port>` or `<http_port>-1</http_port>`
would silently wrap (to `4464` / `65535`) and `SYSTEM START LISTEN` would bring
the listener up on an unexpected port instead of failing.
Validate the effective `tcp_port` / `http_port` value in `LocalServer::startServers`
for every protocol being started, rejecting values outside `0..65535` with
`BAD_ARGUMENTS` (`0` is allowed and means an OS-assigned port). The check is
done in the local-specific `startServers` rather than in the shared
`createServer`, so it does not change `clickhouse-server` behavior, and it runs
lazily only when the user actually requests a listener, so a stray out-of-range
port in a shared config does not break plain `clickhouse-local` invocations that
never listen.
Added `05017_local_system_listen_config_port_range` covering the rejected
out-of-range config values and an accepted config `tcp_port=0`.
Addresses: https://github.com/ClickHouse/ClickHouse/pull/101143#discussion_r3408759429
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
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) <[email protected]>
`--tcp_port` and `--http_port` define the port that `SYSTEM START LISTEN`
binds, but they were parsed as a signed `int` and stored without range
validation. The listener path later does `static_cast<UInt16>(config.getInt(port_name))`
in `createServer`, so values outside `0..65535` silently wrapped instead of
failing: `--tcp_port -1` became `65535` and `--http_port 70000` became `4464`,
exposing the listener on an unexpected port.
Validate both options when they are processed: reject negative and `> 65535`
values with `BAD_ARGUMENTS`, allowing `0` for an OS-assigned port. The error
names the offending option and value.
Added `04338_local_system_listen_port_range_validation` covering the rejected
out-of-range values and the accepted boundary values (`0` and `65535`).
Addresses: https://github.com/ClickHouse/ClickHouse/pull/101143#discussion_r3407941592
Co-Authored-By: Claude Opus 4.8 <[email protected]>
Master now sizes connection-pool thread stacks with
`DEFAULT_THREAD_STACK_SIZE` (8 MiB on Darwin, where the OS default of
512 KiB is too small for JIT codegen) — see #107033. The listener pool
in `clickhouse-local` mirrors the pool configuration of `Server::main`,
so apply the same stack size here.
Co-Authored-By: Claude Fable 5 <[email protected]>
Address review: a positive max_server_memory_usage_to_ram_ratio used to
opt the server into runtime hard-limit shrinking with no way to keep the
previous static cap, while setting the ratio to 0 removed the cap
entirely. The new memory_worker_dynamic_hard_limit server setting
(enabled by default) gates only the runtime adjustment: when disabled,
max_server_memory_usage_to_ram_ratio caps the hard memory limit
statically at startup and on configuration reload, exactly as in
previous versions.
Co-Authored-By: Claude Fable 5 <[email protected]>
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) <[email protected]>
`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: https://github.com/ClickHouse/ClickHouse/issues/101697
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
The all-or-nothing rollback in `LocalServer::startServers` stopped every
listener appended by a failed `SYSTEM START LISTEN` and then erased their
adapters unconditionally. `stop` only prevents new connections: once
`server.start` has returned, an accept thread can already have handed an
external connection to a handler that still references the underlying
`TCPServer`. Erasing the adapter then destroys the `TCPServer` while the
handler is still using it — a use-after-free.
Mirror the normal `stopServers` lifetime rule: stop every appended adapter,
then erase only those with no active connections. A still-busy adapter stays
in `servers` (already stopped, so it accepts nothing new and is not reported
as an active listener) and is drained and destroyed by `cleanup` at teardown.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
`LocalServer::cleanup` runs from the `SCOPE_EXIT` in `LocalServer::main`,
which can execute while the stack is unwinding from a failed batch query.
When external listener connections remained after the drain window, the
forced-shutdown branch hardcoded `safeExit(0)`, exiting before the
surrounding `catch (DB::Exception & e)` could return the query's nonzero
error code. As a result `clickhouse-local` could report success even
though the local query failed.
Use `safeExit(std::uncaught_exceptions() ? 1 : 0)` so a failure is still
reported when the forced shutdown happens during exception unwinding.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Address review on `LocalServer::startServers`:
- Defer `Context::registerServerPort` until after the per-protocol
verification passes. Previously a successful early bind registered its
port immediately, and when a later bind failed the rollback closed the
listener but left `server_ports` pointing at the now-closed port, so
`getServerPort('tcp_port')` returned a stale value diverging from the
all-or-nothing listener state. The ports are now collected and published
only on full success.
- Set the process-list limit from `max_concurrent_queries` instead of a
hard-coded `0`. Once `clickhouse-local` is turned into a server via
`SYSTEM START LISTEN` it accepts external connections and must honor the
configured concurrency limit like `clickhouse-server`; the default of `0`
preserves the historical "no limit" behavior for ordinary local usage.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
New keeper-only system table with one row per on-disk Raft log file
(changelog_<from>_<to>.bin[.zstd]) tracked by the in-process Keeper.
Mirrors the plumbing of system.keeper_snapshots.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Address review: the shared `setupEchoAndHighlightSettings` made `--verbose`
imply query echoing for all clients, but historically only `clickhouse-local`
behaved that way. `clickhouse-client` and the embedded client defaulted
`echo_queries` from `--echo` only, so `clickhouse-client --verbose --query ...`
emitted just the result.
Make the implication opt-in via a `verbose_implies_echo` parameter (passed
`true` only from `LocalServer`), restoring the prior contract that existing
behavior stays identical unless the new echo options are used.
Extend `04308_echo_options_granular` to cover both cases: `--verbose` echoes
in `clickhouse-local` but not in `clickhouse-client`.
