Commit fdc6d42b authored by Marcin Siodelski's avatar Marcin Siodelski

[3975] Support for unwarned-reclaim-cycles parameter added.

parent 356f7e1f
......@@ -277,9 +277,11 @@ void ControlledDhcpv4Srv::sessionReader(void) {
void
ControlledDhcpv4Srv::reclaimExpiredLeases(const size_t max_leases,
const uint16_t timeout,
const bool remove_lease) {
const bool remove_lease,
const uint16_t max_unwarned_cycles) {
server_->alloc_engine_->reclaimExpiredLeases4(max_leases, timeout,
remove_lease);
remove_lease,
max_unwarned_cycles);
// We're using the ONE_SHOT timer so there is a need to re-schedule it.
TimerMgr::instance()->setup(CfgExpiration::RECLAIM_EXPIRED_TIMER_NAME);
}
......
......@@ -163,8 +163,13 @@ private:
/// @param remove_lease A boolean value indicating if the lease should
/// be removed when it is reclaimed (if true) or it should be left in the
/// database in the "expired-reclaimed" state (if false).
/// @param max_unwarned_cycles A number of consecutive processing cycles
/// of expired leases, after which the system issues a warning if there
/// are still expired leases in the database. If this value is 0, the
/// warning is never issued.
void reclaimExpiredLeases(const size_t max_leases, const uint16_t timeout,
const bool remove_lease);
const bool remove_lease,
const uint16_t max_unwarned_cycles);
/// @brief Deletes reclaimed leases and reschedules the timer.
///
......
......@@ -275,9 +275,11 @@ void ControlledDhcpv6Srv::sessionReader(void) {
void
ControlledDhcpv6Srv::reclaimExpiredLeases(const size_t max_leases,
const uint16_t timeout,
const bool remove_lease) {
const bool remove_lease,
const uint16_t max_unwarned_cycles) {
server_->alloc_engine_->reclaimExpiredLeases6(max_leases, timeout,
remove_lease);
remove_lease,
max_unwarned_cycles);
// We're using the ONE_SHOT timer so there is a need to re-schedule it.
TimerMgr::instance()->setup(CfgExpiration::RECLAIM_EXPIRED_TIMER_NAME);
}
......
......@@ -162,8 +162,14 @@ private:
/// @param remove_lease A boolean value indicating if the lease should
/// be removed when it is reclaimed (if true) or it should be left in the
/// database in the "expired-reclaimed" state (if false).
/// @param max_unwarned_cycles A number of consecutive processing cycles
/// of expired leases, after which the system issues a warning if there
/// are still expired leases in the database. If this value is 0, the
/// warning is never issued.
void reclaimExpiredLeases(const size_t max_leases, const uint16_t timeout,
const bool remove_lease);
const bool remove_lease,
const uint16_t max_unwarned_cycles);
/// @brief Deletes reclaimed leases and reschedules the timer.
///
......
......@@ -247,7 +247,8 @@ AllocEngine::RandomAllocator::pickAddress(const SubnetPtr&,
AllocEngine::AllocEngine(AllocType engine_type, uint64_t attempts,
bool ipv6)
: attempts_(attempts) {
: attempts_(attempts), incomplete_v4_reclamations_(0),
incomplete_v6_reclamations_(0) {
// Choose the basic (normal address) lease type
Lease::Type basic_type = ipv6 ? Lease::TYPE_NA : Lease::TYPE_V4;
......@@ -1288,7 +1289,8 @@ AllocEngine::updateLeaseData(ClientContext6& ctx, const Lease6Collection& leases
void
AllocEngine::reclaimExpiredLeases6(const size_t max_leases, const uint16_t timeout,
const bool remove_lease) {
const bool remove_lease,
const uint16_t max_unwarned_cycles) {
LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE,
ALLOC_ENGINE_V6_LEASES_RECLAMATION_START)
......@@ -1301,8 +1303,31 @@ AllocEngine::reclaimExpiredLeases6(const size_t max_leases, const uint16_t timeo
LeaseMgr& lease_mgr = LeaseMgrFactory::instance();
// This value indicates if we have been able to deal with all expired
// leases in this pass.
bool incomplete_reclamation = false;
Lease6Collection leases;
lease_mgr.getExpiredLeases6(leases, max_leases);
// The value of 0 has a special meaning - reclaim all.
if (max_leases > 0) {
// If the value is non-zero, the caller has limited the number of
// leases to reclaim. We obtain one lease more to see if there will
// be still leases left after this pass.
lease_mgr.getExpiredLeases6(leases, max_leases + 1);
// There are more leases expired leases than we will process in this
// pass, so we should mark it as an incomplete reclamation. We also
// remove this extra lease (which we don't want to process anyway)
// from the collection.
if (leases.size() > max_leases) {
leases.pop_back();
incomplete_reclamation = true;
}
} else {
// If there is no limitation on the number of leases to reclaim,
// we will try to process all. Hence, we don't mark it as incomplete
// reclamation just yet.
lease_mgr.getExpiredLeases6(leases, max_leases);
}
// Do not initialize the callout handle until we know if there are any
// lease6_expire callouts installed.
......@@ -1388,6 +1413,15 @@ AllocEngine::reclaimExpiredLeases6(const size_t max_leases, const uint16_t timeo
// return if we have. We're checking it here, because we always want to
// allow reclaiming at least one lease.
if ((timeout > 0) && (stopwatch.getTotalMilliseconds() >= timeout)) {
// Timeout. This will likely mean that we haven't been able to process
// all leases we wanted to process. The reclamation pass will be
// probably marked as incomplete.
if (!incomplete_reclamation) {
if (leases_processed < leases.size()) {
incomplete_reclamation = true;
}
}
LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE,
ALLOC_ENGINE_V6_LEASES_RECLAMATION_TIMEOUT)
.arg(timeout);
......@@ -1403,6 +1437,28 @@ AllocEngine::reclaimExpiredLeases6(const size_t max_leases, const uint16_t timeo
ALLOC_ENGINE_V6_LEASES_RECLAMATION_COMPLETE)
.arg(leases_processed)
.arg(stopwatch.logFormatTotalDuration());
// Check if this was an incomplete reclamation and increase the number of
// consecutive incomplete reclamations.
if (incomplete_reclamation) {
++incomplete_v6_reclamations_;
// If the number of incomplete reclamations is beyond the threshold, we
// need to issue a warning.
if ((max_unwarned_cycles > 0) &&
(incomplete_v6_reclamations_ > max_unwarned_cycles)) {
LOG_WARN(alloc_engine_logger, ALLOC_ENGINE_V6_LEASES_RECLAMATION_SLOW)
.arg(max_unwarned_cycles);
// We issued a warning, so let's now reset the counter.
incomplete_v6_reclamations_ = 0;
}
} else {
// This was a complete reclamation, so let's reset the counter.
incomplete_v6_reclamations_ = 0;
LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE,
ALLOC_ENGINE_V6_NO_MORE_EXPIRED_LEASES);
}
}
void
......@@ -1430,7 +1486,8 @@ AllocEngine::deleteExpiredReclaimedLeases6(const uint32_t secs) {
void
AllocEngine::reclaimExpiredLeases4(const size_t max_leases, const uint16_t timeout,
const bool remove_lease) {
const bool remove_lease,
const uint16_t max_unwarned_cycles) {
LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE,
ALLOC_ENGINE_V4_LEASES_RECLAMATION_START)
......@@ -1443,8 +1500,32 @@ AllocEngine::reclaimExpiredLeases4(const size_t max_leases, const uint16_t timeo
LeaseMgr& lease_mgr = LeaseMgrFactory::instance();
// This value indicates if we have been able to deal with all expired
// leases in this pass.
bool incomplete_reclamation = false;
Lease4Collection leases;
lease_mgr.getExpiredLeases4(leases, max_leases);
// The value of 0 has a special meaning - reclaim all.
if (max_leases > 0) {
// If the value is non-zero, the caller has limited the number of
// leases to reclaim. We obtain one lease more to see if there will
// be still leases left after this pass.
lease_mgr.getExpiredLeases4(leases, max_leases + 1);
// There are more leases expired leases than we will process in this
// pass, so we should mark it as an incomplete reclamation. We also
// remove this extra lease (which we don't want to process anyway)
// from the collection.
if (leases.size() > max_leases) {
leases.pop_back();
incomplete_reclamation = true;
}
} else {
// If there is no limitation on the number of leases to reclaim,
// we will try to process all. Hence, we don't mark it as incomplete
// reclamation just yet.
lease_mgr.getExpiredLeases4(leases, max_leases);
}
// Do not initialize the callout handle until we know if there are any
// lease4_expire callouts installed.
......@@ -1540,6 +1621,15 @@ AllocEngine::reclaimExpiredLeases4(const size_t max_leases, const uint16_t timeo
// return if we have. We're checking it here, because we always want to
// allow reclaiming at least one lease.
if ((timeout > 0) && (stopwatch.getTotalMilliseconds() >= timeout)) {
// Timeout. This will likely mean that we haven't been able to process
// all leases we wanted to process. The reclamation pass will be
// probably marked as incomplete.
if (!incomplete_reclamation) {
if (leases_processed < leases.size()) {
incomplete_reclamation = true;
}
}
LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE,
ALLOC_ENGINE_V6_LEASES_RECLAMATION_TIMEOUT)
.arg(timeout);
......@@ -1555,6 +1645,28 @@ AllocEngine::reclaimExpiredLeases4(const size_t max_leases, const uint16_t timeo
ALLOC_ENGINE_V4_LEASES_RECLAMATION_COMPLETE)
.arg(leases_processed)
.arg(stopwatch.logFormatTotalDuration());
// Check if this was an incomplete reclamation and increase the number of
// consecutive incomplete reclamations.
if (incomplete_reclamation) {
++incomplete_v4_reclamations_;
// If the number of incomplete reclamations is beyond the threshold, we
// need to issue a warning.
if ((max_unwarned_cycles > 0) &&
(incomplete_v4_reclamations_ > max_unwarned_cycles)) {
LOG_WARN(alloc_engine_logger, ALLOC_ENGINE_V4_LEASES_RECLAMATION_SLOW)
.arg(max_unwarned_cycles);
// We issued a warning, so let's now reset the counter.
incomplete_v4_reclamations_ = 0;
}
} else {
// This was a complete reclamation, so let's reset the counter.
incomplete_v4_reclamations_ = 0;
LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE,
ALLOC_ENGINE_V4_NO_MORE_EXPIRED_LEASES);
}
}
void
......
......@@ -511,8 +511,13 @@ public:
/// @param remove_lease A boolean value indicating if the lease should
/// be removed when it is reclaimed (if true) or it should be left in the
/// database in the "expired-reclaimed" state (if false).
/// @param max_unwarned_cycles A number of consecutive processing cycles
/// of expired leases, after which the system issues a warning if there
/// are still expired leases in the database. If this value is 0, the
/// warning is never issued.
void reclaimExpiredLeases6(const size_t max_leases, const uint16_t timeout,
const bool remove_lease);
const bool remove_lease,
const uint16_t max_unwarned_cycles = 0);
/// @brief Deletes reclaimed leases expired more than specified amount
/// of time ago.
......@@ -564,8 +569,13 @@ public:
/// @param remove_lease A boolean value indicating if the lease should
/// be removed when it is reclaimed (if true) or it should be left in the
/// database in the "expired-reclaimed" state (if false).
/// @param max_unwarned_cycles A number of consecutive processing cycles
/// of expired leases, after which the system issues a warning if there
/// are still expired leases in the database. If this value is 0, the
/// warning is never issued.
void reclaimExpiredLeases4(const size_t max_leases, const uint16_t timeout,
const bool remove_lease);
const bool remove_lease,
const uint16_t max_unwarned_cycles = 0);
/// @brief Deletes reclaimed leases expired more than specified amount
/// of time ago.
......@@ -1235,6 +1245,16 @@ private:
/// otherwise.
bool conditionalExtendLifetime(Lease& lease) const;
private:
/// @brief Number of consecutive DHCPv4 leases' reclamations after
/// which there are still expired leases in the database.
uint16_t incomplete_v4_reclamations_;
/// @brief Number of consecutive DHCPv6 leases' reclamations after
/// which there are still expired leases in the database.
uint16_t incomplete_v6_reclamations_;
};
/// @brief A pointer to the @c AllocEngine object.
......
......@@ -102,6 +102,22 @@ 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_SLOW expired leases still exist after %1 reclamations
This warning message is issued when the server has been unable to
reclaim all expired leases in a specified number of consecutive
attempts. This indicates that the value of "reclaim-timer-wait-time"
may be too high. However, if this is just a short burst of leases'
expirations the value does not have to be modified and the server
should deal with this in subsequent reclamation attempts. If this
is a result of a permanent increase of the server load, the value
of "reclaim-timer-wait-time" should be decreased, or the
values of "max-reclaim-leases" and "max-reclaim-time" should be
increased to allow processing more leases in a single cycle.
Alternatively, these values may be set to 0 to remove the
limitations on the number of leases and duration. However, this
may result in longer periods of server's unresponsiveness to
DHCP packets, while it processes the expired leases.
% ALLOC_ENGINE_V4_LEASES_RECLAMATION_START starting reclamation of expired leases (limit = %1 leases or %2 seconds)
This debug message is issued when the allocation engine starts the
reclamation of the expired leases. The maximum number of leases to
......@@ -116,6 +132,10 @@ been reclaimed, because of the timeout, will be reclaimed when the
next scheduled reclamation is started. The argument is the timeout
value expressed in milliseconds.
% ALLOC_ENGINE_V4_NO_MORE_EXPIRED_LEASES all expired leases have been reclaimed
This debug message is issued when the server reclaims all expired
DHCPv4 leases in the database.
% ALLOC_ENGINE_V4_OFFER_EXISTING_LEASE allocation engine will try to offer existing lease to the client %1
This message is issued when the allocation engine determines that
the client has a lease in the lease database, it doesn't have
......@@ -328,6 +348,22 @@ 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_SLOW expired leases still exist after %1 reclamations
This warning message is issued when the server has been unable to
reclaim all expired leases in a specified number of consecutive
attempts. This indicates that the value of "reclaim-timer-wait-time"
may be too high. However, if this is just a short burst of leases'
expirations the value does not have to be modified and the server
should deal with this in subsequent reclamation attempts. If this
is a result of a permanent increase of the server load, the value
of "reclaim-timer-wait-time" should be decreased, or the
values of "max-reclaim-leases" and "max-reclaim-time" should be
increased to allow processing more leases in a single cycle.
Alternatively, these values may be set to 0 to remove the
limitations on the number of leases and duration. However, this
may result in longer periods of server's unresponsiveness to
DHCP packets, while it processes the expired leases.
% ALLOC_ENGINE_V6_LEASES_RECLAMATION_START starting reclamation of expired leases (limit = %1 leases or %2 seconds)
This debug message is issued when the allocation engine starts the
reclamation of the expired leases. The maximum number of leases to
......@@ -342,6 +378,10 @@ been reclaimed, because of the timeout, will be reclaimed when the
next scheduled reclamation is started. The argument is the timeout
value expressed in milliseconds.
% ALLOC_ENGINE_V6_NO_MORE_EXPIRED_LEASES all expired leases have been reclaimed
This debug message is issued when the server reclaims all expired
DHCPv6 leases in the database.
% ALLOC_ENGINE_V6_RECLAIMED_LEASES_DELETE begin deletion of reclaimed leases expired more than %1 seconds ago
This debug message is issued when the allocation engine begins
deletion of the reclaimed leases which have expired more than
......
......@@ -227,7 +227,7 @@ public:
/// are implemented.
template<typename Instance>
void setupTimers(void (Instance::*reclaim_fun)(const size_t, const uint16_t,
const bool),
const bool, const uint16_t),
void (Instance::*delete_fun)(const uint32_t),
Instance* instance_ptr) const;
......@@ -284,8 +284,9 @@ typedef boost::shared_ptr<const CfgExpiration> ConstCfgExpirationPtr;
template<typename Instance>
void
CfgExpiration::setupTimers(void (Instance::*reclaim_fun)(const size_t,
const uint16_t,
const bool),
const uint16_t,
const bool,
const uint16_t),
void (Instance::*delete_fun)(const uint32_t),
Instance* instance_ptr) const {
// One of the parameters passed to the leases' reclamation routine
......@@ -307,7 +308,8 @@ CfgExpiration::setupTimers(void (Instance::*reclaim_fun)(const size_t,
boost::bind(reclaim_fun, instance_ptr,
getMaxReclaimLeases(),
getMaxReclaimTime(),
flush_timer_disabled),
flush_timer_disabled,
getUnwarnedReclaimCycles()),
reclaim_interval,
asiolink::IntervalTimer::ONE_SHOT);
timer_mgr_->setup(RECLAIM_EXPIRED_TIMER_NAME);
......
......@@ -190,6 +190,10 @@ public:
/// @brief Boolean flag which indicates if the leases should be removed
/// when reclaimed.
bool remove_lease;
/// @brief Maximum number of reclamation attempts after which all leases
/// should be reclaimed.
uint16_t max_unwarned_cycles;
};
/// @brief Constructor.
......@@ -206,15 +210,19 @@ public:
/// @param timeout Timeout for processing leases in milliseconds.
/// @remove_lease Boolean flag which indicates if the leases should be
/// removed when it is reclaimed.
/// @param Maximum number of reclamation attempts after which all leases
/// should be reclaimed.
void
reclaimExpiredLeases(const size_t max_leases, const uint16_t timeout,
const bool remove_lease) {
const bool remove_lease,
const uint16_t max_unwarned_cycles) {
// Increase calls counter for this method.
++reclaim_calls_count_;
// Record all parameters with which this method has been called.
reclaim_params_.max_leases = max_leases;
reclaim_params_.timeout = timeout;
reclaim_params_.remove_lease = remove_lease;
reclaim_params_.max_unwarned_cycles = max_unwarned_cycles;
// Leases' reclamation routine is responsible for re-scheduling
// the timer.
......@@ -342,6 +350,7 @@ TEST_F(CfgExpirationTimersTest, reclamationParameters) {
cfg_.setMaxReclaimLeases(1000);
cfg_.setMaxReclaimTime(1500);
cfg_.setHoldReclaimedTime(1800);
cfg_.setUnwarnedReclaimCycles(13);
// Run timers for 500ms.
ASSERT_NO_FATAL_FAILURE(setupAndRun(500));
......@@ -352,6 +361,7 @@ TEST_F(CfgExpirationTimersTest, reclamationParameters) {
EXPECT_EQ(1000, stub_->reclaim_params_.max_leases);
EXPECT_EQ(1500, stub_->reclaim_params_.timeout);
EXPECT_FALSE(stub_->reclaim_params_.remove_lease);
EXPECT_EQ(13, stub_->reclaim_params_.max_unwarned_cycles);
// Make sure we had more than one call to the routine which flushes
// expired reclaimed leases.
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment