From b0813ac3b91f6e29d241c5ba49d62756e821aaaf Mon Sep 17 00:00:00 2001 From: Benoit Foucher Date: Fri, 14 Sep 2012 13:44:07 +0200 Subject: Fixed to remove need for calling updateViews after map registration --- cpp/src/Ice/InstrumentationI.cpp | 1 - cpp/src/Ice/MetricsAdminI.cpp | 150 +++++++++++++++++++++++----------- cpp/src/Ice/MetricsAdminI.h | 38 +++++---- cpp/src/Ice/MetricsObserverI.h | 48 ++++++----- cpp/src/IceStorm/InstrumentationI.cpp | 1 - 5 files changed, 150 insertions(+), 88 deletions(-) (limited to 'cpp/src') diff --git a/cpp/src/Ice/InstrumentationI.cpp b/cpp/src/Ice/InstrumentationI.cpp index a5121ff7e23..3b7ae11a495 100644 --- a/cpp/src/Ice/InstrumentationI.cpp +++ b/cpp/src/Ice/InstrumentationI.cpp @@ -750,7 +750,6 @@ CommunicatorObserverI::CommunicatorObserverI(const MetricsAdminIPtr& metrics) : _endpointLookups(metrics, "EndpointLookup") { _invocations.registerSubMap("Remote", &InvocationMetrics::remotes); - _metrics->updateViews(); } void diff --git a/cpp/src/Ice/MetricsAdminI.cpp b/cpp/src/Ice/MetricsAdminI.cpp index 493f0a30e40..53448eb1173 100644 --- a/cpp/src/Ice/MetricsAdminI.cpp +++ b/cpp/src/Ice/MetricsAdminI.cpp @@ -236,11 +236,9 @@ MetricsViewI::MetricsViewI(const string& name) : _name(name) { } -void -MetricsViewI::update(const PropertiesPtr& properties, - const map& factories, - set& updatedMaps, - const Ice::LoggerPtr& logger) +bool +MetricsViewI::addOrUpdateMap(const PropertiesPtr& properties, const string& mapName, + const MetricsMapFactoryPtr& factory, const Ice::LoggerPtr& logger) { // // Add maps to views configured with the given map. @@ -248,53 +246,61 @@ MetricsViewI::update(const PropertiesPtr& properties, const string viewPrefix = "IceMX.Metrics." + _name + "."; const string mapsPrefix = viewPrefix + "Map."; PropertyDict mapsProps = properties->getPropertiesForPrefix(mapsPrefix); - for(map::const_iterator p = factories.begin(); p != factories.end(); ++p) + + string mapPrefix; + PropertyDict mapProps; + if(!mapsProps.empty()) { - const string& mapName = p->first; - string mapPrefix; - PropertyDict mapProps; - if(!mapsProps.empty()) + mapPrefix = mapsPrefix + mapName + "."; + mapProps = properties->getPropertiesForPrefix(mapPrefix); + if(mapProps.empty()) { - mapPrefix = mapsPrefix + mapName + "."; - mapProps = properties->getPropertiesForPrefix(mapPrefix); - if(mapProps.empty()) - { - // This map isn't configured anymore for this view. - updatedMaps.insert(p->second); - _maps.erase(mapName); - continue; - } - } - else - { - mapPrefix = viewPrefix; - mapProps = properties->getPropertiesForPrefix(mapPrefix); + // This map isn't configured for this view. + _maps.erase(mapName); + return true; } + } + else + { + mapPrefix = viewPrefix; + mapProps = properties->getPropertiesForPrefix(mapPrefix); + } - map::iterator q = _maps.find(mapName); - if(q != _maps.end() && q->second->getProperties() == mapProps) - { - continue; // The map configuration didn't change, no need to re-create. - } + map::iterator q = _maps.find(mapName); + if(q != _maps.end() && q->second->getProperties() == mapProps) + { + return false; // The map configuration didn't change, no need to re-create. + } - try - { - _maps[mapName] = p->second->create(mapPrefix, properties); - } - catch(const std::exception& ex) - { - Ice::Warning warn(logger); - warn << "unexpected exception while creating metrics map:\n" << ex; - _maps.erase(mapName); - } - catch(const string& msg) - { - Ice::Warning warn(logger); - warn << msg; - _maps.erase(mapName); - } - updatedMaps.insert(p->second); + try + { + _maps[mapName] = factory->create(mapPrefix, properties); + } + catch(const std::exception& ex) + { + Ice::Warning warn(logger); + warn << "unexpected exception while creating metrics map:\n" << ex; + _maps.erase(mapName); + } + catch(const string& msg) + { + Ice::Warning warn(logger); + warn << msg; + _maps.erase(mapName); } + return true; +} + +bool +MetricsViewI::removeMap(const string& mapName) +{ + map::iterator q = _maps.find(mapName); + if(q != _maps.end()) + { + _maps.erase(q); + return true; + } + return false; } MetricsView @@ -404,7 +410,14 @@ MetricsAdminI::updateViews() { q = views.insert(make_pair(viewName, q->second)).first; } - q->second->update(_properties, _factories, updatedMaps, _logger); + + for(map::const_iterator p = _factories.begin(); p != _factories.end(); ++p) + { + if(q->second->addOrUpdateMap(_properties, p->first, p->second, _logger)) + { + updatedMaps.insert(p->second); + } + } } _views.swap(views); @@ -433,6 +446,28 @@ MetricsAdminI::updateViews() } } +void +MetricsAdminI::unregisterMap(const std::string& mapName) +{ + bool updated; + MetricsMapFactoryPtr factory; + { + Lock sync(*this); + map::iterator p = _factories.find(mapName); + if(p == _factories.end()) + { + return; + } + factory = p->second; + _factories.erase(p); + updated = removeMap(mapName); + } + if(updated) + { + factory->update(); + } +} + StringSeq MetricsAdminI::getMetricsViewNames(const Current&) { @@ -532,3 +567,24 @@ MetricsAdminI::updated(const PropertyDict& props) } } +bool +MetricsAdminI::addOrUpdateMap(const std::string& mapName, const MetricsMapFactoryPtr& factory) +{ + bool updated = false; + for(std::map::const_iterator p = _views.begin(); p != _views.end(); ++p) + { + updated |= p->second->addOrUpdateMap(_properties, mapName, factory, _logger); + } + return updated; +} + +bool +MetricsAdminI::removeMap(const std::string& mapName) +{ + bool updated = false; + for(std::map::const_iterator p = _views.begin(); p != _views.end(); ++p) + { + updated |= p->second->removeMap(mapName); + } + return updated; +} diff --git a/cpp/src/Ice/MetricsAdminI.h b/cpp/src/Ice/MetricsAdminI.h index 260d283a2e2..0ed8a5a050d 100644 --- a/cpp/src/Ice/MetricsAdminI.h +++ b/cpp/src/Ice/MetricsAdminI.h @@ -495,8 +495,9 @@ public: MetricsViewI(const std::string&); - void update(const Ice::PropertiesPtr&, const std::map&, - std::set&, const Ice::LoggerPtr&); + bool addOrUpdateMap(const Ice::PropertiesPtr&, const std::string&, const MetricsMapFactoryPtr&, + const Ice::LoggerPtr&); + bool removeMap(const std::string&); MetricsView getMetrics(); MetricsFailuresSeq getFailures(const std::string&); @@ -524,8 +525,18 @@ public: template void registerMap(const std::string& map, Updater* updater) { - Lock sync(*this); - _factories[map] = new MetricsMapFactoryT(updater); + bool update; + MetricsMapFactoryPtr factory; + { + Lock sync(*this); + factory = new MetricsMapFactoryT(updater); + _factories[map] = factory; + addOrUpdateMap(map, factory); + } + if(update) + { + factory->update(); + } } template void @@ -534,18 +545,14 @@ public: Lock sync(*this); std::map::const_iterator p = _factories.find(map); - assert(p != _factories.end()); - - MetricsMapFactoryT* factory = dynamic_cast*>(p->second.get()); - factory->template registerSubMap(subMap, member); + if(p != _factories.end()) + { + MetricsMapFactoryT* factory = dynamic_cast*>(p->second.get()); + factory->template registerSubMap(subMap, member); + } } - void - unregisterMap(const std::string& map) - { - Lock sync(*this); - _factories.erase(map); - } + void unregisterMap(const std::string&); virtual Ice::StringSeq getMetricsViewNames(const ::Ice::Current&); virtual MetricsView getMetricsView(const std::string&, const ::Ice::Current&); @@ -563,6 +570,9 @@ private: void updated(const Ice::PropertyDict&); + bool addOrUpdateMap(const std::string&, const MetricsMapFactoryPtr&); + bool removeMap(const std::string&); + std::map _views; std::map _factories; diff --git a/cpp/src/Ice/MetricsObserverI.h b/cpp/src/Ice/MetricsObserverI.h index 653b0535d93..07b19567652 100644 --- a/cpp/src/Ice/MetricsObserverI.h +++ b/cpp/src/Ice/MetricsObserverI.h @@ -31,14 +31,6 @@ public: virtual std::string operator()(const std::string&) const = 0; }; -class Updater : public IceUtil::Shared -{ -public: - - virtual void update() = 0; -}; -typedef IceUtil::Handle UpdaterPtr; - template class MetricsHelperT : public MetricsHelper { public: @@ -268,6 +260,14 @@ protected: }; }; +class Updater : public IceUtil::Shared +{ +public: + + virtual void update() = 0; +}; +typedef IceUtil::Handle UpdaterPtr; + template class UpdaterT : public Updater { public: @@ -288,7 +288,17 @@ private: void (T::*_fn)(); }; -class ObserverI; +template Updater* +newUpdater(const IceUtil::Handle& updater, void (T::*fn)()) +{ + return new UpdaterT(updater.get(), fn); +} + +template Updater* +newUpdater(const IceInternal::Handle& updater, void (T::*fn)()) +{ + return new UpdaterT(updater.get(), fn); +} template class ObserverT : virtual public Ice::Instrumentation::Observer { @@ -413,22 +423,6 @@ private: IceUtilInternal::StopWatch _watch; }; -class ObserverI : virtual public Ice::Instrumentation::Observer, public ObserverT -{ -}; - -template Updater* -newUpdater(const IceUtil::Handle& updater, void (T::*fn)()) -{ - return new UpdaterT(updater.get(), fn); -} - -template Updater* -newUpdater(const IceInternal::Handle& updater, void (T::*fn)()) -{ - return new UpdaterT(updater.get(), fn); -} - template class ObserverFactoryT : public Updater, private IceUtil::Mutex { @@ -566,6 +560,10 @@ private: UpdaterPtr _updater; }; +class ObserverI : virtual public Ice::Instrumentation::Observer, public ObserverT +{ +}; + } #endif diff --git a/cpp/src/IceStorm/InstrumentationI.cpp b/cpp/src/IceStorm/InstrumentationI.cpp index b7bbae60410..e310557dbce 100644 --- a/cpp/src/IceStorm/InstrumentationI.cpp +++ b/cpp/src/IceStorm/InstrumentationI.cpp @@ -317,7 +317,6 @@ TopicManagerObserverI::TopicManagerObserverI(const MetricsAdminIPtr& metrics) : _topics(metrics, "Topic"), _subscribers(metrics, "Subscriber") { - _metrics->updateViews(); } void -- cgit v1.2.3