Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Catch exceptions from create_directories in fs cache #49203

Merged
merged 1 commit into from Apr 27, 2023

Conversation

kssenii
Copy link
Member

@kssenii kssenii commented Apr 26, 2023

Changelog category (leave one):

  • Improvement

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

Catch exception from create_directories in filesystem cache.

@robot-clickhouse robot-clickhouse added the pr-bugfix Pull request with bugfix, not backported by default label Apr 26, 2023
@devcrafter devcrafter self-assigned this Apr 26, 2023
@kssenii kssenii added pr-improvement Pull request with some product improvements and removed pr-bugfix Pull request with bugfix, not backported by default labels Apr 26, 2023
@kssenii
Copy link
Member Author

kssenii commented Apr 27, 2023

Integration tests (release) [2/4]

test_merge_tree_s3/test.py::test_store_cleanup_disk_s3
error

 DB::Exception: Data directory for table already containing data parts - probably it was unclean DROP table or manual intervention. You must either clear directory by hand or use ATTACH TABLE instead of CREATE TABLE if you need to use that parts..

seems unrelated, moreover in this test cache is not used

Integration tests (tsan) [4/6]

test_user_ip_restrictions/test.py - not related

Upgrade check (tsan)

#49258

@kssenii kssenii merged commit 28dee37 into ClickHouse:master Apr 27, 2023
255 of 259 checks passed
if (offsets.empty())
{
auto key_path = getPathInLocalCache(key);

if (!fs::exists(key_path))
fs::create_directories(key_path);
{
try
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense to add a comment about the possible reasons - why sometimes creating a directory fails and why we should catch the exception here instead of letting it propagate and be caught in the upper layer.

Copy link
Member Author

@kssenii kssenii Apr 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, will add a comment, the initial reason for this PR was that create_directories can fail with "no space left on device". And it is less convenient to propagate exception to the upper level, because we want to skip cache in this case, which should be done here by assigning a file segment the state SKIP_CACHE, which was done in this PR.

 Received exception from server (version 23.3.1):
Code: 1001. DB::Exception: Received from . DB::Exception: Received from . DB::Exception: std::__1::__fs::filesystem::filesystem_error: filesystem error: in create_directories: No space left on device ["/mnt/clickhouse-cache/s3disk/hash”]. Stack trace:

0. std::system_error::system_error(std::error_code, String const&) @ 0x1a417012 in /usr/bin/clickhouse
1. ? @ 0xe100a63 in /usr/bin/clickhouse
2. ? @ 0x1a3bc6da in /usr/bin/clickhouse
3. ? @ 0x1a3bfe36 in /usr/bin/clickhouse
4. std::__fs::filesystem::__create_directories(std::__fs::filesystem::path const&, std::error_code*) @ 0x1a3c0669 in /usr/bin/clickhouse
5. DB::FileCache::addCell(DB::FileCacheKey const&, unsigned long, unsigned long, DB::FileSegment::State, DB::CreateFileSegmentSettings const&, std::lock_guard<std::mutex>&) @ 0x13a4e300 in /usr/bin/clickhouse
6. DB::FileCache::getOrSet(DB::FileCacheKey const&, unsigned long, unsigned long, DB::CreateFileSegmentSettings const&) @ 0x13a4f14c in /usr/bin/clickhouse

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-improvement Pull request with some product improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants