diff options
| -rw-r--r-- | src/hooks/dhcp/perfmon/alarm_store.cc | 10 | ||||
| -rw-r--r-- | src/hooks/dhcp/perfmon/alarm_store.h | 18 | ||||
| -rw-r--r-- | src/hooks/dhcp/perfmon/monitored_duration.cc | 33 | ||||
| -rw-r--r-- | src/hooks/dhcp/perfmon/monitored_duration.h | 2 | ||||
| -rw-r--r-- | src/hooks/dhcp/perfmon/monitored_duration_store.cc | 12 | ||||
| -rw-r--r-- | src/hooks/dhcp/perfmon/monitored_duration_store.h | 19 | ||||
| -rw-r--r-- | src/hooks/dhcp/perfmon/tests/alarm_store_unittests.cc | 33 | ||||
| -rw-r--r-- | src/hooks/dhcp/perfmon/tests/monitored_duration_store_unittests.cc | 17 | ||||
| -rw-r--r-- | src/hooks/dhcp/perfmon/tests/monitored_duration_unittests.cc | 8 |
9 files changed, 76 insertions, 76 deletions
diff --git a/src/hooks/dhcp/perfmon/alarm_store.cc b/src/hooks/dhcp/perfmon/alarm_store.cc index 41b76c06d5..cf9a0d05a6 100644 --- a/src/hooks/dhcp/perfmon/alarm_store.cc +++ b/src/hooks/dhcp/perfmon/alarm_store.cc @@ -48,8 +48,8 @@ AlarmStore::checkDurationSample(DurationKeyPtr key, const Duration& sample, auto& index = alarms_.get<AlarmPrimaryKeyTag>(); auto alarm_iter = index.find(*key); - // If we find an alarm the check the sample. Alarm::checkSample() - //does not alter the key so it can be done in-place. + // If we find an alarm then we check the sample. Alarm::checkSample() + // does not alter the key so it can be done in-place. if (alarm_iter != index.end()) { bool should_report = false; bool modified = index.modify(alarm_iter, @@ -87,7 +87,7 @@ AlarmStore::addAlarm(AlarmPtr alarm) { AlarmPtr AlarmStore::addAlarm(DurationKeyPtr key, const Duration& low_water, - const Duration& high_water, bool enabled /* = true */) { + const Duration& high_water, bool enabled /* = true */) { validateKey("addAlarm", key); // Create the alarm instance. @@ -149,8 +149,8 @@ AlarmStore::getAll() { MultiThreadingLock lock(*mutex_); const auto& index = alarms_.get<AlarmPrimaryKeyTag>(); AlarmCollectionPtr collection(new AlarmCollection()); - for (auto alarm_iter = index.begin(); alarm_iter != index.end(); ++alarm_iter) { - collection->push_back(AlarmPtr(new Alarm(**alarm_iter))); + for (auto const& alarm : index) { + collection->push_back(AlarmPtr(new Alarm(*alarm))); } return (collection); diff --git a/src/hooks/dhcp/perfmon/alarm_store.h b/src/hooks/dhcp/perfmon/alarm_store.h index 103f53ea2f..dfaacce5f6 100644 --- a/src/hooks/dhcp/perfmon/alarm_store.h +++ b/src/hooks/dhcp/perfmon/alarm_store.h @@ -36,7 +36,7 @@ public: /// @brief Tag for index by primary key (DurationKey). struct AlarmPrimaryKeyTag { }; -/// @brief A multi index container holding pointers to Alarms. +/// @brief A multi index container holding pointers to alarms. /// /// The alarms in the container may be accessed using different indexes: /// - using the index on DurationKey members, AlarmPrimaryKeyTag @@ -64,10 +64,10 @@ typedef std::vector<AlarmPtr> AlarmCollection; /// @brief Type for a pointer to a collection of AlarmPtrs. typedef boost::shared_ptr<AlarmCollection> AlarmCollectionPtr; -/// @brief Maintains an in-memory store of Alarms +/// @brief Maintains an in-memory store of alarms /// /// Provides essential CRUD functions for managing a collection of -/// Alarms. Additionally there are finders that can return +/// alarms. Additionally there are finders that can return /// durations by DurationKey (others are TBD). /// All finders return copies of the durations found, rather than the /// stored duration itself. @@ -102,22 +102,22 @@ public: AlarmPtr checkDurationSample(DurationKeyPtr key, const Duration& sample, const Duration& report_interval); - /// @brief Creates a new Alarm and adds it to the store + /// @brief Creates a new alarm and adds it to the store /// - /// @param key key value of the Alarm to create. + /// @param key key value of the alarm to create. /// @param low_water threshold below which the average duration must fall to clear the alarm /// @param high_water threshold above which the average duration must rise to trigger the alarm. /// @param enabled true sets state to CLEAR, otherwise DISABLED, defaults to true. /// - /// @return pointer to the newly created Alarm. + /// @return pointer to the newly created alarm. /// @throw DuplicateAlarm if a duration for the given key already exists in /// the store. AlarmPtr addAlarm(DurationKeyPtr key, const Duration& low_water, const Duration& high_water, bool enabled = true); - /// @brief Adds an Alarm to the store. + /// @brief Adds an alarm to the store. /// - /// @return pointer to a copy of the Alarm added. + /// @return pointer to a copy of the alarm added. AlarmPtr addAlarm(AlarmPtr alarm); /// @brief Fetches a duration from the store for a given key. @@ -162,7 +162,7 @@ private: /// @brief Convenience method to verify a key is valid for an operation. /// /// @param label description of where the check is being made, appears in exception text. - /// @param key key to validate + /// @param key key to validate. /// /// @throw BadValue if the key is either empty or its family does not /// match the store. diff --git a/src/hooks/dhcp/perfmon/monitored_duration.cc b/src/hooks/dhcp/perfmon/monitored_duration.cc index d81b7c638b..ec6bb9d5df 100644 --- a/src/hooks/dhcp/perfmon/monitored_duration.cc +++ b/src/hooks/dhcp/perfmon/monitored_duration.cc @@ -23,7 +23,10 @@ namespace perfmon { DurationDataInterval::DurationDataInterval(const Timestamp& start_time /* = PktEvent::now()*/) : start_time_(start_time), occurrences_(0), min_duration_(pos_infin), max_duration_(neg_infin), - total_duration_(microseconds(0)) { } void + total_duration_(microseconds(0)) { +} + +void DurationDataInterval::addDuration(const Duration& duration) { ++occurrences_; if (duration < min_duration_) { @@ -48,13 +51,11 @@ DurationDataInterval::getAverageDuration() const { bool DurationDataInterval::operator==(const DurationDataInterval& other) const { - return ( - (start_time_ == other.start_time_) && - (occurrences_ == other.occurrences_) && - (min_duration_ == other.min_duration_) && - (max_duration_ == other.max_duration_) && - (total_duration_ == other.total_duration_) - ); + return ((start_time_ == other.start_time_) && + (occurrences_ == other.occurrences_) && + (min_duration_ == other.min_duration_) && + (max_duration_ == other.max_duration_) && + (total_duration_ == other.total_duration_)); } // DurationKey methods @@ -190,13 +191,11 @@ DurationKey::operator!=(const DurationKey& other) const { bool DurationKey::operator<(const DurationKey& other) const { - return ( - (query_type_ < other.query_type_) || - (response_type_ < other.response_type_) || - (start_event_label_ < other.start_event_label_) || - (stop_event_label_ < other.stop_event_label_) || - (subnet_id_ < other.subnet_id_) - ); + return ((query_type_ < other.query_type_) || + (response_type_ < other.response_type_) || + (start_event_label_ < other.start_event_label_) || + (stop_event_label_ < other.stop_event_label_) || + (subnet_id_ < other.subnet_id_)); } @@ -237,11 +236,11 @@ MonitoredDuration::MonitoredDuration(const MonitoredDuration& rhs) current_interval_(0), previous_interval_(0) { if (rhs.current_interval_) { - current_interval_.reset(new DurationDataInterval(*rhs.current_interval_)); + current_interval_.reset(new DurationDataInterval(*rhs.current_interval_)); } if (rhs.previous_interval_) { - previous_interval_.reset(new DurationDataInterval(*rhs.previous_interval_)); + previous_interval_.reset(new DurationDataInterval(*rhs.previous_interval_)); } } diff --git a/src/hooks/dhcp/perfmon/monitored_duration.h b/src/hooks/dhcp/perfmon/monitored_duration.h index 2893d82650..6efa261498 100644 --- a/src/hooks/dhcp/perfmon/monitored_duration.h +++ b/src/hooks/dhcp/perfmon/monitored_duration.h @@ -237,7 +237,7 @@ public: /// /// less than operator to compare two DurationKey objects. /// @param other DurationKey to be compared against. - /// @return True other is less than this key + /// @return True key is less than the other key bool operator<(const DurationKey& other) const; protected: diff --git a/src/hooks/dhcp/perfmon/monitored_duration_store.cc b/src/hooks/dhcp/perfmon/monitored_duration_store.cc index 3827da7519..6c0e5504b8 100644 --- a/src/hooks/dhcp/perfmon/monitored_duration_store.cc +++ b/src/hooks/dhcp/perfmon/monitored_duration_store.cc @@ -59,7 +59,7 @@ MonitoredDurationStore::addDurationSample(DurationKeyPtr key, const Duration& sa bool should_report = false; // Modify updates in place and only re-indexes if keys change. bool modified = index.modify(duration_iter, - [sample, &should_report](MonitoredDurationPtr mond){ + [sample, &should_report](MonitoredDurationPtr mond) { should_report = mond->addSample(sample); }); @@ -126,7 +126,7 @@ MonitoredDurationStore::getDuration(DurationKeyPtr key) { const auto& index = durations_.get<DurationKeyTag>(); auto duration_iter = index.find(*key); return (duration_iter == index.end() ? MonitoredDurationPtr() - : MonitoredDurationPtr(new MonitoredDuration(**duration_iter))); + : MonitoredDurationPtr(new MonitoredDuration(**duration_iter))); } void @@ -166,8 +166,8 @@ MonitoredDurationStore::getAll() { MultiThreadingLock lock(*mutex_); const auto& index = durations_.get<DurationKeyTag>(); MonitoredDurationCollectionPtr collection(new MonitoredDurationCollection()); - for (auto duration_iter = index.begin(); duration_iter != index.end(); ++duration_iter) { - collection->push_back(MonitoredDurationPtr(new MonitoredDuration(**duration_iter))); + for (auto const& mond : index) { + collection->push_back(MonitoredDurationPtr(new MonitoredDuration(*mond))); } return (collection); @@ -186,7 +186,7 @@ MonitoredDurationStore::getReportsNext() { // We want to find the oldest interval that is less than interval_duration in the past. auto duration_iter = index.lower_bound(dhcp::PktEvent::now() - interval_duration_); return (duration_iter == index.end() ? MonitoredDurationPtr() - : MonitoredDurationPtr(new MonitoredDuration(**duration_iter))); + : MonitoredDurationPtr(new MonitoredDuration(**duration_iter))); } MonitoredDurationCollectionPtr @@ -195,7 +195,7 @@ MonitoredDurationStore::getOverdueReports(const Timestamp& since /* = PktEvent:: // We use a lower bound of MIN + 1us to avoid dormant durations static Timestamp lower_limit_time(PktEvent::MIN_TIME() + microseconds(1)); - // We want to find anything since the start of time that who's start time + // We want to find anything since the start of time who's start time // is more than interval_duration_ in the past. const auto& index = durations_.get<IntervalStartTag>(); auto lower_limit = index.lower_bound(lower_limit_time); diff --git a/src/hooks/dhcp/perfmon/monitored_duration_store.h b/src/hooks/dhcp/perfmon/monitored_duration_store.h index 6bb24e6c46..292782ecf4 100644 --- a/src/hooks/dhcp/perfmon/monitored_duration_store.h +++ b/src/hooks/dhcp/perfmon/monitored_duration_store.h @@ -39,7 +39,7 @@ struct DurationKeyTag { }; /// @brief Tag for index by interval start time. struct IntervalStartTag { }; -/// @brief A multi index container holding pointers to MonitoredDurations. +/// @brief A multi index container holding pointers to durations. /// /// The durations in the container may be accessed using different indexes: /// - using the index on DurationKey members, DurationKeyTag @@ -75,10 +75,10 @@ typedef std::vector<MonitoredDurationPtr> MonitoredDurationCollection; /// @brief Type for a pointer to a collection of MonitoredDurationPtrs. typedef boost::shared_ptr<MonitoredDurationCollection> MonitoredDurationCollectionPtr; -/// @brief Maintains an in-memory store of MonitoredDurations +/// @brief Maintains an in-memory store of durations /// /// Provides essential CRUD functions for managing a collection of -/// MonitoredDurations. Additionally there are finders that can return +/// durations. Additionally there are finders that can return /// durations by DurationKey (others are TBD) /// All finders return copies of the durations found, rather than the /// stored duration itself. @@ -100,7 +100,7 @@ public: /// condition, then a copy of the in-store duration is returned, otherwise an empty /// pointer is returned. /// - /// If The duration does not exist in the store, then one is created and inserted + /// If the duration does not exist in the store, then one is created and inserted /// into the store after adding the sample. An empty pointer is returned. /// /// This function does not/must not modify any index keys. @@ -112,7 +112,7 @@ public: /// pointer otherwise. MonitoredDurationPtr addDurationSample(DurationKeyPtr key, const Duration& sample); - /// @brief Creates a new MonitoredDuration and adds it to the store + /// @brief Creates a new duration and adds it to the store /// /// @param key key value of the duration to create. /// @@ -134,7 +134,7 @@ public: /// /// @param duration duration to update. /// - /// @throw InvalidOperation if MonitoredDuration does not exist in the store. + /// @throw InvalidOperation if duration does not exist in the store. void updateDuration(MonitoredDurationPtr& duration); /// @brief Removes the duration from the store. @@ -161,8 +161,7 @@ public: /// /// @return a collection of the matching durations, ordered by current interval /// start time. - MonitoredDurationCollectionPtr getOverdueReports(const Timestamp& since - = dhcp::PktEvent::now()); + MonitoredDurationCollectionPtr getOverdueReports(const Timestamp& since = dhcp::PktEvent::now()); /// @brief Removes all durations from the store. void clear(); @@ -178,7 +177,7 @@ private: /// @brief Convenience method to verify a key is valid for an operation. /// /// @param label description of where the check is being made, appears in exception text. - /// @param key key to validate + /// @param key key to validate. /// /// @throw BadValue if the key is either empty or its family does not /// match the store. @@ -187,7 +186,7 @@ private: /// @brief Protocol family AF_INET or AF_INET6. uint16_t family_; - /// @brief The length of time over data for a single MonitoredDuration is + /// @brief The length of time over data for a single duration is /// accumulated before reporting. Duration interval_duration_; diff --git a/src/hooks/dhcp/perfmon/tests/alarm_store_unittests.cc b/src/hooks/dhcp/perfmon/tests/alarm_store_unittests.cc index 5564e26b9e..270f0a9c25 100644 --- a/src/hooks/dhcp/perfmon/tests/alarm_store_unittests.cc +++ b/src/hooks/dhcp/perfmon/tests/alarm_store_unittests.cc @@ -130,13 +130,14 @@ public: alarms = store.getAll(); ASSERT_TRUE(alarms->empty()); } + /// @brief Verifies that duplicate alarms cannot be added to the store. /// /// @param family protocol family to test, AF_INET or AF_INET6 void addAlarmDuplicateTest(uint16_t family) { AlarmStore store(family); - // Add a duration. + // Add an alarm. AlarmPtr alarm; ASSERT_NO_THROW_LOG(alarm = store.addAlarm(makeKey(family), milliseconds(10), milliseconds(250))); @@ -146,7 +147,7 @@ public: ASSERT_THROW(store.addAlarm(alarm), DuplicateAlarm); } - /// @brief Verifies that duration key must be valid to add a duration to the store. + /// @brief Verifies that duration key must be valid to add an alarm to the store. /// /// Tests both v4 and v6. void addAlarmInvalidTest() { @@ -155,13 +156,13 @@ public: // Attempting to add with an empty key should throw. ASSERT_THROW_MSG(store->addAlarm(DurationKeyPtr(), - milliseconds(10), milliseconds(250)), + milliseconds(10), milliseconds(250)), BadValue, "AlarmStore::addAlarm - key is empty"); - // Attempting to a v6 key should fail. + // Attempting to add a v6 key should fail. ASSERT_THROW_MSG(store->addAlarm(makeKey(AF_INET6), - milliseconds(10), milliseconds(250)), + milliseconds(10), milliseconds(250)), BadValue, "AlarmStore::addAlarm" " - family mismatch, key is v6, store is v4"); @@ -171,7 +172,7 @@ public: // Attempting to add a v4 key should fail. ASSERT_THROW_MSG(store->addAlarm(makeKey(AF_INET), - milliseconds(10), milliseconds(250)), + milliseconds(10), milliseconds(250)), BadValue, "AlarmStore::addAlarm" " - family mismatch, key is v4, store is v6"); @@ -196,7 +197,7 @@ public: auto alarms = store.getAll(); ASSERT_EQ(alarms->size(), 3); - // Fetch the second duration. + // Fetch the second alarm. AlarmPtr alarm; ASSERT_NO_THROW_LOG(alarm = store.getAlarm(keys[1])); ASSERT_TRUE(alarm); @@ -218,11 +219,11 @@ public: ASSERT_EQ(alarms->size(), 2); } - /// @brief Verify an invalid duration key on delete is detected. + /// @brief Verify an invalid alarm key on delete is detected. /// /// Tests both v4 and v6. void deleteAlarmInvalidTest() { - // Create a v4 store. + // Create a v4 store. AlarmStorePtr store(new AlarmStore(AF_INET)); // Attempting to delete an empty key should throw. @@ -253,10 +254,10 @@ public: void updateAlarmTest(uint16_t family) { AlarmStore store(family); - // Add the duration to the store. + // Add the alarm to the store. AlarmPtr alarm; ASSERT_NO_THROW_LOG(alarm.reset(new Alarm(*makeKey(family), milliseconds(10), - milliseconds(250)))); + milliseconds(250)))); ASSERT_NO_THROW_LOG(store.addAlarm(alarm)); // Fetch it. @@ -281,13 +282,13 @@ public: EXPECT_EQ(found->getHighWater(), milliseconds(500)); } - /// @brief Verify an invalid duration key on update is detected. + /// @brief Verify an invalid alarm key on update is detected. /// /// Tests both v4 and v6. void updateAlarmInvalidTest() { AlarmPtr alarm; - // Create a v4 store. + // Create a v4 store. AlarmStorePtr store(new AlarmStore(AF_INET)); // Attempting to update an empty key should throw. @@ -297,7 +298,7 @@ public: // Create a v6 alarm. ASSERT_NO_THROW_LOG(alarm.reset(new Alarm(*makeKey(AF_INET6), milliseconds(10), - milliseconds(250)))); + milliseconds(250)))); // Attempting to update v6 alarm to a v4 store should fail. ASSERT_THROW_MSG(store->updateAlarm(alarm), @@ -316,9 +317,9 @@ public: // Create a v4 alarm. ASSERT_NO_THROW_LOG(alarm.reset(new Alarm(*makeKey(AF_INET), milliseconds(10), - milliseconds(250)))); + milliseconds(250)))); - // Attempting to update v4 duration to a v6 store fail. + // Attempting to update v4 alarm to a v6 store fail. ASSERT_THROW_MSG(store->updateAlarm(alarm), BadValue, "AlarmStore::updateAlarm" diff --git a/src/hooks/dhcp/perfmon/tests/monitored_duration_store_unittests.cc b/src/hooks/dhcp/perfmon/tests/monitored_duration_store_unittests.cc index 1dea08b6f7..ae79933f8b 100644 --- a/src/hooks/dhcp/perfmon/tests/monitored_duration_store_unittests.cc +++ b/src/hooks/dhcp/perfmon/tests/monitored_duration_store_unittests.cc @@ -11,6 +11,7 @@ #include <testutils/gtest_utils.h> #include <testutils/multi_threading_utils.h> +#include <boost/range/adaptor/reversed.hpp> #include <gtest/gtest.h> #include <sstream> @@ -156,7 +157,7 @@ public: BadValue, "MonitoredDurationStore::addDuration - key is empty"); - // Attempting to a v6 key should fail. + // Attempting to add a v6 key should fail. ASSERT_THROW_MSG(store->addDuration(makeKey(AF_INET6)), BadValue, "MonitoredDurationStore::addDuration" @@ -217,7 +218,7 @@ public: /// /// Tests both v4 and v6. void deleteDurationInvalidTest() { - // Create a v4 store. + // Create a v4 store. Duration interval_duration(milliseconds(10)); MonitoredDurationStorePtr store(new MonitoredDurationStore(AF_INET, interval_duration)); @@ -291,7 +292,7 @@ public: Duration interval_duration(seconds(60)); MonitoredDurationPtr mond; - // Create a v4 store. + // Create a v4 store. MonitoredDurationStorePtr store(new MonitoredDurationStore(AF_INET, interval_duration)); // Attempting to update an empty key should throw. @@ -428,9 +429,9 @@ public: // key[2] - interval start = now + 2ms // key[3] - interval start = now auto five_ms(milliseconds(5)); - for (auto k = keys.rbegin(); k != keys.rend(); ++k ) { - ASSERT_NO_THROW_LOG(store.addDurationSample(*k, five_ms)); - if ((*k)->getSubnetId() != 2) { + for (auto const& k : boost::adaptors::reverse(keys)) { + ASSERT_NO_THROW_LOG(store.addDurationSample(k, five_ms)); + if (k->getSubnetId() != 2) { usleep(2 * 1000); // put 2ms gap between them } else { usleep(50 * 1000); // put 50ms gap between them. @@ -525,7 +526,7 @@ public: EXPECT_EQ(durations->size(), num_subnets); std::cout << "report count: " << report_count << std::endl - << "add keys time : " << (add_keys_time - start_time) << std::endl + << "add keys time: " << (add_keys_time - start_time) << std::endl << "add samples time: " << (add_samples_time - add_keys_time) << std::endl << "time per sample: " << (add_samples_time - add_keys_time) / (num_subnets * num_passes) << std::endl; @@ -659,7 +660,7 @@ TEST_F(MonitoredDurationStoreTest, reportDueMultiThreading) { } TEST_F(MonitoredDurationStoreTest, reportDue6) { - reportDueTest(AF_INET); + reportDueTest(AF_INET6); } TEST_F(MonitoredDurationStoreTest, reportDue6MultiThreading) { diff --git a/src/hooks/dhcp/perfmon/tests/monitored_duration_unittests.cc b/src/hooks/dhcp/perfmon/tests/monitored_duration_unittests.cc index 09aa77683b..8aeea12678 100644 --- a/src/hooks/dhcp/perfmon/tests/monitored_duration_unittests.cc +++ b/src/hooks/dhcp/perfmon/tests/monitored_duration_unittests.cc @@ -289,7 +289,7 @@ TEST(MonitoredDuration, validConstructors) { EXPECT_FALSE(mond->getPreviousInterval()); } -// Verifies Copy construction. Since current and preivous intervals are not +// Verifies Copy construction. Since current and previous intervals are not // exposed, this test relies on addSample() to alter them. TEST(MonitoredDuration, copyConstructors) { MonitoredDurationPtr mond; @@ -297,7 +297,7 @@ TEST(MonitoredDuration, copyConstructors) { // Create valid v4 duration, verify contents and label. ASSERT_NO_THROW_LOG(mond.reset(new MonitoredDuration(AF_INET, DHCPDISCOVER, DHCPOFFER, - "process_started", "process_completed", + "process_started", "process_completed", SUBNET_ID_GLOBAL, interval_duration))); // Make a copy. @@ -307,11 +307,11 @@ TEST(MonitoredDuration, copyConstructors) { // Should have different pointers. EXPECT_NE(duplicate, mond); - // Key values should be equal (DurationKey::== operator). + // Key values should be equal (DurationKey::operator==). EXPECT_EQ(*duplicate, *mond); // Check non-key members. - EXPECT_EQ(duplicate->getIntervalDuration(),mond->getIntervalDuration()); + EXPECT_EQ(duplicate->getIntervalDuration(), mond->getIntervalDuration()); EXPECT_FALSE(duplicate->getCurrentInterval()); EXPECT_FALSE(duplicate->getPreviousInterval()); |