1
0
mirror of https://github.com/mariadb-corporation/mariadb-columnstore-engine.git synced 2025-12-10 22:42:30 +03:00

Fixed another race. Also shortened the interval Sync creates new jobs.

In this one, if a read, then a delete happened on an object being flushed
by makeSpace(), it would leave the toBeDeleted struct with an invalid
list iterator.
This commit is contained in:
Patrick LeBlanc
2019-07-03 17:18:14 -05:00
parent 56a51605af
commit aa65090a61
2 changed files with 7 additions and 17 deletions

View File

@@ -276,13 +276,6 @@ void Cache::read(const vector<string> &keys)
{ {
addToDNE(mit->lit); addToDNE(mit->lit);
lru.splice(lru.end(), lru, mit->lit); // move them to the back so they are last to pick for eviction lru.splice(lru.end(), lru, mit->lit); // move them to the back so they are last to pick for eviction
// if it's about to be deleted, stop that
TBD_t::iterator tbd_it = toBeDeleted.find(mit->lit);
if (tbd_it != toBeDeleted.end())
{
//cout << "Saved one from being deleted" << endl;
toBeDeleted.erase(tbd_it);
}
} }
else else
{ {
@@ -448,7 +441,7 @@ void Cache::deletedObject(const string &key, size_t size)
boost::unique_lock<boost::mutex> s(lru_mutex); boost::unique_lock<boost::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()); // TODO: 5/16/19 - got this assertion using S3 by running test000, then test000 again. assert(mit != m_lru.end());
// if it's being flushed, let makeSpace() do the deleting // if it's being flushed, let makeSpace() do the deleting
if (toBeDeleted.find(mit->lit) == toBeDeleted.end()) if (toBeDeleted.find(mit->lit) == toBeDeleted.end())
@@ -507,7 +500,7 @@ void Cache::_makeSpace(size_t size)
} }
if (!bf::exists(prefix / *it)) if (!bf::exists(prefix / *it))
cout << prefix / *it << " doesn't exist, WTF?" << endl; cout << prefix / *it << " doesn't exist, WTF?" << endl; // Have gotten this a couple times
assert(bf::exists(prefix / *it)); assert(bf::exists(prefix / *it));
/* /*
tell Synchronizer that this key will be evicted tell Synchronizer that this key will be evicted
@@ -518,7 +511,7 @@ void Cache::_makeSpace(size_t size)
//logger->log(LOG_WARNING, "Cache: flushing!"); //logger->log(LOG_WARNING, "Cache: flushing!");
toBeDeleted.insert(it); toBeDeleted.insert(it);
string key = *it; // need to make a copy; it could be in the process of being renamed string key = *it; // need to make a copy; it could get changed after unlocking.
lru_mutex.unlock(); lru_mutex.unlock();
try try
@@ -534,15 +527,12 @@ void Cache::_makeSpace(size_t size)
} }
lru_mutex.lock(); lru_mutex.lock();
TBD_t::iterator tbd_it = toBeDeleted.find(it); // check doNotEvict again in case this object is now being read
if (tbd_it != toBeDeleted.end()) if (doNotEvict.find(it) == doNotEvict.end())
{ {
// if it's still in toBeDeleted then it is safe to delete.
// if read() happened to access it while it was flushing, it will not
// be in that set.
bf::path cachedFile = prefix / *it; bf::path cachedFile = prefix / *it;
m_lru.erase(*it); m_lru.erase(*it);
toBeDeleted.erase(tbd_it); toBeDeleted.erase(it);
lru.erase(it); lru.erase(it);
size_t newSize = bf::file_size(cachedFile); size_t newSize = bf::file_size(cachedFile);
replicator->remove(cachedFile, Replicator::LOCAL_ONLY); replicator->remove(cachedFile, Replicator::LOCAL_ONLY);

View File

@@ -85,7 +85,7 @@ class Synchronizer : public boost::noncopyable
// this thread will start jobs for entries in pendingOps every 10 seconds // this thread will start jobs for entries in pendingOps every 10 seconds
bool die; bool die;
boost::thread syncThread; boost::thread syncThread;
const boost::chrono::seconds syncInterval = boost::chrono::seconds(10); const boost::chrono::seconds syncInterval = boost::chrono::seconds(1);
void periodicSync(); void periodicSync();
SMLogging *logger; SMLogging *logger;