1
0
mirror of https://github.com/mariadb-corporation/mariadb-columnstore-engine.git synced 2025-12-13 23:02:14 +03:00

Realized Cache::_makeSpace() could deadlock, made a mutex a recursive_mutex.

This commit is contained in:
Patrick LeBlanc
2019-04-08 10:39:30 -05:00
parent 6f234f45c0
commit 64bbf44227
3 changed files with 16 additions and 15 deletions

View File

@@ -59,12 +59,12 @@ link_directories(${CMAKE_CURRENT_BINARY_DIR}/lib)
add_executable(StorageManager src/main.cpp ${storagemanager_SRCS}) add_executable(StorageManager src/main.cpp ${storagemanager_SRCS})
add_dependencies(StorageManager ms3) add_dependencies(StorageManager ms3)
target_link_libraries(StorageManager boost_system boost_thread boost_filesystem boost_regex ${S3API_DEPS}) target_link_libraries(StorageManager boost_system boost_thread boost_filesystem boost_regex pthread ${S3API_DEPS})
set_property(TARGET StorageManager PROPERTY CXX_STANDARD 11) set_property(TARGET StorageManager PROPERTY CXX_STANDARD 11)
add_executable(unit_tests src/unit_tests.cpp ${storagemanager_SRCS}) add_executable(unit_tests src/unit_tests.cpp ${storagemanager_SRCS})
add_dependencies(unit_tests ms3) add_dependencies(unit_tests ms3)
target_link_libraries(unit_tests boost_system boost_thread boost_filesystem boost_regex ${S3API_DEPS}) target_link_libraries(unit_tests boost_system boost_thread boost_filesystem boost_regex pthread ${S3API_DEPS})
set_property(TARGET unit_tests PROPERTY CXX_STANDARD 11) set_property(TARGET unit_tests PROPERTY CXX_STANDARD 11)
#install(TARGETS StorageManager DESTINATION ${ENGINE_BINDIR} COMPONENT platform) #install(TARGETS StorageManager DESTINATION ${ENGINE_BINDIR} COMPONENT platform)

View File

