From 27266ca374d6c9156066abe7f40c0e9a3a9efe04 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Tue, 13 Oct 2015 21:43:38 +0200 Subject: [PATCH 1/3] [4075] Account for the usleep wake up delays on VMs. This change affects the reclaimExpiredLeasesTimeout test. --- .../tests/alloc_engine_expiration_unittest.cc | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc index fc78088351..1472b3c4ac 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc @@ -503,11 +503,11 @@ public: } /// @brief Implements "lease{4,6}_expire callout, which lasts at least - /// 2ms. + /// 40ms. /// /// This callout is useful to test scenarios where the reclamation of the /// lease needs to take a known amount of time. If the callout is installed - /// it will take at least 2ms for each lease. It is then possible to calculate + /// it will take at least 40ms for each lease. It is then possible to calculate /// the approximate time that the reclamation of all leases would take and /// test that the timeouts for the leases' reclamation work as expected. /// @@ -515,8 +515,8 @@ public: /// @return Zero. static int leaseExpireWithDelayCallout(CalloutHandle& callout_handle) { leaseExpireCallout(callout_handle); - // Delay the return from the callout by 2ms. - usleep(2000); + // Delay the return from the callout by 40ms. + usleep(40000); return (0); } @@ -874,7 +874,7 @@ public: HooksManager::loadLibraries(libraries); // Install a callout: lease4_expire or lease6_expire. Each callout - // takes at least 2ms to run (it uses usleep). + // takes at least 40ms to run (it uses usleep). std::ostringstream callout_name; callout_name << callout_argument_name << "_expire"; EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout( @@ -883,8 +883,8 @@ public: // Reclaim leases with timeout. ASSERT_NO_THROW(reclaimExpiredLeases(0, timeout, false)); - // We reclaimed at most (timeout / 2ms) leases. - const uint16_t theoretical_reclaimed = static_cast(timeout / 2); + // We reclaimed at most (timeout / 40ms) leases. + const uint16_t theoretical_reclaimed = static_cast(timeout / 40); // The actual number of leases reclaimed is likely to be lower than // the theoretical number. For low theoretical number the adjusted @@ -1267,8 +1267,8 @@ TEST_F(ExpirationAllocEngine6Test, reclaimExpiredLeasesHooksWithSkip) { TEST_F(ExpirationAllocEngine6Test, reclaimExpiredLeasesTimeout) { // This test needs at least 40 leases to make sense. BOOST_STATIC_ASSERT(TEST_LEASES_NUM >= 40); - // Run with timeout of 60ms. - testReclaimExpiredLeasesTimeout(60); + // Run with timeout of 1.2s. + testReclaimExpiredLeasesTimeout(1200); } // This test verifies that at least one lease is reclaimed if the timeout @@ -1631,8 +1631,8 @@ TEST_F(ExpirationAllocEngine4Test, reclaimExpiredLeasesHooksWithSkip) { TEST_F(ExpirationAllocEngine4Test, reclaimExpiredLeasesTimeout) { // This test needs at least 40 leases to make sense. BOOST_STATIC_ASSERT(TEST_LEASES_NUM >= 40); - // Run with timeout of 60ms. - testReclaimExpiredLeasesTimeout(60); + // Run with timeout of 1.2s. + testReclaimExpiredLeasesTimeout(1200); } // This test verifies that at least one lease is reclaimed if the timeout From a045551c70df031afe42d65abeb7d0c8dd26acea Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Tue, 13 Oct 2015 21:59:12 +0200 Subject: [PATCH 2/3] [4075] Added comment in the unit test about the usleep time selection. --- src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc index 1472b3c4ac..f955109350 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc @@ -511,6 +511,12 @@ public: /// the approximate time that the reclamation of all leases would take and /// test that the timeouts for the leases' reclamation work as expected. /// + /// The value of 40ms is relatively high, but it has been selected to + /// mitigate the problems with usleep on some virtual machines. On those + /// machines the wakeup from usleep may take significant amount of time, + /// i.e. usually around 10ms. Thus, the sleep time should be considerably + /// higher than this delay. + /// /// @param callout_handle Callout handle. /// @return Zero. static int leaseExpireWithDelayCallout(CalloutHandle& callout_handle) { From 390cf0aeee549b53a753c9951f7194f9ccc09020 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Tue, 13 Oct 2015 22:01:00 +0200 Subject: [PATCH 3/3] [4075] Corrected seconds to milliseconds in the log message. --- src/lib/dhcpsrv/alloc_engine_messages.mes | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/dhcpsrv/alloc_engine_messages.mes b/src/lib/dhcpsrv/alloc_engine_messages.mes index e41d45c2e6..7270b92022 100644 --- a/src/lib/dhcpsrv/alloc_engine_messages.mes +++ b/src/lib/dhcpsrv/alloc_engine_messages.mes @@ -83,7 +83,7 @@ the number of reclaimed leases may also be limited by the timeout value, configured with 'max-reclaim-time'. The message includes the number of reclaimed leases and the total time. -% ALLOC_ENGINE_V4_LEASES_RECLAMATION_START starting reclamation of expired leases (limit = %1 leases or %2 seconds) +% ALLOC_ENGINE_V4_LEASES_RECLAMATION_START starting reclamation of expired leases (limit = %1 leases or %2 milliseconds) This debug message is issued when the allocation engine starts the reclamation of the expired leases. The maximum number of leases to be reclaimed and the timeout is included in the message. If any of @@ -309,7 +309,7 @@ the number of reclaimed leases may also be limited by the timeout value, configured with 'max-reclaim-time'. The message includes the number of reclaimed leases and the total time. -% ALLOC_ENGINE_V6_LEASES_RECLAMATION_START starting reclamation of expired leases (limit = %1 leases or %2 seconds) +% ALLOC_ENGINE_V6_LEASES_RECLAMATION_START starting reclamation of expired leases (limit = %1 leases or %2 milliseconds) This debug message is issued when the allocation engine starts the reclamation of the expired leases. The maximum number of leases to be reclaimed and the timeout is included in the message. If any of