From 8d6e0a5838c9addf8e09f1e6ccd2809d35f0e62b Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Mon, 15 Jun 2015 13:18:42 +0200 Subject: [PATCH] [3798] Changes after review: - {update,remove}Statistics moved from CfgMgr to SrvConfig,CfgSubnets4 --- src/lib/dhcpsrv/cfg_subnets4.cc | 35 ++++++++++++++++++++++++++- src/lib/dhcpsrv/cfg_subnets4.h | 17 +++++++++++++ src/lib/dhcpsrv/cfgmgr.cc | 42 ++++----------------------------- src/lib/dhcpsrv/cfgmgr.h | 17 ------------- src/lib/dhcpsrv/srv_config.cc | 15 ++++++++++++ src/lib/dhcpsrv/srv_config.h | 11 +++++++++ 6 files changed, 82 insertions(+), 55 deletions(-) diff --git a/src/lib/dhcpsrv/cfg_subnets4.cc b/src/lib/dhcpsrv/cfg_subnets4.cc index 3a27e4a6fe..5ea1a8b024 100644 --- a/src/lib/dhcpsrv/cfg_subnets4.cc +++ b/src/lib/dhcpsrv/cfg_subnets4.cc @@ -17,6 +17,7 @@ #include #include #include +#include using namespace isc::asiolink; @@ -109,7 +110,7 @@ CfgSubnets4::selectSubnet(const SubnetSelector& selector) const { } // We have identified an address in the client's packet that can be - // used for subnet selection. Match this packet with the subnets. + // used for subnet selection. Match this packet with the subnets. return (selectSubnet(address, selector.client_classes_)); } @@ -145,8 +146,40 @@ CfgSubnets4::isDuplicate(const Subnet4& subnet) const { return (false); } +void +CfgSubnets4::removeStatistics() { + using namespace isc::stats; + + // For each v4 subnet currently configured, remove the statistic. + /// @todo: May move this to CfgSubnets4 class if there will be more + /// statistics here. + for (Subnet4Collection::const_iterator subnet4 = subnets_.begin(); + subnet4 != subnets_.end(); ++subnet4) { + + StatsMgr::instance().del(StatsMgr::generateName("subnet", + (*subnet4)->getID(), + "total-addresses")); + + StatsMgr::instance().del(StatsMgr::generateName("subnet", + (*subnet4)->getID(), + "assigned-addresses")); + } +} +void +CfgSubnets4::updateStatistics() { + using namespace isc::stats; + /// @todo: May move this to CfgSubnets4 class if there will be more + /// statistics here. + for (Subnet4Collection::const_iterator subnet = subnets_.begin(); + subnet != subnets_.end(); ++subnet) { + + StatsMgr::instance().setValue( + StatsMgr::generateName("subnet", (*subnet)->getID(), "total-addresses"), + static_cast((*subnet)->getPoolCapacity(Lease::TYPE_V4))); + } +} } // end of namespace isc::dhcp } // end of namespace isc diff --git a/src/lib/dhcpsrv/cfg_subnets4.h b/src/lib/dhcpsrv/cfg_subnets4.h index a39ae5fae1..0c6c16c2c5 100644 --- a/src/lib/dhcpsrv/cfg_subnets4.h +++ b/src/lib/dhcpsrv/cfg_subnets4.h @@ -125,6 +125,23 @@ public: const ClientClasses& client_classes = ClientClasses()) const; + /// @brief Updates statistics. + /// + /// This method updates statistics that are affected by the newly committed + /// configuration. In particular, it updates the number of available addresses + /// in each subnet. Other statistics may be added in the future. In general, + /// these are statistics that are dependant only on configuration, so they are + /// not expected to change until the next reconfiguration event. + void updateStatistics(); + + /// @brief Removes statistics. + /// + /// During commitment of a new configuration, we need to get rid of the old + /// statistics for the old configuration. In particular, we need to remove + /// anything related to subnets, as there may be fewer subnets in the new + /// configuration and also subnet-ids may change. + void removeStatistics(); + private: /// @brief Checks that the IPv4 subnet with the given id already exists. diff --git a/src/lib/dhcpsrv/cfgmgr.cc b/src/lib/dhcpsrv/cfgmgr.cc index caa707de50..4fe4c6b19c 100644 --- a/src/lib/dhcpsrv/cfgmgr.cc +++ b/src/lib/dhcpsrv/cfgmgr.cc @@ -120,12 +120,14 @@ CfgMgr::clear() { void CfgMgr::commit() { + + ensureCurrentAllocated(); + // First we need to remove statistics. The new configuration can have fewer // subnets. Also, it may change subnet-ids. So we need to remove them all // and add it back. - removeStatistics(); + configuration_->removeStatistics(); - ensureCurrentAllocated(); if (!configs_.back()->sequenceEquals(*configuration_)) { configuration_ = configs_.back(); // Keep track of the maximum size of the configs history. Before adding @@ -138,7 +140,7 @@ CfgMgr::commit() { } // Now we need to set the statistics back. - updateStatistics(); + configuration_->updateStatistics(); } void @@ -198,40 +200,6 @@ CfgMgr::getStagingCfg() { return (configs_.back()); } -void -CfgMgr::removeStatistics() { - const Subnet4Collection* subnets = getCurrentCfg()->getCfgSubnets4()->getAll(); - - // For each subnet currently configured, remove the statistic - for (Subnet4Collection::const_iterator subnet = subnets->begin(); - subnet != subnets->end(); ++subnet) { - - /// @todo: Once stat contexts are implemented, we'll need to remove all - /// statistics from the subnet[subnet-id] context. - std::stringstream stat1; - stat1 << "subnet[" << (*subnet)->getID() << "].total-addresses"; - StatsMgr::instance().del(stat1.str()); - - std::stringstream stat2; - stat2 << "subnet[" << (*subnet)->getID() << "].assigned-addresses"; - isc::stats::StatsMgr::instance().del(stat2.str()); - } -} - -void -CfgMgr::updateStatistics() { - const Subnet4Collection* subnets = getCurrentCfg()->getCfgSubnets4()->getAll(); - - for (Subnet4Collection::const_iterator subnet = subnets->begin(); - subnet != subnets->end(); ++subnet) { - std::stringstream name; - name << "subnet[" << (*subnet)->getID() << "].total-addresses"; - - StatsMgr::instance().setValue(name.str(), - static_cast((*subnet)->getPoolCapacity(Lease::TYPE_V4))); - } -} - CfgMgr::CfgMgr() : datadir_(DHCP_DATA_DIR), echo_v4_client_id_(true), d2_client_mgr_(), verbose_mode_(false) { diff --git a/src/lib/dhcpsrv/cfgmgr.h b/src/lib/dhcpsrv/cfgmgr.h index 4d84976af8..77d8fd996a 100644 --- a/src/lib/dhcpsrv/cfgmgr.h +++ b/src/lib/dhcpsrv/cfgmgr.h @@ -339,23 +339,6 @@ private: /// @return true if the duplicate subnet exists. bool isDuplicate(const Subnet6& subnet) const; - /// @brief Updates statistics. - /// - /// This method updates statistics that are affected by the newly committed - /// configuration. In particular, it updates the number of available addresses - /// in each subnet. Other statistics may be added in the future. In general, - /// these are statistics that are dependant only on configuration, so they are - /// not expected to change until the next reconfiguration event. - void updateStatistics(); - - /// @brief Removes statistics. - /// - /// During commitment of a new configuration, we need to get rid of the old - /// statistics for the old configuration. In particular, we need to remove - /// anything related to subnets, as there may be fewer subnets in the new - /// configuration and also subnet-ids may change. - void removeStatistics(); - /// @brief Container for defined DHCPv6 option spaces. OptionSpaceCollection spaces6_; diff --git a/src/lib/dhcpsrv/srv_config.cc b/src/lib/dhcpsrv/srv_config.cc index 2108645c2f..577deca404 100644 --- a/src/lib/dhcpsrv/srv_config.cc +++ b/src/lib/dhcpsrv/srv_config.cc @@ -145,5 +145,20 @@ SrvConfig::equals(const SrvConfig& other) const { (*cfg_option_ == *other.cfg_option_)); } +void +SrvConfig::removeStatistics() { + + // For now, this method only removes statistics for v4 subnets, but in the + // near future, we'll also get statistics for v6 subnets. + getCfgSubnets4()->removeStatistics(); +} + +void +SrvConfig::updateStatistics() { + // For now, this method only updates statistics for v4 subnets, but in the + // near future, we'll also get statistics for v6 subnets. + getCfgSubnets4()->updateStatistics(); +} + } } diff --git a/src/lib/dhcpsrv/srv_config.h b/src/lib/dhcpsrv/srv_config.h index e27f9dd122..d13c48add0 100644 --- a/src/lib/dhcpsrv/srv_config.h +++ b/src/lib/dhcpsrv/srv_config.h @@ -349,6 +349,17 @@ public: //@} + /// @brief Updates statistics. + /// + /// This method calls appropriate methods in child objects that update + /// related statistics. See @ref CfgSubnets4::updateStatistics for details. + void updateStatistics(); + + /// @brief Removes statistics. + /// + /// This method calls appropriate methods in child objects that remove + /// related statistics. See @ref CfgSubnets4::removeStatistics for details. + void removeStatistics(); private: -- GitLab