@@ -167,7 +167,7 @@ void Cache::read(const vector<string> &keys)
fetch keys that do not exist fetch keys that do not exist
after fetching, move all keys from do-not-evict to the back of the LRU after fetching, move all keys from do-not-evict to the back of the LRU
*/ */
boost::unique_lock<boost::mutex> s(lru_mutex); boost::unique_lock<boost::recursive_mutex> s(lru_mutex);
vector<const string *> keysToFetch; vector<const string *> keysToFetch;
uint i; uint i;
@@ -270,20 +270,20 @@ const bf::path & Cache::getJournalPath()
void Cache::exists(const vector<string> &keys, vector<bool> *out) const void Cache::exists(const vector<string> &keys, vector<bool> *out) const
{ {
out->resize(keys.size()); out->resize(keys.size());
boost::unique_lock<boost::mutex> s(lru_mutex); boost::unique_lock<boost::recursive_mutex> s(lru_mutex);
for (uint i = 0; i < keys.size(); i++) for (uint i = 0; i < keys.size(); i++)
(*out)[i] = (m_lru.find(keys[i]) != m_lru.end()); (*out)[i] = (m_lru.find(keys[i]) != m_lru.end());
} }
bool Cache::exists(const string &key) const bool Cache::exists(const string &key) const
{ {
boost::unique_lock<boost::mutex> s(lru_mutex); boost::unique_lock<boost::recursive_mutex> s(lru_mutex);
return m_lru.find(key) != m_lru.end(); return m_lru.find(key) != m_lru.end();
} }
void Cache::newObject(const string &key, size_t size) void Cache::newObject(const string &key, size_t size)
{ {
boost::unique_lock<boost::mutex> s(lru_mutex); boost::unique_lock<boost::recursive_mutex> s(lru_mutex);
assert(m_lru.find(key) == m_lru.end()); assert(m_lru.find(key) == m_lru.end());
_makeSpace(size); _makeSpace(size);
lru.push_back(key); lru.push_back(key);
@@ -294,21 +294,21 @@ void Cache::newObject(const string &key, size_t size)
void Cache::newJournalEntry(size_t size) void Cache::newJournalEntry(size_t size)
{ {
boost::unique_lock<boost::mutex> s(lru_mutex); boost::unique_lock<boost::recursive_mutex> s(lru_mutex);
_makeSpace(size); _makeSpace(size);
currentCacheSize += size; currentCacheSize += size;
} }
void Cache::deletedJournal(size_t size) void Cache::deletedJournal(size_t size)
{ {
boost::unique_lock<boost::mutex> s(lru_mutex); boost::unique_lock<boost::recursive_mutex> s(lru_mutex);
assert(currentCacheSize >= size); assert(currentCacheSize >= size);
currentCacheSize -= size; currentCacheSize -= size;
} }
void Cache::deletedObject(const string &key, size_t size) void Cache::deletedObject(const string &key, size_t size)
{ {
boost::unique_lock<boost::mutex> s(lru_mutex); boost::unique_lock<boost::recursive_mutex> s(lru_mutex);
assert(currentCacheSize >= size); assert(currentCacheSize >= size);
M_LRU_t::iterator mit = m_lru.find(key); M_LRU_t::iterator mit = m_lru.find(key);
assert(mit != m_lru.end()); assert(mit != m_lru.end());
@@ -320,7 +320,7 @@ void Cache::deletedObject(const string &key, size_t size)
void Cache::setMaxCacheSize(size_t size) void Cache::setMaxCacheSize(size_t size)
{ {
boost::unique_lock<boost::mutex> s(lru_mutex); boost::unique_lock<boost::recursive_mutex> s(lru_mutex);
if (size < maxCacheSize) if (size < maxCacheSize)
_makeSpace(maxCacheSize - size); _makeSpace(maxCacheSize - size);
maxCacheSize = size; maxCacheSize = size;
@@ -328,7 +328,7 @@ void Cache::setMaxCacheSize(size_t size)
void Cache::makeSpace(size_t size) void Cache::makeSpace(size_t size)
{ {
boost::unique_lock<boost::mutex> s(lru_mutex); boost::unique_lock<boost::recursive_mutex> s(lru_mutex);
_makeSpace(size); _makeSpace(size);
} }
@@ -358,7 +358,8 @@ void Cache::_makeSpace(size_t size)
int err = stat(cachedFile.string().c_str(), &statbuf); int err = stat(cachedFile.string().c_str(), &statbuf);
if (err) if (err)
{ {
logger->log(LOG_WARNING, "Cache::makeSpace(): There seems to be a cached file that couldn't be stat'ed: %s", cachedFile.string().c_str()); logger->log(LOG_WARNING, "Cache::makeSpace(): There seems to be a cached file that couldn't be stat'ed: %s",
cachedFile.string().c_str());
++it; ++it;
continue; continue;
} }
@@ -382,7 +383,7 @@ void Cache::_makeSpace(size_t size)
void Cache::rename(const string &oldKey, const string &newKey, ssize_t sizediff) void Cache::rename(const string &oldKey, const string &newKey, ssize_t sizediff)
{ {
boost::unique_lock<boost::mutex> s(lru_mutex); boost::unique_lock<boost::recursive_mutex> s(lru_mutex);
auto it = m_lru.find(oldKey); auto it = m_lru.find(oldKey);
//assert(it != m_lru.end()); //assert(it != m_lru.end());
if (it == m_lru.end()) if (it == m_lru.end())
@@ -405,7 +406,7 @@ size_t Cache::getCurrentCacheSize() const
void Cache::reset() void Cache::reset()
{ {
boost::unique_lock<boost::mutex> s(lru_mutex); boost::unique_lock<boost::recursive_mutex> s(lru_mutex);
m_lru.clear(); m_lru.clear();
lru.clear(); lru.clear();

View File

@@ -110,7 +110,7 @@ class Cache : public boost::noncopyable
DNE_t doNotEvict; DNE_t doNotEvict;
void addToDNE(const LRU_t::iterator &key); void addToDNE(const LRU_t::iterator &key);
void removeFromDNE(const LRU_t::iterator &key); void removeFromDNE(const LRU_t::iterator &key);
mutable boost::mutex lru_mutex; // protects the main cache structures & the do-not-evict set mutable boost::recursive_mutex lru_mutex; // protects the main cache structures & the do-not-evict set
}; };