Repository navigation
Conversation
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>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Run the creator of a
disk(...)custom disk outside the server-widestorage_policies_mutex.Context::getOrCreateDiskran 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 everygetDisk,getDisksMapandgetStoragePolicycall 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
getOrCreateDiskreleases 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 intoDiskSelector, and configuration reload.Poco::Exception; the dynamic type is kept for exception types that overriderethrow(ErrnoException,NetException). Other exceptions are rethrown unchanged.Risks / notes
DiskSelector::initialize.Testing
src/Disks/tests/gtest_cas_startup_stale_mounts.cpp, all also with--gtest_repeat=3:getDisksMapcall during a stale-mount wait returns under the 1.15 s bound instead of after 3.6 s.ErrnoExceptioncopy.Changelog category (leave one):
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 CASdisk(...)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
No documentation change: no setting, syntax or documented behavior changes.
CI/CD Options
Exclude tests:
Regression jobs to run: