Skip to content

Commit e424954

Browse files
azatazat
authored
Merge pull request ClickHouse#90070 from ClickHouse/backport/25.8/89640
Backport ClickHouse#89640 to 25.8: Remove duplicated filesystem caches from asynchronous metrics, SYSTEM queries
2 parents e6e71a4 + 40c0977 commit e424954

File tree

8 files changed

+60
-39
lines changed

8 files changed

+60
-39
lines changed

‎src/Interpreters/Cache/FileCache.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ FileCache::FileCache(const std::string & cache_name, const FileCacheSettings & s
125125
, keep_current_size_to_max_ratio(1 - settings[FileCacheSetting::keep_free_space_size_ratio])
126126
, keep_current_elements_to_max_ratio(1 - settings[FileCacheSetting::keep_free_space_elements_ratio])
127127
, keep_up_free_space_remove_batch(settings[FileCacheSetting::keep_free_space_remove_batch])
128+
, name(cache_name)
128129
, log(getLogger("FileCache(" + cache_name + ")"))
129130
, metadata(settings[FileCacheSetting::path],
130131
settings[FileCacheSetting::background_download_queue_size_limit],

‎src/Interpreters/Cache/FileCache.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,8 @@ class FileCache : private boost::noncopyable
209209

210210
void freeSpaceRatioKeepingThreadFunc();
211211

212+
const String & getName() const { return name; }
213+
212214
private:
213215
using KeyAndOffset = FileCacheKeyAndOffset;
214216

@@ -228,6 +230,7 @@ class FileCache : private boost::noncopyable
228230
const double keep_current_elements_to_max_ratio;
229231
const size_t keep_up_free_space_remove_batch;
230232

233+
String name;
231234
LoggerPtr log;
232235

233236
std::exception_ptr init_exception;

‎src/Interpreters/Cache/FileCacheFactory.cpp‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,15 @@ FileCacheFactory::CacheByName FileCacheFactory::getAll()
5252
return caches_by_name;
5353
}
5454

55+
FileCacheFactory::Caches FileCacheFactory::getUniqueInstances()
56+
{
57+
std::lock_guard lock(mutex);
58+
Caches caches;
59+
for (const auto & [_, cache_data] : caches_by_name)
60+
caches.insert(cache_data);
61+
return caches;
62+
}
63+
5564
FileCachePtr FileCacheFactory::get(const std::string & cache_name)
5665
{
5766
std::lock_guard lock(mutex);

‎src/Interpreters/Cache/FileCacheFactory.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
#include <boost/noncopyable.hpp>
77
#include <unordered_map>
8+
#include <unordered_set>
89
#include <mutex>
910

1011
namespace DB
@@ -36,6 +37,7 @@ class FileCacheFactory final : private boost::noncopyable
3637

3738
using FileCacheDataPtr = std::shared_ptr<FileCacheData>;
3839
using CacheByName = std::unordered_map<std::string, FileCacheDataPtr>;
40+
using Caches = std::unordered_set<FileCacheDataPtr>;
3941

4042
static FileCacheFactory & instance();
4143

@@ -52,6 +54,7 @@ class FileCacheFactory final : private boost::noncopyable
5254
const std::string & config_path);
5355

5456
CacheByName getAll();
57+
Caches getUniqueInstances();
5558

5659
FileCacheDataPtr getByName(const std::string & cache_name);
5760

‎src/Interpreters/Context.cpp‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -882,9 +882,8 @@ struct ContextSharedPart : boost::noncopyable
882882
/// Background operations in cache use background schedule pool.
883883
/// Deactivate them before destructing it.
884884
LOG_TRACE(log, "Shutting down caches");
885-
const auto & caches = FileCacheFactory::instance().getAll();
886-
for (const auto & [_, cache] : caches)
887-
cache->cache->deactivateBackgroundOperations();
885+
for (const auto & cache_data : FileCacheFactory::instance().getUniqueInstances())
886+
cache_data->cache->deactivateBackgroundOperations();
888887
FileCacheFactory::instance().clear();
889888

890889
{

‎src/Interpreters/InterpreterSystemQuery.cpp‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -461,8 +461,7 @@ BlockIO InterpreterSystemQuery::execute()
461461

462462
if (query.filesystem_cache_name.empty())
463463
{
464-
auto caches = FileCacheFactory::instance().getAll();
465-
for (const auto & [_, cache_data] : caches)
464+
for (const auto & cache_data : FileCacheFactory::instance().getUniqueInstances())
466465
{
467466
if (!cache_data->cache->isInitialized())
468467
continue;
@@ -523,11 +522,10 @@ BlockIO InterpreterSystemQuery::execute()
523522

524523
if (query.filesystem_cache_name.empty())
525524
{
526-
auto caches = FileCacheFactory::instance().getAll();
527-
for (const auto & [cache_name, cache_data] : caches)
525+
for (const auto & cache_data : FileCacheFactory::instance().getUniqueInstances())
528526
{
529527
auto file_segments = cache_data->cache->sync();
530-
fill_data(cache_name, cache_data->cache, file_segments);
528+
fill_data(cache_data->cache->getName(), cache_data->cache, file_segments);
531529
}
532530
}
533531
else

‎src/Interpreters/ServerAsynchronousMetrics.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,12 +73,11 @@ ServerAsynchronousMetrics::~ServerAsynchronousMetrics()
7373
void ServerAsynchronousMetrics::updateImpl(TimePoint update_time, TimePoint current_time, bool force_update, bool first_run, AsynchronousMetricValues & new_values)
7474
{
7575
{
76-
auto caches = FileCacheFactory::instance().getAll();
7776
size_t total_bytes = 0;
7877
size_t max_bytes = 0;
7978
size_t total_files = 0;
8079

81-
for (const auto & [_, cache_data] : caches)
80+
for (const auto & cache_data : FileCacheFactory::instance().getUniqueInstances())
8281
{
8382
total_bytes += cache_data->cache->getUsedCacheSize();
8483
max_bytes += cache_data->cache->getMaxCacheSize();

‎src/Storages/System/StorageSystemFilesystemCache.cpp‎

Lines changed: 38 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -45,46 +45,55 @@ StorageSystemFilesystemCache::StorageSystemFilesystemCache(const StorageID & tab
4545

4646
void StorageSystemFilesystemCache::fillData(MutableColumns & res_columns, ContextPtr, const ActionsDAG::Node *, std::vector<UInt8>) const
4747
{
48-
auto caches = FileCacheFactory::instance().getAll();
48+
auto caches_by_name = FileCacheFactory::instance().getAll();
49+
std::unordered_set<FileCacheFactory::FileCacheDataPtr> caches;
50+
for (const auto & [_, cache_data] : caches_by_name)
51+
caches.insert(cache_data);
52+
std::unordered_map<FileCacheFactory::FileCacheDataPtr, std::vector<std::string>> caches_by_instance;
53+
for (const auto & [cache_name, cache_data] : caches_by_name)
54+
caches_by_instance[cache_data].push_back(cache_name);
4955

50-
for (const auto & [cache_name, cache_data] : caches)
56+
for (const auto & cache_data : caches)
5157
{
5258
const auto & cache = cache_data->cache;
5359
if (!cache->isInitialized())
5460
continue;
5561

5662
cache->iterate([&](const FileSegment::Info & file_segment)
5763
{
58-
size_t i = 0;
59-
res_columns[i++]->insert(cache_name);
60-
res_columns[i++]->insert(cache->getBasePath());
64+
for (const auto & cache_name : caches_by_instance.at(cache_data))
65+
{
66+
size_t i = 0;
67+
res_columns[i++]->insert(cache_name);
68+
res_columns[i++]->insert(cache->getBasePath());
6169

62-
/// Do not use `file_segment->getPath` here because it will lead to nullptr dereference
63-
/// (because file_segments in getSnapshot doesn't have `cache` field set)
70+
/// Do not use `file_segment->getPath` here because it will lead to nullptr dereference
71+
/// (because file_segments in getSnapshot doesn't have `cache` field set)
6472

65-
const auto path = cache->getFileSegmentPath(
66-
file_segment.key, file_segment.offset, file_segment.kind,
67-
FileCache::UserInfo(file_segment.user_id, file_segment.user_weight));
68-
res_columns[i++]->insert(path);
69-
res_columns[i++]->insert(file_segment.key.toString());
70-
res_columns[i++]->insert(file_segment.range_left);
71-
res_columns[i++]->insert(file_segment.range_right);
72-
res_columns[i++]->insert(file_segment.size);
73-
res_columns[i++]->insert(FileSegment::stateToString(file_segment.state));
74-
res_columns[i++]->insert(file_segment.download_finished_time);
75-
res_columns[i++]->insert(file_segment.cache_hits);
76-
res_columns[i++]->insert(file_segment.references);
77-
res_columns[i++]->insert(file_segment.downloaded_size);
78-
res_columns[i++]->insert(toString(file_segment.kind));
79-
res_columns[i++]->insert(file_segment.is_unbound);
80-
res_columns[i++]->insert(file_segment.user_id);
73+
const auto path = cache->getFileSegmentPath(
74+
file_segment.key, file_segment.offset, file_segment.kind,
75+
FileCache::UserInfo(file_segment.user_id, file_segment.user_weight));
76+
res_columns[i++]->insert(path);
77+
res_columns[i++]->insert(file_segment.key.toString());
78+
res_columns[i++]->insert(file_segment.range_left);
79+
res_columns[i++]->insert(file_segment.range_right);
80+
res_columns[i++]->insert(file_segment.size);
81+
res_columns[i++]->insert(FileSegment::stateToString(file_segment.state));
82+
res_columns[i++]->insert(file_segment.download_finished_time);
83+
res_columns[i++]->insert(file_segment.cache_hits);
84+
res_columns[i++]->insert(file_segment.references);
85+
res_columns[i++]->insert(file_segment.downloaded_size);
86+
res_columns[i++]->insert(toString(file_segment.kind));
87+
res_columns[i++]->insert(file_segment.is_unbound);
88+
res_columns[i++]->insert(file_segment.user_id);
8189

82-
std::error_code ec;
83-
auto size = fs::file_size(path, ec);
84-
if (!ec)
85-
res_columns[i++]->insert(size);
86-
else
87-
res_columns[i++]->insertDefault();
90+
std::error_code ec;
91+
auto size = fs::file_size(path, ec);
92+
if (!ec)
93+
res_columns[i++]->insert(size);
94+
else
95+
res_columns[i++]->insertDefault();
96+
}
8897
}, FileCache::getCommonUser().user_id);
8998
}
9099
}

0 commit comments

Comments
 (0)