diff --git a/src/Cache.cpp b/src/Cache.cpp index 401c0cd4e..c53f39924 100644 --- a/src/Cache.cpp +++ b/src/Cache.cpp @@ -103,22 +103,26 @@ void Cache::read(const vector &keys) else // not in the cache, put it in the list to download keysToFetch.push_back(&key); - } - - // TODO: get the sizes of the objects to download and make space - // For now using an estimate - makeSpace(keys.size() * objectSize); - + } s.unlock(); // start downloading the keys to fetch int dl_err; vector dl_errnos; + vector sizes; if (!keysToFetch.empty()) - dl_err = downloader.download(keysToFetch, &dl_errnos); + dl_err = downloader.download(keysToFetch, &dl_errnos, &sizes); + + size_t sum_sizes = 0; + for (size_t &size : sizes) + sum_sizes += size; s.lock(); - + // do makespace() before downloading. Problem is, until the download is finished, this fcn can't tell which + // downloads it was responsible for. Need Downloader to make the call...? + makeSpace(sum_sizes); + currentCacheSize += sum_sizes; + // move all keys to the back of the LRU for (i = 0; i < keys.size(); i++) { @@ -131,7 +135,8 @@ void Cache::read(const vector &keys) else if (dl_errnos[i] == 0) // successful download { lru.push_back(keys[i]); - m_lru.insert(M_LRU_element_t(&(lru.back()), lru.end()--)); + LRU_t::iterator it = lru.end(); + m_lru.insert(M_LRU_element_t(--it)); } else { @@ -189,14 +194,31 @@ void Cache::exists(const vector &keys, vector *out) void Cache::newObject(const string &key, size_t size) { + boost::unique_lock s(lru_mutex); + assert(m_lru.find(key) == m_lru.end()); + makeSpace(size); + lru.push_back(key); + LRU_t::iterator back = lru.end(); + m_lru.insert(--back); + currentCacheSize += size; } void Cache::deletedObject(const string &key, size_t size) { + boost::unique_lock s(lru_mutex); + M_LRU_t::iterator mit = m_lru.find(key); + assert(mit != m_lru.end()); + assert(doNotEvict.find(mit->lit) == doNotEvict.end()); + lru.erase(mit->lit); + m_lru.erase(mit); + currentCacheSize -= size; } void Cache::setMaxCacheSize(size_t size) { + if (size < maxCacheSize) + makeSpace(maxCacheSize - size); + maxCacheSize = size; } // call this holding lru_mutex @@ -251,7 +273,7 @@ Cache::M_LRU_element_t::M_LRU_element_t(const string *k) : key(k) Cache::M_LRU_element_t::M_LRU_element_t(const string &k) : key(&k) {} -Cache::M_LRU_element_t::M_LRU_element_t(const string *k, const LRU_t::iterator &i) : key(k), lit(i) +Cache::M_LRU_element_t::M_LRU_element_t(const LRU_t::iterator &i) : key(&(*i)), lit(i) {} inline size_t Cache::KeyHasher::operator()(const M_LRU_element_t &l) const diff --git a/src/Cache.h b/src/Cache.h index 555fa0aa5..12e751b75 100644 --- a/src/Cache.h +++ b/src/Cache.h @@ -52,7 +52,7 @@ class Cache : public boost::noncopyable { M_LRU_element_t(const std::string &); M_LRU_element_t(const std::string *); - M_LRU_element_t(const std::string *, const LRU_t::iterator &); + M_LRU_element_t(const LRU_t::iterator &); const std::string *key; LRU_t::iterator lit; }; diff --git a/src/CloudStorage.h b/src/CloudStorage.h index 43e69af12..3bd9296de 100644 --- a/src/CloudStorage.h +++ b/src/CloudStorage.h @@ -11,7 +11,7 @@ class CloudStorage { public: /* These behave like syscalls. return code -1 means an error, and errno is set */ - virtual int getObject(const std::string &sourceKey, const std::string &destFile) = 0; + virtual int getObject(const std::string &sourceKey, const std::string &destFile, size_t *size = NULL) = 0; virtual int putObject(const std::string &sourceFile, const std::string &destKey) = 0; virtual void deleteObject(const std::string &key) = 0; virtual int copyObject(const std::string &sourceKey, const std::string &destKey) = 0; diff --git a/src/Downloader.cpp b/src/Downloader.cpp index a248aefa6..32092beea 100644 --- a/src/Downloader.cpp +++ b/src/Downloader.cpp @@ -36,9 +36,10 @@ inline boost::mutex & Downloader::getDownloadMutex() return download_mutex; } -int Downloader::download(const vector &keys, vector *errnos) +int Downloader::download(const vector &keys, vector *errnos, vector *sizes) { - volatile uint counter = keys.size(); + //volatile uint counter = keys.size(); + uint counter = keys.size(); boost::condition condvar; boost::mutex m; DownloadListener listener(&counter, &condvar, &m); @@ -71,19 +72,27 @@ int Downloader::download(const vector &keys, vector *errnos (*iterators[i])->listeners.push_back(&listener); } } - s.unlock(); // wait for the downloads to finish boost::unique_lock dl_lock(m); + s.unlock(); while (counter > 0) condvar.wait(dl_lock); dl_lock.unlock(); // remove the entries inserted by this call + sizes->resize(keys.size()); s.lock(); for (i = 0; i < keys.size(); i++) + { if (inserted[i]) + { + (*sizes)[i] = (*iterators[i])->size; downloads.erase(iterators[i]); + } + else + (*sizes)[i] = 0; + } s.unlock(); // check for errors & propagate @@ -113,14 +122,14 @@ inline const string & Downloader::getDownloadPath() const } /* The helper fcns */ -Downloader::Download::Download(const string *source, Downloader *dl) : key(source), dler(dl), dl_errno(0) +Downloader::Download::Download(const string *source, Downloader *dl) : key(source), dler(dl), dl_errno(0), size(0) { } void Downloader::Download::operator()() { CloudStorage *storage = CloudStorage::get(); - int err = storage->getObject(*key, dler->getDownloadPath() + "/" + *key); + int err = storage->getObject(*key, dler->getDownloadPath() + "/" + *key, &size); if (err != 0) dl_errno = errno; @@ -129,7 +138,8 @@ void Downloader::Download::operator()() listener->downloadFinished(); } -Downloader::DownloadListener::DownloadListener(volatile uint *counter, boost::condition *condvar, boost::mutex *m) : count(counter), cond(condvar), mutex(m) +//Downloader::DownloadListener::DownloadListener(volatile uint *counter, boost::condition *condvar, boost::mutex *m) : count(counter), cond(condvar), mutex(m) +Downloader::DownloadListener::DownloadListener(uint *counter, boost::condition *condvar, boost::mutex *m) : count(counter), cond(condvar), mutex(m) { } diff --git a/src/Downloader.h b/src/Downloader.h index bd300d13f..411ecab8f 100644 --- a/src/Downloader.h +++ b/src/Downloader.h @@ -24,7 +24,7 @@ class Downloader // returns 0 on success. If != 0, errnos will contains the errno associated with the failure // caller owns the memory for the strings. - int download(const std::vector &keys, std::vector *errnos); + int download(const std::vector &keys, std::vector *errnos, std::vector *sizes); void setDownloadPath(const std::string &path); const std::string & getDownloadPath() const; @@ -35,10 +35,12 @@ class Downloader class DownloadListener { public: - DownloadListener(volatile uint *counter, boost::condition *condvar, boost::mutex *m); + DownloadListener(uint *counter, boost::condition *condvar, boost::mutex *m); + //DownloadListener(volatile uint *counter, boost::condition *condvar, boost::mutex *m); void downloadFinished(); private: - volatile uint *count; + uint *count; + //volatile uint *count; boost::condition *cond; boost::mutex *mutex; }; @@ -50,6 +52,7 @@ class Downloader Downloader *dler; const std::string *key; int dl_errno; // to propagate errors from the download job to the caller + size_t size; std::vector listeners; }; diff --git a/src/LocalStorage.cpp b/src/LocalStorage.cpp index 7f9e4246f..1b8377cd6 100644 --- a/src/LocalStorage.cpp +++ b/src/LocalStorage.cpp @@ -43,7 +43,7 @@ int LocalStorage::copy(const path &source, const path &dest) { boost::system::error_code err; copy_file(source, dest, copy_option::fail_if_exists, err); - if (!err) + if (err) { errno = err.value(); return -1; @@ -58,9 +58,14 @@ path operator+(const path &p1, const path &p2) return ret; } -int LocalStorage::getObject(const string &source, const string &dest) +int LocalStorage::getObject(const string &source, const string &dest, size_t *size) { - return copy(prefix + source, dest); + int ret = copy(prefix + source, dest); + if (ret) + return ret; + if (size) + *size = boost::filesystem::file_size(dest); + return ret; } int LocalStorage::putObject(const string &source, const string &dest) diff --git a/src/LocalStorage.h b/src/LocalStorage.h index 4fc9bb6d1..b129a06df 100644 --- a/src/LocalStorage.h +++ b/src/LocalStorage.h @@ -14,7 +14,7 @@ class LocalStorage : public CloudStorage LocalStorage(); virtual ~LocalStorage(); - int getObject(const std::string &sourceKey, const std::string &destFile); + int getObject(const std::string &sourceKey, const std::string &destFile, size_t *size = NULL); int putObject(const std::string &sourceFile, const std::string &destKey); void deleteObject(const std::string &key); int copyObject(const std::string &sourceKey, const std::string &destKey); diff --git a/src/S3Storage.cpp b/src/S3Storage.cpp index 1a5b74a2b..86fd2370a 100644 --- a/src/S3Storage.cpp +++ b/src/S3Storage.cpp @@ -14,7 +14,7 @@ S3Storage::~S3Storage() { } -int S3Storage::getObject(const string &sourceKey, const string &destFile) +int S3Storage::getObject(const string &sourceKey, const string &destFile, size_t *size) { return 0; } diff --git a/src/S3Storage.h b/src/S3Storage.h index 04eb2ff59..e91b649cb 100644 --- a/src/S3Storage.h +++ b/src/S3Storage.h @@ -14,7 +14,7 @@ class S3Storage : public CloudStorage S3Storage(); virtual ~S3Storage(); - int getObject(const std::string &sourceKey, const std::string &destFile); + int getObject(const std::string &sourceKey, const std::string &destFile, size_t *size = NULL); int putObject(const std::string &sourceFile, const std::string &destKey); void deleteObject(const std::string &key); int copyObject(const std::string &sourceKey, const std::string &destKey);