From: Thomas Markwalder Date: Mon, 1 Apr 2024 19:46:15 +0000 (-0400) Subject: [#3297] PerfMonMgr::processPktEventStack tests X-Git-Tag: Kea-2.5.8~100 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=24e18479862d10028b392dba040e52f2dd561547;p=thirdparty%2Fkea.git [#3297] PerfMonMgr::processPktEventStack tests src/hooks/dhcp/perfmon/monitored_duration_store.* Retooled to use composite key instead of DurationKey operators src/hooks/dhcp/perfmon/perfmon_mgr.cc Altered some exception throws src/hooks/dhcp/perfmon/tests/monitored_duration_store_unittests.cc TEST_F(MonitoredDurationStoreTest, adjacentEvent) TEST_F(MonitoredDurationStoreTest, adjacentEvent6) - new tests to check adjacent event ordering src/hooks/dhcp/perfmon/tests/perfmon_mgr_unittests.cc TEST_F(PerfMonMgrTest4, invalidProcessPktEventStack) TEST_F(PerfMonMgrTest6, invalidProcessPktEventStack) TEST_F(PerfMonMgrTest4, processPktEventStack) TEST_F(PerfMonMgrTest6, processPktEventStack) - new tests --- diff --git a/src/hooks/dhcp/perfmon/monitored_duration_store.cc b/src/hooks/dhcp/perfmon/monitored_duration_store.cc index 6c0e5504b8..d0f0b31738 100644 --- a/src/hooks/dhcp/perfmon/monitored_duration_store.cc +++ b/src/hooks/dhcp/perfmon/monitored_duration_store.cc @@ -52,11 +52,18 @@ MonitoredDurationPtr MonitoredDurationStore::addDurationSample(DurationKeyPtr key, const Duration& sample) { validateKey("addDurationSample", key); + MultiThreadingLock lock(*mutex_); auto& index = durations_.get(); - auto duration_iter = index.find(*key); + auto duration_iter = index.find(boost::make_tuple(key->getQueryType(), + key->getResponseType(), + key->getStartEventLabel(), + key->getStopEventLabel(), + key->getSubnetId())); + if (duration_iter != index.end()) { 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) { @@ -83,7 +90,7 @@ MonitoredDurationStore::addDurationSample(DurationKeyPtr key, const Duration& sa // Shouldn't be possible. isc_throw(DuplicateDurationKey, "MonitoredDurationStore::addDurationSample: duration already exists for: " - << key->getLabel()); + << mond->getLabel()); } // Nothing to report. @@ -124,7 +131,12 @@ MonitoredDurationStore::getDuration(DurationKeyPtr key) { MultiThreadingLock lock(*mutex_); const auto& index = durations_.get(); - auto duration_iter = index.find(*key); + auto duration_iter = index.find(boost::make_tuple(key->getQueryType(), + key->getResponseType(), + key->getStartEventLabel(), + key->getStopEventLabel(), + key->getSubnetId())); + return (duration_iter == index.end() ? MonitoredDurationPtr() : MonitoredDurationPtr(new MonitoredDuration(**duration_iter))); } @@ -135,7 +147,12 @@ MonitoredDurationStore::updateDuration(MonitoredDurationPtr& duration) { MultiThreadingLock lock(*mutex_); auto& index = durations_.get(); - auto duration_iter = index.find(*duration); + auto duration_iter = index.find(boost::make_tuple(duration->getQueryType(), + duration->getResponseType(), + duration->getStartEventLabel(), + duration->getStopEventLabel(), + duration->getSubnetId())); + if (duration_iter == index.end()) { isc_throw(InvalidOperation, "MonitoredDurationStore::updateDuration duration not found: " << duration->getLabel()); @@ -151,7 +168,12 @@ MonitoredDurationStore::deleteDuration(DurationKeyPtr key) { MultiThreadingLock lock(*mutex_); auto& index = durations_.get(); - auto duration_iter = index.find(*key); + auto duration_iter = index.find(boost::make_tuple(key->getQueryType(), + key->getResponseType(), + key->getStartEventLabel(), + key->getStopEventLabel(), + key->getSubnetId())); + if (duration_iter == index.end()) { // Not there, just return. return; diff --git a/src/hooks/dhcp/perfmon/monitored_duration_store.h b/src/hooks/dhcp/perfmon/monitored_duration_store.h index 292782ecf4..2e884dcec5 100644 --- a/src/hooks/dhcp/perfmon/monitored_duration_store.h +++ b/src/hooks/dhcp/perfmon/monitored_duration_store.h @@ -53,10 +53,27 @@ typedef boost::multi_index_container< MonitoredDurationPtr, boost::multi_index::indexed_by< // Specification of the first index starts here. - // This index sorts using DurationKey::operators + // This index sorts using composite key boost::multi_index::ordered_unique< boost::multi_index::tag, - boost::multi_index::identity + boost::multi_index::composite_key< + MonitoredDuration, + // The query packet type + boost::multi_index::const_mem_fun, + // The response packet type + boost::multi_index::const_mem_fun, + // The start event label + boost::multi_index::const_mem_fun, + // The end event label + boost::multi_index::const_mem_fun, + // The subnet id + boost::multi_index::const_mem_fun + > >, // Specification of the second index starts here. diff --git a/src/hooks/dhcp/perfmon/perfmon_mgr.cc b/src/hooks/dhcp/perfmon/perfmon_mgr.cc index ca97a88e62..79b7c1bea2 100644 --- a/src/hooks/dhcp/perfmon/perfmon_mgr.cc +++ b/src/hooks/dhcp/perfmon/perfmon_mgr.cc @@ -68,7 +68,7 @@ PerfMonMgr::processPktEventStack(isc::dhcp::PktPtr query, isc::dhcp::PktPtr response, const isc::dhcp::SubnetID& subnet_id) { if (!query) { - isc_throw(Unexpected, "processPktEventStack - query is empty!"); + isc_throw(Unexpected, "PerfMonMgr::processPktEventStack - query is empty!"); } uint16_t query_type = query->getType(); @@ -87,10 +87,12 @@ PerfMonMgr::processPktEventStack(isc::dhcp::PktPtr query, auto events = query->getPktEvents(); if (events.size() < 2) { - isc_throw(Unexpected, "processPtkEventStack - incomplete stack, size: " + isc_throw(Unexpected, "PerfMonMgr::processPtkEventStack - incomplete stack, size: " << events.size()); } + // no subnet id? + boost::posix_time::ptime start_time; boost::posix_time::ptime prev_time; std::string prev_event_label; 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 f466e6fdb2..1f05864103 100644 --- a/src/hooks/dhcp/perfmon/tests/monitored_duration_store_unittests.cc +++ b/src/hooks/dhcp/perfmon/tests/monitored_duration_store_unittests.cc @@ -531,6 +531,41 @@ public: << "time per sample: " << (add_samples_time - add_keys_time) / (num_subnets * num_passes) << std::endl; } + + /// @brief Verifies that composite key index compares correctly with adjacent events. + /// + /// @param family protocol family to test, AF_INET or AF_INET6 + void adjacentEventTest(uint16_t family) { + Duration interval_duration(milliseconds(10)); + MonitoredDurationStore store(family, interval_duration); + + // Create two keys where the stop event for the first key is the start + // event for the second key. + DurationKeyPtr key1(new DurationKey(family, 0, 0, "socket_received", "buffer_read", 1)); + DurationKeyPtr key2(new DurationKey(family, 0, 0, "buffer_read", "process_started", 1)); + + // Make multiple calls to addDurationSample() for each key, starting with key1. + for (int i = 0; i < 4; ++i) { + MonitoredDurationPtr mond; + ASSERT_NO_THROW_LOG(mond = store.addDurationSample(key1, milliseconds(1))); + ASSERT_NO_THROW_LOG(mond = store.addDurationSample(key2, milliseconds(2))); + } + + // Get all should retrieve all four in ascending order. + MonitoredDurationCollectionPtr durations = store.getAll(); + ASSERT_EQ(durations->size(), 2); + + auto mond = (*durations)[0]; + ASSERT_EQ(*key2, *mond); + ASSERT_TRUE(mond->getCurrentInterval()); + EXPECT_EQ(milliseconds(8), mond->getCurrentInterval()->getTotalDuration()); + + mond = (*durations)[1]; + ASSERT_EQ(*key1, *mond); + ASSERT_TRUE(mond->getCurrentInterval()); + EXPECT_EQ(milliseconds(4), mond->getCurrentInterval()->getTotalDuration()); + } + }; TEST_F(MonitoredDurationStoreTest, addDuration) { @@ -668,4 +703,17 @@ TEST_F(MonitoredDurationStoreTest, reportDue6MultiThreading) { reportDueTest(AF_INET6); } +TEST_F(MonitoredDurationStoreTest, adjacentEvent) { + adjacentEventTest(AF_INET); +} + +TEST_F(MonitoredDurationStoreTest, adjacentEvent6) { + adjacentEventTest(AF_INET6); +} + +TEST_F(MonitoredDurationStoreTest, speed) { + speedCheck(AF_INET); +} + } // end of anonymous namespace + diff --git a/src/hooks/dhcp/perfmon/tests/perfmon_mgr_unittests.cc b/src/hooks/dhcp/perfmon/tests/perfmon_mgr_unittests.cc index f390ae9608..e51a4573d8 100644 --- a/src/hooks/dhcp/perfmon/tests/perfmon_mgr_unittests.cc +++ b/src/hooks/dhcp/perfmon/tests/perfmon_mgr_unittests.cc @@ -8,6 +8,8 @@ #include #include #include +#include +#include #include #include #include @@ -469,9 +471,138 @@ public: EXPECT_TRUE(checkFile()); } - /// @brief Exercises PerfMonMgr::procssPacketEventStack(). - void testProcesPacketEventStack() { - /* YOU ARE HERE */ + /// @brief Exercises error handling in PerfMonMgr::procssPktEventStack(). + void testInvalidProcessPktEventStack() { + // Minimal valid config. + std::string valid_config = + R"({ "enable-monitoring": true })"; + + ASSERT_NO_THROW_LOG(createMgr(valid_config)); + PktPtr query; + PktPtr response; + SubnetID subnet_id = SUBNET_ID_GLOBAL; + // No query + ASSERT_THROW_MSG(mgr_->processPktEventStack(query, response, subnet_id), Unexpected, + "PerfMonMgr::processPktEventStack - query is empty!"); + + // Invalid message pair + query = makeFamilyQuery(); + response = makeFamilyResponse(); + + std::ostringstream oss; + oss << "Query type not supported by monitoring: " + << (family_ == AF_INET ? Pkt4::getName(response->getType()) + : Pkt6::getName(response->getType())); + + ASSERT_THROW_MSG(mgr_->processPktEventStack(response, query, subnet_id), BadValue, + oss.str()); + + // Packet stack size less than 2. + query->addPktEvent("just one"); + ASSERT_THROW_MSG(mgr_->processPktEventStack(query, response, subnet_id), Unexpected, + "PerfMonMgr::processPtkEventStack - incomplete stack, size: 1"); + } + + /// @brief Exercises PerfMonMgr::procssPktEventStack(). + void testProcessPktEventStack() { + // Minimal valid configuration. + std::string valid_config = + R"({ "enable-monitoring": true })"; + + ASSERT_NO_THROW_LOG(createMgr(valid_config)); + + // Create family-specific packets. + PktPtr query = makeFamilyQuery(); + PktPtr response = makeFamilyResponse(); + + // Populate the query's packet event stack with some events. + auto event_time = PktEvent::now(); + query->addPktEvent("socket_received", event_time); + event_time += milliseconds(5); + + query->addPktEvent("buffer_read", event_time); + event_time += milliseconds(4); + + query->addPktEvent("process_started", event_time); + event_time += milliseconds(3); + + query->addPktEvent("process_completed", event_time); + + // Now process the packets on two different subnets. + ASSERT_NO_THROW_LOG(mgr_->processPktEventStack(query, response, 22)); + ASSERT_NO_THROW_LOG(mgr_->processPktEventStack(query, response, 33)); + + // Fetch all the durations in primary key order. + MonitoredDurationCollectionPtr durations = mgr_->getDurationStore()->getAll(); + ASSERT_EQ(durations->size(), 12); + + // Contains the expected values for single duration. + struct ExpectedDuration { + std::string start_event_; + std::string stop_event_; + dhcp::SubnetID subnet_id_; + uint64_t occurrences_; + uint64_t total_duration_; + }; + + // Specifies the expected durations in the order they should be returned. + std::list exp_data_rows { + { "buffer_read", "process_started", 0, 2, 8 }, + { "buffer_read", "process_started", 22, 1, 4 }, + { "buffer_read", "process_started", 33, 1, 4 }, + + { "composite", "total_response", 0, 2, 24 }, + { "composite", "total_response", 22, 1, 12 }, + { "composite", "total_response", 33, 1, 12 }, + + { "process_started", "process_completed", 0, 2, 6 }, + { "process_started", "process_completed", 22, 1, 3 }, + { "process_started", "process_completed", 33, 1, 3 }, + + { "socket_received", "buffer_read", 0, 2, 10 }, + { "socket_received", "buffer_read", 22, 1, 5 }, + { "socket_received", "buffer_read", 33, 1, 5 } + }; + + // Verify the returne collection is as expected. + DurationKeyPtr exp_key; + auto exp_row = exp_data_rows.begin(); + for (auto const& d : *durations) { + EXPECT_EQ(query->getType(), d->getQueryType()); + EXPECT_EQ(response->getType(), d->getResponseType()); + EXPECT_EQ((*exp_row).start_event_, d->getStartEventLabel()); + EXPECT_EQ((*exp_row).stop_event_, d->getStopEventLabel()); + EXPECT_EQ((*exp_row).subnet_id_, d->getSubnetId()); + ASSERT_TRUE(d->getCurrentInterval()); + EXPECT_EQ((*exp_row).occurrences_, d->getCurrentInterval()->getOccurrences()); + EXPECT_EQ(milliseconds((*exp_row).total_duration_), + d->getCurrentInterval()->getTotalDuration()); + ++exp_row; + } + } + + /// @brief Make a valid family-specific query. + /// + /// @return if family is AF_INET return a pointer to a DHCPDISCOVER + /// otherwise a pointer to a DHCPV6_SOLICIT. + PktPtr makeFamilyQuery() { + if (family_ == AF_INET) { + return (PktPtr(new Pkt4(DHCPDISCOVER, 7788))); + } + + return (PktPtr(new Pkt6(DHCPV6_SOLICIT, 7788))); + } + + /// @brief Make a valid family-specific response. + /// + /// @return if family is AF_INET return a pointer to a DHCPOFFER + /// otherwise a pointer to a DHCPV6_ADVERTISE. + PktPtr makeFamilyResponse() { + if (family_ == AF_INET) { + return (PktPtr(new Pkt4(DHCPOFFER, 7788))); + } + + return (PktPtr(new Pkt6(DHCPV6_ADVERTISE, 7788))); } /// @brief Protocol family AF_INET or AF_INET6 @@ -527,20 +658,36 @@ TEST_F(PerfMonMgrTest6, invalidConfig) { testInvalidConfig(); } -TEST_F(PerfMonMgrTest4, testReportToStatsMgr) { +TEST_F(PerfMonMgrTest4, reportToStatsMgr) { testReportToStatsMgr(); } -TEST_F(PerfMonMgrTest6, testReportToStatsMgr) { +TEST_F(PerfMonMgrTest6, reportToStatsMgr) { testReportToStatsMgr(); } -TEST_F(PerfMonMgrTest4, testAddDurationSample) { +TEST_F(PerfMonMgrTest4, addDurationSample) { testAddDurationSample(); } -TEST_F(PerfMonMgrTest6, testAddDurationSample) { +TEST_F(PerfMonMgrTest6, addDurationSample) { testAddDurationSample(); } +TEST_F(PerfMonMgrTest4, invalidProcessPktEventStack) { + testInvalidProcessPktEventStack(); +} + +TEST_F(PerfMonMgrTest6, invalidProcessPktEventStack) { + testInvalidProcessPktEventStack(); +} + +TEST_F(PerfMonMgrTest4, processPktEventStack) { + testProcessPktEventStack(); +} + +TEST_F(PerfMonMgrTest6, processPktEventStack) { + testProcessPktEventStack(); +} + } // end of anonymous namespace