thevar1able/ClickHouse
1
0
Fork 0
mirror of https://github.com/ClickHouse/ClickHouse.git synced 2024-12-15 10:52:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix NASTY C++ PITFALL: Function try blocks implicitly rethrow the exception

Browse Source 
https://stackoverflow.com/a/5612508

Function try blocks silently rethrow the exception in ctors/dtors, even
with "catch(...){}". Changed to standard try/catch.

Now also logging exceptions.
This commit is contained in:
Robert Schulze 2023-01-03 20:39:08 +00:00
parent 0dda4921ba
commit 4bdab0d5e8
No known key found for this signature in database
GPG Key ID: 26703B55FB13728A
1 changed files with 56 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

109
src/Interpreters/Cache/QueryResultCache.cpp
View File

@ -177,71 +177,74 @@ QueryResultCache::Writer::Writer(std::mutex & mutex_, Cache & cache_, const Key
} }
QueryResultCache::Writer::~Writer() QueryResultCache::Writer::~Writer()
try
{ {
if (skip_insert) try
return;
if (std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::system_clock::now() - query_start_time) < min_query_runtime)
return;
auto to_single_chunk = [](const Chunks & chunks_) -> Chunk
{ {
if (chunks_.empty()) if (skip_insert)
return {}; return;
Chunk res = chunks_[0].clone(); if (std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::system_clock::now() - query_start_time) < min_query_runtime)
for (size_t i = 1; i != chunks_.size(); ++i) return;
res.append(chunks_[i]);
return res;
};
auto query_result = to_single_chunk(partial_query_results); auto to_single_chunk = [](const Chunks & chunks_) -> Chunk
new_entry_size_in_bytes = query_result.allocatedBytes(); // updated because compression potentially affects the size of the single chunk vs the aggregate size of individual chunks {
if (chunks_.empty())
return {};
std::lock_guard lock(mutex); Chunk res = chunks_[0].clone();
for (size_t i = 1; i != chunks_.size(); ++i)
res.append(chunks_[i]);
return res;
};
if (auto it = cache.find(key); it != cache.end() && !is_stale(it->first)) auto query_result = to_single_chunk(partial_query_results);
return; /// same check as in ctor because a parallel Writer could have inserted the current key in the meantime new_entry_size_in_bytes = query_result.allocatedBytes(); // updated because compression potentially affects the size of the single chunk vs the aggregate size of individual chunks
auto sufficient_space_in_cache = [this]() TSA_REQUIRES(mutex) std::lock_guard lock(mutex);
{
return (cache_size_in_bytes + new_entry_size_in_bytes <= max_cache_size_in_bytes) && (cache.size() + 1 <= max_cache_entries);
};
if (!sufficient_space_in_cache()) if (auto it = cache.find(key); it != cache.end() && !is_stale(it->first))
{ return; /// same check as in ctor because a parallel Writer could have inserted the current key in the meantime
size_t removed_items = 0;
/// Remove stale entries auto sufficient_space_in_cache = [this]() TSA_REQUIRES(mutex)
for (auto it = cache.begin(); it != cache.end();) {
if (is_stale(it->first)) return (cache_size_in_bytes + new_entry_size_in_bytes <= max_cache_size_in_bytes) && (cache.size() + 1 <= max_cache_entries);
{ };
cache_size_in_bytes -= it->second.allocatedBytes();
it = cache.erase(it); if (!sufficient_space_in_cache())
++removed_items; {
} size_t removed_items = 0;
else /// Remove stale entries
++it; for (auto it = cache.begin(); it != cache.end();)
LOG_DEBUG(&Poco::Logger::get("QueryResultCache"), "Removed {} stale entries", removed_items); if (is_stale(it->first))
{
cache_size_in_bytes -= it->second.allocatedBytes();
it = cache.erase(it);
++removed_items;
}
else
++it;
LOG_DEBUG(&Poco::Logger::get("QueryResultCache"), "Removed {} stale entries", removed_items);
}
/// Insert or replace if enough space
if (sufficient_space_in_cache())
{
cache_size_in_bytes += query_result.allocatedBytes();
if (auto it = cache.find(key); it != cache.end())
cache_size_in_bytes -= it->second.allocatedBytes(); /// key replacement
/// cache[key] = query_result; /// does no replacement for unclear reasons
cache.erase(key);
cache[key] = std::move(query_result);
LOG_DEBUG(&Poco::Logger::get("QueryResultCache"), "Stored result of query {}", key.queryStringFromAst());
}
} }
catch (...)
/// Insert or replace if enough space
if (sufficient_space_in_cache())
{ {
cache_size_in_bytes += query_result.allocatedBytes(); tryLogCurrentException(&Poco::Logger::get("QueryResultCache"), __PRETTY_FUNCTION__);
if (auto it = cache.find(key); it != cache.end())
cache_size_in_bytes -= it->second.allocatedBytes(); /// key replacement
/// cache[key] = query_result; /// does no replacement for unclear reasons
cache.erase(key);
cache[key] = std::move(query_result);
LOG_DEBUG(&Poco::Logger::get("QueryResultCache"), "Stored result of query {}", key.queryStringFromAst());
} }
} }
catch (const std::exception &)
{
}
void QueryResultCache::Writer::buffer(Chunk && partial_query_result) void QueryResultCache::Writer::buffer(Chunk && partial_query_result)
{ {