From a8f85f25086f5cc9bc91a334fb788beadcf51a6a Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Fri, 12 Aug 2016 14:02:35 -0400 Subject: [4294] Memfile and MySql now support recalulating IPv4 lease statistics src/lib/dhcpsrv/cfg_subnets4.cc CfgSubnets4::removeStatistics() - added removal of all lease statistics per subnet, and global declined address stats CfgSubnets4::updateStatistics() - added call to LeaseMgr::recountAddressStats4 src/lib/dhcpsrv/lease.cc src/lib/dhcpsrv/lease.h Replaces lease state constants with LeaseState enumeration. src/lib/dhcpsrv/lease_mgr.cc src/lib/dhcpsrv/lease_mgr.h struct AddressStatsRow4 - contains the content of one row of the IPv4 lease statistical data result set class AddressStatsQuery4 - base class for constructing the IPv4 lease statistical data result set for an IPv4 lease storage LeaseMgr::recountAddressStats4() - new method which recalculates per-subnet and global stats for IPv4 leases LeaseMgr::startAddressStatsQuery4() - new virtual method that fetches the IPv4 lease statistical data result set src/lib/dhcpsrv/lease_mgr_factory.h src/lib/dhcpsrv/lease_mgr_factory.cc LeaseMgrFactory::haveInstance() - new static method which indicates whether or not the lease manager singleton exists src/lib/dhcpsrv/memfile_lease_mgr.h src/lib/dhcpsrv/memfile_lease_mgr.cc MemfileAddressStatsQuery4 - Derivation of AddressStatsQuery4, it constructs the IPv4 lease statistical data by iterating over IPv4 lease storage Memfile_LeaseMgr::startAddressStatsQuery4() - new virtual method which creates, starts, and returns a MemfileAddressStatsQuery4 src/lib/dhcpsrv/memfile_lease_storage.h Added an a per subnet_ID index to IPv4 storage src/lib/dhcpsrv/mysql_lease_mgr.h src/lib/dhcpsrv/mysql_lease_mgr.cc - Added RECOUNT_LEASE4_STATS query MySqlAddressStatsQuery4 Derivation of AddressStatsQuery4, it constructs the IPv4 lease statistical data by executing RECOUNT_LEASE4_STATS MySqlLeaseMgr::startAddressStatsQuery4() - new virtual method which creates, starts, and returns a MySqlAddressStatsQuery4 src/lib/dhcpsrv/tests/alloc_engine_utils.cc AllocEngine6Test::AllocEngine6Test() AllocEngine4Test::AllocEngine4Test() - moved lease mgr create up above configuration commit src/lib/dhcpsrv/tests/cfg_db_access_unittest.cc ~CfgMySQLDbAccessTest() - added destruction of lease manager singleton, otherwise subsequent tests can fail src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc GenericLeaseMgrTest::checkStat() - new method for comparing a stat GenericLeaseMgrTest::checkAddressStats4() - new method for comparing a list of stats GenericLeaseMgrTest::makeLease4() - new method for making a minimal lease GenericLeaseMgrTest::testRecountAddressStats4() - new method which tests a lease manager's ability to recalculate the IPv4 lease statistics src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc TEST_F(MemfileLeaseMgrTest, recountAddressStats4) - new test which tests Memfile_LeaseMgr's ability to recalculate IPv4 lease statistics src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc TEST_F(MySqlLeaseMgrTest, recountAddressStats4) - new test which tests MySqlLeaseMgr's ability to recalculate IPv4 lease statistics --- src/lib/dhcpsrv/cfg_subnets4.cc | 41 +++-- src/lib/dhcpsrv/dhcpsrv_messages.mes | 3 + src/lib/dhcpsrv/lease.cc | 4 - src/lib/dhcpsrv/lease.h | 22 +-- src/lib/dhcpsrv/lease_mgr.cc | 79 ++++++++++ src/lib/dhcpsrv/lease_mgr.h | 94 +++++++++++ src/lib/dhcpsrv/lease_mgr_factory.cc | 5 + src/lib/dhcpsrv/lease_mgr_factory.h | 5 +- src/lib/dhcpsrv/memfile_lease_mgr.cc | 145 +++++++++++++++++ src/lib/dhcpsrv/memfile_lease_mgr.h | 9 ++ src/lib/dhcpsrv/memfile_lease_storage.h | 17 +- src/lib/dhcpsrv/mysql_lease_mgr.cc | 141 ++++++++++++++++- src/lib/dhcpsrv/mysql_lease_mgr.h | 11 +- src/lib/dhcpsrv/tests/alloc_engine_utils.cc | 7 +- src/lib/dhcpsrv/tests/cfg_db_access_unittest.cc | 1 + .../dhcpsrv/tests/generic_lease_mgr_unittest.cc | 175 ++++++++++++++++++++- src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h | 43 ++++- .../dhcpsrv/tests/memfile_lease_mgr_unittest.cc | 6 +- src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc | 5 + 19 files changed, 773 insertions(+), 40 deletions(-) diff --git a/src/lib/dhcpsrv/cfg_subnets4.cc b/src/lib/dhcpsrv/cfg_subnets4.cc index 6c6fd408d3..43b1883053 100644 --- a/src/lib/dhcpsrv/cfg_subnets4.cc +++ b/src/lib/dhcpsrv/cfg_subnets4.cc @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -232,18 +233,21 @@ 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. + StatsMgr& stats_mgr = StatsMgr::instance(); for (Subnet4Collection::const_iterator subnet4 = subnets_.begin(); subnet4 != subnets_.end(); ++subnet4) { + SubnetID subnet_id = (*subnet4)->getID(); + stats_mgr.del(StatsMgr::generateName("subnet", subnet_id, + "total-addresses")); - StatsMgr::instance().del(StatsMgr::generateName("subnet", - (*subnet4)->getID(), - "total-addresses")); + stats_mgr.del(StatsMgr::generateName("subnet", subnet_id, + "assigned-addresses")); - StatsMgr::instance().del(StatsMgr::generateName("subnet", - (*subnet4)->getID(), - "assigned-addresses")); + stats_mgr.del(StatsMgr::generateName("subnet", subnet_id, + "declined-addresses")); + + stats_mgr.del(StatsMgr::generateName("subnet", subnet_id, + "declined-reclaimed-addresses")); } } @@ -251,14 +255,21 @@ 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& stats_mgr = StatsMgr::instance(); + for (Subnet4Collection::const_iterator subnet4 = subnets_.begin(); + subnet4 != subnets_.end(); ++subnet4) { + SubnetID subnet_id = (*subnet4)->getID(); + + stats_mgr.setValue(StatsMgr:: + generateName("subnet", subnet_id, "total-addresses"), + static_cast + ((*subnet4)->getPoolCapacity(Lease:: + TYPE_V4))); + } - StatsMgr::instance().setValue( - StatsMgr::generateName("subnet", (*subnet)->getID(), "total-addresses"), - static_cast((*subnet)->getPoolCapacity(Lease::TYPE_V4))); + // If we have subnets and a lease mgr, recount the least statistics + if (subnets_.begin() != subnets_.end() && LeaseMgrFactory::haveInstance()) { + LeaseMgrFactory::instance().recountAddressStats4(); } } diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.mes b/src/lib/dhcpsrv/dhcpsrv_messages.mes index 714148548c..db03ed0bb0 100644 --- a/src/lib/dhcpsrv/dhcpsrv_messages.mes +++ b/src/lib/dhcpsrv/dhcpsrv_messages.mes @@ -923,3 +923,6 @@ lease from the Cassandra database for the specified address. % DHCPSRV_CQL_UPDATE_ADDR6 updating IPv6 lease for address %1 A debug message issued when the server is attempting to update IPv6 lease from the Cassandra database for the specified address. + +% TOMS_UTILITY_MESSAGE %1 +Handy log message that should be deleted diff --git a/src/lib/dhcpsrv/lease.cc b/src/lib/dhcpsrv/lease.cc index 5854f06900..d8d15613d1 100644 --- a/src/lib/dhcpsrv/lease.cc +++ b/src/lib/dhcpsrv/lease.cc @@ -15,10 +15,6 @@ using namespace std; namespace isc { namespace dhcp { -const uint32_t Lease::STATE_DEFAULT = 0x0; -const uint32_t Lease::STATE_DECLINED = 0x1; -const uint32_t Lease::STATE_EXPIRED_RECLAIMED = 0x2; - Lease::Lease(const isc::asiolink::IOAddress& addr, uint32_t t1, uint32_t t2, uint32_t valid_lft, SubnetID subnet_id, time_t cltt, const bool fqdn_fwd, const bool fqdn_rev, diff --git a/src/lib/dhcpsrv/lease.h b/src/lib/dhcpsrv/lease.h index d1ab300f11..365cb06f53 100644 --- a/src/lib/dhcpsrv/lease.h +++ b/src/lib/dhcpsrv/lease.h @@ -40,18 +40,18 @@ struct Lease { /// @return text decription static std::string typeToText(Type type); - /// @name Common lease states constants. + /// @name Enumeration of lease states //@{ - /// - /// @brief A lease in the default state. - static const uint32_t STATE_DEFAULT; - - /// @brief Declined lease. - static const uint32_t STATE_DECLINED; - - /// @brief Expired and reclaimed lease. - static const uint32_t STATE_EXPIRED_RECLAIMED; - + typedef enum { + /// @brief A lease in the default (assigned) state. + STATE_DEFAULT, + /// @brief Declined lease. + STATE_DECLINED, + /// @brief Expired and reclaimed lease. + STATE_EXPIRED_RECLAIMED, + /// @brief The number of defined lease states. + NUM_LEASE_STATES + } LeaseState; //@} /// @brief Returns name(s) of the basic lease state(s). diff --git a/src/lib/dhcpsrv/lease_mgr.cc b/src/lib/dhcpsrv/lease_mgr.cc index 58f95d7dc9..491c5a0010 100644 --- a/src/lib/dhcpsrv/lease_mgr.cc +++ b/src/lib/dhcpsrv/lease_mgr.cc @@ -6,8 +6,11 @@ #include +#include +#include #include #include +#include #include #include @@ -44,6 +47,82 @@ LeaseMgr::getLease6(Lease::Type type, const DUID& duid, return (*col.begin()); } +void +LeaseMgr::recountAddressStats4() { + using namespace stats; + + StatsMgr& stats_mgr = StatsMgr::instance(); + + AddressStatsQuery4Ptr query = startAddressStatsQuery4(); + if (!query) { + /// NULL means not backend does not support recounting. + return; + } + + // Zero out the global stats. (Ok, so currently there's only one + // that should be cleared. "reclaimed-declined-addresses" never + // gets zeroed. @todo discuss with Tomek the rational of not + // clearing it when we clear the rest. + int64_t zero = 0; + stats_mgr.setValue("declined-addresses", zero); + stats_mgr.setValue("declined-reclaimed-addresses", zero); + + // Clear subnet level stats. This ensures we don't end up with corner + // cases that leave stale values in place. + const Subnet4Collection* subnets = + CfgMgr::instance().getCurrentCfg()->getCfgSubnets4()->getAll(); + + for (Subnet4Collection::const_iterator subnet = subnets->begin(); + subnet != subnets->end(); ++subnet) { + SubnetID subnet_id = (*subnet)->getID(); + stats_mgr.setValue(StatsMgr::generateName("subnet", subnet_id, + "assigned-addresses"), + zero); + + stats_mgr.setValue(StatsMgr::generateName("subnet", subnet_id, + "declined-addresses"), + zero); + stats_mgr.setValue(StatsMgr::generateName("subnet", subnet_id, + "declined-reclaimed-addresses"), + zero); + } + + // Get counts per state per subnet. Iterate over the result set + // updating the subnet and global values. + AddressStatsRow4 row; + while (query->getNextRow(row)) { + switch(row.lease_state_) { + case Lease::STATE_DEFAULT: + // Set subnet level value. + stats_mgr.setValue(StatsMgr::generateName("subnet", + row.subnet_id_, + "assigned-addresses"), + row.state_count_); + break; + + case Lease::STATE_DECLINED: + // Set subnet level value. + stats_mgr.setValue(StatsMgr::generateName("subnet", + row.subnet_id_, + "declined-addresses"), + row.state_count_); + + // Add to the global value. + stats_mgr.addValue("declined-addresses", row.state_count_); + break; + + default: + // Not one we're tracking. + break; + } + } +} + +AddressStatsQuery4Ptr +LeaseMgr::startAddressStatsQuery4() { + return(AddressStatsQuery4Ptr()); +} + std::string LeaseMgr::getDBVersion() { isc_throw(NotImplemented, "LeaseMgr::getDBVersion() called"); diff --git a/src/lib/dhcpsrv/lease_mgr.h b/src/lib/dhcpsrv/lease_mgr.h index c31f2dfa12..e8123e840f 100644 --- a/src/lib/dhcpsrv/lease_mgr.h +++ b/src/lib/dhcpsrv/lease_mgr.h @@ -146,6 +146,70 @@ public: virtual ~SqlExchange() {}; ExchangeColumnInfoContainer parameters_; ///< Column names and types }; + +/// @brief Contains a single row of IPv4 lease statistical data +/// +/// The contents of the row consist of a subnet ID, a lease state, +/// and the number of leases in that state for that subnet ID. +struct AddressStatsRow4 { + /// @brief Default constructor + AddressStatsRow4() : + subnet_id_(0), lease_state_(Lease::STATE_DEFAULT), state_count_(0) { + } + + /// @brief Constructor + /// + /// @param subnet_id The subnet id to which this data applies + /// @param lease_state The lease state counted + /// @param state_count The count of leases in the lease state + AddressStatsRow4(const SubnetID& subnet_id, + const Lease::LeaseState& lease_state, + const int64_t state_count) + : subnet_id_(subnet_id), lease_state_(lease_state), + state_count_(state_count) { + } + + /// @brief The subnet ID to which this data applies + SubnetID subnet_id_; + /// @brief The lease_state to which the count applies + uint32_t lease_state_; + /// @brief state_count The count of leases in the lease state + int64_t state_count_; +}; + +/// @brief Base class for fulfilling IPv4 statistical lease data query +/// +/// LeaseMgr derivations implement this class such that it provides +/// upto date IPv4 statistical lease data organized as rows of +/// AddressStatsRow4 instances. The rows must be accessible in +/// ascending order by subnet id. +class AddressStatsQuery4 { +public: + /// @brief Default constructor + AddressStatsQuery4() {}; + + /// @brief virtual destructor + virtual ~AddressStatsQuery4() {}; + + /// @brief Executes the query + /// + /// This method should conduct whatever steps are required to + /// calculate the IPv4 lease statistical data by examining the + /// IPv4 lease data and making that results available row by row. + virtual void start() {}; + + /// @brief Fetches the next row of data + /// + /// @param[out] row Storage into which the row is fetched + /// + /// @return True if a row was fetched, false if there are no + /// more rows. + virtual bool getNextRow(AddressStatsRow4& row) { return(false); }; +}; + +/// @brief Defines a pointer to an AddressStatsQuery4. +typedef boost::shared_ptr AddressStatsQuery4Ptr; + /// @brief Abstract Lease Manager /// /// This is an abstract API for lease database backends. It provides unified @@ -397,6 +461,36 @@ public: /// @return Number of leases deleted. virtual uint64_t deleteExpiredReclaimedLeases6(const uint32_t secs) = 0; + /// @brief Recalculates per-subnet and global stats for IPv4 leases + /// + /// This method recalculates the following statistics: + /// per-subnet: + /// - assigned-addresses + /// - declined-addresses + /// - declined-reclaimed-addresses (reset to zero) + /// global: + /// - declined-addresses + /// - declined-reclaimed-addresses (reset to zero) + /// + /// It invokes the virtual method, startAddressStatsQuery4(), which + /// returns an instance of an AddressStats4Qry. The query + /// query contains a "result set" where each row is an AddressStatRow4 + /// that contains a subnet id, a lease state, the number of leases in that + /// state and is ordered by subnet id. The method iterates over the + /// result set rows, setting the appropriate statistic per subnet and + /// adding to the approporate global statistic. + void recountAddressStats4(); + + /// @brief Virtual method which creates and runs the IPv4 lease stats query + /// + /// LeaseMgr derivations implement this method such that it creates and + /// returns an instance of an AddressStatsQuery whose result set has been + /// populated with upto date IPv4 lease statistical data. Each row of the + /// result set is an AddressStatRow4 which ordered ascending by subnet ID. + /// + /// @return A populated AddressStatsQuery4 + virtual AddressStatsQuery4Ptr startAddressStatsQuery4(); + /// @brief Return backend type /// /// Returns the type of the backend (e.g. "mysql", "memfile" etc.) diff --git a/src/lib/dhcpsrv/lease_mgr_factory.cc b/src/lib/dhcpsrv/lease_mgr_factory.cc index 5f2f33a4f3..a9f1507b2b 100644 --- a/src/lib/dhcpsrv/lease_mgr_factory.cc +++ b/src/lib/dhcpsrv/lease_mgr_factory.cc @@ -101,6 +101,11 @@ LeaseMgrFactory::destroy() { getLeaseMgrPtr().reset(); } +bool +LeaseMgrFactory::haveInstance() { + return (getLeaseMgrPtr().get()); +} + LeaseMgr& LeaseMgrFactory::instance() { LeaseMgr* lmptr = getLeaseMgrPtr().get(); diff --git a/src/lib/dhcpsrv/lease_mgr_factory.h b/src/lib/dhcpsrv/lease_mgr_factory.h index 279058bed8..4c6baf1270 100644 --- a/src/lib/dhcpsrv/lease_mgr_factory.h +++ b/src/lib/dhcpsrv/lease_mgr_factory.h @@ -85,7 +85,10 @@ public: /// create() to create one before calling this method. static LeaseMgr& instance(); - + /// @brief Indicates if the lease manager has been instantiated. + /// + /// @return True if the lease manager instance exists, false otherwise. + static bool haveInstance(); private: /// @brief Hold pointer to lease manager diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.cc b/src/lib/dhcpsrv/memfile_lease_mgr.cc index 51314b4cd8..1ad41cd1c2 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.cc +++ b/src/lib/dhcpsrv/memfile_lease_mgr.cc @@ -256,6 +256,143 @@ LFCSetup::getExitStatus() const { return (process_->getExitStatus(pid_)); } + +/// @brief Memfile derivation of the IPv4 statistical lease data query +/// +/// This class is used to recalculate IPv4 lease statistics for Memfile +/// lease storage. It does so by iterating over the given storage, +/// accumulating counts of leases in each of the monitored lease states +/// for each subnet and storing these counts in an internal collection. +/// The populated result set will contain one entry per monitored state +/// per subnet. +/// +class MemfileAddressStatsQuery4 : public AddressStatsQuery4 { +public: + /// @brief Constructor + /// + /// @param storage4 A pointer to the v4 lease storage to be counted + MemfileAddressStatsQuery4(Lease4Storage& storage4); + + /// @brief Destructor + virtual ~MemfileAddressStatsQuery4() {}; + + /// @brief Creates the IPv4 lease statistical data result set + /// + /// The result is populated by iterating over the IPv4 leases in storage, + /// in ascending order by subnet ID, accumulating the lease state counts. + /// At the completion of all entries for a given subnet, the counts are + /// used to create AddressStatsRow4 instances which are appended to an + /// internal vector. The process results in a vector containing one entry + /// per state per subnet. + /// + /// Currently the states counted are: + /// + /// - Lease::STATE_DEFAULT (i.e. assigned) + /// - Lease::STATE_DECLINED + virtual void start(); + + /// @brief Fetches the next row in the result set + /// + /// Once the internal result set has been populated by invoking the + /// the start() method, this method is used to iterate over the + /// result set rows. Once the last row has been fetched, subsequent + /// calls will return false. + /// @param row Storage for the fetched row + /// + /// @return True if the fetch succeeded, false if there are no more + /// rows to fetch. + virtual bool getNextRow(AddressStatsRow4& row); + + /// @brief Returns the number of rows in the result set + /// @todo, should this be a virtual member of the base class? + int getRowCount(); + +private: + /// @brief The Memfile storage containing the IPv4 leases to analyze + Lease4Storage& storage4_; + + /// @brief A vector containing the "result set" + std::vector rows_; + + /// @brief An iterator for accessing the next row within the result set + std::vector::iterator next_pos_; +}; + +MemfileAddressStatsQuery4::MemfileAddressStatsQuery4(Lease4Storage& storage4) + : storage4_(storage4), rows_(0), next_pos_(rows_.end()) {}; + +void +MemfileAddressStatsQuery4::start() { + // Get the subnet_id index + const Lease4StorageSubnetIdIndex& idx = storage4_.get(); + + // Iterate over the leases in order by subnet, accumulating per + // subnet counts for each state of interest. As we finish each + // subnet, add the appropriate rows to our result set. + SubnetID cur_id = 0; + int64_t assigned = 0; + int64_t declined = 0; + for(Lease4StorageSubnetIdIndex::const_iterator lease = idx.begin(); + lease != idx.end(); ++lease) { + + // If we've hit the next subnet, add rows for the current subnet + // and wipe the accumulators + if ((*lease)->subnet_id_ > cur_id) { + if (cur_id > 0) { + rows_.push_back(AddressStatsRow4(cur_id,Lease::STATE_DEFAULT, + assigned)); + assigned = 0; + rows_.push_back(AddressStatsRow4(cur_id, Lease::STATE_DECLINED, + declined)); + declined = 0; + } + + // Update current subnet id + cur_id = (*lease)->subnet_id_; + } + + // Bump the appropriate accumulator + switch ((*lease)->state_) { + case Lease::STATE_DEFAULT: + ++assigned; + break; + case Lease::STATE_DECLINED: + ++declined; + break; + default: + // Not one we're tracking. + break; + } + } + + // Make the rows for last subnet, unless there were no rows + if (idx.begin() != idx.end()) { + rows_.push_back(AddressStatsRow4(cur_id, Lease::STATE_DEFAULT, + assigned)); + rows_.push_back(AddressStatsRow4(cur_id, Lease::STATE_DECLINED, + declined)); + } + + // Set the next row position to the beginning of the rows. + next_pos_ = rows_.begin(); +} + +bool +MemfileAddressStatsQuery4::getNextRow(AddressStatsRow4& row) { + if (next_pos_ == rows_.end()) { + return (false); + } + + row = *next_pos_; + ++next_pos_; + return (true); +} + +int +MemfileAddressStatsQuery4::getRowCount() { + return (rows_.size()); +} + // Explicit definition of class static constants. Values are given in the // declaration so they're not needed here. const int Memfile_LeaseMgr::MAJOR_VERSION; @@ -299,6 +436,7 @@ Memfile_LeaseMgr::Memfile_LeaseMgr(const DatabaseConnection::ParameterMap& param } lfcSetup(conversion_needed); } + } Memfile_LeaseMgr::~Memfile_LeaseMgr() { @@ -1048,5 +1186,12 @@ void Memfile_LeaseMgr::lfcExecute(boost::shared_ptr& lease_file) } } +AddressStatsQuery4Ptr +Memfile_LeaseMgr::startAddressStatsQuery4() { + AddressStatsQuery4Ptr query(new MemfileAddressStatsQuery4(storage4_)); + query->start(); + return(query); +} + } // end of namespace isc::dhcp } // end of namespace isc diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.h b/src/lib/dhcpsrv/memfile_lease_mgr.h index 1ace364cf7..8bc4c72b85 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.h +++ b/src/lib/dhcpsrv/memfile_lease_mgr.h @@ -92,6 +92,7 @@ public: /// @} + /// @brief Specifies universe (V4, V6) /// /// This enumeration is used by various functions in Memfile %Lease Manager, @@ -594,6 +595,14 @@ public: int getLFCExitStatus() const; //@} + /// @brief Creates and runs the IPv4 lease stats query + /// + /// It creates an instance of a MemfileAddressStatsQuery4 and then + /// invokes it's start method in which the query constructs its + /// statistical data result set. The query object is then returned. + /// + /// @return The populated query as a pointer to an AddressStatsQuery4 + virtual AddressStatsQuery4Ptr startAddressStatsQuery4(); /// @name Protected methods used for %Lease File Cleanup. /// The following methods are protected so as they can be accessed and diff --git a/src/lib/dhcpsrv/memfile_lease_storage.h b/src/lib/dhcpsrv/memfile_lease_storage.h index 566a74d7c5..fba499672f 100644 --- a/src/lib/dhcpsrv/memfile_lease_storage.h +++ b/src/lib/dhcpsrv/memfile_lease_storage.h @@ -1,4 +1,4 @@ -// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -27,6 +27,9 @@ namespace dhcp { /// @brief Tag for indexes by address. struct AddressIndexTag { }; +/// @brief Tag for indexes by subnet id. +struct SubnetIdIndexTag { }; + /// @brief Tag for indexes by DUID, IAID, lease type tuple. struct DuidIaidTypeIndexTag { }; @@ -135,6 +138,15 @@ typedef boost::multi_index_container< boost::multi_index::member >, + boost::multi_index::ordered_non_unique< + boost::multi_index::tag, + // The subnet id is held in the subnet_id_ member of Lease4 + // class. Note that the subnet_id_ is defined in the base + // class (Lease) so we have to point to this class rather + // than derived class: Lease4. + boost::multi_index::member + >, + // Specification of the second index starts here. boost::multi_index::ordered_non_unique< boost::multi_index::tag, @@ -232,6 +244,9 @@ typedef Lease6Storage::index::type Lease6StorageExpirationIn /// @brief DHCPv4 lease storage index by address. typedef Lease4Storage::index::type Lease4StorageAddressIndex; +/// @brief DHCPv4 lease storage index by subnet id. +typedef Lease4Storage::index::type Lease4StorageSubnetIdIndex; + /// @brief DHCPv4 lease storage index by exiration time. typedef Lease4Storage::index::type Lease4StorageExpirationIndex; diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc index 410880af36..87925d1f52 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.cc +++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc @@ -205,6 +205,9 @@ TaggedStatement tagged_statements[] = { "hostname = ?, hwaddr = ?, hwtype = ?, hwaddr_source = ?, " "state = ? " "WHERE address = ?"}, + {MySqlLeaseMgr::RECOUNT_LEASE4_STATS, + "SELECT subnet_id, state, count(state) as state_count " + "FROM lease4 group by subnet_id, state"}, // End of list sentinel {MySqlLeaseMgr::NUM_STATEMENTS, NULL} }; @@ -1214,6 +1217,143 @@ private: uint32_t state_; ///< Lease state. }; +/// @brief MySql derivation of the IPv4 statistical lease data query +/// +/// This class is used to recalculate IPv4 lease statistics for MySQL +/// lease storage. It does so by executing a query which returns a result +/// containining contain one row per monitored state per subnet, ordered +/// by subnet id in ascending order. +/// +class MySqlAddressStatsQuery4 : public AddressStatsQuery4 { +public: + /// @brief Constructor + /// + /// @param conn A open connection to the database housing the lease data + MySqlAddressStatsQuery4(MySqlConnection& conn); + + /// @brief Destructor + virtual ~MySqlAddressStatsQuery4(); + + /// @brief Creates the IPv4 lease statistical data result set + /// + /// The result set is populated by executing an SQL query against the + /// lease4 table which sums the leases per lease state per subnet id. + /// The query used is the prepared statement identified by + /// MySqlLeaseMgr::RECOUNT_LEASE4_STATS. This method creates the binds + /// the statement to the output bind array and then executes the + /// statement. + void start(); + + /// @brief Fetches the next row in the result set + /// + /// Once the internal result set has been populated by invoking the + /// the start() method, this method is used to iterate over the + /// result set rows. Once the last row has been fetched, subsequent + /// calls will return false. + /// + /// @param row Storage for the fetched row + /// + /// @return True if the fetch succeeded, false if there are no more + /// rows to fetch. + bool getNextRow(AddressStatsRow4& row); + +private: + + /// @brief Analyzes the given statement outcome status + /// + /// Wrapper method around the MySqlConnection:checkError() that is + /// used to generate the appropriate exception if the status indicates + /// an error. + //// + /// a DbOperation error + /// @param status The MySQL statement execution outcome status + /// @param what invocation context message which will be included in + /// any exception + void checkError(int status, const char* what) const; + + /// @brief Database connection to use to execute the query + MySqlConnection& conn_; + + /// @brief The query's prepared statement + MYSQL_STMT *statement_; + + /// @brief Bind array used to store the query result set; + std::vector bind_; + + /// @brief Member struct that is bound to the statement; + AddressStatsRow4 stat_row_; +}; + +MySqlAddressStatsQuery4::MySqlAddressStatsQuery4(MySqlConnection& conn) + : conn_(conn), statement_(conn_.statements_[MySqlLeaseMgr + ::RECOUNT_LEASE4_STATS]), + bind_(3) { +} + +MySqlAddressStatsQuery4::~MySqlAddressStatsQuery4() { + (void) mysql_stmt_free_result(statement_); +} + + +void +MySqlAddressStatsQuery4::start() { + // subnet_id: unsigned int + bind_[0].buffer_type = MYSQL_TYPE_LONG; + bind_[0].buffer = reinterpret_cast(&stat_row_.subnet_id_); + bind_[0].is_unsigned = MLM_TRUE; + + // state: uint32_t + bind_[1].buffer_type = MYSQL_TYPE_LONG; + bind_[1].buffer = reinterpret_cast(&stat_row_.lease_state_); + bind_[1].is_unsigned = MLM_TRUE; + + // state_count_: uint32_t + bind_[2].buffer_type = MYSQL_TYPE_LONG; + bind_[2].buffer = reinterpret_cast(&stat_row_.state_count_); + bind_[2].is_unsigned = MLM_TRUE; + + // Set up the MYSQL_BIND array for the data being returned + // and bind it to the statement. + int status = mysql_stmt_bind_result(statement_, &bind_[0]); + checkError(status, "RECOUNT_LEASE4_STATS: outbound binding failed"); + + // Execute the statement + status = mysql_stmt_execute(statement_); + checkError(status, "RECOUNT_LEASE4_STATS: unable to execute"); + + // Ensure that all the lease information is retrieved in one go to avoid + // overhead of going back and forth between client and server. + status = mysql_stmt_store_result(statement_); + checkError(status, "RECOUNT_LEASE4_STATS: results storage setup failed"); +} + +bool +MySqlAddressStatsQuery4::getNextRow(AddressStatsRow4& row) { + bool have_row = false; + int status = mysql_stmt_fetch(statement_); + if (status == MLM_MYSQL_FETCH_SUCCESS) { + row = stat_row_; + have_row = true; + } else if (status != MYSQL_NO_DATA) { + checkError(status, "RECOUNT_LEASE4_STATS: getNextRow failed"); + } + + return (have_row); +} + +void +MySqlAddressStatsQuery4::checkError(int status, const char* what) const { + conn_.checkError(status, MySqlLeaseMgr::RECOUNT_LEASE4_STATS, what); +} + +AddressStatsQuery4Ptr +MySqlLeaseMgr::startAddressStatsQuery4() { + AddressStatsQuery4Ptr query(new MySqlAddressStatsQuery4(conn_)); + query->start(); + return(query); +} + + @@ -2025,7 +2165,6 @@ MySqlLeaseMgr::getVersion() const { return (std::make_pair(major, minor)); } - void MySqlLeaseMgr::commit() { LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, DHCPSRV_MYSQL_COMMIT); diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.h b/src/lib/dhcpsrv/mysql_lease_mgr.h index a5824becbb..b65c52a31b 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.h +++ b/src/lib/dhcpsrv/mysql_lease_mgr.h @@ -25,7 +25,6 @@ namespace dhcp { class MySqlLease4Exchange; class MySqlLease6Exchange; - /// @brief MySQL Lease Manager /// /// This class provides the \ref isc::dhcp::LeaseMgr interface to the MySQL @@ -410,6 +409,7 @@ public: INSERT_LEASE6, // Add entry to lease6 table UPDATE_LEASE4, // Update a Lease4 entry UPDATE_LEASE6, // Update a Lease6 entry + RECOUNT_LEASE4_STATS, // Fetches address statisics NUM_STATEMENTS // Number of statements }; @@ -590,6 +590,15 @@ private: uint64_t deleteExpiredReclaimedLeasesCommon(const uint32_t secs, StatementIndex statement_index); + /// @brief Creates and runs the IPv4 lease stats query + /// + /// It creates an instance of a MySqlAddressStatsQuery4 and then + /// invokes its start method, which fetches its statistical data + /// result set by executing the RECOUNT_LEASE_STATS4 query. + /// The query object is then returned. + /// + /// @return The populated query as a pointer to an AddressStatsQuery4 + virtual AddressStatsQuery4Ptr startAddressStatsQuery4(); /// @brief Check Error and Throw Exception /// diff --git a/src/lib/dhcpsrv/tests/alloc_engine_utils.cc b/src/lib/dhcpsrv/tests/alloc_engine_utils.cc index 8f86f6a5e9..9beb306609 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine_utils.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine_utils.cc @@ -123,6 +123,8 @@ AllocEngine4Test::generateDeclinedLease(const std::string& addr, AllocEngine6Test::AllocEngine6Test() { CfgMgr::instance().clear(); + factory_.create("type=memfile universe=6 persist=false"); + duid_ = DuidPtr(new DUID(std::vector(8, 0x42))); iaid_ = 42; @@ -141,7 +143,6 @@ AllocEngine6Test::AllocEngine6Test() { initFqdn("", false, false); - factory_.create("type=memfile universe=6 persist=false"); } void @@ -525,6 +526,9 @@ AllocEngine4Test::initSubnet(const asiolink::IOAddress& pool_start, } AllocEngine4Test::AllocEngine4Test() { + + factory_.create("type=memfile universe=4 persist=false"); + // Create fresh instance of the HostMgr, and drop any previous HostMgr state. HostMgr::instance().create(); @@ -548,7 +552,6 @@ AllocEngine4Test::AllocEngine4Test() { initSubnet(IOAddress("192.0.2.100"), IOAddress("192.0.2.109")); cfg_mgr.commit(); - factory_.create("type=memfile universe=4 persist=false"); // Create a default context. Note that remaining parameters must be // assigned when needed. diff --git a/src/lib/dhcpsrv/tests/cfg_db_access_unittest.cc b/src/lib/dhcpsrv/tests/cfg_db_access_unittest.cc index 49d3a378ff..25e7c053c3 100644 --- a/src/lib/dhcpsrv/tests/cfg_db_access_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfg_db_access_unittest.cc @@ -88,6 +88,7 @@ public: /// @brief Destructor. virtual ~CfgMySQLDbAccessTest() { destroyMySQLSchema(); + LeaseMgrFactory::destroy(); } }; diff --git a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc index a94a3a2b5c..d8ced0e5f2 100644 --- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc @@ -5,11 +5,18 @@ // file, You can obtain one at http://mozilla.org/MPL/2.0/. #include + +#include +#include +#include #include #include -#include -#include +#include + +#include + #include + #include using namespace std; @@ -57,6 +64,7 @@ GenericLeaseMgrTest::GenericLeaseMgrTest() /// a template leasetype6_.push_back(LEASETYPE6[i]); } + } GenericLeaseMgrTest::~GenericLeaseMgrTest() { @@ -2381,6 +2389,169 @@ GenericLeaseMgrTest::testGetDeclinedLeases6() { } } +void +GenericLeaseMgrTest::checkStat(const std::string& name, + const int64_t expected_value) { + stats::ObservationPtr obs = + stats::StatsMgr::instance().getObservation(name); + + ASSERT_TRUE(obs) << " stat: " << name << " not found "; + ASSERT_EQ(expected_value, obs->getInteger().first) + << " stat: " << name << " value wrong"; +} + +void +GenericLeaseMgrTest::checkAddressStats4(const StatValMapList& expectedStats) { + // Global accumulators + int64_t declined_addresses = 0; + int64_t declined_reclaimed_addresses = 0; + +#if 0 + isc::data::ConstElementPtr allstats = stats::StatsMgr::instance().getAll(); + std::cout << "ALL: "; + allstats->toJSON(std::cout); + std::cout << std::endl; +#endif + + // Iterate over all stats for each subnet + for (int subnet_idx = 0; subnet_idx < expectedStats.size(); ++subnet_idx) { + BOOST_FOREACH(StatValPair expectedStat, expectedStats[subnet_idx]) { + // Verify the per subnet value. + checkStat(stats::StatsMgr::generateName("subnet", subnet_idx+1, + expectedStat.first), + expectedStat.second); + + // Add the value to globals as needed. + if (expectedStat.first == "declined-addresses") { + declined_addresses += expectedStat.second; + } else if (expectedStat.first == "declined-reclaimed-addresses") { + declined_reclaimed_addresses += expectedStat.second; + } + } + } + + // Verify the globals. + checkStat("declined-addresses", declined_addresses); + checkStat("declined-reclaimed-addresses", declined_reclaimed_addresses); +} + +void +GenericLeaseMgrTest::makeLease4(const std::string& address, + const SubnetID& subnet_id, + const Lease::LeaseState& state) { + Lease4Ptr lease(new Lease4()); + + // set the address + lease->addr_ = IOAddress(address); + + // make a MAC from the address + std::vector hwaddr = lease->addr_.toBytes(); + hwaddr.push_back(0); + hwaddr.push_back(0); + + lease->hwaddr_.reset(new HWAddr(hwaddr, HTYPE_ETHER)); + lease->valid_lft_ = 86400; + lease->cltt_ = 168256; + lease->subnet_id_ = subnet_id; + lease->state_ = state; + ASSERT_TRUE(lmptr_->addLease(lease)); +} + +void +GenericLeaseMgrTest::testRecountAddressStats4() { + using namespace stats; + + StatsMgr::instance().removeAll(); + + // create subnets + CfgSubnets4Ptr cfg = + CfgMgr::instance().getStagingCfg()->getCfgSubnets4(); + + // Create 3 subnets. + Subnet4Ptr subnet; + Pool4Ptr pool; + + subnet.reset(new Subnet4(IOAddress("192.0.1.0"), 24, 1, 2, 3, 1)); + pool.reset(new Pool4(IOAddress("192.0.1.0"), 24)); + subnet->addPool(pool); + cfg->add(subnet); + + subnet.reset(new Subnet4(IOAddress("192.0.2.0"), 24, 1, 2, 3, 2)); + pool.reset(new Pool4(IOAddress("192.0.2.0"), 24)); + subnet->addPool(pool); + cfg->add(subnet); + + int num_subnets = 2; + + ASSERT_NO_THROW(CfgMgr::instance().commit()); + + // Create the expected stats list. At this point, the only stat + // that should be non-zero is total-addresses. + StatValMapList expectedStats(num_subnets); + for (int i = 0; i < num_subnets; ++i) { + expectedStats[i]["total-addresses"] = 256; + expectedStats[i]["assigned-addresses"] = 0; + expectedStats[i]["declined-addresses"] = 0; + expectedStats[i]["declined-reclaimed-addresses"] = 0; + } + + // Make sure stats are as expected. + ASSERT_NO_FATAL_FAILURE(checkAddressStats4(expectedStats)); + + // Recount stats. We should have the same results. + ASSERT_NO_THROW(lmptr_->recountAddressStats4()); + + // Make sure stats are as expected. + ASSERT_NO_FATAL_FAILURE(checkAddressStats4(expectedStats)); + + // Now let's insert some leases into subnet 1. + int subnet_id = 1; + + // Insert one lease in default state, i.e. assigned. + makeLease4("192.0.1.1", subnet_id); + + // Insert one lease in declined state. + makeLease4("192.0.1.2", subnet_id, Lease::STATE_DECLINED); + + // Insert one lease in the expired state. + makeLease4("192.0.1.3", subnet_id, Lease::STATE_EXPIRED_RECLAIMED); + + // Insert another lease in default state, i.e. assigned. + makeLease4("192.0.1.4", subnet_id); + + // Update the expected stats list for subnet 1. + expectedStats[subnet_id - 1]["assigned-addresses"] = 2; + expectedStats[subnet_id - 1]["declined-addresses"] = 1; + + // Now let's add leases to subnet 2. + subnet_id = 2; + + // Insert one delined lease. + makeLease4("192.0.2.2", subnet_id, Lease::STATE_DECLINED); + + // Update the expected stats. + expectedStats[subnet_id - 1]["declined-addresses"] = 1; + + // Now Recount the stats. + ASSERT_NO_THROW(lmptr_->recountAddressStats4()); + + // Make sure stats are as expected. + ASSERT_NO_FATAL_FAILURE(checkAddressStats4(expectedStats)); + + // Delete some leases from subnet, and update the expected stats. + EXPECT_TRUE(lmptr_->deleteLease(IOAddress("192.0.1.1"))); + expectedStats[0]["assigned-addresses"] = 1; + + EXPECT_TRUE(lmptr_->deleteLease(IOAddress("192.0.1.2"))); + expectedStats[0]["declined-addresses"] = 0; + + // Recount the stats. + ASSERT_NO_THROW(lmptr_->recountAddressStats4()); + + // Make sure stats are as expected. + ASSERT_NO_FATAL_FAILURE(checkAddressStats4(expectedStats)); +} + }; // namespace test }; // namespace dhcp }; // namespace isc diff --git a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h index 3ef1405503..f646412db3 100644 --- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h +++ b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-2016 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -15,6 +15,12 @@ namespace isc { namespace dhcp { namespace test { + +/// @brief typedefs to simplify lease statistic testing +typedef std::map StatValMap; +typedef std::pair StatValPair; +typedef std::vector StatValMapList; + /// @brief Test Fixture class with utility functions for LeaseMgr backends /// /// It contains utility functions, like dummy lease creation. @@ -94,6 +100,34 @@ public: /// @return vector Vector of pointers to leases std::vector createLeases6(); + /// @brief Compares a StatsMgr statistic to an expected value + /// + /// Attempt to fetch the named statistic from the StatsMg and if + /// found, compare its observed value to the given value. + /// Fails if the stat is not found or if the values do not match. + /// + /// @param name StatsMgr name for the statistic to check + /// @param expected_value expected value of the statistic + void checkStat(const std::string& name, const int64_t expected_value); + + /// @brief Compares StatsMgr statistics against an expected list of values + /// + /// Iterates over a list of statistic names and expectec values, attempting + /// to fetch each from the StatsMgr and if found, compare its observed value + /// to the expected value. Fails any the stat is not found or if the values + /// do not match. + /// + /// @param expected_stats Map of expected static names and values. + void checkAddressStats4(const StatValMapList& expected_stats); + + /// @brief Constructs a minimal IPv4 lease and adds it to the lease storage + /// + /// @param address - IPv4 address for the lease + /// @param subnet_id - subnet ID to which the lease belongs + /// @param state - the state of the lease + void makeLease4(const std::string& address, const SubnetID& subnet_id, + const Lease::LeaseState& state = Lease::STATE_DEFAULT); + /// @brief checks that addLease, getLease4(addr) and deleteLease() works void testBasicLease4(); @@ -313,6 +347,13 @@ public: /// leases can be removed. void testDeleteExpiredReclaimedLeases4(); + /// @brief Check that the IPv4 lease statistics can be recounted + /// + /// This test creates two subnets and several leases associated with + /// them, then verifies that lease statistics are recalculated correctly + /// after altering the lease states in various ways. + void testRecountAddressStats4(); + /// @brief String forms of IPv4 addresses std::vector straddress4_; diff --git a/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc index a01a54457c..3cf45d58d6 100644 --- a/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc @@ -1883,6 +1883,10 @@ TEST_F(MemfileLeaseMgrTest, lease6ContainerIndexUpdate) { } } - +// Verifies that IPv4 lease statistics can be recalculated. +TEST_F(MemfileLeaseMgrTest, recountAddressStats4) { + startBackend(V4); + testRecountAddressStats4(); +} }; // end of anonymous namespace diff --git a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc index 33549b415a..bbe80f9b60 100644 --- a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc @@ -477,4 +477,9 @@ TEST_F(MySqlLeaseMgrTest, deleteExpiredReclaimedLeases4) { testDeleteExpiredReclaimedLeases4(); } +// Verifies that IPv4 lease statistics can be recalculated. +TEST_F(MySqlLeaseMgrTest, recountAddressStats4) { + testRecountAddressStats4(); +} + }; // Of anonymous namespace -- cgit v1.2.3 From 64c23c76ca53a29be709f4eade7d139782a0603e Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Mon, 15 Aug 2016 10:25:25 -0400 Subject: [4294] PostgreSQL now supports IPv4 lease stats recount src/lib/dhcpsrv/cfg_subnets4.cc CfgSubnets4::updateStatistics() - removed lease mgr instance check src/lib/dhcpsrv/mysql_lease_mgr.cc Added "ORDER by subnet_id" to RECOUNT_LEASE4_STATS src/lib/dhcpsrv/pgsql_lease_mgr.cc Added tagged statement RECOUNT_LEASE4_STATS PgSqlAddressStatsQuery4 - new class, PostgreSQL derivation of AddressStatsQuery4 PgSqlLeaseMgr::startAddressStatsQuery4() - PostgreSQL impl of virtual method src/lib/dhcpsrv/srv_config.cc - SrvConfig::updateStatistics() - Added LeaseMgr singleton check around calls subnet statistics updates src/lib/dhcpsrv/tests/cfgmgr_unittest.cc CfgMgrTest: ~CfgMgrTest() - now destroys LeaseMgr singleton startBackend(int family = AF_INET) - new method to create memfile lease mgr TEST_F(CfgMgrTest, commitStats4) TEST_F(CfgMgrTest, commitStats6) - added call to startBackend() src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc TEST_F(PgSqlLeaseMgrTest, recountAddressStats4) - new test --- src/lib/dhcpsrv/cfg_subnets4.cc | 4 +- src/lib/dhcpsrv/mysql_lease_mgr.cc | 2 +- src/lib/dhcpsrv/pgsql_lease_mgr.cc | 124 +++++++++++++++++++++- src/lib/dhcpsrv/pgsql_lease_mgr.h | 11 ++ src/lib/dhcpsrv/srv_config.cc | 17 ++- src/lib/dhcpsrv/tests/cfgmgr_unittest.cc | 20 ++++ src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc | 6 ++ 7 files changed, 175 insertions(+), 9 deletions(-) diff --git a/src/lib/dhcpsrv/cfg_subnets4.cc b/src/lib/dhcpsrv/cfg_subnets4.cc index 43b1883053..f33b40e958 100644 --- a/src/lib/dhcpsrv/cfg_subnets4.cc +++ b/src/lib/dhcpsrv/cfg_subnets4.cc @@ -267,8 +267,8 @@ CfgSubnets4::updateStatistics() { TYPE_V4))); } - // If we have subnets and a lease mgr, recount the least statistics - if (subnets_.begin() != subnets_.end() && LeaseMgrFactory::haveInstance()) { + // Only recount the stats if we have subnets. + if (subnets_.begin() != subnets_.end()) { LeaseMgrFactory::instance().recountAddressStats4(); } } diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc index 87925d1f52..5169ef073a 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.cc +++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc @@ -207,7 +207,7 @@ TaggedStatement tagged_statements[] = { "WHERE address = ?"}, {MySqlLeaseMgr::RECOUNT_LEASE4_STATS, "SELECT subnet_id, state, count(state) as state_count " - "FROM lease4 group by subnet_id, state"}, + "FROM lease4 GROUP BY subnet_id, state ORDER BY subnet_id"}, // End of list sentinel {MySqlLeaseMgr::NUM_STATEMENTS, NULL} }; diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.cc b/src/lib/dhcpsrv/pgsql_lease_mgr.cc index f9a5ad28eb..174237df70 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.cc +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.cc @@ -26,7 +26,7 @@ using namespace std; namespace { -/// @todo TKM lease6 needs to accomodate hwaddr,hwtype, and hwaddr source +/// @todo TKM lease6 needs to accomodate hwaddr,hwtype, and hwaddr source /// columns. This is coverd by tickets #3557, #4530, and PR#9. /// @brief Catalog of all the SQL statements currently supported. Note @@ -201,6 +201,12 @@ PgSqlTaggedStatement tagged_statements[] = { "state = $13 " "WHERE address = $14"}, + // RECOUNT_LEASE4_STATS, + { 0, { OID_NONE }, + "recount_lease4_stats", + "SELECT subnet_id, state, count(state) as state_count " + "FROM lease4 GROUP BY subnet_id, state ORDER BY subnet_id"}, + // End of list sentinel { 0, { 0 }, NULL, NULL} }; @@ -681,6 +687,122 @@ private: //@} }; +/// @brief PgSql derivation of the IPv4 statistical lease data query +/// +/// This class is used to recalculate IPv4 lease statistics for MySQL +/// lease storage. It does so by executing a query which returns a result +/// containining contain one row per monitored state per subnet, ordered +/// by subnet id in ascending order. +/// +class PgSqlAddressStatsQuery4 : public AddressStatsQuery4 { +public: + /// @brief Constructor + /// + /// @param conn A open connection to the database housing the lease data + PgSqlAddressStatsQuery4(PgSqlConnection& conn); + + /// @brief Destructor + virtual ~PgSqlAddressStatsQuery4() {}; + + /// @brief Creates the IPv4 lease statistical data result set + /// + /// The result set is populated by executing an SQL query against the + /// lease4 table which sums the leases per lease state per subnet id. + /// The query used is the prepared statement identified by + /// PgSqlLeaseMgr::RECOUNT_LEASE4_STATS. This method executes the + /// statement which creates the result set. + void start(); + + /// @brief Fetches the next row in the result set + /// + /// Once the internal result set has been populated by invoking the + /// the start() method, this method is used to iterate over the + /// result set rows. Once the last row has been fetched, subsequent + /// calls will return false. + /// + /// @param row Storage for the fetched row + /// + /// @return True if the fetch succeeded, false if there are no more + /// rows to fetch. + bool getNextRow(AddressStatsRow4& row); + +private: + + /// @brief Analyzes the given statement outcome status + /// + /// Wrapper method around the PgSqlConnection:checkError() that is + /// used to generate the appropriate exception if the status indicates + /// an error. + //// + /// a DbOperation error + /// @param status The MySQL statement execution outcome status + /// @param what invocation context message which will be included in + /// any exception + void checkError(int status, const char* what) const; + + /// @brief Database connection to use to execute the query + PgSqlConnection& conn_; + + /// @brief The query's prepared statement + PgSqlTaggedStatement& statement_; + + /// @brief The result set returned by Postgres. + boost::shared_ptr result_set_; + + /// @brief Index of the next row to fetch + uint32_t next_row_; +}; + +PgSqlAddressStatsQuery4::PgSqlAddressStatsQuery4(PgSqlConnection& conn) + : conn_(conn), statement_(tagged_statements[PgSqlLeaseMgr + ::RECOUNT_LEASE4_STATS]), + result_set_(), next_row_(0) { +} + +void +PgSqlAddressStatsQuery4::start() { + // The query has no parameters, so we only need it's name. + result_set_.reset(new PgSqlResult(PQexecPrepared(conn_, statement_.name, + 0, NULL, NULL, NULL, 0))); + + conn_.checkStatementError(*result_set_, statement_); +} + +bool +PgSqlAddressStatsQuery4::getNextRow(AddressStatsRow4& row) { + // If we're past the end, punt. + if (next_row_ >= result_set_->getRows()) { + return (false); + } + + // Fetch the subnet id. + uint32_t col = 0; + uint32_t subnet_id; + PgSqlExchange::getColumnValue(*result_set_, next_row_, 0, subnet_id); + row.subnet_id_ = static_cast(subnet_id); + + // Fetch the lease state. + uint32_t state; + PgSqlExchange::getColumnValue(*result_set_, next_row_ , 1, state); + row.lease_state_ = static_cast(state); + + // Fetch the state count. + PgSqlExchange::getColumnValue(*result_set_, next_row_, 2, row.state_count_); + + // Point to the next row. + ++next_row_; + + return (true); +} + +AddressStatsQuery4Ptr +PgSqlLeaseMgr::startAddressStatsQuery4() { + AddressStatsQuery4Ptr query(new PgSqlAddressStatsQuery4(conn_)); + query->start(); + return(query); +} + + PgSqlLeaseMgr::PgSqlLeaseMgr(const DatabaseConnection::ParameterMap& parameters) : LeaseMgr(), exchange4_(new PgSqlLease4Exchange()), exchange6_(new PgSqlLease6Exchange()), conn_(parameters) { diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.h b/src/lib/dhcpsrv/pgsql_lease_mgr.h index d316f1b9f0..9511c0e237 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.h +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.h @@ -317,6 +317,16 @@ public: /// @return Number of leases deleted. virtual uint64_t deleteExpiredReclaimedLeases6(const uint32_t secs); + /// @brief Creates and runs the IPv4 lease stats query + /// + /// It creates an instance of a PgSqlAddressStatsQuery4 and then + /// invokes its start method, which fetches its statistical data + /// result set by executing the RECOUNT_LEASE_STATS4 query. + /// The query object is then returned. + /// + /// @return The populated query as a pointer to an AddressStatsQuery4 + virtual AddressStatsQuery4Ptr startAddressStatsQuery4(); + /// @brief Return backend type /// /// Returns the type of the backend (e.g. "mysql", "memfile" etc.) @@ -385,6 +395,7 @@ public: INSERT_LEASE6, // Add entry to lease6 table UPDATE_LEASE4, // Update a Lease4 entry UPDATE_LEASE6, // Update a Lease6 entry + RECOUNT_LEASE4_STATS, // Fetch IPv4 lease statistical data NUM_STATEMENTS // Number of statements }; diff --git a/src/lib/dhcpsrv/srv_config.cc b/src/lib/dhcpsrv/srv_config.cc index 84e29bd6fc..7c7830966f 100644 --- a/src/lib/dhcpsrv/srv_config.cc +++ b/src/lib/dhcpsrv/srv_config.cc @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include // Needed for HWADDR_SOURCE_* @@ -164,11 +165,17 @@ SrvConfig::removeStatistics() { void SrvConfig::updateStatistics() { - - // Updates statistics for v4 and v6 subnets - getCfgSubnets4()->updateStatistics(); - - getCfgSubnets6()->updateStatistics(); + // Updating subnet statistics involves updating lease statistics, which + // is done by the LeaseMgr. Since servers with subnets, must have a + // LeaseMgr, we do not bother updating subnet stats for servers without + // a lease manager, such as D2. @todo We should probably examine why + // "SrvConfig" is being used by D2. + if (LeaseMgrFactory::haveInstance()) { + // Updates statistics for v4 and v6 subnets + getCfgSubnets4()->updateStatistics(); + + getCfgSubnets6()->updateStatistics(); + } } } diff --git a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc index c9edb88768..a6de68d7dd 100644 --- a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -280,6 +281,23 @@ public: void clear() { CfgMgr::instance().setVerbose(false); CfgMgr::instance().clear(); + LeaseMgrFactory::destroy(); + } + + /// @brief Creates instance of the backend. + /// + /// @param family AF_INET for v4, AF_INET6 for v6 + void startBackend(int family = AF_INET) { + try { + std::ostringstream s; + s << "type=memfile persist=false " << (family == AF_INET6 ? + "universe=6" : "universe=4"); + LeaseMgrFactory::create(s.str()); + } catch (const std::exception& ex) { + std::cerr << "*** ERROR: unable to create instance of the Memfile\n" + " lease database backend: " << ex.what() << std::endl; + throw; + } } /// used in client classification (or just empty container for other tests) @@ -575,6 +593,7 @@ TEST_F(CfgMgrTest, verbosity) { TEST_F(CfgMgrTest, commitStats4) { CfgMgr& cfg_mgr = CfgMgr::instance(); StatsMgr& stats_mgr = StatsMgr::instance(); + startBackend(AF_INET); // Let's prepare the "old" configuration: a subnet with id 123 // and pretend there were addresses assigned, so statistics are non-zero. @@ -641,6 +660,7 @@ TEST_F(CfgMgrTest, clearStats4) { TEST_F(CfgMgrTest, commitStats6) { CfgMgr& cfg_mgr = CfgMgr::instance(); StatsMgr& stats_mgr = StatsMgr::instance(); + startBackend(AF_INET6); // Let's prepare the "old" configuration: a subnet with id 123 // and pretend there were addresses assigned, so statistics are non-zero. diff --git a/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc index 9234881863..762d37c253 100644 --- a/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc @@ -402,4 +402,10 @@ TEST_F(PgSqlLeaseMgrTest, getExpiredLeases6) { testGetExpiredLeases6(); } +// Verifies that IPv4 lease statistics can be recalculated. +TEST_F(PgSqlLeaseMgrTest, recountAddressStats4) { + testRecountAddressStats4(); +} + + }; // namespace -- cgit v1.2.3 From 61bc7b004aefc683e239917a6afa717c14744456 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Mon, 15 Aug 2016 13:57:13 -0400 Subject: [4294] Added abstract support for IPv6 lease stats recount to LeaseMgr src/lib/dhcpsrv/lease_mgr.h src/lib/dhcpsrv/lease_mgr.cc AddressStatsRow6 - new struct that contains a single row of IPv6 lease statistical data AddressStatsQuery6 - new base class for fulfilling the IPv6 statistical lease data query LeaseMgr::recountAddressStats6() - new method that recalculates per-subnet and global stats for IPv6 leases LeaseMgr::startAddressStatsQuery6() - new virtual method which creates then executes the IPv6 lease stats query --- src/lib/dhcpsrv/lease_mgr.cc | 120 +++++++++++++++++++- src/lib/dhcpsrv/lease_mgr.h | 123 ++++++++++++++++++--- .../dhcpsrv/tests/generic_lease_mgr_unittest.cc | 7 -- 3 files changed, 224 insertions(+), 26 deletions(-) diff --git a/src/lib/dhcpsrv/lease_mgr.cc b/src/lib/dhcpsrv/lease_mgr.cc index 491c5a0010..341b285c78 100644 --- a/src/lib/dhcpsrv/lease_mgr.cc +++ b/src/lib/dhcpsrv/lease_mgr.cc @@ -47,7 +47,7 @@ LeaseMgr::getLease6(Lease::Type type, const DUID& duid, return (*col.begin()); } -void +void LeaseMgr::recountAddressStats4() { using namespace stats; @@ -68,8 +68,8 @@ LeaseMgr::recountAddressStats4() { stats_mgr.setValue("declined-reclaimed-addresses", zero); // Clear subnet level stats. This ensures we don't end up with corner - // cases that leave stale values in place. - const Subnet4Collection* subnets = + // cases that leave stale values in place. + const Subnet4Collection* subnets = CfgMgr::instance().getCurrentCfg()->getCfgSubnets4()->getAll(); for (Subnet4Collection::const_iterator subnet = subnets->begin(); @@ -86,15 +86,15 @@ LeaseMgr::recountAddressStats4() { "declined-reclaimed-addresses"), zero); } - + // Get counts per state per subnet. Iterate over the result set - // updating the subnet and global values. + // updating the subnet and global values. AddressStatsRow4 row; while (query->getNextRow(row)) { switch(row.lease_state_) { case Lease::STATE_DEFAULT: // Set subnet level value. - stats_mgr.setValue(StatsMgr::generateName("subnet", + stats_mgr.setValue(StatsMgr::generateName("subnet", row.subnet_id_, "assigned-addresses"), row.state_count_); @@ -123,6 +123,114 @@ LeaseMgr::startAddressStatsQuery4() { return(AddressStatsQuery4Ptr()); } +void +LeaseMgr::recountAddressStats6() { + using namespace stats; + + StatsMgr& stats_mgr = StatsMgr::instance(); + + AddressStatsQuery6Ptr query = startAddressStatsQuery6(); + if (!query) { + /// NULL means not backend does not support recounting. + return; + } + + // Zero out the global stats. (Ok, so currently there's only one + // that should be cleared. "reclaimed-declined-addresses" never + // gets zeroed. @todo discuss with Tomek the rational of not + // clearing it when we clear the rest. + int64_t zero = 0; + stats_mgr.setValue("declined-addresses", zero); + stats_mgr.setValue("declined-reclaimed-addresses", zero); + + // Clear subnet level stats. This ensures we don't end up with corner + // cases that leave stale values in place. + const Subnet6Collection* subnets = + CfgMgr::instance().getCurrentCfg()->getCfgSubnets6()->getAll(); + + for (Subnet6Collection::const_iterator subnet = subnets->begin(); + subnet != subnets->end(); ++subnet) { + SubnetID subnet_id = (*subnet)->getID(); + stats_mgr.setValue(StatsMgr::generateName("subnet", subnet_id, + "assigned-nas"), + zero); + + stats_mgr.setValue(StatsMgr::generateName("subnet", subnet_id, + "declined-nas"), + zero); + + stats_mgr.setValue(StatsMgr:: + generateName("subnet", subnet_id, + "declined-reclaimed-addresses"), + zero); + + stats_mgr.setValue(StatsMgr::generateName("subnet", subnet_id, + "assigned-pds"), + zero); + } + + // Get counts per state per subnet. Iterate over the result set + // updating the subnet and global values. + AddressStatsRow6 row; + while (query->getNextRow(row)) { + switch(row.lease_type_) { + case Lease::TYPE_NA: + switch(row.lease_state_) { + case Lease::STATE_DEFAULT: + // Set subnet level value. + stats_mgr.setValue(StatsMgr:: + generateName("subnet", + row.subnet_id_, + "assigned-nas"), + row.state_count_); + break; + case Lease::STATE_DECLINED: + // Set subnet level value. + stats_mgr.setValue(StatsMgr:: + generateName("subnet", + row.subnet_id_, + "declined-nas"), + row.state_count_); + + // Add to the global value. + stats_mgr.addValue("declined-addresses", + row.state_count_); + break; + default: + // Not one we're tracking. + break; + } + break; + + case Lease::TYPE_PD: + switch(row.lease_state_) { + case Lease::STATE_DEFAULT: + // Set subnet level value. + stats_mgr.setValue(StatsMgr:: + generateName("subnet", + row.subnet_id_, + "assigned-pds"), + row.state_count_); + break; + default: + // Not one we're tracking. + break; + } + break; + + default: + // We dont' support TYPE_TAs yet + break; + } + } +} + +AddressStatsQuery6Ptr +LeaseMgr::startAddressStatsQuery6() { + return(AddressStatsQuery6Ptr()); +} + + std::string LeaseMgr::getDBVersion() { isc_throw(NotImplemented, "LeaseMgr::getDBVersion() called"); diff --git a/src/lib/dhcpsrv/lease_mgr.h b/src/lib/dhcpsrv/lease_mgr.h index e8123e840f..140a7d1c72 100644 --- a/src/lib/dhcpsrv/lease_mgr.h +++ b/src/lib/dhcpsrv/lease_mgr.h @@ -153,7 +153,7 @@ public: /// and the number of leases in that state for that subnet ID. struct AddressStatsRow4 { /// @brief Default constructor - AddressStatsRow4() : + AddressStatsRow4() : subnet_id_(0), lease_state_(Lease::STATE_DEFAULT), state_count_(0) { } @@ -164,15 +164,15 @@ struct AddressStatsRow4 { /// @param state_count The count of leases in the lease state AddressStatsRow4(const SubnetID& subnet_id, const Lease::LeaseState& lease_state, - const int64_t state_count) - : subnet_id_(subnet_id), lease_state_(lease_state), + const int64_t state_count) + : subnet_id_(subnet_id), lease_state_(lease_state), state_count_(state_count) { } /// @brief The subnet ID to which this data applies SubnetID subnet_id_; /// @brief The lease_state to which the count applies - uint32_t lease_state_; + Lease::LeaseState lease_state_; /// @brief state_count The count of leases in the lease state int64_t state_count_; }; @@ -188,12 +188,12 @@ public: /// @brief Default constructor AddressStatsQuery4() {}; - /// @brief virtual destructor + /// @brief virtual destructor virtual ~AddressStatsQuery4() {}; /// @brief Executes the query /// - /// This method should conduct whatever steps are required to + /// This method should conduct whatever steps are required to /// calculate the IPv4 lease statistical data by examining the /// IPv4 lease data and making that results available row by row. virtual void start() {}; @@ -210,6 +210,72 @@ public: /// @brief Defines a pointer to an AddressStatsQuery4. typedef boost::shared_ptr AddressStatsQuery4Ptr; +/// @brief Contains a single row of IPv6 lease statistical data +/// +/// The contents of the row consist of a subnet ID, a lease state, +/// and the number of leases in that state for that subnet ID. +struct AddressStatsRow6 { + /// @brief Default constructor + AddressStatsRow6() : + subnet_id_(0), lease_type_(Lease::TYPE_NA), + lease_state_(Lease::STATE_DEFAULT), state_count_(0) { + } + + /// @brief Constructor + /// + /// @param subnet_id The subnet id to which this data applies + /// @param lease_state The lease state counted + /// @param state_count The count of leases in the lease state + AddressStatsRow6(const SubnetID& subnet_id, const Lease::Type& lease_type, + const Lease::LeaseState& lease_state, + const int64_t state_count) + : subnet_id_(subnet_id), lease_state_(lease_state), + state_count_(state_count) { + } + + /// @brief The subnet ID to which this data applies + SubnetID subnet_id_; + /// @brief The lease_state to which the count applies + Lease::Type lease_type_; + /// @brief The lease_state to which the count applies + uint32_t lease_state_; + /// @brief state_count The count of leases in the lease state + int64_t state_count_; +}; + +/// @brief Base class for fulfilling IPv6 statistical lease data query +/// +/// LeaseMgr derivations implement this class such that it provides +/// upto date IPv6 statistical lease data organized as rows of +/// AddressStatsRow6 instances. The rows must be accessible in +/// ascending order by subnet id. +class AddressStatsQuery6 { +public: + /// @brief Default constructor + AddressStatsQuery6() {}; + + /// @brief virtual destructor + virtual ~AddressStatsQuery6() {}; + + /// @brief Executes the query + /// + /// This method should conduct whatever steps are required to + /// calculate the IPv6 lease statistical data by examining the + /// IPv6 lease data and making that results available row by row. + virtual void start() {}; + + /// @brief Fetches the next row of data + /// + /// @param[out] row Storage into which the row is fetched + /// + /// @return True if a row was fetched, false if there are no + /// more rows. + virtual bool getNextRow(AddressStatsRow6& row) { return (false); }; +}; + +/// @brief Defines a pointer to an AddressStatsQuery6. +typedef boost::shared_ptr AddressStatsQuery6Ptr; + /// @brief Abstract Lease Manager /// /// This is an abstract API for lease database backends. It provides unified @@ -463,7 +529,7 @@ public: /// @brief Recalculates per-subnet and global stats for IPv4 leases /// - /// This method recalculates the following statistics: + /// This method recalculates the following statistics: /// per-subnet: /// - assigned-addresses /// - declined-addresses @@ -476,21 +542,52 @@ public: /// returns an instance of an AddressStats4Qry. The query /// query contains a "result set" where each row is an AddressStatRow4 /// that contains a subnet id, a lease state, the number of leases in that - /// state and is ordered by subnet id. The method iterates over the + /// state and is ordered by subnet id. The method iterates over the /// result set rows, setting the appropriate statistic per subnet and - /// adding to the approporate global statistic. + /// adding to the approporate global statistic. void recountAddressStats4(); - /// @brief Virtual method which creates and runs the IPv4 lease stats query + /// @brief Virtual method which creates and runs the IPv4 lease stats query /// /// LeaseMgr derivations implement this method such that it creates and - /// returns an instance of an AddressStatsQuery whose result set has been - /// populated with upto date IPv4 lease statistical data. Each row of the - /// result set is an AddressStatRow4 which ordered ascending by subnet ID. + /// returns an instance of an AddressStatsQuery whose result set has been + /// populated with upto date IPv4 lease statistical data. Each row of the + /// result set is an AddressStatRow4 which ordered ascending by subnet ID. /// /// @return A populated AddressStatsQuery4 virtual AddressStatsQuery4Ptr startAddressStatsQuery4(); + /// @brief Recalculates per-subnet and global stats for IPv6 leases + /// + /// This method recalculates the following statistics: + /// per-subnet: + /// - assigned-addresses + /// - declined-addresses + /// - declined-reclaimed-addresses (reset to zero) + /// - assigned-pds + /// global: + /// - declined-addresses + /// - declined-reclaimed-addresses (reset to zero) + /// + /// It invokes the virtual method, startAddressStatsQuery6(), which + /// returns an instance of an AddressStats6Qry. The query + /// query contains a "result set" where each row is an AddressStatRow6 + /// that contains a subnet id, a lease state, the number of leases in that + /// state and is ordered by subnet id. The method iterates over the + /// result set rows, setting the appropriate statistic per subnet and + /// adding to the approporate global statistic. + void recountAddressStats6(); + + /// @brief Virtual method which creates and runs the IPv6 lease stats query + /// + /// LeaseMgr derivations implement this method such that it creates and + /// returns an instance of an AddressStatsQuery whose result set has been + /// populated with upto date IPv6 lease statistical data. Each row of the + /// result set is an AddressStatRow6 which ordered ascending by subnet ID. + /// + /// @return A populated AddressStatsQuery6 + virtual AddressStatsQuery6Ptr startAddressStatsQuery6(); + /// @brief Return backend type /// /// Returns the type of the backend (e.g. "mysql", "memfile" etc.) diff --git a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc index d8ced0e5f2..aa3ff1a11b 100644 --- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc @@ -2406,13 +2406,6 @@ GenericLeaseMgrTest::checkAddressStats4(const StatValMapList& expectedStats) { int64_t declined_addresses = 0; int64_t declined_reclaimed_addresses = 0; -#if 0 - isc::data::ConstElementPtr allstats = stats::StatsMgr::instance().getAll(); - std::cout << "ALL: "; - allstats->toJSON(std::cout); - std::cout << std::endl; -#endif - // Iterate over all stats for each subnet for (int subnet_idx = 0; subnet_idx < expectedStats.size(); ++subnet_idx) { BOOST_FOREACH(StatValPair expectedStat, expectedStats[subnet_idx]) { -- cgit v1.2.3 From 6f56be5aa27dd88581a9b7cc919cf45d1c967417 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Tue, 16 Aug 2016 11:13:17 -0400 Subject: [4294] Unit tests and MySql now support IPv6 lease stat recounting src/lib/dhcpsrv/cfg_subnets6.cc - CfgSubnets6::removeStatistics() - added removal of declined stats - CfgSubnets6::updateStatistics() - added call to recountAddressStats6() src/lib/dhcpsrv/mysql_lease_mgr.h src/lib/dhcpsrv/mysql_lease_mgr.cc - Added TaggedStatement RECOUNT_LEASE6_STATS - MySqlAddressStatsQuery6 - new MySql derivation of AddressStatsQuery6 - MySqlLeaseMgr::startAddressStatsQuery6() - new virtual method which creates and starts a MySqlAddressStatsQuery6 src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc - GenericLeaseMgrTest::checkAddressStats4 renamed to checkAddressStats as it applies to either v4 or v6 - GenericLeaseMgrTest::makeLease6() - new method which creates a minimal IPv6 lease and adds it to lease storage - GenericLeaseMgrTest::testRecountAddressStats6() - new method which checks IPv6 lease stats recounting src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc - TEST_F(MySqlLeaseMgrTest, recountAddressStats6) - new test --- src/lib/dhcpsrv/cfg_subnets6.cc | 54 ++++--- src/lib/dhcpsrv/lease_mgr.cc | 4 +- src/lib/dhcpsrv/mysql_lease_mgr.cc | 157 ++++++++++++++++++++- src/lib/dhcpsrv/mysql_lease_mgr.h | 15 +- .../dhcpsrv/tests/generic_lease_mgr_unittest.cc | 154 +++++++++++++++++++- src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h | 22 ++- src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc | 5 + 7 files changed, 377 insertions(+), 34 deletions(-) diff --git a/src/lib/dhcpsrv/cfg_subnets6.cc b/src/lib/dhcpsrv/cfg_subnets6.cc index ad8ed4c6c6..e8e36b4f5d 100644 --- a/src/lib/dhcpsrv/cfg_subnets6.cc +++ b/src/lib/dhcpsrv/cfg_subnets6.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-2016 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -176,25 +177,26 @@ void CfgSubnets6::removeStatistics() { using namespace isc::stats; + StatsMgr& stats_mgr = StatsMgr::instance(); // For each v6 subnet currently configured, remove the statistics. for (Subnet6Collection::const_iterator subnet6 = subnets_.begin(); subnet6 != subnets_.end(); ++subnet6) { + SubnetID subnet_id = (*subnet6)->getID(); + stats_mgr.del(StatsMgr::generateName("subnet", subnet_id, "total-nas")); - StatsMgr::instance().del(StatsMgr::generateName("subnet", - (*subnet6)->getID(), - "total-nas")); + stats_mgr.del(StatsMgr::generateName("subnet", subnet_id, + "assigned-nas")); - StatsMgr::instance().del(StatsMgr::generateName("subnet", - (*subnet6)->getID(), - "assigned-nas")); + stats_mgr.del(StatsMgr::generateName("subnet", subnet_id, "total-pds")); - StatsMgr::instance().del(StatsMgr::generateName("subnet", - (*subnet6)->getID(), - "total-pds")); + stats_mgr.del(StatsMgr::generateName("subnet", subnet_id, + "assigned-pds")); - StatsMgr::instance().del(StatsMgr::generateName("subnet", - (*subnet6)->getID(), - "assigned-pds")); + stats_mgr.del(StatsMgr::generateName("subnet", subnet_id, + "declined-addresses")); + + stats_mgr.del(StatsMgr::generateName("subnet", subnet_id, + "declined-reclaimed-addresses")); } } @@ -202,16 +204,26 @@ void CfgSubnets6::updateStatistics() { using namespace isc::stats; - for (Subnet6Collection::const_iterator subnet = subnets_.begin(); - subnet != subnets_.end(); ++subnet) { + StatsMgr& stats_mgr = StatsMgr::instance(); + // For each v6 subnet currently configured, calculate totals + for (Subnet6Collection::const_iterator subnet6 = subnets_.begin(); + subnet6 != subnets_.end(); ++subnet6) { + SubnetID subnet_id = (*subnet6)->getID(); + + stats_mgr.setValue(StatsMgr::generateName("subnet", subnet_id, + "total-nas"), + static_cast + ((*subnet6)->getPoolCapacity(Lease::TYPE_NA))); - StatsMgr::instance().setValue( - StatsMgr::generateName("subnet", (*subnet)->getID(), "total-nas"), - static_cast((*subnet)->getPoolCapacity(Lease::TYPE_NA))); + stats_mgr.setValue(StatsMgr::generateName("subnet", subnet_id, + "total-pds"), + static_cast + ((*subnet6)->getPoolCapacity(Lease::TYPE_PD))); + } - StatsMgr::instance().setValue( - StatsMgr::generateName("subnet", (*subnet)->getID(), "total-pds"), - static_cast((*subnet)->getPoolCapacity(Lease::TYPE_PD))); + // Only recount the stats if we have subnets. + if (subnets_.begin() != subnets_.end()) { + LeaseMgrFactory::instance().recountAddressStats6(); } } diff --git a/src/lib/dhcpsrv/lease_mgr.cc b/src/lib/dhcpsrv/lease_mgr.cc index 341b285c78..d5aa90dfdc 100644 --- a/src/lib/dhcpsrv/lease_mgr.cc +++ b/src/lib/dhcpsrv/lease_mgr.cc @@ -156,7 +156,7 @@ LeaseMgr::recountAddressStats6() { zero); stats_mgr.setValue(StatsMgr::generateName("subnet", subnet_id, - "declined-nas"), + "declined-addresses"), zero); stats_mgr.setValue(StatsMgr:: @@ -189,7 +189,7 @@ LeaseMgr::recountAddressStats6() { stats_mgr.setValue(StatsMgr:: generateName("subnet", row.subnet_id_, - "declined-nas"), + "declined-addresses"), row.state_count_); // Add to the global value. diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc index 5169ef073a..cda0626988 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.cc +++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2012-2016 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -206,8 +206,14 @@ TaggedStatement tagged_statements[] = { "state = ? " "WHERE address = ?"}, {MySqlLeaseMgr::RECOUNT_LEASE4_STATS, - "SELECT subnet_id, state, count(state) as state_count " - "FROM lease4 GROUP BY subnet_id, state ORDER BY subnet_id"}, + "SELECT subnet_id, state, count(state) as state_count " + " FROM lease4 GROUP BY subnet_id, state ORDER BY subnet_id"}, + + {MySqlLeaseMgr::RECOUNT_LEASE6_STATS, + "SELECT subnet_id, lease_type, state, count(state) as state_count" + " FROM lease6 GROUP BY subnet_id, lease_type, state " + " ORDER BY subnet_id" }, + // End of list sentinel {MySqlLeaseMgr::NUM_STATEMENTS, NULL} }; @@ -1353,9 +1359,154 @@ MySqlLeaseMgr::startAddressStatsQuery4() { return(query); } +/// @brief MySql derivation of the IPv6 statistical lease data query +/// +/// This class is used to recalculate IPv6 lease statistics for MySQL +/// lease storage. It does so by executing a query which returns a result +/// containining contain one row per monitored state per subnet, ordered +/// by subnet id in ascending order. +/// +class MySqlAddressStatsQuery6 : public AddressStatsQuery6 { +public: + /// @brief Constructor + /// + /// @param conn A open connection to the database housing the lease data + MySqlAddressStatsQuery6(MySqlConnection& conn); + /// @brief Destructor + virtual ~MySqlAddressStatsQuery6(); + /// @brief Creates the IPv6 lease statistical data result set + /// + /// The result set is populated by executing an SQL query against the + /// lease4 table which sums the leases per lease state per subnet id. + /// The query used is the prepared statement identified by + /// MySqlLeaseMgr::RECOUNT_LEASE6_STATS. This method creates the binds + /// the statement to the output bind array and then executes the + /// statement. + void start(); + /// @brief Fetches the next row in the result set + /// + /// Once the internal result set has been populated by invoking the + /// the start() method, this method is used to iterate over the + /// result set rows. Once the last row has been fetched, subsequent + /// calls will return false. + /// + /// @param row Storage for the fetched row + /// + /// @return True if the fetch succeeded, false if there are no more + /// rows to fetch. + bool getNextRow(AddressStatsRow6& row); + +private: + + /// @brief Analyzes the given statement outcome status + /// + /// Wrapper method around the MySqlConnection:checkError() that is + /// used to generate the appropriate exception if the status indicates + /// an error. + //// + /// a DbOperation error + /// @param status The MySQL statement execution outcome status + /// @param what invocation context message which will be included in + /// any exception + void checkError(int status, const char* what) const; + + /// @brief Database connection to use to execute the query + MySqlConnection& conn_; + + /// @brief The query's prepared statement + MYSQL_STMT *statement_; + + /// @brief Bind array used to store the query result set; + std::vector bind_; + + /// @brief Receives subnet ID when fetching a row + uint32_t subnet_id_; + /// @brief Receives the lease type when fetching a row + uint32_t lease_type_; + /// @brief Receives the lease state when fetching a row + uint32_t lease_state_; + /// @brief Receives the state count when fetching a row + uint32_t state_count_; +}; + +MySqlAddressStatsQuery6::MySqlAddressStatsQuery6(MySqlConnection& conn) + : conn_(conn), statement_(conn_.statements_[MySqlLeaseMgr + ::RECOUNT_LEASE6_STATS]), + bind_(4) { +} + +MySqlAddressStatsQuery6::~MySqlAddressStatsQuery6() { + (void) mysql_stmt_free_result(statement_); +} + +void +MySqlAddressStatsQuery6::start() { + // subnet_id: unsigned int + bind_[0].buffer_type = MYSQL_TYPE_LONG; + bind_[0].buffer = reinterpret_cast(&subnet_id_); + bind_[0].is_unsigned = MLM_TRUE; + + // lease type: uint32_t + bind_[1].buffer_type = MYSQL_TYPE_LONG; + bind_[1].buffer = reinterpret_cast(&lease_type_); + bind_[1].is_unsigned = MLM_TRUE; + + // state: uint32_t + bind_[2].buffer_type = MYSQL_TYPE_LONG; + bind_[2].buffer = reinterpret_cast(&lease_state_); + bind_[2].is_unsigned = MLM_TRUE; + + // state_count_: uint32_t + bind_[3].buffer_type = MYSQL_TYPE_LONG; + bind_[3].buffer = reinterpret_cast(&state_count_); + bind_[3].is_unsigned = MLM_TRUE; + + // Set up the MYSQL_BIND array for the data being returned + // and bind it to the statement. + int status = mysql_stmt_bind_result(statement_, &bind_[0]); + checkError(status, "RECOUNT_LEASE6_STATS: outbound binding failed"); + + // Execute the statement + status = mysql_stmt_execute(statement_); + checkError(status, "RECOUNT_LEASE6_STATS: unable to execute"); + + // Ensure that all the lease information is retrieved in one go to avoid + // overhead of going back and forth between client and server. + status = mysql_stmt_store_result(statement_); + checkError(status, "RECOUNT_LEASE6_STATS: results storage setup failed"); +} + +bool +MySqlAddressStatsQuery6::getNextRow(AddressStatsRow6& row) { + bool have_row = false; + int status = mysql_stmt_fetch(statement_); + if (status == MLM_MYSQL_FETCH_SUCCESS) { + row.subnet_id_ = static_cast(subnet_id_); + row.lease_type_ = static_cast(lease_type_); + row.lease_state_ = static_cast(lease_state_); + row.state_count_ = state_count_; + have_row = true; + } else if (status != MYSQL_NO_DATA) { + checkError(status, "RECOUNT_LEASE6_STATS: getNextRow failed"); + } + + return (have_row); +} + +void +MySqlAddressStatsQuery6::checkError(int status, const char* what) const { + conn_.checkError(status, MySqlLeaseMgr::RECOUNT_LEASE6_STATS, what); +} + +AddressStatsQuery6Ptr +MySqlLeaseMgr::startAddressStatsQuery6() { + AddressStatsQuery6Ptr query(new MySqlAddressStatsQuery6(conn_)); + query->start(); + return(query); +} // MySqlLeaseMgr Constructor and Destructor diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.h b/src/lib/dhcpsrv/mysql_lease_mgr.h index b65c52a31b..06f1a3fee4 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.h +++ b/src/lib/dhcpsrv/mysql_lease_mgr.h @@ -409,7 +409,8 @@ public: INSERT_LEASE6, // Add entry to lease6 table UPDATE_LEASE4, // Update a Lease4 entry UPDATE_LEASE6, // Update a Lease6 entry - RECOUNT_LEASE4_STATS, // Fetches address statisics + RECOUNT_LEASE4_STATS, // Fetches IPv4 address statisics + RECOUNT_LEASE6_STATS, // Fetches IPv6 address statisics NUM_STATEMENTS // Number of statements }; @@ -596,10 +597,20 @@ private: /// invokes its start method, which fetches its statistical data /// result set by executing the RECOUNT_LEASE_STATS4 query. /// The query object is then returned. - /// + /// /// @return The populated query as a pointer to an AddressStatsQuery4 virtual AddressStatsQuery4Ptr startAddressStatsQuery4(); + /// @brief Creates and runs the IPv6 lease stats query + /// + /// It creates an instance of a MySqlAddressStatsQuery6 and then + /// invokes its start method, which fetches its statistical data + /// result set by executing the RECOUNT_LEASE_STATS4 query. + /// The query object is then returned. + /// + /// @return The populated query as a pointer to an AddressStatsQuery6 + virtual AddressStatsQuery6Ptr startAddressStatsQuery6(); + /// @brief Check Error and Throw Exception /// /// This method invokes @ref MySqlConnection::checkError. diff --git a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc index aa3ff1a11b..3d411db9f0 100644 --- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc @@ -2401,7 +2401,7 @@ GenericLeaseMgrTest::checkStat(const std::string& name, } void -GenericLeaseMgrTest::checkAddressStats4(const StatValMapList& expectedStats) { +GenericLeaseMgrTest::checkAddressStats(const StatValMapList& expectedStats) { // Global accumulators int64_t declined_addresses = 0; int64_t declined_reclaimed_addresses = 0; @@ -2450,6 +2450,26 @@ GenericLeaseMgrTest::makeLease4(const std::string& address, ASSERT_TRUE(lmptr_->addLease(lease)); } +void +GenericLeaseMgrTest::makeLease6(const Lease::Type& type, + const std::string& address, + uint8_t prefix_len, + const SubnetID& subnet_id, + const Lease::LeaseState& state) { + IOAddress addr(address); + + // make a DUID from the address + std::vector bytes = addr.toBytes(); + bytes.push_back(prefix_len); + + Lease6Ptr lease(new Lease6(type, addr, DuidPtr(new DUID(bytes)), 77, + 16000, 24000, 0, 0, subnet_id, HWAddrPtr(), + prefix_len)); + lease->state_ = state; + ASSERT_TRUE(lmptr_->addLease(lease)); +} + + void GenericLeaseMgrTest::testRecountAddressStats4() { using namespace stats; @@ -2489,13 +2509,13 @@ GenericLeaseMgrTest::testRecountAddressStats4() { } // Make sure stats are as expected. - ASSERT_NO_FATAL_FAILURE(checkAddressStats4(expectedStats)); + ASSERT_NO_FATAL_FAILURE(checkAddressStats(expectedStats)); // Recount stats. We should have the same results. ASSERT_NO_THROW(lmptr_->recountAddressStats4()); // Make sure stats are as expected. - ASSERT_NO_FATAL_FAILURE(checkAddressStats4(expectedStats)); + ASSERT_NO_FATAL_FAILURE(checkAddressStats(expectedStats)); // Now let's insert some leases into subnet 1. int subnet_id = 1; @@ -2529,7 +2549,7 @@ GenericLeaseMgrTest::testRecountAddressStats4() { ASSERT_NO_THROW(lmptr_->recountAddressStats4()); // Make sure stats are as expected. - ASSERT_NO_FATAL_FAILURE(checkAddressStats4(expectedStats)); + ASSERT_NO_FATAL_FAILURE(checkAddressStats(expectedStats)); // Delete some leases from subnet, and update the expected stats. EXPECT_TRUE(lmptr_->deleteLease(IOAddress("192.0.1.1"))); @@ -2542,9 +2562,133 @@ GenericLeaseMgrTest::testRecountAddressStats4() { ASSERT_NO_THROW(lmptr_->recountAddressStats4()); // Make sure stats are as expected. - ASSERT_NO_FATAL_FAILURE(checkAddressStats4(expectedStats)); + ASSERT_NO_FATAL_FAILURE(checkAddressStats(expectedStats)); } +void +GenericLeaseMgrTest::testRecountAddressStats6() { + using namespace stats; + + StatsMgr::instance().removeAll(); + + // create subnets + CfgSubnets6Ptr cfg = + CfgMgr::instance().getStagingCfg()->getCfgSubnets6(); + + // Create 3 subnets. + Subnet6Ptr subnet; + Pool6Ptr pool; + int num_subnets = 2; + StatValMapList expectedStats(num_subnets); + + int subnet_id = 1; + subnet.reset(new Subnet6(IOAddress("3001:1::"), 64, 1, 2, 3, 4, subnet_id)); + pool.reset(new Pool6(Lease::TYPE_NA, IOAddress("3001:1::"), + IOAddress("3001:1::FF"))); + subnet->addPool(pool); + expectedStats[subnet_id - 1]["total-nas"] = 256; + + pool.reset(new Pool6(Lease::TYPE_PD, IOAddress("3001:1:2::"),96,112)); + subnet->addPool(pool); + expectedStats[subnet_id - 1]["total-pds"] = 65536; + cfg->add(subnet); + + ++subnet_id; + subnet.reset(new Subnet6(IOAddress("2001:db8:1::"), 64, 1, 2, 3, 4, + subnet_id)); + pool.reset(new Pool6(Lease::TYPE_NA, IOAddress("2001:db8:1::"), 120)); + subnet->addPool(pool); + expectedStats[subnet_id - 1]["total-nas"] = 256; + expectedStats[subnet_id - 1]["total-pds"] = 0; + cfg->add(subnet); + + ASSERT_NO_THROW(CfgMgr::instance().commit()); + + + // Create the expected stats list. At this point, the only stat + // that should be non-zero is total-nas/total-pds. + for (int i = 0; i < num_subnets; ++i) { + expectedStats[i]["assigned-nas"] = 0; + expectedStats[i]["declined-addresses"] = 0; + expectedStats[i]["declined-reclaimed-addresses"] = 0; + expectedStats[i]["assigned-pds"] = 0; + } + + // Make sure stats are as expected. + ASSERT_NO_FATAL_FAILURE(checkAddressStats(expectedStats)); + + + // Recount stats. We should have the same results. + ASSERT_NO_THROW(lmptr_->recountAddressStats4()); + + // Make sure stats are as expected. + ASSERT_NO_FATAL_FAILURE(checkAddressStats(expectedStats)); + + // Now let's insert some leases into subnet 1. + subnet_id = 1; + + // Insert three assigned NAs. + makeLease6(Lease::TYPE_NA, "3001:1::1", 0, subnet_id); + makeLease6(Lease::TYPE_NA, "3001:1::2", 0, subnet_id); + makeLease6(Lease::TYPE_NA, "3001:1::3", 0, subnet_id); + expectedStats[subnet_id - 1]["assigned-nas"] = 3; + + // Insert two declined NAs. + makeLease6(Lease::TYPE_NA, "3001:1::4", 0, subnet_id, + Lease::STATE_DECLINED); + makeLease6(Lease::TYPE_NA, "3001:1::5", 0, subnet_id, + Lease::STATE_DECLINED); + expectedStats[subnet_id - 1]["declined-addresses"] = 2; + + // Insert one expired NA. + makeLease6(Lease::TYPE_NA, "3001:1::6", 0, subnet_id, + Lease::STATE_EXPIRED_RECLAIMED); + + // Insert two assigned PDs. + makeLease6(Lease::TYPE_PD, "3001:1:2:0100::", 112, subnet_id); + makeLease6(Lease::TYPE_PD, "3001:1:2:0200::", 112, subnet_id); + expectedStats[subnet_id - 1]["assigned-pds"] = 2; + + // Insert two expired PDs. + makeLease6(Lease::TYPE_PD, "3001:1:2:0300::", 112, subnet_id, + Lease::STATE_EXPIRED_RECLAIMED); + makeLease6(Lease::TYPE_PD, "3001:1:2:0400::", 112, subnet_id, + Lease::STATE_EXPIRED_RECLAIMED); + + // Now let's add leases to subnet 2. + subnet_id = 2; + + // Insert two assigned NAs. + makeLease6(Lease::TYPE_NA, "2001:db81::1", 0, subnet_id); + makeLease6(Lease::TYPE_NA, "2001:db81::2", 0, subnet_id); + expectedStats[subnet_id - 1]["assigned-nas"] = 2; + + // Insert one declined NA. + makeLease6(Lease::TYPE_NA, "2001:db81::3", 0, subnet_id, + Lease::STATE_DECLINED); + expectedStats[subnet_id - 1]["declined-addresses"] = 1; + + // Now Recount the stats. + ASSERT_NO_THROW(lmptr_->recountAddressStats6()); + + // Make sure stats are as expected. + ASSERT_NO_FATAL_FAILURE(checkAddressStats(expectedStats)); + + // Delete some leases and update the expected stats. + EXPECT_TRUE(lmptr_->deleteLease(IOAddress("3001:1::2"))); + expectedStats[0]["assigned-nas"] = 2; + + EXPECT_TRUE(lmptr_->deleteLease(IOAddress("2001:db81::3"))); + expectedStats[1]["declined-addresses"] = 0; + + // Recount the stats. + ASSERT_NO_THROW(lmptr_->recountAddressStats6()); + + // Make sure stats are as expected. + ASSERT_NO_FATAL_FAILURE(checkAddressStats(expectedStats)); +} + + }; // namespace test }; // namespace dhcp }; // namespace isc diff --git a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h index f646412db3..6a7ae4d640 100644 --- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h +++ b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h @@ -118,7 +118,7 @@ public: /// do not match. /// /// @param expected_stats Map of expected static names and values. - void checkAddressStats4(const StatValMapList& expected_stats); + void checkAddressStats(const StatValMapList& expected_stats); /// @brief Constructs a minimal IPv4 lease and adds it to the lease storage /// @@ -128,6 +128,19 @@ public: void makeLease4(const std::string& address, const SubnetID& subnet_id, const Lease::LeaseState& state = Lease::STATE_DEFAULT); + /// @brief Constructs a minimal IPv6 lease and adds it to the lease storage + /// + /// The DUID is constructed from the address and prefix length. + /// + /// @param type - type of lease to create (TYPE_NA, TYPE_PD...) + /// @param address - IPv6 address/prefix for the lease + /// @param prefix_len = length of the prefix (should be 0 for TYPE_NA) + /// @param subnet_id - subnet ID to which the lease belongs + /// @param state - the state of the lease + void makeLease6(const Lease::Type& type, const std::string& address, + uint8_t prefix_len, const SubnetID& subnet_id, + const Lease::LeaseState& state = Lease::STATE_DEFAULT); + /// @brief checks that addLease, getLease4(addr) and deleteLease() works void testBasicLease4(); @@ -354,6 +367,13 @@ public: /// after altering the lease states in various ways. void testRecountAddressStats4(); + /// @brief Check that the IPv6 lease statistics can be recounted + /// + /// This test creates two subnets and several leases associated with + /// them, then verifies that lease statistics are recalculated correctly + /// after altering the lease states in various ways. + void testRecountAddressStats6(); + /// @brief String forms of IPv4 addresses std::vector straddress4_; diff --git a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc index bbe80f9b60..4e6b583a05 100644 --- a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc @@ -482,4 +482,9 @@ TEST_F(MySqlLeaseMgrTest, recountAddressStats4) { testRecountAddressStats4(); } +// Verifies that IPv6 lease statistics can be recalculated. +TEST_F(MySqlLeaseMgrTest, recountAddressStats6) { + testRecountAddressStats6(); +} + }; // Of anonymous namespace -- cgit v1.2.3 From efbf437f6dd84a03b1a8af1d017fada9ba02a492 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Tue, 16 Aug 2016 12:01:21 -0400 Subject: [4294] PostgreSQL now supports IPv6 lease stats recounting src/lib/dhcpsrv/pgsql_lease_mgr.h src/lib/dhcpsrv/pgsql_lease_mgr.cc - Added TaggedStatement RECOUNT_LEASE6_STATS - PgSqlAddressStatsQuery6 - new class PgSql derivation of the IPv6 statistical lease data query - PgSqlLeaseMgr::startAddressStatsQuery6() - new virtual method which creates and runs the IPv6 lease stats query src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc TEST_F(PgSqlLeaseMgrTest, recountAddressStats6) - new test --- src/lib/dhcpsrv/mysql_lease_mgr.cc | 4 +- src/lib/dhcpsrv/pgsql_lease_mgr.cc | 138 +++++++++++++++++++++- src/lib/dhcpsrv/pgsql_lease_mgr.h | 11 ++ src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc | 4 + 4 files changed, 152 insertions(+), 5 deletions(-) diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc index cda0626988..1e4d826ddf 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.cc +++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc @@ -1379,8 +1379,8 @@ public: /// @brief Creates the IPv6 lease statistical data result set /// /// The result set is populated by executing an SQL query against the - /// lease4 table which sums the leases per lease state per subnet id. - /// The query used is the prepared statement identified by + /// lease6 table which sums the leases per lease state per lease type + /// per subnet id. The query used is the prepared statement identified by /// MySqlLeaseMgr::RECOUNT_LEASE6_STATS. This method creates the binds /// the statement to the output bind array and then executes the /// statement. diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.cc b/src/lib/dhcpsrv/pgsql_lease_mgr.cc index 174237df70..751f636004 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.cc +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.cc @@ -207,6 +207,13 @@ PgSqlTaggedStatement tagged_statements[] = { "SELECT subnet_id, state, count(state) as state_count " "FROM lease4 GROUP BY subnet_id, state ORDER BY subnet_id"}, + // RECOUNT_LEASE6_STATS, + { 0, { OID_NONE }, + "recount_lease6_stats", + "SELECT subnet_id, lease_type, state, count(state) as state_count " + "FROM lease6 GROUP BY subnet_id, lease_type, state " + "ORDER BY subnet_id"}, + // End of list sentinel { 0, { 0 }, NULL, NULL} }; @@ -778,16 +785,19 @@ PgSqlAddressStatsQuery4::getNextRow(AddressStatsRow4& row) { // Fetch the subnet id. uint32_t col = 0; uint32_t subnet_id; - PgSqlExchange::getColumnValue(*result_set_, next_row_, 0, subnet_id); + PgSqlExchange::getColumnValue(*result_set_, next_row_, col, subnet_id); row.subnet_id_ = static_cast(subnet_id); + ++col; // Fetch the lease state. uint32_t state; - PgSqlExchange::getColumnValue(*result_set_, next_row_ , 1, state); + PgSqlExchange::getColumnValue(*result_set_, next_row_ , col, state); row.lease_state_ = static_cast(state); + ++col; // Fetch the state count. - PgSqlExchange::getColumnValue(*result_set_, next_row_, 2, row.state_count_); + PgSqlExchange::getColumnValue(*result_set_, next_row_, col, + row.state_count_); // Point to the next row. ++next_row_; @@ -802,6 +812,128 @@ PgSqlLeaseMgr::startAddressStatsQuery4() { return(query); } +/// @brief PgSql derivation of the IPv6 statistical lease data query +/// +/// This class is used to recalculate IPv6 lease statistics for MySQL +/// lease storage. It does so by executing a query which returns a result +/// containining contain one row per monitored state per subnet, ordered +/// by subnet id in ascending order. +/// +class PgSqlAddressStatsQuery6 : public AddressStatsQuery6 { +public: + /// @brief Constructor + /// + /// @param conn A open connection to the database housing the lease data + PgSqlAddressStatsQuery6(PgSqlConnection& conn); + + /// @brief Destructor + virtual ~PgSqlAddressStatsQuery6() {}; + + /// @brief Creates the IPv6 lease statistical data result set + /// + /// The result set is populated by executing an SQL query against the + /// lease6 table which sums the leases per lease state per lease type + /// per subnet id. The query used is the prepared statement identified by + /// PgSqlLeaseMgr::RECOUNT_LEASE6_STATS. This method executes the + /// statement which creates the result set. + void start(); + + /// @brief Fetches the next row in the result set + /// + /// Once the internal result set has been populated by invoking the + /// the start() method, this method is used to iterate over the + /// result set rows. Once the last row has been fetched, subsequent + /// calls will return false. + /// + /// @param row Storage for the fetched row + /// + /// @return True if the fetch succeeded, false if there are no more + /// rows to fetch. + bool getNextRow(AddressStatsRow6& row); + +private: + + /// @brief Analyzes the given statement outcome status + /// + /// Wrapper method around the PgSqlConnection:checkError() that is + /// used to generate the appropriate exception if the status indicates + /// an error. + //// + /// a DbOperation error + /// @param status The MySQL statement execution outcome status + /// @param what invocation context message which will be included in + /// any exception + void checkError(int status, const char* what) const; + + /// @brief Database connection to use to execute the query + PgSqlConnection& conn_; + + /// @brief The query's prepared statement + PgSqlTaggedStatement& statement_; + + /// @brief The result set returned by Postgres. + boost::shared_ptr result_set_; + + /// @brief Index of the next row to fetch + uint32_t next_row_; +}; + +PgSqlAddressStatsQuery6::PgSqlAddressStatsQuery6(PgSqlConnection& conn) + : conn_(conn), statement_(tagged_statements[PgSqlLeaseMgr + ::RECOUNT_LEASE6_STATS]), + result_set_(), next_row_(0) { +} + +void +PgSqlAddressStatsQuery6::start() { + // The query has no parameters, so we only need it's name. + result_set_.reset(new PgSqlResult(PQexecPrepared(conn_, statement_.name, + 0, NULL, NULL, NULL, 0))); + + conn_.checkStatementError(*result_set_, statement_); +} + +bool +PgSqlAddressStatsQuery6::getNextRow(AddressStatsRow6& row) { + // If we're past the end, punt. + if (next_row_ >= result_set_->getRows()) { + return (false); + } + + // Fetch the subnet id. + uint32_t col = 0; + uint32_t subnet_id; + PgSqlExchange::getColumnValue(*result_set_, next_row_, col, subnet_id); + row.subnet_id_ = static_cast(subnet_id); + ++col; + + // Fetch the lease type. + uint32_t lease_type; + PgSqlExchange::getColumnValue(*result_set_, next_row_ , col, lease_type); + row.lease_type_ = static_cast(lease_type); + ++col; + + // Fetch the lease state. + uint32_t state; + PgSqlExchange::getColumnValue(*result_set_, next_row_ , col, state); + row.lease_state_ = static_cast(state); + ++col; + + // Fetch the state count. + PgSqlExchange::getColumnValue(*result_set_, next_row_, col, row.state_count_); + + // Point to the next row. + ++next_row_; + + return (true); +} + +AddressStatsQuery6Ptr +PgSqlLeaseMgr::startAddressStatsQuery6() { + AddressStatsQuery6Ptr query(new PgSqlAddressStatsQuery6(conn_)); + query->start(); + return(query); +} PgSqlLeaseMgr::PgSqlLeaseMgr(const DatabaseConnection::ParameterMap& parameters) : LeaseMgr(), exchange4_(new PgSqlLease4Exchange()), diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.h b/src/lib/dhcpsrv/pgsql_lease_mgr.h index 9511c0e237..a8a40813a5 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.h +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.h @@ -327,6 +327,16 @@ public: /// @return The populated query as a pointer to an AddressStatsQuery4 virtual AddressStatsQuery4Ptr startAddressStatsQuery4(); + /// @brief Creates and runs the IPv6 lease stats query + /// + /// It creates an instance of a PgSqlAddressStatsQuery6 and then + /// invokes its start method, which fetches its statistical data + /// result set by executing the RECOUNT_LEASE_STATS6 query. + /// The query object is then returned. + /// + /// @return The populated query as a pointer to an AddressStatsQuery6 + virtual AddressStatsQuery6Ptr startAddressStatsQuery6(); + /// @brief Return backend type /// /// Returns the type of the backend (e.g. "mysql", "memfile" etc.) @@ -396,6 +406,7 @@ public: UPDATE_LEASE4, // Update a Lease4 entry UPDATE_LEASE6, // Update a Lease6 entry RECOUNT_LEASE4_STATS, // Fetch IPv4 lease statistical data + RECOUNT_LEASE6_STATS, // Fetch IPv4 lease statistical data NUM_STATEMENTS // Number of statements }; diff --git a/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc index 762d37c253..fb473a7830 100644 --- a/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc @@ -407,5 +407,9 @@ TEST_F(PgSqlLeaseMgrTest, recountAddressStats4) { testRecountAddressStats4(); } +// Verifies that IPv6 lease statistics can be recalculated. +TEST_F(PgSqlLeaseMgrTest, recountAddressStats6) { + testRecountAddressStats6(); +} }; // namespace -- cgit v1.2.3 From 4a94a1f7b9b46dcecbf1c0fd2ceb4532cbd80be7 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Tue, 16 Aug 2016 14:15:25 -0400 Subject: [4294] Memfile now supports IPv6 lease stats recounting src/lib/dhcpsrv/memfile_lease_mgr.h src/lib/dhcpsrv/memfile_lease_mgr.cc - MemfileAddressStatsQuery6 - new class, Memfile derivation of the IPv6 statistical lease data query - Memfile_LeaseMgr::startAddressStatsQuery6() - new virtual method that creates and runs the IPv6 lease stats query src/lib/dhcpsrv/memfile_lease_storage.h - Added non-unique index on subnet ID to Lease6Storage src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc TEST_F(MemfileLeaseMgrTest, recountAddressStats6) - new test --- src/lib/dhcpsrv/lease_mgr.h | 4 +- src/lib/dhcpsrv/memfile_lease_mgr.cc | 169 +++++++++++++++++++++ src/lib/dhcpsrv/memfile_lease_mgr.h | 9 ++ src/lib/dhcpsrv/memfile_lease_storage.h | 13 ++ .../dhcpsrv/tests/memfile_lease_mgr_unittest.cc | 6 + 5 files changed, 199 insertions(+), 2 deletions(-) diff --git a/src/lib/dhcpsrv/lease_mgr.h b/src/lib/dhcpsrv/lease_mgr.h index 140a7d1c72..f0a58f7aef 100644 --- a/src/lib/dhcpsrv/lease_mgr.h +++ b/src/lib/dhcpsrv/lease_mgr.h @@ -229,8 +229,8 @@ struct AddressStatsRow6 { AddressStatsRow6(const SubnetID& subnet_id, const Lease::Type& lease_type, const Lease::LeaseState& lease_state, const int64_t state_count) - : subnet_id_(subnet_id), lease_state_(lease_state), - state_count_(state_count) { + : subnet_id_(subnet_id), lease_type_(lease_type), + lease_state_(lease_state), state_count_(state_count) { } /// @brief The subnet ID to which this data applies diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.cc b/src/lib/dhcpsrv/memfile_lease_mgr.cc index 1ad41cd1c2..ceed6f473d 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.cc +++ b/src/lib/dhcpsrv/memfile_lease_mgr.cc @@ -393,6 +393,168 @@ MemfileAddressStatsQuery4::getRowCount() { return (rows_.size()); } +/// @brief Memfile derivation of the IPv6 statistical lease data query +/// +/// This class is used to recalculate IPv6 lease statistics for Memfile +/// lease storage. It does so by iterating over the given storage, +/// accumulating counts of leases in each of the monitored lease states +/// for each subnet and storing these counts in an internal collection. +/// The populated result set will contain one entry per monitored state +/// per subnet. +/// +class MemfileAddressStatsQuery6 : public AddressStatsQuery6 { +public: + /// @brief Constructor + /// + /// @param storage6 A pointer to the v6 lease storage to be counted + MemfileAddressStatsQuery6(Lease6Storage& storage6); + + /// @brief Destructor + virtual ~MemfileAddressStatsQuery6() {}; + + /// @brief Creates the IPv6 lease statistical data result set + /// + /// The result is populated by iterating over the IPv6 leases in storage, + /// in ascending order by subnet ID, accumulating the lease state counts + /// per lease type. At the completion of all entries for a given subnet, + /// the counts are used to create AddressStatsRow5 instances which are + /// appended to an internal vector. The process results in a vector + /// containing one entry per state per lease type per subnet. + /// + /// Currently the states counted are: + /// + /// - Lease::STATE_DEFAULT (i.e. assigned) + /// - Lease::STATE_DECLINED + virtual void start(); + + /// @brief Fetches the next row in the result set + /// + /// Once the internal result set has been populated by invoking the + /// the start() method, this method is used to iterate over the + /// result set rows. Once the last row has been fetched, subsequent + /// calls will return false. + /// @param row Storage for the fetched row + /// + /// @return True if the fetch succeeded, false if there are no more + /// rows to fetch. + virtual bool getNextRow(AddressStatsRow6& row); + + /// @brief Returns the number of rows in the result set + /// @todo, should this be a virtual member of the base class? + int getRowCount(); + +private: + /// @brief The Memfile storage containing the IPv6 leases to analyze + Lease6Storage& storage6_; + + /// @brief A vector containing the "result set" + std::vector rows_; + + /// @brief An iterator for accessing the next row within the result set + std::vector::iterator next_pos_; +}; + +MemfileAddressStatsQuery6::MemfileAddressStatsQuery6(Lease6Storage& storage6) + : storage6_(storage6), rows_(0), next_pos_(rows_.end()) {}; + +void +MemfileAddressStatsQuery6::start() { + // Get the subnet_id index + const Lease6StorageSubnetIdIndex& idx = storage6_.get(); + + // Iterate over the leases in order by subnet, accumulating per + // subnet counts for each state of interest. As we finish each + // subnet, add the appropriate rows to our result set. + SubnetID cur_id = 0; + int64_t assigned = 0; + int64_t declined = 0; + int64_t assigned_pds = 0; + + for(Lease6StorageSubnetIdIndex::const_iterator lease = idx.begin(); + lease != idx.end(); ++lease) { + + // If we've hit the next subnet, add rows for the current subnet + // and wipe the accumulators + if ((*lease)->subnet_id_ > cur_id) { + if (cur_id > 0) { + rows_.push_back(AddressStatsRow6(cur_id, Lease::TYPE_NA, + Lease::STATE_DEFAULT, + assigned)); + assigned = 0; + rows_.push_back(AddressStatsRow6(cur_id, Lease::TYPE_NA, + Lease::STATE_DECLINED, + declined)); + declined = 0; + rows_.push_back(AddressStatsRow6(cur_id, Lease::TYPE_PD, + Lease::STATE_DEFAULT, + assigned_pds)); + assigned_pds = 0; + } + + // Update current subnet id + cur_id = (*lease)->subnet_id_; + } + + // Bump the appropriate accumulator + switch ((*lease)->state_) { + case Lease::STATE_DEFAULT: + switch((*lease)->type_) { + case Lease::TYPE_NA: + ++assigned; + break; + case Lease::TYPE_PD: + ++assigned_pds; + break; + default: + break; + } + break; + case Lease::STATE_DECLINED: + // In theory only NAs can be declined + if (((*lease)->type_) == Lease::TYPE_NA) { + ++declined; + } + break; + default: + // Not one we're tracking. + break; + } + } + + // Make the rows for last subnet, unless there were no rows + if (idx.begin() != idx.end()) { + rows_.push_back(AddressStatsRow6(cur_id, Lease::TYPE_NA, + Lease::STATE_DEFAULT, + assigned)); + rows_.push_back(AddressStatsRow6(cur_id, Lease::TYPE_NA, + Lease::STATE_DECLINED, + declined)); + rows_.push_back(AddressStatsRow6(cur_id, Lease::TYPE_PD, + Lease::STATE_DEFAULT, + assigned_pds)); + } + + // Set the next row position to the beginning of the rows. + next_pos_ = rows_.begin(); +} + +bool +MemfileAddressStatsQuery6::getNextRow(AddressStatsRow6& row) { + if (next_pos_ == rows_.end()) { + return (false); + } + + row = *next_pos_; + ++next_pos_; + return (true); +} + +int +MemfileAddressStatsQuery6::getRowCount() { + return (rows_.size()); +} + + // Explicit definition of class static constants. Values are given in the // declaration so they're not needed here. const int Memfile_LeaseMgr::MAJOR_VERSION; @@ -1193,5 +1355,12 @@ Memfile_LeaseMgr::startAddressStatsQuery4() { return(query); } +AddressStatsQuery6Ptr +Memfile_LeaseMgr::startAddressStatsQuery6() { + AddressStatsQuery6Ptr query(new MemfileAddressStatsQuery6(storage6_)); + query->start(); + return(query); +} + } // end of namespace isc::dhcp } // end of namespace isc diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.h b/src/lib/dhcpsrv/memfile_lease_mgr.h index 8bc4c72b85..d4e7c1a94a 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.h +++ b/src/lib/dhcpsrv/memfile_lease_mgr.h @@ -604,6 +604,15 @@ public: /// @return The populated query as a pointer to an AddressStatsQuery4 virtual AddressStatsQuery4Ptr startAddressStatsQuery4(); + /// @brief Creates and runs the IPv6 lease stats query + /// + /// It creates an instance of a MemfileAddressStatsQuery6 and then + /// invokes it's start method in which the query constructs its + /// statistical data result set. The query object is then returned. + /// + /// @return The populated query as a pointer to an AddressStatsQuery6 + virtual AddressStatsQuery6Ptr startAddressStatsQuery6(); + /// @name Protected methods used for %Lease File Cleanup. /// The following methods are protected so as they can be accessed and /// tested by unit tests. diff --git a/src/lib/dhcpsrv/memfile_lease_storage.h b/src/lib/dhcpsrv/memfile_lease_storage.h index fba499672f..5c09f35d56 100644 --- a/src/lib/dhcpsrv/memfile_lease_storage.h +++ b/src/lib/dhcpsrv/memfile_lease_storage.h @@ -106,7 +106,17 @@ typedef boost::multi_index_container< boost::multi_index::const_mem_fun > + >, + + boost::multi_index::ordered_non_unique< + boost::multi_index::tag, + // The subnet id is held in the subnet_id_ member of Lease6 + // class. Note that the subnet_id_ is defined in the base + // class (Lease) so we have to point to this class rather + // than derived class: Lease6. + boost::multi_index::member > + > > Lease6Storage; // Specify the type name of this container. @@ -241,6 +251,9 @@ typedef Lease6Storage::index::type Lease6StorageDuidIaidTy /// @brief DHCPv6 lease storage index by expiration time. typedef Lease6Storage::index::type Lease6StorageExpirationIndex; +/// @brief DHCPv6 lease storage index by subnet id. +typedef Lease6Storage::index::type Lease6StorageSubnetIdIndex; + /// @brief DHCPv4 lease storage index by address. typedef Lease4Storage::index::type Lease4StorageAddressIndex; diff --git a/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc index 3cf45d58d6..1379cf1c14 100644 --- a/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc @@ -1889,4 +1889,10 @@ TEST_F(MemfileLeaseMgrTest, recountAddressStats4) { testRecountAddressStats4(); } +// Verifies that IPv6 lease statistics can be recalculated. +TEST_F(MemfileLeaseMgrTest, recountAddressStats6) { + startBackend(V6); + testRecountAddressStats6(); +} + }; // end of anonymous namespace -- cgit v1.2.3 From 45d4cdc6899695b9fcfea4c51ecf5e8d3c482af8 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Tue, 16 Aug 2016 15:23:33 -0400 Subject: [4294] Minor cleanup --- src/lib/dhcpsrv/mysql_lease_mgr.cc | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc index 1e4d826ddf..52ed485d51 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.cc +++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc @@ -1286,8 +1286,12 @@ private: /// @brief Bind array used to store the query result set; std::vector bind_; - /// @brief Member struct that is bound to the statement; - AddressStatsRow4 stat_row_; + /// @brief Receives subnet ID when fetching a row + uint32_t subnet_id_; + /// @brief Receives the lease state when fetching a row + uint32_t lease_state_; + /// @brief Receives the state count when fetching a row + uint32_t state_count_; }; MySqlAddressStatsQuery4::MySqlAddressStatsQuery4(MySqlConnection& conn) @@ -1305,17 +1309,17 @@ void MySqlAddressStatsQuery4::start() { // subnet_id: unsigned int bind_[0].buffer_type = MYSQL_TYPE_LONG; - bind_[0].buffer = reinterpret_cast(&stat_row_.subnet_id_); + bind_[0].buffer = reinterpret_cast(&subnet_id_); bind_[0].is_unsigned = MLM_TRUE; // state: uint32_t bind_[1].buffer_type = MYSQL_TYPE_LONG; - bind_[1].buffer = reinterpret_cast(&stat_row_.lease_state_); + bind_[1].buffer = reinterpret_cast(&lease_state_); bind_[1].is_unsigned = MLM_TRUE; // state_count_: uint32_t bind_[2].buffer_type = MYSQL_TYPE_LONG; - bind_[2].buffer = reinterpret_cast(&stat_row_.state_count_); + bind_[2].buffer = reinterpret_cast(&state_count_); bind_[2].is_unsigned = MLM_TRUE; // Set up the MYSQL_BIND array for the data being returned @@ -1338,7 +1342,9 @@ MySqlAddressStatsQuery4::getNextRow(AddressStatsRow4& row) { bool have_row = false; int status = mysql_stmt_fetch(statement_); if (status == MLM_MYSQL_FETCH_SUCCESS) { - row = stat_row_; + row.subnet_id_ = static_cast(subnet_id_); + row.lease_state_ = static_cast(lease_state_); + row.state_count_ = state_count_; have_row = true; } else if (status != MYSQL_NO_DATA) { checkError(status, "RECOUNT_LEASE4_STATS: getNextRow failed"); -- cgit v1.2.3 From d619d5e38b7fce28316619779684ae8e758eef7e Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Wed, 17 Aug 2016 09:48:32 -0400 Subject: [4294] More clean up --- src/lib/dhcpsrv/memfile_lease_mgr.cc | 378 +++++++++++++++++------------------ src/lib/dhcpsrv/mysql_lease_mgr.cc | 290 ++++++++++++--------------- src/lib/dhcpsrv/pgsql_lease_mgr.cc | 242 ++++++++++------------ 3 files changed, 414 insertions(+), 496 deletions(-) diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.cc b/src/lib/dhcpsrv/memfile_lease_mgr.cc index ceed6f473d..2ffaa7c114 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.cc +++ b/src/lib/dhcpsrv/memfile_lease_mgr.cc @@ -271,7 +271,9 @@ public: /// @brief Constructor /// /// @param storage4 A pointer to the v4 lease storage to be counted - MemfileAddressStatsQuery4(Lease4Storage& storage4); + MemfileAddressStatsQuery4(Lease4Storage& storage4) + : storage4_(storage4), rows_(0), next_pos_(rows_.end()) { + }; /// @brief Destructor virtual ~MemfileAddressStatsQuery4() {}; @@ -280,118 +282,109 @@ public: /// /// The result is populated by iterating over the IPv4 leases in storage, /// in ascending order by subnet ID, accumulating the lease state counts. - /// At the completion of all entries for a given subnet, the counts are - /// used to create AddressStatsRow4 instances which are appended to an + /// At the completion of all entries for a given subnet, the counts are + /// used to create AddressStatsRow4 instances which are appended to an /// internal vector. The process results in a vector containing one entry /// per state per subnet. /// /// Currently the states counted are: /// - /// - Lease::STATE_DEFAULT (i.e. assigned) - /// - Lease::STATE_DECLINED - virtual void start(); + /// - Lease::STATE_DEFAULT (i.e. assigned) + /// - Lease::STATE_DECLINED + void start() { + // Get the subnet_id index + const Lease4StorageSubnetIdIndex& idx + = storage4_.get(); + + // Iterate over the leases in order by subnet, accumulating per + // subnet counts for each state of interest. As we finish each + // subnet, add the appropriate rows to our result set. + SubnetID cur_id = 0; + int64_t assigned = 0; + int64_t declined = 0; + for(Lease4StorageSubnetIdIndex::const_iterator lease = idx.begin(); + lease != idx.end(); ++lease) { + + // If we've hit the next subnet, add rows for the current subnet + // and wipe the accumulators + if ((*lease)->subnet_id_ > cur_id) { + if (cur_id > 0) { + rows_.push_back(AddressStatsRow4(cur_id, + Lease::STATE_DEFAULT, + assigned)); + assigned = 0; + rows_.push_back(AddressStatsRow4(cur_id, + Lease::STATE_DECLINED, + declined)); + declined = 0; + } + + // Update current subnet id + cur_id = (*lease)->subnet_id_; + } + + // Bump the appropriate accumulator + switch ((*lease)->state_) { + case Lease::STATE_DEFAULT: + ++assigned; + break; + case Lease::STATE_DECLINED: + ++declined; + break; + default: + // Not one we're tracking. + break; + } + } + + // Make the rows for last subnet, unless there were no rows + if (idx.begin() != idx.end()) { + rows_.push_back(AddressStatsRow4(cur_id, Lease::STATE_DEFAULT, + assigned)); + rows_.push_back(AddressStatsRow4(cur_id, Lease::STATE_DECLINED, + declined)); + } + + // Set the next row position to the beginning of the rows. + next_pos_ = rows_.begin(); + } /// @brief Fetches the next row in the result set /// /// Once the internal result set has been populated by invoking the /// the start() method, this method is used to iterate over the /// result set rows. Once the last row has been fetched, subsequent - /// calls will return false. + /// calls will return false. /// @param row Storage for the fetched row /// /// @return True if the fetch succeeded, false if there are no more /// rows to fetch. - virtual bool getNextRow(AddressStatsRow4& row); + virtual bool getNextRow(AddressStatsRow4& row) { + if (next_pos_ == rows_.end()) { + return (false); + } + + row = *next_pos_; + ++next_pos_; + return (true); + } /// @brief Returns the number of rows in the result set - /// @todo, should this be a virtual member of the base class? - int getRowCount(); + int getRowCount() const { + return (rows_.size()); + } private: /// @brief The Memfile storage containing the IPv4 leases to analyze Lease4Storage& storage4_; - /// @brief A vector containing the "result set" + /// @brief A vector containing the "result set" std::vector rows_; /// @brief An iterator for accessing the next row within the result set std::vector::iterator next_pos_; }; -MemfileAddressStatsQuery4::MemfileAddressStatsQuery4(Lease4Storage& storage4) - : storage4_(storage4), rows_(0), next_pos_(rows_.end()) {}; - -void -MemfileAddressStatsQuery4::start() { - // Get the subnet_id index - const Lease4StorageSubnetIdIndex& idx = storage4_.get(); - - // Iterate over the leases in order by subnet, accumulating per - // subnet counts for each state of interest. As we finish each - // subnet, add the appropriate rows to our result set. - SubnetID cur_id = 0; - int64_t assigned = 0; - int64_t declined = 0; - for(Lease4StorageSubnetIdIndex::const_iterator lease = idx.begin(); - lease != idx.end(); ++lease) { - - // If we've hit the next subnet, add rows for the current subnet - // and wipe the accumulators - if ((*lease)->subnet_id_ > cur_id) { - if (cur_id > 0) { - rows_.push_back(AddressStatsRow4(cur_id,Lease::STATE_DEFAULT, - assigned)); - assigned = 0; - rows_.push_back(AddressStatsRow4(cur_id, Lease::STATE_DECLINED, - declined)); - declined = 0; - } - - // Update current subnet id - cur_id = (*lease)->subnet_id_; - } - - // Bump the appropriate accumulator - switch ((*lease)->state_) { - case Lease::STATE_DEFAULT: - ++assigned; - break; - case Lease::STATE_DECLINED: - ++declined; - break; - default: - // Not one we're tracking. - break; - } - } - - // Make the rows for last subnet, unless there were no rows - if (idx.begin() != idx.end()) { - rows_.push_back(AddressStatsRow4(cur_id, Lease::STATE_DEFAULT, - assigned)); - rows_.push_back(AddressStatsRow4(cur_id, Lease::STATE_DECLINED, - declined)); - } - - // Set the next row position to the beginning of the rows. - next_pos_ = rows_.begin(); -} - -bool -MemfileAddressStatsQuery4::getNextRow(AddressStatsRow4& row) { - if (next_pos_ == rows_.end()) { - return (false); - } - - row = *next_pos_; - ++next_pos_; - return (true); -} - -int -MemfileAddressStatsQuery4::getRowCount() { - return (rows_.size()); -} /// @brief Memfile derivation of the IPv6 statistical lease data query /// @@ -407,7 +400,9 @@ public: /// @brief Constructor /// /// @param storage6 A pointer to the v6 lease storage to be counted - MemfileAddressStatsQuery6(Lease6Storage& storage6); + MemfileAddressStatsQuery6(Lease6Storage& storage6) + : storage6_(storage6), rows_(0), next_pos_(rows_.end()) { + }; /// @brief Destructor virtual ~MemfileAddressStatsQuery6() {}; @@ -416,145 +411,132 @@ public: /// /// The result is populated by iterating over the IPv6 leases in storage, /// in ascending order by subnet ID, accumulating the lease state counts - /// per lease type. At the completion of all entries for a given subnet, - /// the counts are used to create AddressStatsRow5 instances which are - /// appended to an internal vector. The process results in a vector + /// per lease type. At the completion of all entries for a given subnet, + /// the counts are used to create AddressStatsRow5 instances which are + /// appended to an internal vector. The process results in a vector /// containing one entry per state per lease type per subnet. /// /// Currently the states counted are: /// - /// - Lease::STATE_DEFAULT (i.e. assigned) - /// - Lease::STATE_DECLINED - virtual void start(); + /// - Lease::STATE_DEFAULT (i.e. assigned) + /// - Lease::STATE_DECLINED + virtual void start() { + // Get the subnet_id index + const Lease6StorageSubnetIdIndex& idx + = storage6_.get(); + + // Iterate over the leases in order by subnet, accumulating per + // subnet counts for each state of interest. As we finish each + // subnet, add the appropriate rows to our result set. + SubnetID cur_id = 0; + int64_t assigned = 0; + int64_t declined = 0; + int64_t assigned_pds = 0; + + for(Lease6StorageSubnetIdIndex::const_iterator lease = idx.begin(); + lease != idx.end(); ++lease) { + + // If we've hit the next subnet, add rows for the current subnet + // and wipe the accumulators + if ((*lease)->subnet_id_ > cur_id) { + if (cur_id > 0) { + rows_.push_back(AddressStatsRow6(cur_id, Lease::TYPE_NA, + Lease::STATE_DEFAULT, + assigned)); + assigned = 0; + rows_.push_back(AddressStatsRow6(cur_id, Lease::TYPE_NA, + Lease::STATE_DECLINED, + declined)); + declined = 0; + rows_.push_back(AddressStatsRow6(cur_id, Lease::TYPE_PD, + Lease::STATE_DEFAULT, + assigned_pds)); + assigned_pds = 0; + } + + // Update current subnet id + cur_id = (*lease)->subnet_id_; + } + + // Bump the appropriate accumulator + switch ((*lease)->state_) { + case Lease::STATE_DEFAULT: + switch((*lease)->type_) { + case Lease::TYPE_NA: + ++assigned; + break; + case Lease::TYPE_PD: + ++assigned_pds; + break; + default: + break; + } + break; + case Lease::STATE_DECLINED: + // In theory only NAs can be declined + if (((*lease)->type_) == Lease::TYPE_NA) { + ++declined; + } + break; + default: + // Not one we're tracking. + break; + } + } + + // Make the rows for last subnet, unless there were no rows + if (idx.begin() != idx.end()) { + rows_.push_back(AddressStatsRow6(cur_id, Lease::TYPE_NA, + Lease::STATE_DEFAULT, + assigned)); + rows_.push_back(AddressStatsRow6(cur_id, Lease::TYPE_NA, + Lease::STATE_DECLINED, + declined)); + rows_.push_back(AddressStatsRow6(cur_id, Lease::TYPE_PD, + Lease::STATE_DEFAULT, + assigned_pds)); + } + + // Set the next row position to the beginning of the rows. + next_pos_ = rows_.begin(); + } /// @brief Fetches the next row in the result set /// /// Once the internal result set has been populated by invoking the /// the start() method, this method is used to iterate over the /// result set rows. Once the last row has been fetched, subsequent - /// calls will return false. + /// calls will return false. /// @param row Storage for the fetched row /// /// @return True if the fetch succeeded, false if there are no more /// rows to fetch. - virtual bool getNextRow(AddressStatsRow6& row); + virtual bool getNextRow(AddressStatsRow6& row) { + if (next_pos_ == rows_.end()) { + return (false); + } + + row = *next_pos_; + ++next_pos_; + return (true); + } /// @brief Returns the number of rows in the result set - /// @todo, should this be a virtual member of the base class? - int getRowCount(); + int getRowCount() { + return (rows_.size()); + } private: /// @brief The Memfile storage containing the IPv6 leases to analyze Lease6Storage& storage6_; - /// @brief A vector containing the "result set" + /// @brief A vector containing the "result set" std::vector rows_; /// @brief An iterator for accessing the next row within the result set std::vector::iterator next_pos_; }; -MemfileAddressStatsQuery6::MemfileAddressStatsQuery6(Lease6Storage& storage6) - : storage6_(storage6), rows_(0), next_pos_(rows_.end()) {}; - -void -MemfileAddressStatsQuery6::start() { - // Get the subnet_id index - const Lease6StorageSubnetIdIndex& idx = storage6_.get(); - - // Iterate over the leases in order by subnet, accumulating per - // subnet counts for each state of interest. As we finish each - // subnet, add the appropriate rows to our result set. - SubnetID cur_id = 0; - int64_t assigned = 0; - int64_t declined = 0; - int64_t assigned_pds = 0; - - for(Lease6StorageSubnetIdIndex::const_iterator lease = idx.begin(); - lease != idx.end(); ++lease) { - - // If we've hit the next subnet, add rows for the current subnet - // and wipe the accumulators - if ((*lease)->subnet_id_ > cur_id) { - if (cur_id > 0) { - rows_.push_back(AddressStatsRow6(cur_id, Lease::TYPE_NA, - Lease::STATE_DEFAULT, - assigned)); - assigned = 0; - rows_.push_back(AddressStatsRow6(cur_id, Lease::TYPE_NA, - Lease::STATE_DECLINED, - declined)); - declined = 0; - rows_.push_back(AddressStatsRow6(cur_id, Lease::TYPE_PD, - Lease::STATE_DEFAULT, - assigned_pds)); - assigned_pds = 0; - } - - // Update current subnet id - cur_id = (*lease)->subnet_id_; - } - - // Bump the appropriate accumulator - switch ((*lease)->state_) { - case Lease::STATE_DEFAULT: - switch((*lease)->type_) { - case Lease::TYPE_NA: - ++assigned; - break; - case Lease::TYPE_PD: - ++assigned_pds; - break; - default: - break; - } - break; - case Lease::STATE_DECLINED: - // In theory only NAs can be declined - if (((*lease)->type_) == Lease::TYPE_NA) { - ++declined; - } - break; - default: - // Not one we're tracking. - break; - } - } - - // Make the rows for last subnet, unless there were no rows - if (idx.begin() != idx.end()) { - rows_.push_back(AddressStatsRow6(cur_id, Lease::TYPE_NA, - Lease::STATE_DEFAULT, - assigned)); - rows_.push_back(AddressStatsRow6(cur_id, Lease::TYPE_NA, - Lease::STATE_DECLINED, - declined)); - rows_.push_back(AddressStatsRow6(cur_id, Lease::TYPE_PD, - Lease::STATE_DEFAULT, - assigned_pds)); - } - - // Set the next row position to the beginning of the rows. - next_pos_ = rows_.begin(); -} - -bool -MemfileAddressStatsQuery6::getNextRow(AddressStatsRow6& row) { - if (next_pos_ == rows_.end()) { - return (false); - } - - row = *next_pos_; - ++next_pos_; - return (true); -} - -int -MemfileAddressStatsQuery6::getRowCount() { - return (rows_.size()); -} - - // Explicit definition of class static constants. Values are given in the // declaration so they're not needed here. const int Memfile_LeaseMgr::MAJOR_VERSION; diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc index 52ed485d51..199911767a 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.cc +++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc @@ -1235,10 +1235,16 @@ public: /// @brief Constructor /// /// @param conn A open connection to the database housing the lease data - MySqlAddressStatsQuery4(MySqlConnection& conn); + MySqlAddressStatsQuery4(MySqlConnection& conn) + : conn_(conn), statement_(conn_.statements_[MySqlLeaseMgr + ::RECOUNT_LEASE4_STATS]), + bind_(3) { + } /// @brief Destructor - virtual ~MySqlAddressStatsQuery4(); + virtual ~MySqlAddressStatsQuery4() { + (void) mysql_stmt_free_result(statement_); + } /// @brief Creates the IPv4 lease statistical data result set /// @@ -1248,7 +1254,37 @@ public: /// MySqlLeaseMgr::RECOUNT_LEASE4_STATS. This method creates the binds /// the statement to the output bind array and then executes the /// statement. - void start(); + void start() { + // subnet_id: unsigned int + bind_[0].buffer_type = MYSQL_TYPE_LONG; + bind_[0].buffer = reinterpret_cast(&subnet_id_); + bind_[0].is_unsigned = MLM_TRUE; + + // state: uint32_t + bind_[1].buffer_type = MYSQL_TYPE_LONG; + bind_[1].buffer = reinterpret_cast(&lease_state_); + bind_[1].is_unsigned = MLM_TRUE; + + // state_count_: uint32_t + bind_[2].buffer_type = MYSQL_TYPE_LONG; + bind_[2].buffer = reinterpret_cast(&state_count_); + bind_[2].is_unsigned = MLM_TRUE; + + // Set up the MYSQL_BIND array for the data being returned + // and bind it to the statement. + int status = mysql_stmt_bind_result(statement_, &bind_[0]); + checkError(status, "RECOUNT_LEASE4_STATS: outbound binding failed"); + + // Execute the statement + status = mysql_stmt_execute(statement_); + checkError(status, "RECOUNT_LEASE4_STATS: unable to execute"); + + // Ensure that all the lease information is retrieved in one go to avoid + // overhead of going back and forth between client and server. + status = mysql_stmt_store_result(statement_); + checkError(status, "RECOUNT_LEASE4_STATS: results storage failed"); + } + /// @brief Fetches the next row in the result set /// @@ -1261,7 +1297,20 @@ public: /// /// @return True if the fetch succeeded, false if there are no more /// rows to fetch. - bool getNextRow(AddressStatsRow4& row); + bool getNextRow(AddressStatsRow4& row) { + bool have_row = false; + int status = mysql_stmt_fetch(statement_); + if (status == MLM_MYSQL_FETCH_SUCCESS) { + row.subnet_id_ = static_cast(subnet_id_); + row.lease_state_ = static_cast(lease_state_); + row.state_count_ = state_count_; + have_row = true; + } else if (status != MYSQL_NO_DATA) { + checkError(status, "RECOUNT_LEASE4_STATS: getNextRow failed"); + } + + return (have_row); + } private: @@ -1275,7 +1324,9 @@ private: /// @param status The MySQL statement execution outcome status /// @param what invocation context message which will be included in /// any exception - void checkError(int status, const char* what) const; + void checkError(int status, const char* what) const { + conn_.checkError(status, MySqlLeaseMgr::RECOUNT_LEASE4_STATS, what); + } /// @brief Database connection to use to execute the query MySqlConnection& conn_; @@ -1294,77 +1345,6 @@ private: uint32_t state_count_; }; -MySqlAddressStatsQuery4::MySqlAddressStatsQuery4(MySqlConnection& conn) - : conn_(conn), statement_(conn_.statements_[MySqlLeaseMgr - ::RECOUNT_LEASE4_STATS]), - bind_(3) { -} - -MySqlAddressStatsQuery4::~MySqlAddressStatsQuery4() { - (void) mysql_stmt_free_result(statement_); -} - - -void -MySqlAddressStatsQuery4::start() { - // subnet_id: unsigned int - bind_[0].buffer_type = MYSQL_TYPE_LONG; - bind_[0].buffer = reinterpret_cast(&subnet_id_); - bind_[0].is_unsigned = MLM_TRUE; - - // state: uint32_t - bind_[1].buffer_type = MYSQL_TYPE_LONG; - bind_[1].buffer = reinterpret_cast(&lease_state_); - bind_[1].is_unsigned = MLM_TRUE; - - // state_count_: uint32_t - bind_[2].buffer_type = MYSQL_TYPE_LONG; - bind_[2].buffer = reinterpret_cast(&state_count_); - bind_[2].is_unsigned = MLM_TRUE; - - // Set up the MYSQL_BIND array for the data being returned - // and bind it to the statement. - int status = mysql_stmt_bind_result(statement_, &bind_[0]); - checkError(status, "RECOUNT_LEASE4_STATS: outbound binding failed"); - - // Execute the statement - status = mysql_stmt_execute(statement_); - checkError(status, "RECOUNT_LEASE4_STATS: unable to execute"); - - // Ensure that all the lease information is retrieved in one go to avoid - // overhead of going back and forth between client and server. - status = mysql_stmt_store_result(statement_); - checkError(status, "RECOUNT_LEASE4_STATS: results storage setup failed"); -} - -bool -MySqlAddressStatsQuery4::getNextRow(AddressStatsRow4& row) { - bool have_row = false; - int status = mysql_stmt_fetch(statement_); - if (status == MLM_MYSQL_FETCH_SUCCESS) { - row.subnet_id_ = static_cast(subnet_id_); - row.lease_state_ = static_cast(lease_state_); - row.state_count_ = state_count_; - have_row = true; - } else if (status != MYSQL_NO_DATA) { - checkError(status, "RECOUNT_LEASE4_STATS: getNextRow failed"); - } - - return (have_row); -} - -void -MySqlAddressStatsQuery4::checkError(int status, const char* what) const { - conn_.checkError(status, MySqlLeaseMgr::RECOUNT_LEASE4_STATS, what); -} - -AddressStatsQuery4Ptr -MySqlLeaseMgr::startAddressStatsQuery4() { - AddressStatsQuery4Ptr query(new MySqlAddressStatsQuery4(conn_)); - query->start(); - return(query); -} - /// @brief MySql derivation of the IPv6 statistical lease data query /// /// This class is used to recalculate IPv6 lease statistics for MySQL @@ -1377,10 +1357,16 @@ public: /// @brief Constructor /// /// @param conn A open connection to the database housing the lease data - MySqlAddressStatsQuery6(MySqlConnection& conn); + MySqlAddressStatsQuery6(MySqlConnection& conn) + : conn_(conn), statement_(conn_.statements_[MySqlLeaseMgr + ::RECOUNT_LEASE6_STATS]), + bind_(4) { + } /// @brief Destructor - virtual ~MySqlAddressStatsQuery6(); + virtual ~MySqlAddressStatsQuery6() { + (void) mysql_stmt_free_result(statement_); + } /// @brief Creates the IPv6 lease statistical data result set /// @@ -1390,7 +1376,42 @@ public: /// MySqlLeaseMgr::RECOUNT_LEASE6_STATS. This method creates the binds /// the statement to the output bind array and then executes the /// statement. - void start(); + void start() { + // subnet_id: unsigned int + bind_[0].buffer_type = MYSQL_TYPE_LONG; + bind_[0].buffer = reinterpret_cast(&subnet_id_); + bind_[0].is_unsigned = MLM_TRUE; + + // lease type: uint32_t + bind_[1].buffer_type = MYSQL_TYPE_LONG; + bind_[1].buffer = reinterpret_cast(&lease_type_); + bind_[1].is_unsigned = MLM_TRUE; + + // state: uint32_t + bind_[2].buffer_type = MYSQL_TYPE_LONG; + bind_[2].buffer = reinterpret_cast(&lease_state_); + bind_[2].is_unsigned = MLM_TRUE; + + // state_count_: uint32_t + bind_[3].buffer_type = MYSQL_TYPE_LONG; + bind_[3].buffer = reinterpret_cast(&state_count_); + bind_[3].is_unsigned = MLM_TRUE; + + // Set up the MYSQL_BIND array for the data being returned + // and bind it to the statement. + int status = mysql_stmt_bind_result(statement_, &bind_[0]); + checkError(status, "RECOUNT_LEASE6_STATS: outbound binding failed"); + + // Execute the statement + status = mysql_stmt_execute(statement_); + checkError(status, "RECOUNT_LEASE6_STATS: unable to execute"); + + // Ensure that all the lease information is retrieved in one go to avoid + // overhead of going back and forth between client and server. + status = mysql_stmt_store_result(statement_); + checkError(status, "RECOUNT_LEASE6_STATS: results storage failed"); + } + /// @brief Fetches the next row in the result set /// @@ -1403,7 +1424,22 @@ public: /// /// @return True if the fetch succeeded, false if there are no more /// rows to fetch. - bool getNextRow(AddressStatsRow6& row); + bool getNextRow(AddressStatsRow6& row) { + bool have_row = false; + int status = mysql_stmt_fetch(statement_); + if (status == MLM_MYSQL_FETCH_SUCCESS) { + row.subnet_id_ = static_cast(subnet_id_); + row.lease_type_ = static_cast(lease_type_); + row.lease_state_ = static_cast(lease_state_); + row.state_count_ = state_count_; + have_row = true; + } else if (status != MYSQL_NO_DATA) { + checkError(status, "RECOUNT_LEASE6_STATS: getNextRow failed"); + } + + return (have_row); + } + private: @@ -1417,7 +1453,9 @@ private: /// @param status The MySQL statement execution outcome status /// @param what invocation context message which will be included in /// any exception - void checkError(int status, const char* what) const; + void checkError(int status, const char* what) const { + conn_.checkError(status, MySqlLeaseMgr::RECOUNT_LEASE6_STATS, what); + } /// @brief Database connection to use to execute the query MySqlConnection& conn_; @@ -1438,82 +1476,6 @@ private: uint32_t state_count_; }; -MySqlAddressStatsQuery6::MySqlAddressStatsQuery6(MySqlConnection& conn) - : conn_(conn), statement_(conn_.statements_[MySqlLeaseMgr - ::RECOUNT_LEASE6_STATS]), - bind_(4) { -} - -MySqlAddressStatsQuery6::~MySqlAddressStatsQuery6() { - (void) mysql_stmt_free_result(statement_); -} - -void -MySqlAddressStatsQuery6::start() { - // subnet_id: unsigned int - bind_[0].buffer_type = MYSQL_TYPE_LONG; - bind_[0].buffer = reinterpret_cast(&subnet_id_); - bind_[0].is_unsigned = MLM_TRUE; - - // lease type: uint32_t - bind_[1].buffer_type = MYSQL_TYPE_LONG; - bind_[1].buffer = reinterpret_cast(&lease_type_); - bind_[1].is_unsigned = MLM_TRUE; - - // state: uint32_t - bind_[2].buffer_type = MYSQL_TYPE_LONG; - bind_[2].buffer = reinterpret_cast(&lease_state_); - bind_[2].is_unsigned = MLM_TRUE; - - // state_count_: uint32_t - bind_[3].buffer_type = MYSQL_TYPE_LONG; - bind_[3].buffer = reinterpret_cast(&state_count_); - bind_[3].is_unsigned = MLM_TRUE; - - // Set up the MYSQL_BIND array for the data being returned - // and bind it to the statement. - int status = mysql_stmt_bind_result(statement_, &bind_[0]); - checkError(status, "RECOUNT_LEASE6_STATS: outbound binding failed"); - - // Execute the statement - status = mysql_stmt_execute(statement_); - checkError(status, "RECOUNT_LEASE6_STATS: unable to execute"); - - // Ensure that all the lease information is retrieved in one go to avoid - // overhead of going back and forth between client and server. - status = mysql_stmt_store_result(statement_); - checkError(status, "RECOUNT_LEASE6_STATS: results storage setup failed"); -} - -bool -MySqlAddressStatsQuery6::getNextRow(AddressStatsRow6& row) { - bool have_row = false; - int status = mysql_stmt_fetch(statement_); - if (status == MLM_MYSQL_FETCH_SUCCESS) { - row.subnet_id_ = static_cast(subnet_id_); - row.lease_type_ = static_cast(lease_type_); - row.lease_state_ = static_cast(lease_state_); - row.state_count_ = state_count_; - have_row = true; - } else if (status != MYSQL_NO_DATA) { - checkError(status, "RECOUNT_LEASE6_STATS: getNextRow failed"); - } - - return (have_row); -} - -void -MySqlAddressStatsQuery6::checkError(int status, const char* what) const { - conn_.checkError(status, MySqlLeaseMgr::RECOUNT_LEASE6_STATS, what); -} - -AddressStatsQuery6Ptr -MySqlLeaseMgr::startAddressStatsQuery6() { - AddressStatsQuery6Ptr query(new MySqlAddressStatsQuery6(conn_)); - query->start(); - return(query); -} - // MySqlLeaseMgr Constructor and Destructor MySqlLeaseMgr::MySqlLeaseMgr(const MySqlConnection::ParameterMap& parameters) @@ -2322,6 +2284,20 @@ MySqlLeaseMgr::getVersion() const { return (std::make_pair(major, minor)); } +AddressStatsQuery4Ptr +MySqlLeaseMgr::startAddressStatsQuery4() { + AddressStatsQuery4Ptr query(new MySqlAddressStatsQuery4(conn_)); + query->start(); + return(query); +} + +AddressStatsQuery6Ptr +MySqlLeaseMgr::startAddressStatsQuery6() { + AddressStatsQuery6Ptr query(new MySqlAddressStatsQuery6(conn_)); + query->start(); + return(query); +} + void MySqlLeaseMgr::commit() { LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, DHCPSRV_MYSQL_COMMIT); diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.cc b/src/lib/dhcpsrv/pgsql_lease_mgr.cc index 751f636004..13c3c2ee4d 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.cc +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.cc @@ -706,7 +706,11 @@ public: /// @brief Constructor /// /// @param conn A open connection to the database housing the lease data - PgSqlAddressStatsQuery4(PgSqlConnection& conn); + PgSqlAddressStatsQuery4(PgSqlConnection& conn) + : conn_(conn), statement_(tagged_statements[PgSqlLeaseMgr + ::RECOUNT_LEASE4_STATS]), + result_set_(), next_row_(0) { + } /// @brief Destructor virtual ~PgSqlAddressStatsQuery4() {}; @@ -718,7 +722,13 @@ public: /// The query used is the prepared statement identified by /// PgSqlLeaseMgr::RECOUNT_LEASE4_STATS. This method executes the /// statement which creates the result set. - void start(); + void start() { + // The query has no parameters, so we only need it's name. + result_set_.reset(new PgSqlResult(PQexecPrepared(conn_, statement_.name, + 0, NULL, NULL, NULL, 0))); + + conn_.checkStatementError(*result_set_, statement_); + } /// @brief Fetches the next row in the result set /// @@ -731,22 +741,35 @@ public: /// /// @return True if the fetch succeeded, false if there are no more /// rows to fetch. - bool getNextRow(AddressStatsRow4& row); - -private: + bool getNextRow(AddressStatsRow4& row) { + // If we're past the end, punt. + if (next_row_ >= result_set_->getRows()) { + return (false); + } - /// @brief Analyzes the given statement outcome status - /// - /// Wrapper method around the PgSqlConnection:checkError() that is - /// used to generate the appropriate exception if the status indicates - /// an error. - //// - /// a DbOperation error - /// @param status The MySQL statement execution outcome status - /// @param what invocation context message which will be included in - /// any exception - void checkError(int status, const char* what) const; + // Fetch the subnet id. + uint32_t col = 0; + uint32_t subnet_id; + PgSqlExchange::getColumnValue(*result_set_, next_row_, col, subnet_id); + row.subnet_id_ = static_cast(subnet_id); + ++col; + + // Fetch the lease state. + uint32_t state; + PgSqlExchange::getColumnValue(*result_set_, next_row_ , col, state); + row.lease_state_ = static_cast(state); + ++col; + + // Fetch the state count. + PgSqlExchange::getColumnValue(*result_set_, next_row_, col, + row.state_count_); + + // Point to the next row. + ++next_row_; + return (true); + } +private: /// @brief Database connection to use to execute the query PgSqlConnection& conn_; @@ -760,58 +783,6 @@ private: uint32_t next_row_; }; -PgSqlAddressStatsQuery4::PgSqlAddressStatsQuery4(PgSqlConnection& conn) - : conn_(conn), statement_(tagged_statements[PgSqlLeaseMgr - ::RECOUNT_LEASE4_STATS]), - result_set_(), next_row_(0) { -} - -void -PgSqlAddressStatsQuery4::start() { - // The query has no parameters, so we only need it's name. - result_set_.reset(new PgSqlResult(PQexecPrepared(conn_, statement_.name, - 0, NULL, NULL, NULL, 0))); - - conn_.checkStatementError(*result_set_, statement_); -} - -bool -PgSqlAddressStatsQuery4::getNextRow(AddressStatsRow4& row) { - // If we're past the end, punt. - if (next_row_ >= result_set_->getRows()) { - return (false); - } - - // Fetch the subnet id. - uint32_t col = 0; - uint32_t subnet_id; - PgSqlExchange::getColumnValue(*result_set_, next_row_, col, subnet_id); - row.subnet_id_ = static_cast(subnet_id); - ++col; - - // Fetch the lease state. - uint32_t state; - PgSqlExchange::getColumnValue(*result_set_, next_row_ , col, state); - row.lease_state_ = static_cast(state); - ++col; - - // Fetch the state count. - PgSqlExchange::getColumnValue(*result_set_, next_row_, col, - row.state_count_); - - // Point to the next row. - ++next_row_; - - return (true); -} - -AddressStatsQuery4Ptr -PgSqlLeaseMgr::startAddressStatsQuery4() { - AddressStatsQuery4Ptr query(new PgSqlAddressStatsQuery4(conn_)); - query->start(); - return(query); -} - /// @brief PgSql derivation of the IPv6 statistical lease data query /// /// This class is used to recalculate IPv6 lease statistics for MySQL @@ -824,7 +795,11 @@ public: /// @brief Constructor /// /// @param conn A open connection to the database housing the lease data - PgSqlAddressStatsQuery6(PgSqlConnection& conn); + PgSqlAddressStatsQuery6(PgSqlConnection& conn) + : conn_(conn), statement_(tagged_statements[PgSqlLeaseMgr + ::RECOUNT_LEASE6_STATS]), + result_set_(), next_row_(0) { + } /// @brief Destructor virtual ~PgSqlAddressStatsQuery6() {}; @@ -836,7 +811,14 @@ public: /// per subnet id. The query used is the prepared statement identified by /// PgSqlLeaseMgr::RECOUNT_LEASE6_STATS. This method executes the /// statement which creates the result set. - void start(); + void start() { + // The query has no parameters, so we only need it's name. + result_set_.reset(new PgSqlResult(PQexecPrepared(conn_, statement_.name, + 0, NULL, NULL, NULL, + 0))); + + conn_.checkStatementError(*result_set_, statement_); + } /// @brief Fetches the next row in the result set /// @@ -849,22 +831,43 @@ public: /// /// @return True if the fetch succeeded, false if there are no more /// rows to fetch. - bool getNextRow(AddressStatsRow6& row); - -private: + bool getNextRow(AddressStatsRow6& row) { + // If we're past the end, punt. + if (next_row_ >= result_set_->getRows()) { + return (false); + } - /// @brief Analyzes the given statement outcome status - /// - /// Wrapper method around the PgSqlConnection:checkError() that is - /// used to generate the appropriate exception if the status indicates - /// an error. - //// - /// a DbOperation error - /// @param status The MySQL statement execution outcome status - /// @param what invocation context message which will be included in - /// any exception - void checkError(int status, const char* what) const; + // Fetch the subnet id. + uint32_t col = 0; + uint32_t subnet_id; + PgSqlExchange::getColumnValue(*result_set_, next_row_, col, subnet_id); + row.subnet_id_ = static_cast(subnet_id); + ++col; + + // Fetch the lease type. + uint32_t lease_type; + PgSqlExchange::getColumnValue(*result_set_, next_row_ , col, + lease_type); + row.lease_type_ = static_cast(lease_type); + ++col; + + // Fetch the lease state. + uint32_t state; + PgSqlExchange::getColumnValue(*result_set_, next_row_ , col, state); + row.lease_state_ = static_cast(state); + ++col; + + // Fetch the state count. + PgSqlExchange::getColumnValue(*result_set_, next_row_, col, + row.state_count_); + + // Point to the next row. + ++next_row_; + + return (true); + } +private: /// @brief Database connection to use to execute the query PgSqlConnection& conn_; @@ -878,63 +881,6 @@ private: uint32_t next_row_; }; -PgSqlAddressStatsQuery6::PgSqlAddressStatsQuery6(PgSqlConnection& conn) - : conn_(conn), statement_(tagged_statements[PgSqlLeaseMgr - ::RECOUNT_LEASE6_STATS]), - result_set_(), next_row_(0) { -} - -void -PgSqlAddressStatsQuery6::start() { - // The query has no parameters, so we only need it's name. - result_set_.reset(new PgSqlResult(PQexecPrepared(conn_, statement_.name, - 0, NULL, NULL, NULL, 0))); - - conn_.checkStatementError(*result_set_, statement_); -} - -bool -PgSqlAddressStatsQuery6::getNextRow(AddressStatsRow6& row) { - // If we're past the end, punt. - if (next_row_ >= result_set_->getRows()) { - return (false); - } - - // Fetch the subnet id. - uint32_t col = 0; - uint32_t subnet_id; - PgSqlExchange::getColumnValue(*result_set_, next_row_, col, subnet_id); - row.subnet_id_ = static_cast(subnet_id); - ++col; - - // Fetch the lease type. - uint32_t lease_type; - PgSqlExchange::getColumnValue(*result_set_, next_row_ , col, lease_type); - row.lease_type_ = static_cast(lease_type); - ++col; - - // Fetch the lease state. - uint32_t state; - PgSqlExchange::getColumnValue(*result_set_, next_row_ , col, state); - row.lease_state_ = static_cast(state); - ++col; - - // Fetch the state count. - PgSqlExchange::getColumnValue(*result_set_, next_row_, col, row.state_count_); - - // Point to the next row. - ++next_row_; - - return (true); -} - -AddressStatsQuery6Ptr -PgSqlLeaseMgr::startAddressStatsQuery6() { - AddressStatsQuery6Ptr query(new PgSqlAddressStatsQuery6(conn_)); - query->start(); - return(query); -} - PgSqlLeaseMgr::PgSqlLeaseMgr(const DatabaseConnection::ParameterMap& parameters) : LeaseMgr(), exchange4_(new PgSqlLease4Exchange()), exchange6_(new PgSqlLease6Exchange()), conn_(parameters) { @@ -1476,6 +1422,20 @@ PgSqlLeaseMgr::deleteExpiredReclaimedLeasesCommon(const uint32_t secs, return (deleteLeaseCommon(statement_index, bind_array)); } +AddressStatsQuery4Ptr +PgSqlLeaseMgr::startAddressStatsQuery4() { + AddressStatsQuery4Ptr query(new PgSqlAddressStatsQuery4(conn_)); + query->start(); + return(query); +} + +AddressStatsQuery6Ptr +PgSqlLeaseMgr::startAddressStatsQuery6() { + AddressStatsQuery6Ptr query(new PgSqlAddressStatsQuery6(conn_)); + query->start(); + return(query); +} + string PgSqlLeaseMgr::getName() const { string name = ""; -- cgit v1.2.3 From 180a0b201d37ce023c3c8f37c4ad69cd175ad217 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Thu, 18 Aug 2016 06:54:58 -0400 Subject: [4294] Fixed a doxygen error --- src/lib/dhcpsrv/lease_mgr.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib/dhcpsrv/lease_mgr.h b/src/lib/dhcpsrv/lease_mgr.h index f0a58f7aef..a8f21b1009 100644 --- a/src/lib/dhcpsrv/lease_mgr.h +++ b/src/lib/dhcpsrv/lease_mgr.h @@ -224,6 +224,7 @@ struct AddressStatsRow6 { /// @brief Constructor /// /// @param subnet_id The subnet id to which this data applies + /// @param lease_type The lease type for this state count /// @param lease_state The lease state counted /// @param state_count The count of leases in the lease state AddressStatsRow6(const SubnetID& subnet_id, const Lease::Type& lease_type, @@ -235,7 +236,7 @@ struct AddressStatsRow6 { /// @brief The subnet ID to which this data applies SubnetID subnet_id_; - /// @brief The lease_state to which the count applies + /// @brief The lease_type to which the count applies Lease::Type lease_type_; /// @brief The lease_state to which the count applies uint32_t lease_state_; -- cgit v1.2.3 From 643ced7b1a13d3683e9fc797def8c3232a97f30c Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Tue, 23 Aug 2016 18:50:26 +0200 Subject: [4294] Compilation fix for unused parameter --- src/lib/dhcpsrv/lease_mgr.cc | 9 +++++++++ src/lib/dhcpsrv/lease_mgr.h | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/lib/dhcpsrv/lease_mgr.cc b/src/lib/dhcpsrv/lease_mgr.cc index d5aa90dfdc..b3c34f13ad 100644 --- a/src/lib/dhcpsrv/lease_mgr.cc +++ b/src/lib/dhcpsrv/lease_mgr.cc @@ -123,6 +123,11 @@ LeaseMgr::startAddressStatsQuery4() { return(AddressStatsQuery4Ptr()); } +bool +AddressStatsQuery4::getNextRow(AddressStatsRow4& /*row*/) { + return (false); +} + void LeaseMgr::recountAddressStats6() { using namespace stats; @@ -230,6 +235,10 @@ LeaseMgr::startAddressStatsQuery6() { return(AddressStatsQuery6Ptr()); } +bool +AddressStatsQuery6::getNextRow(AddressStatsRow6& /*row*/) { + return (false); +} std::string LeaseMgr::getDBVersion() { diff --git a/src/lib/dhcpsrv/lease_mgr.h b/src/lib/dhcpsrv/lease_mgr.h index a8f21b1009..679b7b9b6c 100644 --- a/src/lib/dhcpsrv/lease_mgr.h +++ b/src/lib/dhcpsrv/lease_mgr.h @@ -204,7 +204,7 @@ public: /// /// @return True if a row was fetched, false if there are no /// more rows. - virtual bool getNextRow(AddressStatsRow4& row) { return(false); }; + virtual bool getNextRow(AddressStatsRow4& row); }; /// @brief Defines a pointer to an AddressStatsQuery4. @@ -271,7 +271,7 @@ public: /// /// @return True if a row was fetched, false if there are no /// more rows. - virtual bool getNextRow(AddressStatsRow6& row) { return (false); }; + virtual bool getNextRow(AddressStatsRow6& row); }; /// @brief Defines a pointer to an AddressStatsQuery6. -- cgit v1.2.3 From 86411b64adb0de8ce8b1bd6394c475b08afdb8db Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Tue, 23 Aug 2016 14:16:46 -0400 Subject: [4294] Reverted Lease states back to uin32_t per review comments I had replaced the uint32_t constants for lease states with an enum. Tomek stated that they should remain uin32_t as they may eventually be bitmasks. --- src/lib/dhcpsrv/lease.cc | 4 + src/lib/dhcpsrv/lease.h | 22 +++-- src/lib/dhcpsrv/lease_mgr.cc | 93 ++++++++-------------- src/lib/dhcpsrv/lease_mgr.h | 6 +- src/lib/dhcpsrv/memfile_lease_mgr.cc | 20 +---- src/lib/dhcpsrv/mysql_lease_mgr.cc | 4 +- src/lib/dhcpsrv/pgsql_lease_mgr.cc | 8 +- .../dhcpsrv/tests/generic_lease_mgr_unittest.cc | 4 +- src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h | 4 +- 9 files changed, 65 insertions(+), 100 deletions(-) diff --git a/src/lib/dhcpsrv/lease.cc b/src/lib/dhcpsrv/lease.cc index d8d15613d1..5854f06900 100644 --- a/src/lib/dhcpsrv/lease.cc +++ b/src/lib/dhcpsrv/lease.cc @@ -15,6 +15,10 @@ using namespace std; namespace isc { namespace dhcp { +const uint32_t Lease::STATE_DEFAULT = 0x0; +const uint32_t Lease::STATE_DECLINED = 0x1; +const uint32_t Lease::STATE_EXPIRED_RECLAIMED = 0x2; + Lease::Lease(const isc::asiolink::IOAddress& addr, uint32_t t1, uint32_t t2, uint32_t valid_lft, SubnetID subnet_id, time_t cltt, const bool fqdn_fwd, const bool fqdn_rev, diff --git a/src/lib/dhcpsrv/lease.h b/src/lib/dhcpsrv/lease.h index 365cb06f53..c89ad523a3 100644 --- a/src/lib/dhcpsrv/lease.h +++ b/src/lib/dhcpsrv/lease.h @@ -40,19 +40,17 @@ struct Lease { /// @return text decription static std::string typeToText(Type type); - /// @name Enumeration of lease states + /// @name Common lease states constants. //@{ - typedef enum { - /// @brief A lease in the default (assigned) state. - STATE_DEFAULT, - /// @brief Declined lease. - STATE_DECLINED, - /// @brief Expired and reclaimed lease. - STATE_EXPIRED_RECLAIMED, - /// @brief The number of defined lease states. - NUM_LEASE_STATES - } LeaseState; - //@} + /// + /// @brief A lease in the default state. + static const uint32_t STATE_DEFAULT; + + /// @brief Declined lease. + static const uint32_t STATE_DECLINED; + + /// @brief Expired and reclaimed lease. + static const uint32_t STATE_EXPIRED_RECLAIMED; /// @brief Returns name(s) of the basic lease state(s). /// diff --git a/src/lib/dhcpsrv/lease_mgr.cc b/src/lib/dhcpsrv/lease_mgr.cc index b3c34f13ad..de2bf01153 100644 --- a/src/lib/dhcpsrv/lease_mgr.cc +++ b/src/lib/dhcpsrv/lease_mgr.cc @@ -91,29 +91,19 @@ LeaseMgr::recountAddressStats4() { // updating the subnet and global values. AddressStatsRow4 row; while (query->getNextRow(row)) { - switch(row.lease_state_) { - case Lease::STATE_DEFAULT: - // Set subnet level value. - stats_mgr.setValue(StatsMgr::generateName("subnet", - row.subnet_id_, - "assigned-addresses"), - row.state_count_); - break; - - case Lease::STATE_DECLINED: - // Set subnet level value. - stats_mgr.setValue(StatsMgr::generateName("subnet", - row.subnet_id_, - "declined-addresses"), - row.state_count_); - - // Add to the global value. - stats_mgr.addValue("declined-addresses", row.state_count_); - break; - - default: - // Not one we're tracking. - break; + if (row.lease_state_ == Lease::STATE_DEFAULT) { + // Set subnet level value. + stats_mgr.setValue(StatsMgr::generateName("subnet", row.subnet_id_, + "assigned-addresses"), + row.state_count_); + } else if (row.lease_state_ == Lease::STATE_DECLINED) { + // Set subnet level value. + stats_mgr.setValue(StatsMgr::generateName("subnet", row.subnet_id_, + "declined-addresses"), + row.state_count_); + + // Add to the global value. + stats_mgr.addValue("declined-addresses", row.state_count_); } } } @@ -180,46 +170,31 @@ LeaseMgr::recountAddressStats6() { while (query->getNextRow(row)) { switch(row.lease_type_) { case Lease::TYPE_NA: - switch(row.lease_state_) { - case Lease::STATE_DEFAULT: - // Set subnet level value. - stats_mgr.setValue(StatsMgr:: - generateName("subnet", - row.subnet_id_, - "assigned-nas"), - row.state_count_); - break; - case Lease::STATE_DECLINED: - // Set subnet level value. - stats_mgr.setValue(StatsMgr:: - generateName("subnet", - row.subnet_id_, - "declined-addresses"), - row.state_count_); - - // Add to the global value. - stats_mgr.addValue("declined-addresses", - row.state_count_); - break; - default: - // Not one we're tracking. - break; + if (row.lease_state_ == Lease::STATE_DEFAULT) { + // Set subnet level value. + stats_mgr.setValue(StatsMgr:: + generateName("subnet", row.subnet_id_, + "assigned-nas"), + row.state_count_); + } if (row.lease_state_ == Lease::STATE_DECLINED) { + // Set subnet level value. + stats_mgr.setValue(StatsMgr:: + generateName("subnet", row.subnet_id_, + "declined-addresses"), + row.state_count_); + + // Add to the global value. + stats_mgr.addValue("declined-addresses", row.state_count_); } break; case Lease::TYPE_PD: - switch(row.lease_state_) { - case Lease::STATE_DEFAULT: - // Set subnet level value. - stats_mgr.setValue(StatsMgr:: - generateName("subnet", - row.subnet_id_, - "assigned-pds"), - row.state_count_); - break; - default: - // Not one we're tracking. - break; + if (row.lease_state_ == Lease::STATE_DEFAULT) { + // Set subnet level value. + stats_mgr.setValue(StatsMgr:: + generateName("subnet", row.subnet_id_, + "assigned-pds"), + row.state_count_); } break; diff --git a/src/lib/dhcpsrv/lease_mgr.h b/src/lib/dhcpsrv/lease_mgr.h index 679b7b9b6c..456a237f66 100644 --- a/src/lib/dhcpsrv/lease_mgr.h +++ b/src/lib/dhcpsrv/lease_mgr.h @@ -163,7 +163,7 @@ struct AddressStatsRow4 { /// @param lease_state The lease state counted /// @param state_count The count of leases in the lease state AddressStatsRow4(const SubnetID& subnet_id, - const Lease::LeaseState& lease_state, + const uint32_t lease_state, const int64_t state_count) : subnet_id_(subnet_id), lease_state_(lease_state), state_count_(state_count) { @@ -172,7 +172,7 @@ struct AddressStatsRow4 { /// @brief The subnet ID to which this data applies SubnetID subnet_id_; /// @brief The lease_state to which the count applies - Lease::LeaseState lease_state_; + uint32_t lease_state_; /// @brief state_count The count of leases in the lease state int64_t state_count_; }; @@ -228,7 +228,7 @@ struct AddressStatsRow6 { /// @param lease_state The lease state counted /// @param state_count The count of leases in the lease state AddressStatsRow6(const SubnetID& subnet_id, const Lease::Type& lease_type, - const Lease::LeaseState& lease_state, + const uint32_t lease_state, const int64_t state_count) : subnet_id_(subnet_id), lease_type_(lease_type), lease_state_(lease_state), state_count_(state_count) { diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.cc b/src/lib/dhcpsrv/memfile_lease_mgr.cc index 2ffaa7c114..8f5313a997 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.cc +++ b/src/lib/dhcpsrv/memfile_lease_mgr.cc @@ -324,16 +324,10 @@ public: } // Bump the appropriate accumulator - switch ((*lease)->state_) { - case Lease::STATE_DEFAULT: + if ((*lease)->state_ == Lease::STATE_DEFAULT) { ++assigned; - break; - case Lease::STATE_DECLINED: + } else if ((*lease)->state_ == Lease::STATE_DECLINED) { ++declined; - break; - default: - // Not one we're tracking. - break; } } @@ -459,8 +453,7 @@ public: } // Bump the appropriate accumulator - switch ((*lease)->state_) { - case Lease::STATE_DEFAULT: + if ((*lease)->state_ == Lease::STATE_DEFAULT) { switch((*lease)->type_) { case Lease::TYPE_NA: ++assigned; @@ -471,16 +464,11 @@ public: default: break; } - break; - case Lease::STATE_DECLINED: + } else if ((*lease)->state_ == Lease::STATE_DECLINED) { // In theory only NAs can be declined if (((*lease)->type_) == Lease::TYPE_NA) { ++declined; } - break; - default: - // Not one we're tracking. - break; } } diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc index 199911767a..7f7c1bf3ca 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.cc +++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc @@ -1302,7 +1302,7 @@ public: int status = mysql_stmt_fetch(statement_); if (status == MLM_MYSQL_FETCH_SUCCESS) { row.subnet_id_ = static_cast(subnet_id_); - row.lease_state_ = static_cast(lease_state_); + row.lease_state_ = lease_state_; row.state_count_ = state_count_; have_row = true; } else if (status != MYSQL_NO_DATA) { @@ -1430,7 +1430,7 @@ public: if (status == MLM_MYSQL_FETCH_SUCCESS) { row.subnet_id_ = static_cast(subnet_id_); row.lease_type_ = static_cast(lease_type_); - row.lease_state_ = static_cast(lease_state_); + row.lease_state_ = lease_state_; row.state_count_ = state_count_; have_row = true; } else if (status != MYSQL_NO_DATA) { diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.cc b/src/lib/dhcpsrv/pgsql_lease_mgr.cc index 13c3c2ee4d..2b92e69d6b 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.cc +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.cc @@ -756,8 +756,8 @@ public: // Fetch the lease state. uint32_t state; - PgSqlExchange::getColumnValue(*result_set_, next_row_ , col, state); - row.lease_state_ = static_cast(state); + PgSqlExchange::getColumnValue(*result_set_, next_row_ , col, + row.lease_state_); ++col; // Fetch the state count. @@ -853,8 +853,8 @@ public: // Fetch the lease state. uint32_t state; - PgSqlExchange::getColumnValue(*result_set_, next_row_ , col, state); - row.lease_state_ = static_cast(state); + PgSqlExchange::getColumnValue(*result_set_, next_row_ , col, + row.lease_state_); ++col; // Fetch the state count. diff --git a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc index 3d411db9f0..399533da0e 100644 --- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc @@ -2431,7 +2431,7 @@ GenericLeaseMgrTest::checkAddressStats(const StatValMapList& expectedStats) { void GenericLeaseMgrTest::makeLease4(const std::string& address, const SubnetID& subnet_id, - const Lease::LeaseState& state) { + const uint32_t state) { Lease4Ptr lease(new Lease4()); // set the address @@ -2455,7 +2455,7 @@ GenericLeaseMgrTest::makeLease6(const Lease::Type& type, const std::string& address, uint8_t prefix_len, const SubnetID& subnet_id, - const Lease::LeaseState& state) { + const uint32_t state) { IOAddress addr(address); // make a DUID from the address diff --git a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h index 6a7ae4d640..b3b1f66550 100644 --- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h +++ b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h @@ -126,7 +126,7 @@ public: /// @param subnet_id - subnet ID to which the lease belongs /// @param state - the state of the lease void makeLease4(const std::string& address, const SubnetID& subnet_id, - const Lease::LeaseState& state = Lease::STATE_DEFAULT); + const uint32_t state = Lease::STATE_DEFAULT); /// @brief Constructs a minimal IPv6 lease and adds it to the lease storage /// @@ -139,7 +139,7 @@ public: /// @param state - the state of the lease void makeLease6(const Lease::Type& type, const std::string& address, uint8_t prefix_len, const SubnetID& subnet_id, - const Lease::LeaseState& state = Lease::STATE_DEFAULT); + const uint32_t state = Lease::STATE_DEFAULT); /// @brief checks that addLease, getLease4(addr) and deleteLease() works void testBasicLease4(); -- cgit v1.2.3 From 3de19ec65fdfb0166f3ced78eaab34506338f971 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Tue, 23 Aug 2016 14:53:48 -0400 Subject: [4294] Removed todo comment per review --- src/lib/dhcpsrv/lease_mgr.cc | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/lib/dhcpsrv/lease_mgr.cc b/src/lib/dhcpsrv/lease_mgr.cc index de2bf01153..02aedf5f4a 100644 --- a/src/lib/dhcpsrv/lease_mgr.cc +++ b/src/lib/dhcpsrv/lease_mgr.cc @@ -59,10 +59,7 @@ LeaseMgr::recountAddressStats4() { return; } - // Zero out the global stats. (Ok, so currently there's only one - // that should be cleared. "reclaimed-declined-addresses" never - // gets zeroed. @todo discuss with Tomek the rational of not - // clearing it when we clear the rest. + // Zero out the global stats. int64_t zero = 0; stats_mgr.setValue("declined-addresses", zero); stats_mgr.setValue("declined-reclaimed-addresses", zero); -- cgit v1.2.3 From 9a1249f166f2584a82972c87459ad0b03d940d1f Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Tue, 23 Aug 2016 16:34:28 -0400 Subject: [4294] Refactored AddressStatsRow and AddressStatsQuery classes src/lib/dhcpsrv/cfg_subnets4.cc CfgSubnets4::updateStatistics() - replaced recountAddressStats4() with recountLeaseStats4() src/lib/dhcpsrv/cfg_subnets6.cc CfgSubnets6::updateStatistics() - replaced recountAddressStats6() with recountLeaseStats6() src/lib/dhcpsrv/lease_mgr.cc renamed LeaseMgr::recountAddressStats4() to recountLeaseStats4() renamed LeaseMgr::recountAddressStats6() to recountLeaseStats6() renamed LeaseMgr::startAddressStats4() to startLeaseStats4() renamed LeaseMgr::startAddressStats6() to startLeaseStats6() src/lib/dhcpsrv/lease_mgr.h replaced AddressStatsRow4 and AddressStatsRow6 with single class, LeaseStatsRow replaced AddressStatsQuery4 and AddressStatsQuery6 with single class, AddressStatsQuery src/lib/dhcpsrv/memfile_lease_mgr.h src/lib/dhcpsrv/memfile_lease_mgr.cc Replaced this class heirarchy: AddressStatsQuery4 <-- MemfileAddressStatsQuery4 AddressStatsQuery6 <-- MemfileAddressStatsQuery6 With this one: LeaseStatsQuery | +--- MemfileLeaseStatsQuery | +--- MemfileLeaseStatsQuery4 | +--- MemfileLeaseStatsQuery6 Replaced startAddressStatsQuery4() with startLeaseStatsQuery4() Replaced startAddressStatsQuery6() with startLeaseStatsQuery6() src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc Renamed: checkAddressStats() to checkLeaseStats() testAddressLeaseStats4() to testRecountLeaseStats4() testAddressLeaseStats6() to testRecountLeaseStats6() src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc Renamed tests to recountLeaseStats4 and recountLeaseStats6 --- src/lib/dhcpsrv/cfg_subnets4.cc | 3 +- src/lib/dhcpsrv/cfg_subnets6.cc | 2 +- src/lib/dhcpsrv/lease_mgr.cc | 31 ++-- src/lib/dhcpsrv/lease_mgr.h | 150 ++++++---------- src/lib/dhcpsrv/memfile_lease_mgr.cc | 193 ++++++++++----------- src/lib/dhcpsrv/memfile_lease_mgr.h | 24 +-- .../dhcpsrv/tests/generic_lease_mgr_unittest.cc | 36 ++-- src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h | 6 +- .../dhcpsrv/tests/memfile_lease_mgr_unittest.cc | 8 +- 9 files changed, 192 insertions(+), 261 deletions(-) diff --git a/src/lib/dhcpsrv/cfg_subnets4.cc b/src/lib/dhcpsrv/cfg_subnets4.cc index f33b40e958..5dcb413d4e 100644 --- a/src/lib/dhcpsrv/cfg_subnets4.cc +++ b/src/lib/dhcpsrv/cfg_subnets4.cc @@ -269,7 +269,8 @@ CfgSubnets4::updateStatistics() { // Only recount the stats if we have subnets. if (subnets_.begin() != subnets_.end()) { - LeaseMgrFactory::instance().recountAddressStats4(); + //LeaseMgrFactory::instance().recountAddressStats4(); + LeaseMgrFactory::instance().recountLeaseStats4(); } } diff --git a/src/lib/dhcpsrv/cfg_subnets6.cc b/src/lib/dhcpsrv/cfg_subnets6.cc index e8e36b4f5d..3136763f3c 100644 --- a/src/lib/dhcpsrv/cfg_subnets6.cc +++ b/src/lib/dhcpsrv/cfg_subnets6.cc @@ -223,7 +223,7 @@ CfgSubnets6::updateStatistics() { // Only recount the stats if we have subnets. if (subnets_.begin() != subnets_.end()) { - LeaseMgrFactory::instance().recountAddressStats6(); + LeaseMgrFactory::instance().recountLeaseStats6(); } } diff --git a/src/lib/dhcpsrv/lease_mgr.cc b/src/lib/dhcpsrv/lease_mgr.cc index 02aedf5f4a..cd84a69839 100644 --- a/src/lib/dhcpsrv/lease_mgr.cc +++ b/src/lib/dhcpsrv/lease_mgr.cc @@ -48,12 +48,12 @@ LeaseMgr::getLease6(Lease::Type type, const DUID& duid, } void -LeaseMgr::recountAddressStats4() { +LeaseMgr::recountLeaseStats4() { using namespace stats; StatsMgr& stats_mgr = StatsMgr::instance(); - AddressStatsQuery4Ptr query = startAddressStatsQuery4(); + LeaseStatsQueryPtr query = startLeaseStatsQuery4(); if (!query) { /// NULL means not backend does not support recounting. return; @@ -86,7 +86,7 @@ LeaseMgr::recountAddressStats4() { // Get counts per state per subnet. Iterate over the result set // updating the subnet and global values. - AddressStatsRow4 row; + LeaseStatsRow row; while (query->getNextRow(row)) { if (row.lease_state_ == Lease::STATE_DEFAULT) { // Set subnet level value. @@ -105,23 +105,23 @@ LeaseMgr::recountAddressStats4() { } } -AddressStatsQuery4Ptr -LeaseMgr::startAddressStatsQuery4() { - return(AddressStatsQuery4Ptr()); +LeaseStatsQueryPtr +LeaseMgr::startLeaseStatsQuery4() { + return(LeaseStatsQueryPtr()); } bool -AddressStatsQuery4::getNextRow(AddressStatsRow4& /*row*/) { +LeaseStatsQuery::getNextRow(LeaseStatsRow& /*row*/) { return (false); } void -LeaseMgr::recountAddressStats6() { +LeaseMgr::recountLeaseStats6() { using namespace stats; StatsMgr& stats_mgr = StatsMgr::instance(); - AddressStatsQuery6Ptr query = startAddressStatsQuery6(); + LeaseStatsQueryPtr query = startLeaseStatsQuery6(); if (!query) { /// NULL means not backend does not support recounting. return; @@ -163,7 +163,7 @@ LeaseMgr::recountAddressStats6() { // Get counts per state per subnet. Iterate over the result set // updating the subnet and global values. - AddressStatsRow6 row; + LeaseStatsRow row; while (query->getNextRow(row)) { switch(row.lease_type_) { case Lease::TYPE_NA: @@ -202,14 +202,9 @@ LeaseMgr::recountAddressStats6() { } } -AddressStatsQuery6Ptr -LeaseMgr::startAddressStatsQuery6() { - return(AddressStatsQuery6Ptr()); -} - -bool -AddressStatsQuery6::getNextRow(AddressStatsRow6& /*row*/) { - return (false); +LeaseStatsQueryPtr +LeaseMgr::startLeaseStatsQuery6() { + return(LeaseStatsQueryPtr()); } std::string diff --git a/src/lib/dhcpsrv/lease_mgr.h b/src/lib/dhcpsrv/lease_mgr.h index 456a237f66..c8b4ef4e0d 100644 --- a/src/lib/dhcpsrv/lease_mgr.h +++ b/src/lib/dhcpsrv/lease_mgr.h @@ -147,78 +147,29 @@ public: ExchangeColumnInfoContainer parameters_; ///< Column names and types }; -/// @brief Contains a single row of IPv4 lease statistical data +/// @brief Contains a single row of lease statistical data /// -/// The contents of the row consist of a subnet ID, a lease state, -/// and the number of leases in that state for that subnet ID. -struct AddressStatsRow4 { +/// The contents of the row consist of a subnet ID, a lease +/// type, a lease state, and the number of leases in that state +/// for that type for that subnet ID. +struct LeaseStatsRow { /// @brief Default constructor - AddressStatsRow4() : - subnet_id_(0), lease_state_(Lease::STATE_DEFAULT), state_count_(0) { + LeaseStatsRow() : + subnet_id_(0), lease_type_(Lease::TYPE_NA), + lease_state_(Lease::STATE_DEFAULT), state_count_(0) { } /// @brief Constructor /// + /// Constructor which defaults the type to TYPE_NA. + /// /// @param subnet_id The subnet id to which this data applies /// @param lease_state The lease state counted /// @param state_count The count of leases in the lease state - AddressStatsRow4(const SubnetID& subnet_id, - const uint32_t lease_state, - const int64_t state_count) - : subnet_id_(subnet_id), lease_state_(lease_state), - state_count_(state_count) { - } - - /// @brief The subnet ID to which this data applies - SubnetID subnet_id_; - /// @brief The lease_state to which the count applies - uint32_t lease_state_; - /// @brief state_count The count of leases in the lease state - int64_t state_count_; -}; - -/// @brief Base class for fulfilling IPv4 statistical lease data query -/// -/// LeaseMgr derivations implement this class such that it provides -/// upto date IPv4 statistical lease data organized as rows of -/// AddressStatsRow4 instances. The rows must be accessible in -/// ascending order by subnet id. -class AddressStatsQuery4 { -public: - /// @brief Default constructor - AddressStatsQuery4() {}; - - /// @brief virtual destructor - virtual ~AddressStatsQuery4() {}; - - /// @brief Executes the query - /// - /// This method should conduct whatever steps are required to - /// calculate the IPv4 lease statistical data by examining the - /// IPv4 lease data and making that results available row by row. - virtual void start() {}; - - /// @brief Fetches the next row of data - /// - /// @param[out] row Storage into which the row is fetched - /// - /// @return True if a row was fetched, false if there are no - /// more rows. - virtual bool getNextRow(AddressStatsRow4& row); -}; - -/// @brief Defines a pointer to an AddressStatsQuery4. -typedef boost::shared_ptr AddressStatsQuery4Ptr; - -/// @brief Contains a single row of IPv6 lease statistical data -/// -/// The contents of the row consist of a subnet ID, a lease state, -/// and the number of leases in that state for that subnet ID. -struct AddressStatsRow6 { - /// @brief Default constructor - AddressStatsRow6() : - subnet_id_(0), lease_type_(Lease::TYPE_NA), - lease_state_(Lease::STATE_DEFAULT), state_count_(0) { + LeaseStatsRow(const SubnetID& subnet_id, const uint32_t lease_state, + const int64_t state_count) + : subnet_id_(subnet_id), lease_type_(Lease::TYPE_NA), + lease_state_(lease_state), state_count_(state_count) { } /// @brief Constructor @@ -227,9 +178,8 @@ struct AddressStatsRow6 { /// @param lease_type The lease type for this state count /// @param lease_state The lease state counted /// @param state_count The count of leases in the lease state - AddressStatsRow6(const SubnetID& subnet_id, const Lease::Type& lease_type, - const uint32_t lease_state, - const int64_t state_count) + LeaseStatsRow(const SubnetID& subnet_id, const Lease::Type& lease_type, + const uint32_t lease_state, const int64_t state_count) : subnet_id_(subnet_id), lease_type_(lease_type), lease_state_(lease_state), state_count_(state_count) { } @@ -244,25 +194,24 @@ struct AddressStatsRow6 { int64_t state_count_; }; -/// @brief Base class for fulfilling IPv6 statistical lease data query +/// @brief Base class for fulfilling a statistical lease data query /// /// LeaseMgr derivations implement this class such that it provides -/// upto date IPv6 statistical lease data organized as rows of -/// AddressStatsRow6 instances. The rows must be accessible in -/// ascending order by subnet id. -class AddressStatsQuery6 { +/// upto date statistical lease data organized as rows of LeaseStatsRow +/// instances. The rows must be accessible in ascending order by subnet id. +class LeaseStatsQuery { public: /// @brief Default constructor - AddressStatsQuery6() {}; + LeaseStatsQuery() {}; /// @brief virtual destructor - virtual ~AddressStatsQuery6() {}; + virtual ~LeaseStatsQuery() {}; /// @brief Executes the query /// /// This method should conduct whatever steps are required to - /// calculate the IPv6 lease statistical data by examining the - /// IPv6 lease data and making that results available row by row. + /// calculate the lease statistical data by examining the + /// lease data and making that results available row by row. virtual void start() {}; /// @brief Fetches the next row of data @@ -271,11 +220,11 @@ public: /// /// @return True if a row was fetched, false if there are no /// more rows. - virtual bool getNextRow(AddressStatsRow6& row); + virtual bool getNextRow(LeaseStatsRow& row); }; -/// @brief Defines a pointer to an AddressStatsQuery6. -typedef boost::shared_ptr AddressStatsQuery6Ptr; +/// @brief Defines a pointer to an LeaseStatsQuery. +typedef boost::shared_ptr LeaseStatsQueryPtr; /// @brief Abstract Lease Manager /// @@ -539,24 +488,25 @@ public: /// - declined-addresses /// - declined-reclaimed-addresses (reset to zero) /// - /// It invokes the virtual method, startAddressStatsQuery4(), which - /// returns an instance of an AddressStats4Qry. The query - /// query contains a "result set" where each row is an AddressStatRow4 - /// that contains a subnet id, a lease state, the number of leases in that - /// state and is ordered by subnet id. The method iterates over the + /// It invokes the virtual method, startLeaseStatsQuery4(), which + /// returns an instance of an LeaseStatsQuery. The query + /// query contains a "result set" where each row is an LeaseStatRow + /// that contains a subnet id, a lease type (currently always TYPE_NA), + /// a lease state, and the number of leases of that type, in that state + /// and is ordered by subnet id. The method iterates over the /// result set rows, setting the appropriate statistic per subnet and /// adding to the approporate global statistic. - void recountAddressStats4(); + void recountLeaseStats4(); /// @brief Virtual method which creates and runs the IPv4 lease stats query /// /// LeaseMgr derivations implement this method such that it creates and - /// returns an instance of an AddressStatsQuery whose result set has been + /// returns an instance of an LeaseStatsQuery whose result set has been /// populated with upto date IPv4 lease statistical data. Each row of the - /// result set is an AddressStatRow4 which ordered ascending by subnet ID. + /// result set is an LeaseStatRow which ordered ascending by subnet ID. /// - /// @return A populated AddressStatsQuery4 - virtual AddressStatsQuery4Ptr startAddressStatsQuery4(); + /// @return A populated LeaseStatsQuery + virtual LeaseStatsQueryPtr startLeaseStatsQuery4(); /// @brief Recalculates per-subnet and global stats for IPv6 leases /// @@ -570,24 +520,24 @@ public: /// - declined-addresses /// - declined-reclaimed-addresses (reset to zero) /// - /// It invokes the virtual method, startAddressStatsQuery6(), which - /// returns an instance of an AddressStats6Qry. The query - /// query contains a "result set" where each row is an AddressStatRow6 - /// that contains a subnet id, a lease state, the number of leases in that - /// state and is ordered by subnet id. The method iterates over the - /// result set rows, setting the appropriate statistic per subnet and - /// adding to the approporate global statistic. - void recountAddressStats6(); + /// It invokes the virtual method, startLeaseStatsQuery6(), which + /// returns an instance of an LeaseStatsQuery. The query contains + /// a "result set" where each row is an LeaseStatRow that contains + /// a subnet id, a lease type, a lease state, and the number of leases + /// of that type, in that state and is ordered by subnet id. The method + /// iterates over the result set rows, setting the appropriate statistic + /// per subnet and adding to the approporate global statistic. + void recountLeaseStats6(); /// @brief Virtual method which creates and runs the IPv6 lease stats query /// /// LeaseMgr derivations implement this method such that it creates and - /// returns an instance of an AddressStatsQuery whose result set has been + /// returns an instance of an LeaseStatsQuery whose result set has been /// populated with upto date IPv6 lease statistical data. Each row of the - /// result set is an AddressStatRow6 which ordered ascending by subnet ID. + /// result set is an LeaseStatRow which ordered ascending by subnet ID. /// - /// @return A populated AddressStatsQuery6 - virtual AddressStatsQuery6Ptr startAddressStatsQuery6(); + /// @return A populated LeaseStatsQuery + virtual LeaseStatsQueryPtr startLeaseStatsQuery6(); /// @brief Return backend type /// diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.cc b/src/lib/dhcpsrv/memfile_lease_mgr.cc index 8f5313a997..bfc57dccbf 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.cc +++ b/src/lib/dhcpsrv/memfile_lease_mgr.cc @@ -257,6 +257,55 @@ LFCSetup::getExitStatus() const { } +/// @brief Base Memfile derivation of the statistical lease data query +/// +/// This class provides the functionality such as results storgae and row +/// fetching common to fulfilling the statistical lease data query. +/// +class MemfileLeaseStatsQuery : public LeaseStatsQuery { +public: + /// @brief Constructor + /// + MemfileLeaseStatsQuery() + : rows_(0), next_pos_(rows_.end()) { + }; + + /// @brief Destructor + virtual ~MemfileLeaseStatsQuery() {}; + + /// @brief Fetches the next row in the result set + /// + /// Once the internal result set has been populated by invoking the + /// the start() method, this method is used to iterate over the + /// result set rows. Once the last row has been fetched, subsequent + /// calls will return false. + /// @param row Storage for the fetched row + /// + /// @return True if the fetch succeeded, false if there are no more + /// rows to fetch. + virtual bool getNextRow(LeaseStatsRow& row) { + if (next_pos_ == rows_.end()) { + return (false); + } + + row = *next_pos_; + ++next_pos_; + return (true); + } + + /// @brief Returns the number of rows in the result set + int getRowCount() const { + return (rows_.size()); + } + +protected: + /// @brief A vector containing the "result set" + std::vector rows_; + + /// @brief An iterator for accessing the next row within the result set + std::vector::iterator next_pos_; +}; + /// @brief Memfile derivation of the IPv4 statistical lease data query /// /// This class is used to recalculate IPv4 lease statistics for Memfile @@ -266,17 +315,17 @@ LFCSetup::getExitStatus() const { /// The populated result set will contain one entry per monitored state /// per subnet. /// -class MemfileAddressStatsQuery4 : public AddressStatsQuery4 { +class MemfileLeaseStatsQuery4 : public MemfileLeaseStatsQuery { public: /// @brief Constructor /// /// @param storage4 A pointer to the v4 lease storage to be counted - MemfileAddressStatsQuery4(Lease4Storage& storage4) - : storage4_(storage4), rows_(0), next_pos_(rows_.end()) { + MemfileLeaseStatsQuery4(Lease4Storage& storage4) + : MemfileLeaseStatsQuery(), storage4_(storage4) { }; /// @brief Destructor - virtual ~MemfileAddressStatsQuery4() {}; + virtual ~MemfileLeaseStatsQuery4() {}; /// @brief Creates the IPv4 lease statistical data result set /// @@ -309,13 +358,11 @@ public: // and wipe the accumulators if ((*lease)->subnet_id_ > cur_id) { if (cur_id > 0) { - rows_.push_back(AddressStatsRow4(cur_id, - Lease::STATE_DEFAULT, - assigned)); + rows_.push_back(LeaseStatsRow(cur_id, Lease::STATE_DEFAULT, + assigned)); assigned = 0; - rows_.push_back(AddressStatsRow4(cur_id, - Lease::STATE_DECLINED, - declined)); + rows_.push_back(LeaseStatsRow(cur_id, Lease::STATE_DECLINED, + declined)); declined = 0; } @@ -333,50 +380,19 @@ public: // Make the rows for last subnet, unless there were no rows if (idx.begin() != idx.end()) { - rows_.push_back(AddressStatsRow4(cur_id, Lease::STATE_DEFAULT, - assigned)); - rows_.push_back(AddressStatsRow4(cur_id, Lease::STATE_DECLINED, - declined)); + rows_.push_back(LeaseStatsRow(cur_id, Lease::STATE_DEFAULT, + assigned)); + rows_.push_back(LeaseStatsRow(cur_id, Lease::STATE_DECLINED, + declined)); } // Set the next row position to the beginning of the rows. next_pos_ = rows_.begin(); } - /// @brief Fetches the next row in the result set - /// - /// Once the internal result set has been populated by invoking the - /// the start() method, this method is used to iterate over the - /// result set rows. Once the last row has been fetched, subsequent - /// calls will return false. - /// @param row Storage for the fetched row - /// - /// @return True if the fetch succeeded, false if there are no more - /// rows to fetch. - virtual bool getNextRow(AddressStatsRow4& row) { - if (next_pos_ == rows_.end()) { - return (false); - } - - row = *next_pos_; - ++next_pos_; - return (true); - } - - /// @brief Returns the number of rows in the result set - int getRowCount() const { - return (rows_.size()); - } - private: /// @brief The Memfile storage containing the IPv4 leases to analyze Lease4Storage& storage4_; - - /// @brief A vector containing the "result set" - std::vector rows_; - - /// @brief An iterator for accessing the next row within the result set - std::vector::iterator next_pos_; }; @@ -389,17 +405,17 @@ private: /// The populated result set will contain one entry per monitored state /// per subnet. /// -class MemfileAddressStatsQuery6 : public AddressStatsQuery6 { +class MemfileLeaseStatsQuery6 : public MemfileLeaseStatsQuery { public: /// @brief Constructor /// /// @param storage6 A pointer to the v6 lease storage to be counted - MemfileAddressStatsQuery6(Lease6Storage& storage6) - : storage6_(storage6), rows_(0), next_pos_(rows_.end()) { + MemfileLeaseStatsQuery6(Lease6Storage& storage6) + : MemfileLeaseStatsQuery(), storage6_(storage6) { }; /// @brief Destructor - virtual ~MemfileAddressStatsQuery6() {}; + virtual ~MemfileLeaseStatsQuery6() {}; /// @brief Creates the IPv6 lease statistical data result set /// @@ -434,17 +450,17 @@ public: // and wipe the accumulators if ((*lease)->subnet_id_ > cur_id) { if (cur_id > 0) { - rows_.push_back(AddressStatsRow6(cur_id, Lease::TYPE_NA, - Lease::STATE_DEFAULT, - assigned)); + rows_.push_back(LeaseStatsRow(cur_id, Lease::TYPE_NA, + Lease::STATE_DEFAULT, + assigned)); assigned = 0; - rows_.push_back(AddressStatsRow6(cur_id, Lease::TYPE_NA, - Lease::STATE_DECLINED, - declined)); + rows_.push_back(LeaseStatsRow(cur_id, Lease::TYPE_NA, + Lease::STATE_DECLINED, + declined)); declined = 0; - rows_.push_back(AddressStatsRow6(cur_id, Lease::TYPE_PD, - Lease::STATE_DEFAULT, - assigned_pds)); + rows_.push_back(LeaseStatsRow(cur_id, Lease::TYPE_PD, + Lease::STATE_DEFAULT, + assigned_pds)); assigned_pds = 0; } @@ -474,55 +490,24 @@ public: // Make the rows for last subnet, unless there were no rows if (idx.begin() != idx.end()) { - rows_.push_back(AddressStatsRow6(cur_id, Lease::TYPE_NA, - Lease::STATE_DEFAULT, - assigned)); - rows_.push_back(AddressStatsRow6(cur_id, Lease::TYPE_NA, - Lease::STATE_DECLINED, - declined)); - rows_.push_back(AddressStatsRow6(cur_id, Lease::TYPE_PD, - Lease::STATE_DEFAULT, - assigned_pds)); + rows_.push_back(LeaseStatsRow(cur_id, Lease::TYPE_NA, + Lease::STATE_DEFAULT, + assigned)); + rows_.push_back(LeaseStatsRow(cur_id, Lease::TYPE_NA, + Lease::STATE_DECLINED, + declined)); + rows_.push_back(LeaseStatsRow(cur_id, Lease::TYPE_PD, + Lease::STATE_DEFAULT, + assigned_pds)); } // Set the next row position to the beginning of the rows. next_pos_ = rows_.begin(); } - /// @brief Fetches the next row in the result set - /// - /// Once the internal result set has been populated by invoking the - /// the start() method, this method is used to iterate over the - /// result set rows. Once the last row has been fetched, subsequent - /// calls will return false. - /// @param row Storage for the fetched row - /// - /// @return True if the fetch succeeded, false if there are no more - /// rows to fetch. - virtual bool getNextRow(AddressStatsRow6& row) { - if (next_pos_ == rows_.end()) { - return (false); - } - - row = *next_pos_; - ++next_pos_; - return (true); - } - - /// @brief Returns the number of rows in the result set - int getRowCount() { - return (rows_.size()); - } - private: /// @brief The Memfile storage containing the IPv6 leases to analyze Lease6Storage& storage6_; - - /// @brief A vector containing the "result set" - std::vector rows_; - - /// @brief An iterator for accessing the next row within the result set - std::vector::iterator next_pos_; }; // Explicit definition of class static constants. Values are given in the @@ -1318,16 +1303,16 @@ void Memfile_LeaseMgr::lfcExecute(boost::shared_ptr& lease_file) } } -AddressStatsQuery4Ptr -Memfile_LeaseMgr::startAddressStatsQuery4() { - AddressStatsQuery4Ptr query(new MemfileAddressStatsQuery4(storage4_)); +LeaseStatsQueryPtr +Memfile_LeaseMgr::startLeaseStatsQuery4() { + LeaseStatsQueryPtr query(new MemfileLeaseStatsQuery4(storage4_)); query->start(); return(query); } -AddressStatsQuery6Ptr -Memfile_LeaseMgr::startAddressStatsQuery6() { - AddressStatsQuery6Ptr query(new MemfileAddressStatsQuery6(storage6_)); +LeaseStatsQueryPtr +Memfile_LeaseMgr::startLeaseStatsQuery6() { + LeaseStatsQueryPtr query(new MemfileLeaseStatsQuery6(storage6_)); query->start(); return(query); } diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.h b/src/lib/dhcpsrv/memfile_lease_mgr.h index d4e7c1a94a..d996b45dcb 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.h +++ b/src/lib/dhcpsrv/memfile_lease_mgr.h @@ -595,23 +595,23 @@ public: int getLFCExitStatus() const; //@} - /// @brief Creates and runs the IPv4 lease stats query + /// @brief Creates and runs the IPv4 lease stats query /// - /// It creates an instance of a MemfileAddressStatsQuery4 and then - /// invokes it's start method in which the query constructs its + /// It creates an instance of a MemfileLeaseStatsQuery4 and then + /// invokes it's start method in which the query constructs its /// statistical data result set. The query object is then returned. - /// - /// @return The populated query as a pointer to an AddressStatsQuery4 - virtual AddressStatsQuery4Ptr startAddressStatsQuery4(); + /// + /// @return The populated query as a pointer to an LeaseStatsQuery + virtual LeaseStatsQueryPtr startLeaseStatsQuery4(); - /// @brief Creates and runs the IPv6 lease stats query + /// @brief Creates and runs the IPv6 lease stats query /// - /// It creates an instance of a MemfileAddressStatsQuery6 and then - /// invokes it's start method in which the query constructs its + /// It creates an instance of a MemfileLeaseStatsQuery6 and then + /// invokes it's start method in which the query constructs its /// statistical data result set. The query object is then returned. - /// - /// @return The populated query as a pointer to an AddressStatsQuery6 - virtual AddressStatsQuery6Ptr startAddressStatsQuery6(); + /// + /// @return The populated query as a pointer to an LeaseStatsQuery. + virtual LeaseStatsQueryPtr startLeaseStatsQuery6(); /// @name Protected methods used for %Lease File Cleanup. /// The following methods are protected so as they can be accessed and diff --git a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc index 399533da0e..57511fef8e 100644 --- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc @@ -2401,7 +2401,7 @@ GenericLeaseMgrTest::checkStat(const std::string& name, } void -GenericLeaseMgrTest::checkAddressStats(const StatValMapList& expectedStats) { +GenericLeaseMgrTest::checkLeaseStats(const StatValMapList& expectedStats) { // Global accumulators int64_t declined_addresses = 0; int64_t declined_reclaimed_addresses = 0; @@ -2469,9 +2469,8 @@ GenericLeaseMgrTest::makeLease6(const Lease::Type& type, ASSERT_TRUE(lmptr_->addLease(lease)); } - void -GenericLeaseMgrTest::testRecountAddressStats4() { +GenericLeaseMgrTest::testRecountLeaseStats4() { using namespace stats; StatsMgr::instance().removeAll(); @@ -2509,13 +2508,13 @@ GenericLeaseMgrTest::testRecountAddressStats4() { } // Make sure stats are as expected. - ASSERT_NO_FATAL_FAILURE(checkAddressStats(expectedStats)); + ASSERT_NO_FATAL_FAILURE(checkLeaseStats(expectedStats)); // Recount stats. We should have the same results. - ASSERT_NO_THROW(lmptr_->recountAddressStats4()); + ASSERT_NO_THROW(lmptr_->recountLeaseStats4()); // Make sure stats are as expected. - ASSERT_NO_FATAL_FAILURE(checkAddressStats(expectedStats)); + ASSERT_NO_FATAL_FAILURE(checkLeaseStats(expectedStats)); // Now let's insert some leases into subnet 1. int subnet_id = 1; @@ -2546,10 +2545,10 @@ GenericLeaseMgrTest::testRecountAddressStats4() { expectedStats[subnet_id - 1]["declined-addresses"] = 1; // Now Recount the stats. - ASSERT_NO_THROW(lmptr_->recountAddressStats4()); + ASSERT_NO_THROW(lmptr_->recountLeaseStats4()); // Make sure stats are as expected. - ASSERT_NO_FATAL_FAILURE(checkAddressStats(expectedStats)); + ASSERT_NO_FATAL_FAILURE(checkLeaseStats(expectedStats)); // Delete some leases from subnet, and update the expected stats. EXPECT_TRUE(lmptr_->deleteLease(IOAddress("192.0.1.1"))); @@ -2559,14 +2558,15 @@ GenericLeaseMgrTest::testRecountAddressStats4() { expectedStats[0]["declined-addresses"] = 0; // Recount the stats. - ASSERT_NO_THROW(lmptr_->recountAddressStats4()); + ASSERT_NO_THROW(lmptr_->recountLeaseStats4()); // Make sure stats are as expected. - ASSERT_NO_FATAL_FAILURE(checkAddressStats(expectedStats)); + ASSERT_NO_FATAL_FAILURE(checkLeaseStats(expectedStats)); } + void -GenericLeaseMgrTest::testRecountAddressStats6() { +GenericLeaseMgrTest::testRecountLeaseStats6() { using namespace stats; StatsMgr::instance().removeAll(); @@ -2615,14 +2615,14 @@ GenericLeaseMgrTest::testRecountAddressStats6() { } // Make sure stats are as expected. - ASSERT_NO_FATAL_FAILURE(checkAddressStats(expectedStats)); + ASSERT_NO_FATAL_FAILURE(checkLeaseStats(expectedStats)); // Recount stats. We should have the same results. - ASSERT_NO_THROW(lmptr_->recountAddressStats4()); + ASSERT_NO_THROW(lmptr_->recountLeaseStats4()); // Make sure stats are as expected. - ASSERT_NO_FATAL_FAILURE(checkAddressStats(expectedStats)); + ASSERT_NO_FATAL_FAILURE(checkLeaseStats(expectedStats)); // Now let's insert some leases into subnet 1. subnet_id = 1; @@ -2669,10 +2669,10 @@ GenericLeaseMgrTest::testRecountAddressStats6() { expectedStats[subnet_id - 1]["declined-addresses"] = 1; // Now Recount the stats. - ASSERT_NO_THROW(lmptr_->recountAddressStats6()); + ASSERT_NO_THROW(lmptr_->recountLeaseStats6()); // Make sure stats are as expected. - ASSERT_NO_FATAL_FAILURE(checkAddressStats(expectedStats)); + ASSERT_NO_FATAL_FAILURE(checkLeaseStats(expectedStats)); // Delete some leases and update the expected stats. EXPECT_TRUE(lmptr_->deleteLease(IOAddress("3001:1::2"))); @@ -2682,10 +2682,10 @@ GenericLeaseMgrTest::testRecountAddressStats6() { expectedStats[1]["declined-addresses"] = 0; // Recount the stats. - ASSERT_NO_THROW(lmptr_->recountAddressStats6()); + ASSERT_NO_THROW(lmptr_->recountLeaseStats6()); // Make sure stats are as expected. - ASSERT_NO_FATAL_FAILURE(checkAddressStats(expectedStats)); + ASSERT_NO_FATAL_FAILURE(checkLeaseStats(expectedStats)); } diff --git a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h index b3b1f66550..ec6a4b4837 100644 --- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h +++ b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h @@ -118,7 +118,7 @@ public: /// do not match. /// /// @param expected_stats Map of expected static names and values. - void checkAddressStats(const StatValMapList& expected_stats); + void checkLeaseStats(const StatValMapList& expected_stats); /// @brief Constructs a minimal IPv4 lease and adds it to the lease storage /// @@ -365,14 +365,14 @@ public: /// This test creates two subnets and several leases associated with /// them, then verifies that lease statistics are recalculated correctly /// after altering the lease states in various ways. - void testRecountAddressStats4(); + void testRecountLeaseStats4(); /// @brief Check that the IPv6 lease statistics can be recounted /// /// This test creates two subnets and several leases associated with /// them, then verifies that lease statistics are recalculated correctly /// after altering the lease states in various ways. - void testRecountAddressStats6(); + void testRecountLeaseStats6(); /// @brief String forms of IPv4 addresses std::vector straddress4_; diff --git a/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc index 1379cf1c14..f52f2c1bd3 100644 --- a/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc @@ -1884,15 +1884,15 @@ TEST_F(MemfileLeaseMgrTest, lease6ContainerIndexUpdate) { } // Verifies that IPv4 lease statistics can be recalculated. -TEST_F(MemfileLeaseMgrTest, recountAddressStats4) { +TEST_F(MemfileLeaseMgrTest, recountLeaseStats4) { startBackend(V4); - testRecountAddressStats4(); + testRecountLeaseStats4(); } // Verifies that IPv6 lease statistics can be recalculated. -TEST_F(MemfileLeaseMgrTest, recountAddressStats6) { +TEST_F(MemfileLeaseMgrTest, recountLeaseStats6) { startBackend(V6); - testRecountAddressStats6(); + testRecountLeaseStats6(); } }; // end of anonymous namespace -- cgit v1.2.3 From 92ffb41363724aa752a5dd7961ed0a2b9d11145b Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Wed, 24 Aug 2016 07:23:05 -0400 Subject: [4294] Refactored PgSql stats classes src/lib/dhcpsrv/pgsql_lease_mgr.h src/lib/dhcpsrv/pgsql_lease_mgr.cc Replaced this class heirarchy: AddressStatsQuery4 <-- PgSqlAddressStatsQuery4 AddressStatsQuery6 <-- PgSqlAddressStatsQuery6 With this one: LeaseStatsQuery <-- PgSqlLeaseStatsQuery --- src/lib/dhcpsrv/pgsql_lease_mgr.cc | 163 ++++++---------------- src/lib/dhcpsrv/pgsql_lease_mgr.h | 12 +- src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc | 8 +- 3 files changed, 51 insertions(+), 132 deletions(-) diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.cc b/src/lib/dhcpsrv/pgsql_lease_mgr.cc index 2b92e69d6b..40fb709e7d 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.cc +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.cc @@ -694,34 +694,33 @@ private: //@} }; -/// @brief PgSql derivation of the IPv4 statistical lease data query +/// @brief Base PgSql derivation of the statistical lease data query /// -/// This class is used to recalculate IPv4 lease statistics for MySQL -/// lease storage. It does so by executing a query which returns a result -/// containining contain one row per monitored state per subnet, ordered -/// by subnet id in ascending order. +/// This class provides the functionality such as results storgae and row +/// fetching common to fulfilling the statistical lease data query. /// -class PgSqlAddressStatsQuery4 : public AddressStatsQuery4 { +class PgSqlLeaseStatsQuery : public LeaseStatsQuery { public: /// @brief Constructor /// /// @param conn A open connection to the database housing the lease data - PgSqlAddressStatsQuery4(PgSqlConnection& conn) - : conn_(conn), statement_(tagged_statements[PgSqlLeaseMgr - ::RECOUNT_LEASE4_STATS]), - result_set_(), next_row_(0) { + /// @param statement The lease data SQL prepared statement to execute + /// @param fetch_statement Indicates whether or not lease_type should be + /// fetched from the result set + PgSqlLeaseStatsQuery(PgSqlConnection& conn, PgSqlTaggedStatement& statement, + const bool fetch_type) + : conn_(conn), statement_(statement), result_set_(), next_row_(0), + fetch_type_(fetch_type) { } /// @brief Destructor - virtual ~PgSqlAddressStatsQuery4() {}; + virtual ~PgSqlLeaseStatsQuery() {}; - /// @brief Creates the IPv4 lease statistical data result set + /// @brief Creates the lease statistical data result set /// - /// The result set is populated by executing an SQL query against the - /// lease4 table which sums the leases per lease state per subnet id. - /// The query used is the prepared statement identified by - /// PgSqlLeaseMgr::RECOUNT_LEASE4_STATS. This method executes the - /// statement which creates the result set. + /// The result set is populated by executing a prepared SQL query + /// against the database which sums the leases per lease state per + /// subnet id. void start() { // The query has no parameters, so we only need it's name. result_set_.reset(new PgSqlResult(PQexecPrepared(conn_, statement_.name, @@ -741,7 +740,7 @@ public: /// /// @return True if the fetch succeeded, false if there are no more /// rows to fetch. - bool getNextRow(AddressStatsRow4& row) { + bool getNextRow(LeaseStatsRow& row) { // If we're past the end, punt. if (next_row_ >= result_set_->getRows()) { return (false); @@ -754,6 +753,15 @@ public: row.subnet_id_ = static_cast(subnet_id); ++col; + // Fetch the lease type if we were told to do so. + if (fetch_type_) { + uint32_t lease_type; + PgSqlExchange::getColumnValue(*result_set_, next_row_ , col, + lease_type); + row.lease_type_ = static_cast(lease_type); + ++col; + } + // Fetch the lease state. uint32_t state; PgSqlExchange::getColumnValue(*result_set_, next_row_ , col, @@ -769,7 +777,7 @@ public: return (true); } -private: +protected: /// @brief Database connection to use to execute the query PgSqlConnection& conn_; @@ -781,104 +789,9 @@ private: /// @brief Index of the next row to fetch uint32_t next_row_; -}; - -/// @brief PgSql derivation of the IPv6 statistical lease data query -/// -/// This class is used to recalculate IPv6 lease statistics for MySQL -/// lease storage. It does so by executing a query which returns a result -/// containining contain one row per monitored state per subnet, ordered -/// by subnet id in ascending order. -/// -class PgSqlAddressStatsQuery6 : public AddressStatsQuery6 { -public: - /// @brief Constructor - /// - /// @param conn A open connection to the database housing the lease data - PgSqlAddressStatsQuery6(PgSqlConnection& conn) - : conn_(conn), statement_(tagged_statements[PgSqlLeaseMgr - ::RECOUNT_LEASE6_STATS]), - result_set_(), next_row_(0) { - } - - /// @brief Destructor - virtual ~PgSqlAddressStatsQuery6() {}; - - /// @brief Creates the IPv6 lease statistical data result set - /// - /// The result set is populated by executing an SQL query against the - /// lease6 table which sums the leases per lease state per lease type - /// per subnet id. The query used is the prepared statement identified by - /// PgSqlLeaseMgr::RECOUNT_LEASE6_STATS. This method executes the - /// statement which creates the result set. - void start() { - // The query has no parameters, so we only need it's name. - result_set_.reset(new PgSqlResult(PQexecPrepared(conn_, statement_.name, - 0, NULL, NULL, NULL, - 0))); - - conn_.checkStatementError(*result_set_, statement_); - } - - /// @brief Fetches the next row in the result set - /// - /// Once the internal result set has been populated by invoking the - /// the start() method, this method is used to iterate over the - /// result set rows. Once the last row has been fetched, subsequent - /// calls will return false. - /// - /// @param row Storage for the fetched row - /// - /// @return True if the fetch succeeded, false if there are no more - /// rows to fetch. - bool getNextRow(AddressStatsRow6& row) { - // If we're past the end, punt. - if (next_row_ >= result_set_->getRows()) { - return (false); - } - - // Fetch the subnet id. - uint32_t col = 0; - uint32_t subnet_id; - PgSqlExchange::getColumnValue(*result_set_, next_row_, col, subnet_id); - row.subnet_id_ = static_cast(subnet_id); - ++col; - // Fetch the lease type. - uint32_t lease_type; - PgSqlExchange::getColumnValue(*result_set_, next_row_ , col, - lease_type); - row.lease_type_ = static_cast(lease_type); - ++col; - - // Fetch the lease state. - uint32_t state; - PgSqlExchange::getColumnValue(*result_set_, next_row_ , col, - row.lease_state_); - ++col; - - // Fetch the state count. - PgSqlExchange::getColumnValue(*result_set_, next_row_, col, - row.state_count_); - - // Point to the next row. - ++next_row_; - - return (true); - } - -private: - /// @brief Database connection to use to execute the query - PgSqlConnection& conn_; - - /// @brief The query's prepared statement - PgSqlTaggedStatement& statement_; - - /// @brief The result set returned by Postgres. - boost::shared_ptr result_set_; - - /// @brief Index of the next row to fetch - uint32_t next_row_; + /// @brief Indicates if query supplies lease type + bool fetch_type_; }; PgSqlLeaseMgr::PgSqlLeaseMgr(const DatabaseConnection::ParameterMap& parameters) @@ -1422,16 +1335,22 @@ PgSqlLeaseMgr::deleteExpiredReclaimedLeasesCommon(const uint32_t secs, return (deleteLeaseCommon(statement_index, bind_array)); } -AddressStatsQuery4Ptr -PgSqlLeaseMgr::startAddressStatsQuery4() { - AddressStatsQuery4Ptr query(new PgSqlAddressStatsQuery4(conn_)); +LeaseStatsQueryPtr +PgSqlLeaseMgr::startLeaseStatsQuery4() { + LeaseStatsQueryPtr query( + new PgSqlLeaseStatsQuery(conn_, + tagged_statements[RECOUNT_LEASE4_STATS], + false)); query->start(); return(query); } -AddressStatsQuery6Ptr -PgSqlLeaseMgr::startAddressStatsQuery6() { - AddressStatsQuery6Ptr query(new PgSqlAddressStatsQuery6(conn_)); +LeaseStatsQueryPtr +PgSqlLeaseMgr::startLeaseStatsQuery6() { + LeaseStatsQueryPtr query( + new PgSqlLeaseStatsQuery(conn_, + tagged_statements[RECOUNT_LEASE6_STATS], + true)); query->start(); return(query); } diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.h b/src/lib/dhcpsrv/pgsql_lease_mgr.h index a8a40813a5..69924067a3 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.h +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.h @@ -319,23 +319,23 @@ public: /// @brief Creates and runs the IPv4 lease stats query /// - /// It creates an instance of a PgSqlAddressStatsQuery4 and then + /// It creates an instance of a PgSqlLeaseStatsQuery4 and then /// invokes its start method, which fetches its statistical data /// result set by executing the RECOUNT_LEASE_STATS4 query. /// The query object is then returned. /// - /// @return The populated query as a pointer to an AddressStatsQuery4 - virtual AddressStatsQuery4Ptr startAddressStatsQuery4(); + /// @return The populated query as a pointer to an LeaseStatsQuery + virtual LeaseStatsQueryPtr startLeaseStatsQuery4(); /// @brief Creates and runs the IPv6 lease stats query /// - /// It creates an instance of a PgSqlAddressStatsQuery6 and then + /// It creates an instance of a PgSqlLeaseStatsQuery and then /// invokes its start method, which fetches its statistical data /// result set by executing the RECOUNT_LEASE_STATS6 query. /// The query object is then returned. /// - /// @return The populated query as a pointer to an AddressStatsQuery6 - virtual AddressStatsQuery6Ptr startAddressStatsQuery6(); + /// @return The populated query as a pointer to an LeaseStatsQuery + virtual LeaseStatsQueryPtr startLeaseStatsQuery6(); /// @brief Return backend type /// diff --git a/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc index fb473a7830..0a629d5579 100644 --- a/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc @@ -403,13 +403,13 @@ TEST_F(PgSqlLeaseMgrTest, getExpiredLeases6) { } // Verifies that IPv4 lease statistics can be recalculated. -TEST_F(PgSqlLeaseMgrTest, recountAddressStats4) { - testRecountAddressStats4(); +TEST_F(PgSqlLeaseMgrTest, recountLeaseStats4) { + testRecountLeaseStats4(); } // Verifies that IPv6 lease statistics can be recalculated. -TEST_F(PgSqlLeaseMgrTest, recountAddressStats6) { - testRecountAddressStats6(); +TEST_F(PgSqlLeaseMgrTest, recountLeaseStats6) { + testRecountLeaseStats6(); } }; // namespace -- cgit v1.2.3 From 0833798a33926936f836bd51b1ca4566a8cfcadc Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Wed, 24 Aug 2016 08:50:47 -0400 Subject: [4294] Refactored MySql stats classes src/lib/dhcpsrv/mysql_lease_mgr.h src/lib/dhcpsrv/mysql_lease_mgr.cc Replaced this class heirarchy: AddressStatsQuery4 <-- MySqlAddressStatsQuery4 AddressStatsQuery6 <-- MySqlAddressStatsQuery6 With this one: LeaseStatsQuery <-- MySqlLeaseStatsQuery --- src/lib/dhcpsrv/mysql_lease_mgr.cc | 233 ++++++---------------- src/lib/dhcpsrv/mysql_lease_mgr.h | 14 +- src/lib/dhcpsrv/pgsql_lease_mgr.cc | 2 + src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc | 8 +- 4 files changed, 74 insertions(+), 183 deletions(-) diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc index 7f7c1bf3ca..30af2f0d0f 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.cc +++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc @@ -1223,26 +1223,34 @@ private: uint32_t state_; ///< Lease state. }; -/// @brief MySql derivation of the IPv4 statistical lease data query +/// @brief MySql derivation of the statistical lease data query /// -/// This class is used to recalculate IPv4 lease statistics for MySQL +/// This class is used to recalculate lease statistics for MySQL /// lease storage. It does so by executing a query which returns a result -/// containining contain one row per monitored state per subnet, ordered -/// by subnet id in ascending order. +/// containining contain one row per monitored state per lease type per +/// subnet, ordered by subnet id in ascending order. /// -class MySqlAddressStatsQuery4 : public AddressStatsQuery4 { +class MySqlLeaseStatsQuery : public LeaseStatsQuery { public: /// @brief Constructor /// /// @param conn A open connection to the database housing the lease data - MySqlAddressStatsQuery4(MySqlConnection& conn) - : conn_(conn), statement_(conn_.statements_[MySqlLeaseMgr - ::RECOUNT_LEASE4_STATS]), - bind_(3) { + /// @brief statement_index Index of the query's prepared statement + /// @brief fetch_type Indicates if query supplies lease type + MySqlLeaseStatsQuery(MySqlConnection& conn, const size_t statement_index, + const bool fetch_type) + : conn_(conn), statement_index_(statement_index), statement_(NULL), + fetch_type_(fetch_type), bind_(fetch_type_ ? 4 : 3) { + if (statement_index_ >= MySqlLeaseMgr::NUM_STATEMENTS) { + isc_throw(BadValue, "MySqlLeaseStatsQuery" + " - invalid statement index" << statement_index_); + } + + statement_ = conn.statements_[statement_index_]; } /// @brief Destructor - virtual ~MySqlAddressStatsQuery4() { + virtual ~MySqlLeaseStatsQuery() { (void) mysql_stmt_free_result(statement_); } @@ -1255,161 +1263,48 @@ public: /// the statement to the output bind array and then executes the /// statement. void start() { + int col = 0; // subnet_id: unsigned int - bind_[0].buffer_type = MYSQL_TYPE_LONG; - bind_[0].buffer = reinterpret_cast(&subnet_id_); - bind_[0].is_unsigned = MLM_TRUE; - - // state: uint32_t - bind_[1].buffer_type = MYSQL_TYPE_LONG; - bind_[1].buffer = reinterpret_cast(&lease_state_); - bind_[1].is_unsigned = MLM_TRUE; - - // state_count_: uint32_t - bind_[2].buffer_type = MYSQL_TYPE_LONG; - bind_[2].buffer = reinterpret_cast(&state_count_); - bind_[2].is_unsigned = MLM_TRUE; - - // Set up the MYSQL_BIND array for the data being returned - // and bind it to the statement. - int status = mysql_stmt_bind_result(statement_, &bind_[0]); - checkError(status, "RECOUNT_LEASE4_STATS: outbound binding failed"); - - // Execute the statement - status = mysql_stmt_execute(statement_); - checkError(status, "RECOUNT_LEASE4_STATS: unable to execute"); - - // Ensure that all the lease information is retrieved in one go to avoid - // overhead of going back and forth between client and server. - status = mysql_stmt_store_result(statement_); - checkError(status, "RECOUNT_LEASE4_STATS: results storage failed"); - } - - - /// @brief Fetches the next row in the result set - /// - /// Once the internal result set has been populated by invoking the - /// the start() method, this method is used to iterate over the - /// result set rows. Once the last row has been fetched, subsequent - /// calls will return false. - /// - /// @param row Storage for the fetched row - /// - /// @return True if the fetch succeeded, false if there are no more - /// rows to fetch. - bool getNextRow(AddressStatsRow4& row) { - bool have_row = false; - int status = mysql_stmt_fetch(statement_); - if (status == MLM_MYSQL_FETCH_SUCCESS) { - row.subnet_id_ = static_cast(subnet_id_); - row.lease_state_ = lease_state_; - row.state_count_ = state_count_; - have_row = true; - } else if (status != MYSQL_NO_DATA) { - checkError(status, "RECOUNT_LEASE4_STATS: getNextRow failed"); + bind_[col].buffer_type = MYSQL_TYPE_LONG; + bind_[col].buffer = reinterpret_cast(&subnet_id_); + bind_[col].is_unsigned = MLM_TRUE; + ++col; + + // Fetch the lease type if we were told to do so. + if (fetch_type_) { + // lease type: uint32_t + bind_[col].buffer_type = MYSQL_TYPE_LONG; + bind_[col].buffer = reinterpret_cast(&lease_type_); + bind_[col].is_unsigned = MLM_TRUE; + ++col; + } else { + fetch_type_ = Lease::TYPE_NA; } - return (have_row); - } - -private: - - /// @brief Analyzes the given statement outcome status - /// - /// Wrapper method around the MySqlConnection:checkError() that is - /// used to generate the appropriate exception if the status indicates - /// an error. - //// - /// a DbOperation error - /// @param status The MySQL statement execution outcome status - /// @param what invocation context message which will be included in - /// any exception - void checkError(int status, const char* what) const { - conn_.checkError(status, MySqlLeaseMgr::RECOUNT_LEASE4_STATS, what); - } - - /// @brief Database connection to use to execute the query - MySqlConnection& conn_; - - /// @brief The query's prepared statement - MYSQL_STMT *statement_; - - /// @brief Bind array used to store the query result set; - std::vector bind_; - - /// @brief Receives subnet ID when fetching a row - uint32_t subnet_id_; - /// @brief Receives the lease state when fetching a row - uint32_t lease_state_; - /// @brief Receives the state count when fetching a row - uint32_t state_count_; -}; - -/// @brief MySql derivation of the IPv6 statistical lease data query -/// -/// This class is used to recalculate IPv6 lease statistics for MySQL -/// lease storage. It does so by executing a query which returns a result -/// containining contain one row per monitored state per subnet, ordered -/// by subnet id in ascending order. -/// -class MySqlAddressStatsQuery6 : public AddressStatsQuery6 { -public: - /// @brief Constructor - /// - /// @param conn A open connection to the database housing the lease data - MySqlAddressStatsQuery6(MySqlConnection& conn) - : conn_(conn), statement_(conn_.statements_[MySqlLeaseMgr - ::RECOUNT_LEASE6_STATS]), - bind_(4) { - } - - /// @brief Destructor - virtual ~MySqlAddressStatsQuery6() { - (void) mysql_stmt_free_result(statement_); - } - - /// @brief Creates the IPv6 lease statistical data result set - /// - /// The result set is populated by executing an SQL query against the - /// lease6 table which sums the leases per lease state per lease type - /// per subnet id. The query used is the prepared statement identified by - /// MySqlLeaseMgr::RECOUNT_LEASE6_STATS. This method creates the binds - /// the statement to the output bind array and then executes the - /// statement. - void start() { - // subnet_id: unsigned int - bind_[0].buffer_type = MYSQL_TYPE_LONG; - bind_[0].buffer = reinterpret_cast(&subnet_id_); - bind_[0].is_unsigned = MLM_TRUE; - - // lease type: uint32_t - bind_[1].buffer_type = MYSQL_TYPE_LONG; - bind_[1].buffer = reinterpret_cast(&lease_type_); - bind_[1].is_unsigned = MLM_TRUE; - // state: uint32_t - bind_[2].buffer_type = MYSQL_TYPE_LONG; - bind_[2].buffer = reinterpret_cast(&lease_state_); - bind_[2].is_unsigned = MLM_TRUE; + bind_[col].buffer_type = MYSQL_TYPE_LONG; + bind_[col].buffer = reinterpret_cast(&lease_state_); + bind_[col].is_unsigned = MLM_TRUE; + ++col; // state_count_: uint32_t - bind_[3].buffer_type = MYSQL_TYPE_LONG; - bind_[3].buffer = reinterpret_cast(&state_count_); - bind_[3].is_unsigned = MLM_TRUE; + bind_[col].buffer_type = MYSQL_TYPE_LONG; + bind_[col].buffer = reinterpret_cast(&state_count_); + bind_[col].is_unsigned = MLM_TRUE; // Set up the MYSQL_BIND array for the data being returned // and bind it to the statement. int status = mysql_stmt_bind_result(statement_, &bind_[0]); - checkError(status, "RECOUNT_LEASE6_STATS: outbound binding failed"); + conn_.checkError(status, statement_index_, "outbound binding failed"); // Execute the statement status = mysql_stmt_execute(statement_); - checkError(status, "RECOUNT_LEASE6_STATS: unable to execute"); + conn_.checkError(status, statement_index_, "unable to execute"); // Ensure that all the lease information is retrieved in one go to avoid // overhead of going back and forth between client and server. status = mysql_stmt_store_result(statement_); - checkError(status, "RECOUNT_LEASE6_STATS: results storage failed"); + conn_.checkError(status, statement_index_, "results storage failed"); } @@ -1424,7 +1319,7 @@ public: /// /// @return True if the fetch succeeded, false if there are no more /// rows to fetch. - bool getNextRow(AddressStatsRow6& row) { + bool getNextRow(LeaseStatsRow& row) { bool have_row = false; int status = mysql_stmt_fetch(statement_); if (status == MLM_MYSQL_FETCH_SUCCESS) { @@ -1434,35 +1329,25 @@ public: row.state_count_ = state_count_; have_row = true; } else if (status != MYSQL_NO_DATA) { - checkError(status, "RECOUNT_LEASE6_STATS: getNextRow failed"); + conn_.checkError(status, statement_index_, "getNextRow failed"); } return (have_row); } - private: - - /// @brief Analyzes the given statement outcome status - /// - /// Wrapper method around the MySqlConnection:checkError() that is - /// used to generate the appropriate exception if the status indicates - /// an error. - //// - /// a DbOperation error - /// @param status The MySQL statement execution outcome status - /// @param what invocation context message which will be included in - /// any exception - void checkError(int status, const char* what) const { - conn_.checkError(status, MySqlLeaseMgr::RECOUNT_LEASE6_STATS, what); - } - /// @brief Database connection to use to execute the query MySqlConnection& conn_; + /// @brief Index of the query's prepared statement + size_t statement_index_; + /// @brief The query's prepared statement MYSQL_STMT *statement_; + /// @brief Indicates if query supplies lease type + bool fetch_type_; + /// @brief Bind array used to store the query result set; std::vector bind_; @@ -2284,16 +2169,20 @@ MySqlLeaseMgr::getVersion() const { return (std::make_pair(major, minor)); } -AddressStatsQuery4Ptr -MySqlLeaseMgr::startAddressStatsQuery4() { - AddressStatsQuery4Ptr query(new MySqlAddressStatsQuery4(conn_)); +LeaseStatsQueryPtr +MySqlLeaseMgr::startLeaseStatsQuery4() { + LeaseStatsQueryPtr query(new MySqlLeaseStatsQuery(conn_, + RECOUNT_LEASE4_STATS, + false)); query->start(); return(query); } -AddressStatsQuery6Ptr -MySqlLeaseMgr::startAddressStatsQuery6() { - AddressStatsQuery6Ptr query(new MySqlAddressStatsQuery6(conn_)); +LeaseStatsQueryPtr +MySqlLeaseMgr::startLeaseStatsQuery6() { + LeaseStatsQueryPtr query(new MySqlLeaseStatsQuery(conn_, + RECOUNT_LEASE6_STATS, + true)); query->start(); return(query); } diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.h b/src/lib/dhcpsrv/mysql_lease_mgr.h index 06f1a3fee4..0e4b3af0b2 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.h +++ b/src/lib/dhcpsrv/mysql_lease_mgr.h @@ -593,23 +593,23 @@ private: /// @brief Creates and runs the IPv4 lease stats query /// - /// It creates an instance of a MySqlAddressStatsQuery4 and then + /// It creates an instance of a MySqlLeaseStatsQuery4 and then /// invokes its start method, which fetches its statistical data /// result set by executing the RECOUNT_LEASE_STATS4 query. /// The query object is then returned. /// - /// @return The populated query as a pointer to an AddressStatsQuery4 - virtual AddressStatsQuery4Ptr startAddressStatsQuery4(); + /// @return The populated query as a pointer to an LeaseStatsQuery + virtual LeaseStatsQueryPtr startLeaseStatsQuery4(); /// @brief Creates and runs the IPv6 lease stats query /// - /// It creates an instance of a MySqlAddressStatsQuery6 and then + /// It creates an instance of a MySqlLeaseStatsQuery6 and then /// invokes its start method, which fetches its statistical data - /// result set by executing the RECOUNT_LEASE_STATS4 query. + /// result set by executing the RECOUNT_LEASE_STATS6 query. /// The query object is then returned. /// - /// @return The populated query as a pointer to an AddressStatsQuery6 - virtual AddressStatsQuery6Ptr startAddressStatsQuery6(); + /// @return The populated query as a pointer to an LeaseStatsQuery + virtual LeaseStatsQueryPtr startLeaseStatsQuery6(); /// @brief Check Error and Throw Exception /// diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.cc b/src/lib/dhcpsrv/pgsql_lease_mgr.cc index 40fb709e7d..6b2bb5920e 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.cc +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.cc @@ -760,6 +760,8 @@ public: lease_type); row.lease_type_ = static_cast(lease_type); ++col; + } else { + row.lease_type_ = Lease::TYPE_NA; } // Fetch the lease state. diff --git a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc index 4e6b583a05..b5433ab332 100644 --- a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc @@ -478,13 +478,13 @@ TEST_F(MySqlLeaseMgrTest, deleteExpiredReclaimedLeases4) { } // Verifies that IPv4 lease statistics can be recalculated. -TEST_F(MySqlLeaseMgrTest, recountAddressStats4) { - testRecountAddressStats4(); +TEST_F(MySqlLeaseMgrTest, recountLeaseStats4) { + testRecountLeaseStats4(); } // Verifies that IPv6 lease statistics can be recalculated. -TEST_F(MySqlLeaseMgrTest, recountAddressStats6) { - testRecountAddressStats6(); +TEST_F(MySqlLeaseMgrTest, recountLeaseStats6) { + testRecountLeaseStats6(); } }; // Of anonymous namespace -- cgit v1.2.3 From 569b5a87dce77cb1ff3f858075685fbf6cb40337 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Wed, 24 Aug 2016 10:40:34 -0400 Subject: [4294] Minor clean ups --- src/lib/dhcpsrv/dhcpsrv_messages.mes | 3 --- src/lib/dhcpsrv/memfile_lease_mgr.h | 4 ++-- src/lib/dhcpsrv/mysql_lease_mgr.cc | 16 +++++++++------- src/lib/dhcpsrv/tests/alloc_engine_utils.cc | 2 ++ src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc | 18 ++++++------------ src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h | 6 +++--- 6 files changed, 22 insertions(+), 27 deletions(-) diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.mes b/src/lib/dhcpsrv/dhcpsrv_messages.mes index db03ed0bb0..714148548c 100644 --- a/src/lib/dhcpsrv/dhcpsrv_messages.mes +++ b/src/lib/dhcpsrv/dhcpsrv_messages.mes @@ -923,6 +923,3 @@ lease from the Cassandra database for the specified address. % DHCPSRV_CQL_UPDATE_ADDR6 updating IPv6 lease for address %1 A debug message issued when the server is attempting to update IPv6 lease from the Cassandra database for the specified address. - -% TOMS_UTILITY_MESSAGE %1 -Handy log message that should be deleted diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.h b/src/lib/dhcpsrv/memfile_lease_mgr.h index d996b45dcb..bdbd985511 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.h +++ b/src/lib/dhcpsrv/memfile_lease_mgr.h @@ -598,7 +598,7 @@ public: /// @brief Creates and runs the IPv4 lease stats query /// /// It creates an instance of a MemfileLeaseStatsQuery4 and then - /// invokes it's start method in which the query constructs its + /// invokes its start method in which the query constructs its /// statistical data result set. The query object is then returned. /// /// @return The populated query as a pointer to an LeaseStatsQuery @@ -607,7 +607,7 @@ public: /// @brief Creates and runs the IPv6 lease stats query /// /// It creates an instance of a MemfileLeaseStatsQuery6 and then - /// invokes it's start method in which the query constructs its + /// invokes its start method in which the query constructs its /// statistical data result set. The query object is then returned. /// /// @return The populated query as a pointer to an LeaseStatsQuery. diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc index 30af2f0d0f..0017a690d3 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.cc +++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc @@ -1240,7 +1240,10 @@ public: MySqlLeaseStatsQuery(MySqlConnection& conn, const size_t statement_index, const bool fetch_type) : conn_(conn), statement_index_(statement_index), statement_(NULL), - fetch_type_(fetch_type), bind_(fetch_type_ ? 4 : 3) { + fetch_type_(fetch_type), + // Set the number of columns in the bind array based on fetch_type + // This is the number of columns expected in the result set + bind_(fetch_type_ ? 4 : 3) { if (statement_index_ >= MySqlLeaseMgr::NUM_STATEMENTS) { isc_throw(BadValue, "MySqlLeaseStatsQuery" " - invalid statement index" << statement_index_); @@ -1256,12 +1259,11 @@ public: /// @brief Creates the IPv4 lease statistical data result set /// - /// The result set is populated by executing an SQL query against the - /// lease4 table which sums the leases per lease state per subnet id. - /// The query used is the prepared statement identified by - /// MySqlLeaseMgr::RECOUNT_LEASE4_STATS. This method creates the binds - /// the statement to the output bind array and then executes the - /// statement. + /// The result set is populated by executing a SQL query against the + /// lease(4/6) table which sums the leases per lease state per lease + /// type (v6 only) per subnet id. This method binds the statement to + /// the output bind array and then executes the statement, and fetches + /// entire result set. void start() { int col = 0; // subnet_id: unsigned int diff --git a/src/lib/dhcpsrv/tests/alloc_engine_utils.cc b/src/lib/dhcpsrv/tests/alloc_engine_utils.cc index 9beb306609..764ad60ee1 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine_utils.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine_utils.cc @@ -123,6 +123,7 @@ AllocEngine4Test::generateDeclinedLease(const std::string& addr, AllocEngine6Test::AllocEngine6Test() { CfgMgr::instance().clear(); + // This lease mgr needs to exist to before configuration commits. factory_.create("type=memfile universe=6 persist=false"); duid_ = DuidPtr(new DUID(std::vector(8, 0x42))); @@ -527,6 +528,7 @@ AllocEngine4Test::initSubnet(const asiolink::IOAddress& pool_start, AllocEngine4Test::AllocEngine4Test() { + // This lease mgr needs to exist to before configuration commits. factory_.create("type=memfile universe=4 persist=false"); // Create fresh instance of the HostMgr, and drop any previous HostMgr state. diff --git a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc index 57511fef8e..25d1b2bd07 100644 --- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc @@ -2475,11 +2475,9 @@ GenericLeaseMgrTest::testRecountLeaseStats4() { StatsMgr::instance().removeAll(); - // create subnets - CfgSubnets4Ptr cfg = - CfgMgr::instance().getStagingCfg()->getCfgSubnets4(); - - // Create 3 subnets. + // Create two subnets. + int num_subnets = 2; + CfgSubnets4Ptr cfg = CfgMgr::instance().getStagingCfg()->getCfgSubnets4(); Subnet4Ptr subnet; Pool4Ptr pool; @@ -2493,7 +2491,6 @@ GenericLeaseMgrTest::testRecountLeaseStats4() { subnet->addPool(pool); cfg->add(subnet); - int num_subnets = 2; ASSERT_NO_THROW(CfgMgr::instance().commit()); @@ -2571,14 +2568,11 @@ GenericLeaseMgrTest::testRecountLeaseStats6() { StatsMgr::instance().removeAll(); - // create subnets - CfgSubnets6Ptr cfg = - CfgMgr::instance().getStagingCfg()->getCfgSubnets6(); - - // Create 3 subnets. + // Create two subnets. + int num_subnets = 2; + CfgSubnets6Ptr cfg = CfgMgr::instance().getStagingCfg()->getCfgSubnets6(); Subnet6Ptr subnet; Pool6Ptr pool; - int num_subnets = 2; StatValMapList expectedStats(num_subnets); int subnet_id = 1; diff --git a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h index ec6a4b4837..f426787833 100644 --- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h +++ b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h @@ -112,10 +112,10 @@ public: /// @brief Compares StatsMgr statistics against an expected list of values /// - /// Iterates over a list of statistic names and expectec values, attempting + /// Iterates over a list of statistic names and expected values, attempting /// to fetch each from the StatsMgr and if found, compare its observed value - /// to the expected value. Fails any the stat is not found or if the values - /// do not match. + /// to the expected value. Fails if any of the expected stats are not + /// found or if the values do not match. /// /// @param expected_stats Map of expected static names and values. void checkLeaseStats(const StatValMapList& expected_stats); -- cgit v1.2.3 From 9290dab9955fc3000402536976c75da1c486a573 Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Wed, 24 Aug 2016 23:00:38 +0200 Subject: [4294] Compilation fix, minor clean ups --- src/lib/dhcpsrv/cfg_subnets4.cc | 1 - src/lib/dhcpsrv/memfile_lease_mgr.cc | 2 +- src/lib/dhcpsrv/pgsql_lease_mgr.cc | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/lib/dhcpsrv/cfg_subnets4.cc b/src/lib/dhcpsrv/cfg_subnets4.cc index 5dcb413d4e..f1c7a8f658 100644 --- a/src/lib/dhcpsrv/cfg_subnets4.cc +++ b/src/lib/dhcpsrv/cfg_subnets4.cc @@ -269,7 +269,6 @@ CfgSubnets4::updateStatistics() { // Only recount the stats if we have subnets. if (subnets_.begin() != subnets_.end()) { - //LeaseMgrFactory::instance().recountAddressStats4(); LeaseMgrFactory::instance().recountLeaseStats4(); } } diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.cc b/src/lib/dhcpsrv/memfile_lease_mgr.cc index bfc57dccbf..79c4020f90 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.cc +++ b/src/lib/dhcpsrv/memfile_lease_mgr.cc @@ -259,7 +259,7 @@ LFCSetup::getExitStatus() const { /// @brief Base Memfile derivation of the statistical lease data query /// -/// This class provides the functionality such as results storgae and row +/// This class provides the functionality such as results storage and row /// fetching common to fulfilling the statistical lease data query. /// class MemfileLeaseStatsQuery : public LeaseStatsQuery { diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.cc b/src/lib/dhcpsrv/pgsql_lease_mgr.cc index 6b2bb5920e..3ea7475484 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.cc +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.cc @@ -765,7 +765,6 @@ public: } // Fetch the lease state. - uint32_t state; PgSqlExchange::getColumnValue(*result_set_, next_row_ , col, row.lease_state_); ++col; -- cgit v1.2.3 From 52ff5a4e463007103ae970cc520be7d7a87d64f4 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Thu, 25 Aug 2016 07:15:26 -0400 Subject: [4294] Changed Memfile stats recount to use existing per-address index src/lib/dhcpsrv/memfile_lease_mgr.cc MemfileLeaseStatsQuery4::start() MemfileLeaseStatsQuery6::start() - both now rely on per-address index, rather than adding per subnet_id index. src/lib/dhcpsrv/memfile_lease_storage.h Removed per subnet_id indexes --- src/lib/dhcpsrv/memfile_lease_mgr.cc | 46 +++++++++++++++++++-------------- src/lib/dhcpsrv/memfile_lease_storage.h | 30 +-------------------- 2 files changed, 28 insertions(+), 48 deletions(-) diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.cc b/src/lib/dhcpsrv/memfile_lease_mgr.cc index 79c4020f90..4ff78be9d1 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.cc +++ b/src/lib/dhcpsrv/memfile_lease_mgr.cc @@ -329,10 +329,15 @@ public: /// @brief Creates the IPv4 lease statistical data result set /// - /// The result is populated by iterating over the IPv4 leases in storage, - /// in ascending order by subnet ID, accumulating the lease state counts. + /// The result set is populated by iterating over the IPv4 leases in + /// storage, in ascending order by address, accumulating the lease state + /// counts per subnet. Note that walking the leases by address should + /// inherently group them by subnet, and while this does not gaurantee + /// ascending order of subnet id, it should be sufficient to accumulate + /// state counts per subnet. This avoids introducing an additional + /// subnet_id index. /// At the completion of all entries for a given subnet, the counts are - /// used to create AddressStatsRow4 instances which are appended to an + /// used to create LeaseStatsRow instances which are appended to an /// internal vector. The process results in a vector containing one entry /// per state per subnet. /// @@ -341,9 +346,8 @@ public: /// - Lease::STATE_DEFAULT (i.e. assigned) /// - Lease::STATE_DECLINED void start() { - // Get the subnet_id index - const Lease4StorageSubnetIdIndex& idx - = storage4_.get(); + const Lease4StorageAddressIndex& idx + = storage4_.get(); // Iterate over the leases in order by subnet, accumulating per // subnet counts for each state of interest. As we finish each @@ -351,12 +355,11 @@ public: SubnetID cur_id = 0; int64_t assigned = 0; int64_t declined = 0; - for(Lease4StorageSubnetIdIndex::const_iterator lease = idx.begin(); + for(Lease4StorageAddressIndex::const_iterator lease = idx.begin(); lease != idx.end(); ++lease) { - // If we've hit the next subnet, add rows for the current subnet // and wipe the accumulators - if ((*lease)->subnet_id_ > cur_id) { + if ((*lease)->subnet_id_ != cur_id) { if (cur_id > 0) { rows_.push_back(LeaseStatsRow(cur_id, Lease::STATE_DEFAULT, assigned)); @@ -419,12 +422,17 @@ public: /// @brief Creates the IPv6 lease statistical data result set /// - /// The result is populated by iterating over the IPv6 leases in storage, - /// in ascending order by subnet ID, accumulating the lease state counts - /// per lease type. At the completion of all entries for a given subnet, - /// the counts are used to create AddressStatsRow5 instances which are - /// appended to an internal vector. The process results in a vector - /// containing one entry per state per lease type per subnet. + /// The result set is populated by iterating over the IPv6 leases in + /// storage, in ascending order by address, accumulating the lease state + /// counts per subnet. Note that walking the leases by address should + /// inherently group them by subnet, and while this does not gaurantee + /// ascending order of subnet id, it should be sufficient to accumulate + /// state counts per subnet. This avoids introducing an additional + /// subnet_id index. + /// At the completion of all entries for a given subnet, the counts + /// are used to create LeaseStatsRow instances which are appended to an + /// internal vector. The process results in a vector containing one entry + /// per state per lease type per subnet. /// /// Currently the states counted are: /// @@ -432,8 +440,8 @@ public: /// - Lease::STATE_DECLINED virtual void start() { // Get the subnet_id index - const Lease6StorageSubnetIdIndex& idx - = storage6_.get(); + const Lease6StorageAddressIndex& idx + = storage6_.get(); // Iterate over the leases in order by subnet, accumulating per // subnet counts for each state of interest. As we finish each @@ -443,12 +451,12 @@ public: int64_t declined = 0; int64_t assigned_pds = 0; - for(Lease6StorageSubnetIdIndex::const_iterator lease = idx.begin(); + for(Lease6StorageAddressIndex::const_iterator lease = idx.begin(); lease != idx.end(); ++lease) { // If we've hit the next subnet, add rows for the current subnet // and wipe the accumulators - if ((*lease)->subnet_id_ > cur_id) { + if ((*lease)->subnet_id_ != cur_id) { if (cur_id > 0) { rows_.push_back(LeaseStatsRow(cur_id, Lease::TYPE_NA, Lease::STATE_DEFAULT, diff --git a/src/lib/dhcpsrv/memfile_lease_storage.h b/src/lib/dhcpsrv/memfile_lease_storage.h index 5c09f35d56..3b07fad238 100644 --- a/src/lib/dhcpsrv/memfile_lease_storage.h +++ b/src/lib/dhcpsrv/memfile_lease_storage.h @@ -1,4 +1,4 @@ -// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2015-2016 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -27,9 +27,6 @@ namespace dhcp { /// @brief Tag for indexes by address. struct AddressIndexTag { }; -/// @brief Tag for indexes by subnet id. -struct SubnetIdIndexTag { }; - /// @brief Tag for indexes by DUID, IAID, lease type tuple. struct DuidIaidTypeIndexTag { }; @@ -106,17 +103,7 @@ typedef boost::multi_index_container< boost::multi_index::const_mem_fun > - >, - - boost::multi_index::ordered_non_unique< - boost::multi_index::tag, - // The subnet id is held in the subnet_id_ member of Lease6 - // class. Note that the subnet_id_ is defined in the base - // class (Lease) so we have to point to this class rather - // than derived class: Lease6. - boost::multi_index::member > - > > Lease6Storage; // Specify the type name of this container. @@ -148,15 +135,6 @@ typedef boost::multi_index_container< boost::multi_index::member >, - boost::multi_index::ordered_non_unique< - boost::multi_index::tag, - // The subnet id is held in the subnet_id_ member of Lease4 - // class. Note that the subnet_id_ is defined in the base - // class (Lease) so we have to point to this class rather - // than derived class: Lease4. - boost::multi_index::member - >, - // Specification of the second index starts here. boost::multi_index::ordered_non_unique< boost::multi_index::tag, @@ -251,15 +229,9 @@ typedef Lease6Storage::index::type Lease6StorageDuidIaidTy /// @brief DHCPv6 lease storage index by expiration time. typedef Lease6Storage::index::type Lease6StorageExpirationIndex; -/// @brief DHCPv6 lease storage index by subnet id. -typedef Lease6Storage::index::type Lease6StorageSubnetIdIndex; - /// @brief DHCPv4 lease storage index by address. typedef Lease4Storage::index::type Lease4StorageAddressIndex; -/// @brief DHCPv4 lease storage index by subnet id. -typedef Lease4Storage::index::type Lease4StorageSubnetIdIndex; - /// @brief DHCPv4 lease storage index by exiration time. typedef Lease4Storage::index::type Lease4StorageExpirationIndex; -- cgit v1.2.3