Skip to content

Create custom disks outside storage_policies_mutex - #2573

Open
filimonov wants to merge 4 commits into
antalya-26.6-20261008from
fix/antalya-26.6/cas-392-parallel-stale-mount-wait
Open

filimonov wants to merge 4 commits into
antalya-26.6-20261008from
fix/antalya-26.6/cas-392-parallel-stale-mount-wait

Conversation

@filimonov

Copy link
Copy Markdown
Member

Run the creator of a disk(...) custom disk outside the server-wide storage_policies_mutex.

Context::getOrCreateDisk ran the disk creator under that mutex. A CAS disk whose previous process exited without a farewell waits out the stale mount lease inside the creator (about 36.5 s with the default 30 s lease). While it waited, every other custom-disk creation and every getDisk, getDisksMap and getStoragePolicy call queued behind it. With three stale mounts, server start would take about 110 s; the stress test harness gave up at its limit with "Cannot start clickhouse-server":
#2516 (Stress test (amd_debug), https://altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2516&sha=b7e424247ab997ae2b0d3c06853e10a2c445f2a5&name_0=PR&name_1=Stress%20test%20%28amd_debug%29)

Changes

  • getOrCreateDisk releases the mutex while the creator runs. The mutex still guards selector initialization, lookup, the disks-map snapshot passed to the creator, the in-flight creation map, the final insert into DiskSelector, and configuration reload.
  • Concurrent requests for the same name share one creation. A second open of one CAS root would fail as a live twin.
  • Concurrent callers of a failed creation all get its failure and do not run their own creator; previously each blocked caller ran its creator in turn. Each caller gets its own copy of a Poco::Exception; the dynamic type is kept for exception types that override rethrow (ErrnoException, NetException). Other exceptions are rethrown unchanged.
  • A failed creation is not cached: the next request creates the disk again, as before.
  • A configuration reload skips a configured disk whose name is being created as a custom disk and logs a warning: creating it could open the same storage twice.
  • The stale-mount wait itself is unchanged.

Risks / notes

  • Parallel stale-mount waits apply to custom disks created from independent requests (for example, table loads). Config-defined disks are still created one by one in DiskSelector::initialize.
  • One stale mount still costs about 36.5 s of server start.
  • A configured disk skipped by a reload is not created by that reload. If the custom creation succeeds, the name then refers to the custom disk. If it fails, the configured disk stays absent until the next reload, and storage policies of that reload that use it are not applied.

Testing

  • src/Disks/tests/gtest_cas_startup_stale_mounts.cpp, all also with --gtest_repeat=3:
    • 3 stale disks open in about 3.6 s instead of 10.8 s.
    • A getDisksMap call during a stale-mount wait returns under the 1.15 s bound instead of after 3.6 s.
    • Two concurrent creations of one disk share one open.
    • A failed creation is retried by the next request.
    • Two concurrent requests share one failure: one creator call, each gets its own ErrnoException copy.
    • A configuration reload during a creation does not create the configured disk of the same name.

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):

Creating a disk(...) custom disk no longer blocks other disk creations and disk or storage policy lookups. Server start with several CAS disk(...) custom disks that wait out a stale mount lease now waits for them in parallel instead of one after another.

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

No documentation change: no setting, syntax or documented behavior changes.

CI/CD Options

Exclude tests:

  • Fast test
  • Integration Tests
  • Stateless tests
  • Stateful tests
  • Unit 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)
  • CAS (content-addressed storage; Antalya only)
  • 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)

filimonov and others added 4 commits October 15, 2026 07:41
Server start creates each table's `disk = disk(...)` custom disk inside
`Context::getOrCreateDisk`, which holds `storage_policies_mutex` while the
disk starts. A CAS disk whose previous process left no farewell waits out
the stale mount lease during that start, so N stale mounts cost N
observation windows and every disk or storage-policy lookup in the server
blocks meanwhile.

Both tests fail on the current code: three stale mounts take 10820 ms
against a 3450 ms window, and `getDisksMap` blocks for 3603 ms.

CI report: https://altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2516&sha=b7e424247ab997ae2b0d3c06853e10a2c445f2a5&name_0=PR&name_1=Stress%20test%20%28amd_debug%29
PR: #2516

Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`Context::getOrCreateDisk` ran the disk creator under the server-wide
`storage_policies_mutex`. A CAS disk waiting out a stale mount lease
(about 36.5 s) inside the creator blocked every other disk creation and
every disk and storage policy lookup, so stale mounts were waited out one
after another at server start.

The mutex now guards only the map of in-flight creations and the final
insert into `DiskSelector`. Concurrent requests for one name share one
creation and get the same disk or a copy of the same exception. A failed
creation is still not cached.

CI report: #2516

Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A configuration reload could create a configured disk whose name was
being created as a custom disk, opening the same storage twice. The
reload now skips such names and logs a warning.

Callers of a shared disk creation rethrow a copy of the stored exception
through `Poco::Exception::rethrow`, so `ErrnoException` and other
derived types keep their type and fields. Other exceptions become a
`DB::Exception` with the same code and message.

Tests: shared failure with independent copies, config reload during a
creation, unique disk names so the suite survives `--gtest_repeat`, and
the CAS logger channel is restored only after the disks stop.

Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Use the existing `copyMutableException` helper: `Poco` exceptions are
copied through their virtual `rethrow`, other exceptions are rethrown
unchanged. State the `disks_to_skip` contract in `DiskSelector.h`, wrap
a long log line, and bound the waits in the shared-failure test.

Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Workflow [PR], commit [c905ab3]

@filimonov filimonov added the CAS label Oct 15, 2026

This branch has not been deployed

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant