diff options
Diffstat (limited to 'filedistribution')
16 files changed, 40 insertions, 148 deletions
diff --git a/filedistribution/src/apps/filedistributor/filedistributor.cpp b/filedistribution/src/apps/filedistributor/filedistributor.cpp index 0d007df0f6e..54860465828 100644 --- a/filedistribution/src/apps/filedistributor/filedistributor.cpp +++ b/filedistribution/src/apps/filedistributor/filedistributor.cpp @@ -69,17 +69,15 @@ class FileDistributor : public config::IFetcherCallback<ZookeepersConfig>, Components(const Components &) = delete; Components & operator = (const Components &) = delete; - Components(const std::shared_ptr<ExceptionRethrower>& exceptionRethrower, - const config::ConfigUri & configUri, + Components(const config::ConfigUri & configUri, const ZookeepersConfig& zooKeepersConfig, const FiledistributorConfig& fileDistributorConfig, const FiledistributorrpcConfig& rpcConfig) - :_zk(track(new ZKFacade(zooKeepersConfig.zookeeperserverlist, exceptionRethrower))), + :_zk(track(new ZKFacade(zooKeepersConfig.zookeeperserverlist))), _model(track(new FileDistributionModelImpl( fileDistributorConfig.hostname, fileDistributorConfig.torrentport, - _zk, - exceptionRethrower))), + _zk))), _tracker(track(new FileDistributorTrackerImpl(_model))), _downloader(track(new FileDownloader(_tracker, fileDistributorConfig.hostname, @@ -125,7 +123,6 @@ class FileDistributor : public config::IFetcherCallback<ZookeepersConfig>, std::unique_ptr<FiledistributorConfig> _fileDistributorConfig; std::unique_ptr<FiledistributorrpcConfig> _rpcConfig; - std::shared_ptr<ExceptionRethrower> _exceptionRethrower; std::unique_ptr<Components> _components; public: FileDistributor(const FileDistributor &) = delete; @@ -136,7 +133,6 @@ public: _zooKeepersConfig(), _fileDistributorConfig(), _rpcConfig(), - _exceptionRethrower(), _components() { } @@ -178,33 +174,14 @@ public: void run(const config::ConfigUri & configUri) { while (!askedToShutDown()) { clearReinitializeFlag(); - _exceptionRethrower.reset(new ExceptionRethrower()); runImpl(configUri); - - if (_exceptionRethrower->exceptionStored()) - _exceptionRethrower->rethrow(); - } - } - - static void ensureExceptionsStored(const std::shared_ptr<ExceptionRethrower>& exceptionRethrower) { - //TODO: this is somewhat hackish, refactor to eliminate this later. - LOG(debug, "Waiting for shutdown"); - for (int i=0; i<50 && !exceptionRethrower.unique(); ++i) { - std::this_thread::sleep_for(100ms); - } - LOG(debug, "Done waiting for shutdown"); - - if (!exceptionRethrower.unique()) { - EV_STOPPING(programName, "Forced termination"); - kill(getpid(), SIGKILL); } } - void createComponents(const std::shared_ptr<ExceptionRethrower>& exceptionRethrower, const config::ConfigUri & configUri) { + void createComponents(const config::ConfigUri & configUri) { LockGuard guard(_configMutex); _components.reset( - new Components(exceptionRethrower, - configUri, + new Components(configUri, *_zooKeepersConfig, *_fileDistributorConfig, *_rpcConfig)); @@ -231,13 +208,9 @@ public: #pragma GCC diagnostic ignored "-Wshadow" void runImpl(const config::ConfigUri & configUri) { - BOOST_SCOPE_EXIT((&_components)(&_exceptionRethrower)) { - _components.reset(); - //Ensures that any exception stored during destruction will be available when returning. - ensureExceptionsStored(_exceptionRethrower); - } BOOST_SCOPE_EXIT_END + _components.reset(); - createComponents(_exceptionRethrower, configUri); + createComponents(configUri); // We do not want back to back reinitializing as it gives zero time for serving // some torrents. @@ -245,8 +218,7 @@ public: while (!askedToShutDown() && (postPoneAskedToReinitializedSecs > 0 || !askedToReinitialize()) && - !completeReconfigurationNeeded() && - !_exceptionRethrower->exceptionStored()) { + !completeReconfigurationNeeded()) { postPoneAskedToReinitializedSecs--; std::this_thread::sleep_for(1s); } diff --git a/filedistribution/src/apps/status/status-filedistribution.cpp b/filedistribution/src/apps/status/status-filedistribution.cpp index 2a7e5cfe165..3818be337eb 100644 --- a/filedistribution/src/apps/status/status-filedistribution.cpp +++ b/filedistribution/src/apps/status/status-filedistribution.cpp @@ -10,7 +10,6 @@ LOG_SETUP("status-filedistribution"); #include <boost/program_options.hpp> #include <boost/foreach.hpp> -#include <vespa/filedistribution/common/exceptionrethrower.h> #include <vespa/filedistribution/model/zkfacade.h> #include <vespa/filedistribution/model/filedistributionmodel.h> #include <vespa/filedistribution/model/filedistributionmodelimpl.h> @@ -61,8 +60,7 @@ printWaitingForHosts(const StatusByHostName& notFinishedHosts) //TODO:refactor int printStatus(const std::string& zkservers) { - std::shared_ptr<ExceptionRethrower> exceptionRethrower; - std::shared_ptr<ZKFacade> zk(new ZKFacade(zkservers, exceptionRethrower)); + std::shared_ptr<ZKFacade> zk(new ZKFacade(zkservers)); std::shared_ptr<FileDBModel> model(new ZKFileDBModel(zk)); diff --git a/filedistribution/src/tests/filedbmodelimpl/test-filedistributionmodelimpl.cpp b/filedistribution/src/tests/filedbmodelimpl/test-filedistributionmodelimpl.cpp index 7642798125b..3cf12722d86 100644 --- a/filedistribution/src/tests/filedbmodelimpl/test-filedistributionmodelimpl.cpp +++ b/filedistribution/src/tests/filedbmodelimpl/test-filedistributionmodelimpl.cpp @@ -20,14 +20,12 @@ namespace { struct Fixture { - std::shared_ptr<ExceptionRethrower> _exceptionRethrower; ComponentsDeleter _componentsDeleter; std::shared_ptr<ZKFacade> _zk; std::shared_ptr<FileDistributionModelImpl> _distModel; Fixture() { - _exceptionRethrower.reset(new ExceptionRethrower()); - _zk = _componentsDeleter.track(new ZKFacade("test1-tonyv:2181", _exceptionRethrower)); - _distModel.reset(new FileDistributionModelImpl("hostname", 12345, _zk, _exceptionRethrower)); + _zk = _componentsDeleter.track(new ZKFacade("test1-tonyv:2181")); + _distModel.reset(new FileDistributionModelImpl("hostname", 12345, _zk)); } ~Fixture() { } }; diff --git a/filedistribution/src/tests/filedownloader/testfiledownloader.cpp b/filedistribution/src/tests/filedownloader/testfiledownloader.cpp index 4333055f3b7..51b36745df1 100644 --- a/filedistribution/src/tests/filedownloader/testfiledownloader.cpp +++ b/filedistribution/src/tests/filedownloader/testfiledownloader.cpp @@ -18,7 +18,6 @@ #include <vespa/filedistribution/manager/createtorrent.h> #include <vespa/filedistribution/model/filedistributionmodel.h> -#include <vespa/filedistribution/common/exceptionrethrower.h> #include <vespa/filedistribution/common/componentsdeleter.h> namespace fs = boost::filesystem; @@ -34,12 +33,11 @@ const int downloaderPort = 9112; std::shared_ptr<FileDownloader> createDownloader(ComponentsDeleter& deleter, int port, const fs::path& downloaderPath, - const std::shared_ptr<FileDistributionModel>& model, - const std::shared_ptr<ExceptionRethrower>& exceptionRethrower) + const std::shared_ptr<FileDistributionModel>& model) { - std::shared_ptr<FileDistributorTrackerImpl> tracker(deleter.track(new FileDistributorTrackerImpl(model, exceptionRethrower))); + std::shared_ptr<FileDistributorTrackerImpl> tracker(deleter.track(new FileDistributorTrackerImpl(model))); std::shared_ptr<FileDownloader> downloader(deleter.track(new FileDownloader(tracker, - localHost, port, downloaderPath, exceptionRethrower))); + localHost, port, downloaderPath))); tracker->setDownloader(downloader); return downloader; @@ -99,14 +97,13 @@ BOOST_AUTO_TEST_CASE(fileDownloaderTest) { Buffer buffer(createTorrent.bencode()); ComponentsDeleter deleter; - std::shared_ptr<ExceptionRethrower> exceptionRethrower(new ExceptionRethrower()); std::shared_ptr<FileDistributionModel> model(deleter.track(new MockFileDistributionModel())); std::shared_ptr<FileDownloader> downloader = - createDownloader(deleter, downloaderPort, downloaderPath, model, exceptionRethrower); + createDownloader(deleter, downloaderPort, downloaderPath, model); std::shared_ptr<FileDownloader> uploader = - createDownloader(deleter, uploaderPort, uploaderPath, model, exceptionRethrower); + createDownloader(deleter, uploaderPort, uploaderPath, model); std::thread uploaderThread( [&] () { uploader->runEventLoop(); }); std::thread downloaderThread( [&] () { downloader->runEventLoop(); }); @@ -116,7 +113,6 @@ BOOST_AUTO_TEST_CASE(fileDownloaderTest) { sleep(5); BOOST_CHECK(fs::exists(downloaderPath / fileReference / fileToSend)); - BOOST_CHECK(!exceptionRethrower->exceptionStored()); uploaderThread.interrupt(); uploaderThread.join(); diff --git a/filedistribution/src/tests/status/test-status.cpp b/filedistribution/src/tests/status/test-status.cpp index 7021752f316..cfa3e740093 100644 --- a/filedistribution/src/tests/status/test-status.cpp +++ b/filedistribution/src/tests/status/test-status.cpp @@ -5,7 +5,6 @@ #include <boost/test/unit_test.hpp> #include <boost/foreach.hpp> -#include <vespa/filedistribution/common/exceptionrethrower.h> #include <vespa/filedistribution/model/zkfacade.h> #include <vespa/filedistribution/model/filedistributionmodel.h> #include <vespa/filedistribution/model/filedistributionmodelimpl.h> diff --git a/filedistribution/src/tests/zkfacade/test-zkfacade.cpp b/filedistribution/src/tests/zkfacade/test-zkfacade.cpp index c0b60780ab5..ada601742db 100644 --- a/filedistribution/src/tests/zkfacade/test-zkfacade.cpp +++ b/filedistribution/src/tests/zkfacade/test-zkfacade.cpp @@ -34,16 +34,13 @@ struct Watcher : public ZKFacade::NodeChangedWatcher { }; struct Fixture { - std::shared_ptr<ExceptionRethrower> _exceptionRethrower; ComponentsDeleter _componentsDeleter; std::shared_ptr<ZKFacade> zk; ZKFacade::Path testNode; Fixture() { - _exceptionRethrower.reset(new ExceptionRethrower()); - zoo_set_debug_level(ZOO_LOG_LEVEL_WARN); - zk = _componentsDeleter.track(new ZKFacade("test1-tonyv:2181", _exceptionRethrower)); + zk = _componentsDeleter.track(new ZKFacade("test1-tonyv:2181")); testNode = "/test-node"; zk->removeIfExists(testNode); @@ -155,8 +152,7 @@ BOOST_AUTO_TEST_CASE(addEphemeralNode) zk->removeIfExists(ephemeralNode); //Checked deleter is ok here since we're not installing any watchers - ZKFacade::SP zk2(new ZKFacade("test1-tonyv:2181", _exceptionRethrower), - boost::checked_deleter<ZKFacade>()); + ZKFacade::SP zk2(new ZKFacade("test1-tonyv:2181"), boost::checked_deleter<ZKFacade>()); zk2->addEphemeralNode(ephemeralNode); BOOST_CHECK(zk->hasNode(ephemeralNode)); diff --git a/filedistribution/src/tests/zkfiledbmodel/test-zkfiledbmodel.cpp b/filedistribution/src/tests/zkfiledbmodel/test-zkfiledbmodel.cpp index 2b831501142..6a3a87aac96 100644 --- a/filedistribution/src/tests/zkfiledbmodel/test-zkfiledbmodel.cpp +++ b/filedistribution/src/tests/zkfiledbmodel/test-zkfiledbmodel.cpp @@ -22,16 +22,13 @@ namespace { struct Fixture { - std::shared_ptr<ExceptionRethrower> _exceptionRethrower; ComponentsDeleter _componentsDeleter; std::shared_ptr<ZKFacade> zk; std::shared_ptr<ZKFileDBModel> model; Fixture() { - _exceptionRethrower.reset(new ExceptionRethrower()); - zoo_set_debug_level(ZOO_LOG_LEVEL_WARN); - zk = _componentsDeleter.track(new ZKFacade("test1-tonyv:2181", _exceptionRethrower)); + zk = _componentsDeleter.track(new ZKFacade("test1-tonyv:2181")); zk->setData("/vespa", "", 0); model = _componentsDeleter.track(new ZKFileDBModel(zk)); diff --git a/filedistribution/src/vespa/filedistribution/common/exceptionrethrower.h b/filedistribution/src/vespa/filedistribution/common/exceptionrethrower.h deleted file mode 100644 index 191cd76b8d9..00000000000 --- a/filedistribution/src/vespa/filedistribution/common/exceptionrethrower.h +++ /dev/null @@ -1,46 +0,0 @@ -// Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#pragma once - -#include <mutex> -#include <boost/exception_ptr.hpp> -#include <boost/type_traits/is_polymorphic.hpp> - -namespace filedistribution { - -//used for rethrowing an exceptions in a different context -class ExceptionRethrower { - boost::exception_ptr _exceptionPtr; //not a pod, default constructed to null value - - mutable std::mutex _exceptionMutex; - typedef std::lock_guard<std::mutex> LockGuard; - -public: - void rethrow() const { - LockGuard guard(_exceptionMutex); - - if (_exceptionPtr) - boost::rethrow_exception(_exceptionPtr); - } - - bool exceptionStored() const { - LockGuard guard(_exceptionMutex); - return _exceptionPtr; - } - - template <class T> - void store(const T& exception) { - boost::exception_ptr exceptionPtr = boost::copy_exception(exception); - store(exceptionPtr); - } - - void store(const boost::exception_ptr exceptionPtr) { - LockGuard guard(_exceptionMutex); - - if (!_exceptionPtr) //only store the first exception to be rethrowed. - _exceptionPtr = exceptionPtr; - } -}; - -} //namespace filedistribution - - diff --git a/filedistribution/src/vespa/filedistribution/distributor/filedistributortrackerimpl.cpp b/filedistribution/src/vespa/filedistribution/distributor/filedistributortrackerimpl.cpp index d95c0ddef7f..7eb0ab957ff 100644 --- a/filedistribution/src/vespa/filedistribution/distributor/filedistributortrackerimpl.cpp +++ b/filedistribution/src/vespa/filedistribution/distributor/filedistributortrackerimpl.cpp @@ -15,7 +15,6 @@ using filedistribution::FileDistributorTrackerImpl; using filedistribution::FileDownloader; using filedistribution::FileDistributionModel; using filedistribution::Scheduler; -using filedistribution::ExceptionRethrower; using filedistribution::TorrentSP; typedef FileDistributionModel::PeerEntries PeerEntries; diff --git a/filedistribution/src/vespa/filedistribution/distributor/filedistributortrackerimpl.h b/filedistribution/src/vespa/filedistribution/distributor/filedistributortrackerimpl.h index 331f82f3c9d..bf72a2b80df 100644 --- a/filedistribution/src/vespa/filedistribution/distributor/filedistributortrackerimpl.h +++ b/filedistribution/src/vespa/filedistribution/distributor/filedistributortrackerimpl.h @@ -8,8 +8,8 @@ #include <boost/asio/deadline_timer.hpp> #include <vespa/filedistribution/model/filedistributionmodel.h> -#include <vespa/filedistribution/common/exceptionrethrower.h> #include "scheduler.h" +#include <mutex> namespace filedistribution { class FileDistributionModel; diff --git a/filedistribution/src/vespa/filedistribution/distributor/filedownloader.h b/filedistribution/src/vespa/filedistribution/distributor/filedownloader.h index 6f2124fd10c..38de8ac4357 100644 --- a/filedistribution/src/vespa/filedistribution/distributor/filedownloader.h +++ b/filedistribution/src/vespa/filedistribution/distributor/filedownloader.h @@ -2,6 +2,7 @@ #pragma once #include <vector> +#include <mutex> #include <boost/filesystem/path.hpp> #include <boost/optional.hpp> #include <boost/multi_index_container.hpp> @@ -14,7 +15,6 @@ #include <vespa/filedistribution/rpc/fileprovider.h> #include "hostname.h" #include <vespa/filedistribution/common/buffer.h> -#include <vespa/filedistribution/common/exceptionrethrower.h> #include <vespa/filedistribution/common/exception.h> #include <vespa/filedistribution/model/filedbmodel.h> diff --git a/filedistribution/src/vespa/filedistribution/manager/filedistributionmanager.cpp b/filedistribution/src/vespa/filedistribution/manager/filedistributionmanager.cpp index 70c0e867463..057902327a9 100644 --- a/filedistribution/src/vespa/filedistribution/manager/filedistributionmanager.cpp +++ b/filedistribution/src/vespa/filedistribution/manager/filedistributionmanager.cpp @@ -87,8 +87,7 @@ void initMockFileDBModel(NativeFileDistributionManager& manager) void initFileDBModel(NativeFileDistributionManager& manager, const std::string& zkServers) { //Ignored for now, since we're not installing any watchers. - std::shared_ptr<ExceptionRethrower> ignoredRethrower(new ExceptionRethrower()); - std::shared_ptr<ZKFacade> zk(new ZKFacade(zkServers, ignoredRethrower)); + std::shared_ptr<ZKFacade> zk(new ZKFacade(zkServers)); manager._fileDBModel.reset(new ZKFileDBModel(zk)); } } //end anonymous namespace diff --git a/filedistribution/src/vespa/filedistribution/model/filedistributionmodelimpl.cpp b/filedistribution/src/vespa/filedistribution/model/filedistributionmodelimpl.cpp index c0326db209b..adff69cfc6c 100644 --- a/filedistribution/src/vespa/filedistribution/model/filedistributionmodelimpl.cpp +++ b/filedistribution/src/vespa/filedistribution/model/filedistributionmodelimpl.cpp @@ -223,11 +223,8 @@ FileDistributionModelImpl::addConfigServersAsPeers( void FileDistributionModelImpl::configure(std::unique_ptr<FilereferencesConfig> config) { - try { - const bool changed = updateActiveFileReferences(config->filereferences); - if (changed) - _filesToDownloadChanged(); - } catch(...) { - _exceptionRethrower->store(boost::current_exception()); + const bool changed = updateActiveFileReferences(config->filereferences); + if (changed) { + _filesToDownloadChanged(); } } diff --git a/filedistribution/src/vespa/filedistribution/model/filedistributionmodelimpl.h b/filedistribution/src/vespa/filedistribution/model/filedistributionmodelimpl.h index 95c1ca5c72b..224009822e1 100644 --- a/filedistribution/src/vespa/filedistribution/model/filedistributionmodelimpl.h +++ b/filedistribution/src/vespa/filedistribution/model/filedistributionmodelimpl.h @@ -5,7 +5,6 @@ #include <vespa/filedistribution/model/config-filereferences.h> #include "zkfacade.h" #include "zkfiledbmodel.h" -#include <vespa/filedistribution/common/exceptionrethrower.h> #include <vespa/config/config.h> using cloud::config::filedistribution::FilereferencesConfig; @@ -28,21 +27,16 @@ class FileDistributionModelImpl : public FileDistributionModel, typedef std::lock_guard<std::mutex> LockGuard; std::vector<vespalib::string> _activeFileReferences; - const std::shared_ptr<ExceptionRethrower> _exceptionRethrower; - bool /*changed*/ updateActiveFileReferences(const std::vector<vespalib::string>& fileReferences); ZKFacade::Path getPeerEntryPath(const std::string& fileReference); public: - FileDistributionModelImpl(const std::string& hostName, int port, - const std::shared_ptr<ZKFacade>& zk, - const std::shared_ptr<ExceptionRethrower>& exceptionRethrower) + FileDistributionModelImpl(const std::string& hostName, int port, const std::shared_ptr<ZKFacade>& zk) :_hostName(hostName), _port(port), _zk(zk), - _fileDBModel(_zk), - _exceptionRethrower(exceptionRethrower) + _fileDBModel(_zk) { /* Hack: Force the first call to updateActiveFileReferences to return changed=true when the file references config is empty. diff --git a/filedistribution/src/vespa/filedistribution/model/zkfacade.cpp b/filedistribution/src/vespa/filedistribution/model/zkfacade.cpp index 17ba3d31973..8c67dba3150 100644 --- a/filedistribution/src/vespa/filedistribution/model/zkfacade.cpp +++ b/filedistribution/src/vespa/filedistribution/model/zkfacade.cpp @@ -178,15 +178,15 @@ struct ZKFacade::ZKWatcher { void ZKFacade::stateWatchingFun(zhandle_t*, int type, int state, const char* path, void* context) { (void)path; + (void)context; //The ZKFacade won't expire before zookeeper_close has finished. - ZKFacade* self = (ZKFacade*)context; if (type == ZOO_SESSION_EVENT) { LOGFWD(debug, "Zookeeper session event: %d", state); if (state == ZOO_EXPIRED_SESSION_STATE) { - self->_exceptionRethrower->store(ZKSessionExpired()); + throw ZKSessionExpired(); } else if (state == ZOO_AUTH_FAILED_STATE) { - self->_exceptionRethrower->store(ZKGenericException(ZNOAUTH)); + throw ZKGenericException(ZNOAUTH); } } else { LOGFWD(info, "State watching function: Unexpected event: '%d' -- '%d' ", type, state); @@ -220,30 +220,24 @@ ZKFacade::unregisterWatcher(void* watcherContext) { void ZKFacade::invokeWatcher(void* watcherContext) { - try { - std::shared_ptr<ZKWatcher> watcher = unregisterWatcher(watcherContext); + std::shared_ptr<ZKWatcher> watcher = unregisterWatcher(watcherContext); - if (!_watchersEnabled) - return; + if (!_watchersEnabled) + return; - if (watcher) { - (*watcher->_nodeChangedWatcher)(); - } else { - LOGFWD(error, "Invoke called on expired watcher."); - } - } catch(...) { - _exceptionRethrower->store(boost::current_exception()); + if (watcher) { + (*watcher->_nodeChangedWatcher)(); + } else { + LOGFWD(error, "Invoke called on expired watcher."); } } /********** End live watchers ***************************************/ -ZKFacade::ZKFacade(const std::string& zkservers, - const std::shared_ptr<ExceptionRethrower> &exceptionRethrower) +ZKFacade::ZKFacade(const std::string& zkservers) :_retriesEnabled(true), _watchersEnabled(true), - _exceptionRethrower(exceptionRethrower), _zhandle(zookeeper_init(zkservers.c_str(), &ZKFacade::stateWatchingFun, _zkSessionTimeOut, diff --git a/filedistribution/src/vespa/filedistribution/model/zkfacade.h b/filedistribution/src/vespa/filedistribution/model/zkfacade.h index 35639e56035..7631fa6d9dc 100644 --- a/filedistribution/src/vespa/filedistribution/model/zkfacade.h +++ b/filedistribution/src/vespa/filedistribution/model/zkfacade.h @@ -3,12 +3,12 @@ #include <string> #include <vector> +#include <mutex> #include <boost/filesystem/path.hpp> #include <boost/signals2.hpp> #include <vespa/filedistribution/common/buffer.h> #include <vespa/filedistribution/common/exception.h> -#include <vespa/filedistribution/common/exceptionrethrower.h> struct _zhandle; typedef _zhandle zhandle_t; @@ -62,7 +62,6 @@ class ZKFacade : public std::enable_shared_from_this<ZKFacade> { volatile bool _retriesEnabled; volatile bool _watchersEnabled; - std::shared_ptr<ExceptionRethrower> _exceptionRethrower; zhandle_t* _zhandle; const static int _zkSessionTimeOut = 30 * 1000; const static size_t _maxDataSize = 1024 * 1024; @@ -90,7 +89,7 @@ public: ZKFacade(const ZKFacade &) = delete; ZKFacade & operator = (const ZKFacade &) = delete; - ZKFacade(const std::string& zkservers, const std::shared_ptr<ExceptionRethrower> &); + ZKFacade(const std::string& zkservers); ~ZKFacade(); bool hasNode(const Path&); |