diff --git a/src/Cache.cpp b/src/Cache.cpp index d3e27e786..3fa2049fc 100644 --- a/src/Cache.cpp +++ b/src/Cache.cpp @@ -52,6 +52,7 @@ Cache::Cache() cout << "Cache got prefix " << prefix << endl; downloader.setDownloadPath(prefix.string()); + /* todo: populate structures with existing files in the cache path */ } Cache::~Cache() @@ -84,23 +85,36 @@ void Cache::read(const vector &keys) s.unlock(); // start downloading the keys to fetch + int dl_err; + vector dl_errnos; if (!keysToFetch.empty()) - downloader.download(keysToFetch); + dl_err = downloader.download(keysToFetch, &dl_errnos); s.lock(); // move all keys to the back of the LRU - for (const string &key : keys) + for (i = 0; i < keys.size(); i++) { - mit = m_lru.find(key); + mit = m_lru.find(keys[i]); if (mit != m_lru.end()) - lru.splice(lru.end(), lru, mit->lit); - else { - lru.push_back(key); + lru.splice(lru.end(), lru, mit->lit); + removeFromDNE(lru.end()); + } + else if (dl_errnos[i] == 0) // successful download + { + lru.push_back(keys[i]); m_lru.insert(M_LRU_element_t(&(lru.back()), lru.end()--)); } - removeFromDNE(lru.end()); + else + { + // Downloader already logged it, anything to do here? + /* brainstorming options for handling it. + 1) Should it be handled? The caller will log a file-not-found error, and there will be a download + failure in the log already. + 2) Can't really DO anything can it? + */ + } } } @@ -132,7 +146,10 @@ void Cache::removeFromDNE(const LRU_t::iterator &key) doNotEvict.erase(it); } - +const boost::filesystem::path & Cache::getCachePath() +{ + return prefix; +} void Cache::exists(const vector &keys, vector *out) diff --git a/src/Cache.h b/src/Cache.h index 11df01d71..bc608c730 100644 --- a/src/Cache.h +++ b/src/Cache.h @@ -28,11 +28,14 @@ class Cache : public boost::noncopyable void setCacheSize(size_t size); void makeSpace(size_t size); + // test helpers + const boost::filesystem::path &getCachePath(); private: boost::filesystem::path prefix; size_t maxCacheSize; /* The main cache structures */ + // lru owns the string memory for the filenames it manages. m_lru and DNE point to those strings. typedef std::list LRU_t; LRU_t lru; diff --git a/src/Config.cpp b/src/Config.cpp index 45e7df3a3..bd49db887 100644 --- a/src/Config.cpp +++ b/src/Config.cpp @@ -83,6 +83,7 @@ void Config::reloadThreadFcn() try { reload(); + // TODO: add a listener interface to inform upstream of config changes boost::this_thread::sleep(reloadInterval); } catch (boost::property_tree::ini_parser_error &e) @@ -133,8 +134,14 @@ string expand_numbers(const boost::smatch &match) string Config::getValue(const string §ion, const string &key) const { // if we care, move this envvar substition stuff to where the file is loaded + string ret; boost::unique_lock s(mutex); - string ret = contents.get(section + "." + key); + try { + ret = contents.get(section + "." + key); + } + catch (...) { + return ""; // debating whether it's necessary to tell the caller there was no entry. + } s.unlock(); boost::regex re("\\$\\{(.+)\\}"); diff --git a/src/Downloader.cpp b/src/Downloader.cpp index 65819e193..908f90557 100644 --- a/src/Downloader.cpp +++ b/src/Downloader.cpp @@ -7,20 +7,28 @@ using namespace std; namespace storagemanager { -Downloader::Downloader() +Downloader::Downloader() : maxDownloads(0) { storage = CloudStorage::get(); string sMaxDownloads = Config::get()->getValue("ObjectStorage", "max_concurrent_downloads"); - maxDownloads = stoi(sMaxDownloads); + try + { + maxDownloads = stoul(sMaxDownloads); + } + catch(invalid_argument) + { + // log something + } if (maxDownloads == 0) maxDownloads = 20; + workers.reset(new ThreadPool(maxDownloads)); } Downloader::~Downloader() { } -void Downloader::download(const vector &keys) +int Downloader::download(const vector &keys, vector *errnos) { uint counter = keys.size(); boost::condition condvar; @@ -33,8 +41,8 @@ void Downloader::download(const vector &keys) uint i; - for (const string *key : keys) - dls[i].reset(new Download(key, this)); + for (i = 0; i < keys.size(); i++) + dls[i].reset(new Download(keys[i], this)); boost::unique_lock s(download_mutex); for (i = 0; i < keys.size(); i++) @@ -47,10 +55,13 @@ void Downloader::download(const vector &keys) if (inserted[i]) { dl->listeners.push_back(&listener); - downloaders->addJob(dl); + workers->addJob(dl); } else + { + dl = *(ret.first); // point to the existing download. Redundant with the iterators array. Don't care yet. (*iterators[i])->listeners.push_back(&listener); + } } s.unlock(); @@ -66,7 +77,16 @@ void Downloader::download(const vector &keys) if (inserted[i]) downloads.erase(iterators[i]); - // TODO: check for errors & propagate + // check for errors & propagate + int ret = 0; + errnos->resize(keys.size()); + for (i = 0; i < keys.size(); i++) + { + auto &dl = dls[i]; + (*errnos)[i] = dl->dl_errno; + if (dl->dl_errno != 0) + ret = -1; + } } void Downloader::setDownloadPath(const string &path) @@ -87,7 +107,7 @@ Downloader::Download::Download(const string *source, Downloader *dl) : key(sourc void Downloader::Download::operator()() { CloudStorage *storage = CloudStorage::get(); - int err = storage->getObject(*key, dler->getDownloadPath() + *key); + int err = storage->getObject(*key, dler->getDownloadPath() + "/" + *key); if (err != 0) dl_errno = errno; diff --git a/src/Downloader.h b/src/Downloader.h index d52b3073d..6a0fbb875 100644 --- a/src/Downloader.h +++ b/src/Downloader.h @@ -21,7 +21,9 @@ class Downloader Downloader(); virtual ~Downloader(); - void download(const std::vector &keys); + // 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); void setDownloadPath(const std::string &path); const std::string & getDownloadPath() const; @@ -64,7 +66,7 @@ class Downloader Downloads_t downloads; boost::mutex download_mutex; - boost::scoped_ptr downloaders; + boost::scoped_ptr workers; CloudStorage *storage; }; diff --git a/src/LocalStorage.cpp b/src/LocalStorage.cpp index f410fa01b..6e87fc1ea 100644 --- a/src/LocalStorage.cpp +++ b/src/LocalStorage.cpp @@ -34,6 +34,11 @@ LocalStorage::~LocalStorage() { } +const boost::filesystem::path & LocalStorage::getPrefix() const +{ + return prefix; +} + int LocalStorage::copy(const path &source, const path &dest) { boost::system::error_code err; @@ -49,7 +54,7 @@ int LocalStorage::copy(const path &source, const path &dest) path operator+(const path &p1, const path &p2) { path ret(p1); - ret+=p2; + ret /= p2; return ret; } diff --git a/src/LocalStorage.h b/src/LocalStorage.h index 21ea5fc08..4fc9bb6d1 100644 --- a/src/LocalStorage.h +++ b/src/LocalStorage.h @@ -18,6 +18,8 @@ class LocalStorage : public CloudStorage 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); + + const boost::filesystem::path & getPrefix() const; private: boost::filesystem::path prefix; diff --git a/src/unit_tests.cpp b/src/unit_tests.cpp index f9e7bb8d1..53e32e0b8 100644 --- a/src/unit_tests.cpp +++ b/src/unit_tests.cpp @@ -459,6 +459,47 @@ bool copytask() return true; } +bool localstorageTest1() +{ + LocalStorage ls; + + /* TODO: Some stuff */ + cout << "local storage test 1 OK" << endl; + return true; +} + +bool cacheTest1() +{ + namespace bf = boost::filesystem; + Cache cache; + CloudStorage *cs = CloudStorage::get(); + LocalStorage *ls = dynamic_cast(cs); + if (ls == NULL) { + cout << "Cache test 1 requires using local storage" << endl; + return false; + } + + bf::path storagePath = ls->getPrefix(); + bf::path cachePath = cache.getCachePath(); + vector v_bogus; + + // make sure nothing shows up in the cache path for files that don't exist + v_bogus.push_back("does-not-exist"); + cache.read(v_bogus); + assert(!bf::exists(cachePath / "does-not-exist")); + + // make sure a file that does exist does show up in the cache path + bf::copy_file("storagemanager.cnf", storagePath / "storagemanager.cnf", bf::copy_option::overwrite_if_exists); + v_bogus[0] = "storagemanager.cnf"; + cache.read(v_bogus); + assert(bf::exists(cachePath / "storagemanager.cnf")); + + // cleanup + bf::remove(cachePath / "storagemanager.cnf"); + cout << "cache test 1 OK" << endl; +} + + int main() { cout << "connecting" << endl; @@ -475,12 +516,8 @@ int main() pingtask(); copytask(); - Config *conf = Config::get(); - LocalStorage ls; - Cache cache; - - - + localstorageTest1(); + cacheTest1(); return 0; }