-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Reset downloader for cache file segment in TemporaryFileStream #48386
Changes from all commits
0714777
999a388
50c8472
61c8a95
5966281
6f3fa33
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -562,7 +562,7 @@ TEST_F(FileCacheTest, writeBuffer) | |
DB::FileCache cache(cache_base_path, settings); | ||
cache.initialize(); | ||
|
||
auto write_to_cache = [&cache](const String & key, const Strings & data) | ||
auto write_to_cache = [&cache](const String & key, const Strings & data, bool flush) | ||
{ | ||
CreateFileSegmentSettings segment_settings; | ||
segment_settings.kind = FileSegmentKind::Temporary; | ||
|
@@ -572,22 +572,39 @@ TEST_F(FileCacheTest, writeBuffer) | |
EXPECT_EQ(holder.file_segments.size(), 1); | ||
auto & segment = holder.file_segments.front(); | ||
WriteBufferToFileSegment out(segment.get()); | ||
std::list<std::thread> threads; | ||
std::mutex mu; | ||
for (const auto & s : data) | ||
out.write(s.data(), s.size()); | ||
{ | ||
/// Write from diffetent threads to check | ||
/// that no assertions inside cache related to downloaderId are triggered | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. btw may be make a stateless test too? seems good to have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This case is not to easy to reproduce in stateless. But another task is to enable feature in all s3 stateless tests #48425 (and maybe add some specific ones to that run) |
||
threads.emplace_back([&] | ||
{ | ||
std::unique_lock lock(mu); | ||
out.write(s.data(), s.size()); | ||
/// test different buffering scenarios | ||
if (flush) | ||
{ | ||
out.next(); | ||
} | ||
}); | ||
} | ||
for (auto & t : threads) | ||
t.join(); | ||
return holder; | ||
}; | ||
|
||
std::vector<fs::path> file_segment_paths; | ||
{ | ||
auto holder = write_to_cache("key1", {"abc", "defg"}); | ||
auto holder = write_to_cache("key1", {"abc", "defg"}, false); | ||
file_segment_paths.emplace_back(holder.file_segments.front()->getPathInLocalCache()); | ||
|
||
ASSERT_EQ(fs::file_size(file_segment_paths.back()), 7); | ||
ASSERT_TRUE(holder.file_segments.front()->range() == FileSegment::Range(0, 7)); | ||
ASSERT_EQ(cache.getUsedCacheSize(), 7); | ||
|
||
{ | ||
auto holder2 = write_to_cache("key2", {"1", "22", "333", "4444", "55555"}); | ||
auto holder2 = write_to_cache("key2", {"1", "22", "333", "4444", "55555"}, true); | ||
file_segment_paths.emplace_back(holder2.file_segments.front()->getPathInLocalCache()); | ||
|
||
ASSERT_EQ(fs::file_size(file_segment_paths.back()), 15); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's also add it here https://github.com/ClickHouse/ClickHouse/blob/master/src/Storages/System/StorageSystemFilesystemCache.cpp#L64