2
0
mirror of https://gitlab.isc.org/isc-projects/kea synced 2025-08-31 14:05:33 +00:00

[#1402] LeaseUpdateBacklog is no longer a template

LeaseUpdateBacklog now holds LeasePtr and is no longer a template class
as suggested in review comments. Classes using this class were simplified.
In particular, the HAService class has now only one backlog queue.
This commit is contained in:
Marcin Siodelski
2021-01-07 13:19:55 +01:00
parent a8143a5dac
commit 723eb8f238
10 changed files with 145 additions and 131 deletions

View File

@@ -25,7 +25,7 @@ libha_la_SOURCES += ha_messages.cc ha_messages.h
libha_la_SOURCES += ha_server_type.h
libha_la_SOURCES += ha_service.cc ha_service.h
libha_la_SOURCES += ha_service_states.cc ha_service_states.h
libha_la_SOURCES += lease_update_backlog.h
libha_la_SOURCES += lease_update_backlog.cc lease_update_backlog.h
libha_la_SOURCES += query_filter.cc query_filter.h
libha_la_SOURCES += version.cc

View File

@@ -125,16 +125,16 @@ CommandCreator::createLease6BulkApply(const Lease6CollectionPtr& leases,
}
ConstElementPtr
CommandCreator::createLease6BulkApply(Lease6UpdateBacklog& leases) {
CommandCreator::createLease6BulkApply(LeaseUpdateBacklog& leases) {
ElementPtr deleted_leases_list = Element::createList();
ElementPtr leases_list = Element::createList();
Lease6UpdateBacklog::OpType op_type;
LeaseUpdateBacklog::OpType op_type;
Lease6Ptr lease;
while ((lease = leases.pop(op_type))) {
while ((lease = boost::dynamic_pointer_cast<Lease6>(leases.pop(op_type)))) {
ElementPtr lease_as_json = lease->toElement();
insertLeaseExpireTime(lease_as_json);
if (op_type == Lease6UpdateBacklog::DELETE) {
if (op_type == LeaseUpdateBacklog::DELETE) {
deleted_leases_list->add(lease_as_json);
} else {
leases_list->add(lease_as_json);

View File

@@ -100,7 +100,7 @@ public:
/// @param leases Reference to the collection of DHCPv6 leases backlog.
/// @return Pointer to the JSON representation of the command.
static data::ConstElementPtr
createLease6BulkApply(Lease6UpdateBacklog& leases);
createLease6BulkApply(LeaseUpdateBacklog& leases);
/// @brief Creates lease6-update command.
///

View File

@@ -54,8 +54,7 @@ HAService::HAService(const IOServicePtr& io_service, const NetworkStatePtr& netw
: io_service_(io_service), network_state_(network_state), config_(config),
server_type_(server_type), client_(*io_service), communication_state_(),
query_filter_(config), mutex_(), pending_requests_(),
lease4_update_backlog_(config->getDelayedUpdatesLimit()),
lease6_update_backlog_(config->getDelayedUpdatesLimit()) {
lease_update_backlog_(config->getDelayedUpdatesLimit()) {
if (server_type == HAServerType::DHCPv4) {
communication_state_.reset(new CommunicationState4(io_service_, config));
@@ -239,8 +238,7 @@ HAService::communicationRecoveryHandler() {
// to the normal operation.
if ((communication_state_->getPartnerState() == getNormalState() ||
(communication_state_->getPartnerState() == HA_COMMUNICATION_RECOVERY_ST)) &&
!lease4_update_backlog_.wasOverflown() &&
!lease6_update_backlog_.wasOverflown() &&
!lease_update_backlog_.wasOverflown() &&
sendLeaseUpdatesFromBacklog()) {
// Everything went fine, so we can go back to the normal operation.
verboseTransition(getNormalState());
@@ -257,8 +255,7 @@ HAService::communicationRecoveryHandler() {
// When exiting this state we must ensure that lease updates backlog is cleared.
if (doOnExit()) {
lease4_update_backlog_.clear();
lease6_update_backlog_.clear();
lease_update_backlog_.clear();
}
}
@@ -992,12 +989,12 @@ HAService::asyncSendLeaseUpdates(const dhcp::Pkt4Ptr& query,
if (shouldQueueLeaseUpdates(conf)) {
// Lease updates for deleted leases.
for (auto l = deleted_leases->begin(); l != deleted_leases->end(); ++l) {
lease4_update_backlog_.push(Lease4UpdateBacklog::DELETE, *l);
lease_update_backlog_.push(LeaseUpdateBacklog::DELETE, *l);
}
// Lease updates for new allocations and updated leases.
for (auto l = leases->begin(); l != leases->end(); ++l) {
lease4_update_backlog_.push(Lease4UpdateBacklog::ADD, *l);
lease_update_backlog_.push(LeaseUpdateBacklog::ADD, *l);
}
continue;
@@ -1052,12 +1049,12 @@ HAService::asyncSendLeaseUpdates(const dhcp::Pkt6Ptr& query,
// be sent when the communication is re-established.
if (shouldQueueLeaseUpdates(conf)) {
for (auto l = deleted_leases->begin(); l != deleted_leases->end(); ++l) {
lease6_update_backlog_.push(Lease6UpdateBacklog::DELETE, *l);
lease_update_backlog_.push(LeaseUpdateBacklog::DELETE, *l);
}
// Lease updates for new allocations and updated leases.
for (auto l = leases->begin(); l != leases->end(); ++l) {
lease6_update_backlog_.push(Lease6UpdateBacklog::ADD, *l);
lease_update_backlog_.push(LeaseUpdateBacklog::ADD, *l);
}
continue;
@@ -2062,29 +2059,23 @@ void
HAService::asyncSendLeaseUpdatesFromBacklog(HttpClient& http_client,
const HAConfig::PeerConfigPtr& config,
PostRequestCallback post_request_action) {
auto num_updates = lease4_update_backlog_.size() > 0 ? lease4_update_backlog_.size() :
lease6_update_backlog_.size();
if (num_updates == 0) {
if (lease_update_backlog_.size() == 0) {
post_request_action(true, "");
return;
}
ConstElementPtr command;
if (lease4_update_backlog_.size() > 0) {
Lease4UpdateBacklog::OpType op_type;
Lease4Ptr lease = lease4_update_backlog_.pop(op_type);
if (op_type == Lease4UpdateBacklog::ADD) {
if (server_type_ == HAServerType::DHCPv4) {
LeaseUpdateBacklog::OpType op_type;
Lease4Ptr lease = boost::dynamic_pointer_cast<Lease4>(lease_update_backlog_.pop(op_type));
if (op_type == LeaseUpdateBacklog::ADD) {
command = CommandCreator::createLease4Update(*lease);
} else {
command = CommandCreator::createLease4Delete(*lease);
}
} else if (lease6_update_backlog_.size() > 0) {
command = CommandCreator::createLease6BulkApply(lease6_update_backlog_);
} else {
post_request_action(true, "");
return;
command = CommandCreator::createLease6BulkApply(lease_update_backlog_);
}
// Create HTTP/1.1 request including our command.
@@ -2141,8 +2132,7 @@ HAService::asyncSendLeaseUpdatesFromBacklog(HttpClient& http_client,
bool
HAService::sendLeaseUpdatesFromBacklog() {
auto num_updates = lease4_update_backlog_.size() > 0 ? lease4_update_backlog_.size() :
lease6_update_backlog_.size();
auto num_updates = lease_update_backlog_.size();
if (num_updates == 0) {
LOG_INFO(ha_logger, HA_LEASES_BACKLOG_NOTHING_TO_SEND);
return (true);

View File

@@ -1119,19 +1119,12 @@ private:
protected:
/// @brief Backlog of DHCPv4 lease updates.
/// @brief Backlog of DHCP lease updates.
///
/// Unsent DHCPv4 updates are stored in this queue when the server is in
/// Unsent lease updates are stored in this queue when the server is in
/// the communication-recovery state and is temporarily unable to send
/// lease updates to the partner.
Lease4UpdateBacklog lease4_update_backlog_;
/// @brief Backlog of DHCPv6 lease updates.
///
/// Unsent DHCPv6 updates are stored in this queue when the server is in
/// the communication-recovery state and is temporarily unable to send
/// lease updates to the partner.
Lease6UpdateBacklog lease6_update_backlog_;
LeaseUpdateBacklog lease_update_backlog_;
};
/// @brief Pointer to the @c HAService class.

View File

@@ -0,0 +1,90 @@
// Copyright (C) 2020 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
// file, You can obtain one at http://mozilla.org/MPL/2.0/.
#include <config.h>
#include <lease_update_backlog.h>
#include <util/multi_threading_mgr.h>
using namespace isc::dhcp;
namespace isc {
namespace ha {
LeaseUpdateBacklog::LeaseUpdateBacklog(const size_t limit)
: limit_(limit), overflown_(false), outstanding_updates_() {
}
bool
LeaseUpdateBacklog::push(const LeaseUpdateBacklog::OpType op_type, const LeasePtr& lease) {
if (util::MultiThreadingMgr::instance().getMode()) {
std::lock_guard<std::mutex> lock(mutex_);
return (pushInternal(op_type, lease));
}
return (pushInternal(op_type, lease));
}
LeasePtr
LeaseUpdateBacklog::pop(LeaseUpdateBacklog::OpType& op_type) {
if (util::MultiThreadingMgr::instance().getMode()) {
std::lock_guard<std::mutex> lock(mutex_);
return (popInternal(op_type));
}
return (popInternal(op_type));
}
bool
LeaseUpdateBacklog::wasOverflown() {
if (util::MultiThreadingMgr::instance().getMode()) {
std::lock_guard<std::mutex> lock(mutex_);
return (overflown_);
}
return (overflown_);
}
void
LeaseUpdateBacklog::clear() {
if (util::MultiThreadingMgr::instance().getMode()) {
std::lock_guard<std::mutex> lock(mutex_);
outstanding_updates_.clear();
overflown_ = false;
}
outstanding_updates_.clear();
overflown_ = false;
}
size_t
LeaseUpdateBacklog::size() {
if (util::MultiThreadingMgr::instance().getMode()) {
std::lock_guard<std::mutex> lock(mutex_);
return (outstanding_updates_.size());
}
return (outstanding_updates_.size());
}
bool
LeaseUpdateBacklog::pushInternal(const LeaseUpdateBacklog::OpType op_type, const LeasePtr& lease) {
if (outstanding_updates_.size() >= limit_) {
overflown_ = true;
return (false);
}
outstanding_updates_.push_back(std::make_pair(op_type, lease));
return (true);
}
LeasePtr
LeaseUpdateBacklog::popInternal(LeaseUpdateBacklog::OpType& op_type) {
if (outstanding_updates_.empty()) {
return (LeasePtr());
}
auto item = outstanding_updates_.front();
outstanding_updates_.pop_front();
op_type = item.first;
return (item.second);
}
} // end of namespace isc::ha
} // end of namespace isc

View File

@@ -8,7 +8,6 @@
#define HA_LEASE_BACKLOG_H
#include <dhcpsrv/lease.h>
#include <util/multi_threading_mgr.h>
#include <deque>
#include <mutex>
#include <utility>
@@ -32,10 +31,6 @@ namespace ha {
///
/// There are two types of lease updates: "Add" and "Delete". The type
/// is specified when the lease is appended to the queue.
///
/// @tparam LeaseTypePtr Type of the lease, i.e. @c Lease4Ptr or
/// @c Lease6Ptr.
template<typename LeaseTypePtr>
class LeaseUpdateBacklog {
public:
@@ -49,9 +44,7 @@ public:
///
/// @param limit specifies the maximum number of lease updates which
/// can be stored in the queue.
LeaseUpdateBacklog(const size_t limit)
: limit_(limit), overflown_(false), outstanding_updates_() {
}
LeaseUpdateBacklog(const size_t limit);
/// @brief Appends lease update to the queue.
///
@@ -59,26 +52,14 @@ public:
/// @param lease pointer to the lease being added, or deleted.
/// @return boolean value indicating whether the lease was successfully
/// appended to the queue (if true) or not (if false).
bool push(const OpType op_type, const LeaseTypePtr& lease) {
if (util::MultiThreadingMgr::instance().getMode()) {
std::lock_guard<std::mutex> lock(mutex_);
return (pushInternal(op_type, lease));
}
return (pushInternal(op_type, lease));
}
bool push(const OpType op_type, const dhcp::LeasePtr& lease);
/// @brief Returns the next lease update and removes it from the queue.
///
/// @param [out] op_type reference to the value receiving lease update type.
/// @return pointer to the next lease update in the queue or null pointer
/// when the queue is empty.
LeaseTypePtr pop(OpType& op_type) {
if (util::MultiThreadingMgr::instance().getMode()) {
std::lock_guard<std::mutex> lock(mutex_);
return (popInternal(op_type));
}
return (popInternal(op_type));
}
dhcp::LeasePtr pop(OpType& op_type);
/// @brief Checks if the queue was overflown.
///
@@ -93,35 +74,15 @@ public:
/// This flag is reset to false when @c clear is called.
///
/// @return true if the queue was overflown, false otherwise.
bool wasOverflown() {
if (util::MultiThreadingMgr::instance().getMode()) {
std::lock_guard<std::mutex> lock(mutex_);
return (overflown_);
}
return (overflown_);
}
bool wasOverflown();
/// @brief Removes all lease updates from the queue.
///
/// It also resets the flag indicating that the queue was overflown.
void clear() {
if (util::MultiThreadingMgr::instance().getMode()) {
std::lock_guard<std::mutex> lock(mutex_);
outstanding_updates_.clear();
overflown_ = false;
}
outstanding_updates_.clear();
overflown_ = false;
}
void clear();
/// @brief Returns the current size of the queue.
size_t size() {
if (util::MultiThreadingMgr::instance().getMode()) {
std::lock_guard<std::mutex> lock(mutex_);
return (outstanding_updates_.size());
}
return (outstanding_updates_.size());
}
size_t size();
private:
@@ -131,29 +92,14 @@ private:
/// @param lease pointer to the lease being added, or deleted.
/// @return boolean value indicating whether the lease was successfully
/// appended to the queue (if true) or not (if false).
bool pushInternal(const OpType op_type, const LeaseTypePtr& lease) {
if (outstanding_updates_.size() >= limit_) {
overflown_ = true;
return (false);
}
outstanding_updates_.push_back(std::make_pair(op_type, lease));
return (true);
}
bool pushInternal(const OpType op_type, const dhcp::LeasePtr& lease);
/// @brief Returns the next lease update and removes it from the queue (thread unsafe).
///
/// @param [out] op_type reference to the value receiving lease update type.
/// @return pointer to the next lease update in the queue or null pointer
/// when the queue is empty.
LeaseTypePtr popInternal(OpType& op_type) {
if (outstanding_updates_.empty()) {
return (LeaseTypePtr());
}
auto item = outstanding_updates_.front();
outstanding_updates_.pop_front();
op_type = item.first;
return (item.second);
}
dhcp::LeasePtr popInternal(OpType& op_type);
/// @brief Holds the queue size limit.
size_t limit_;
@@ -162,18 +108,12 @@ private:
bool overflown_;
/// @brief Actual queue of lease updates and their types.
std::deque<std::pair<OpType, LeaseTypePtr> > outstanding_updates_;
std::deque<std::pair<OpType, dhcp::LeasePtr> > outstanding_updates_;
/// @brief Mutex to protect internal state.
std::mutex mutex_;
};
/// @brief Pointer to a backlog of DHCPv4 lease updates.
typedef LeaseUpdateBacklog<dhcp::Lease4Ptr> Lease4UpdateBacklog;
/// @brief Pointer to a backlog of DHCPv6 lease updates.
typedef LeaseUpdateBacklog<dhcp::Lease6Ptr> Lease6UpdateBacklog;
} // end of namespace isc::ha
} // end of namespace isc

View File

@@ -343,9 +343,9 @@ TEST(CommandCreatorTest, createLease6BulkApplyFromBacklog) {
Lease6Ptr lease = createLease6();
Lease6Ptr deleted_lease = createLease6();
Lease6UpdateBacklog backlog(100);
backlog.push(Lease6UpdateBacklog::ADD, lease);
backlog.push(Lease6UpdateBacklog::DELETE, deleted_lease);
LeaseUpdateBacklog backlog(100);
backlog.push(LeaseUpdateBacklog::ADD, lease);
backlog.push(LeaseUpdateBacklog::DELETE, deleted_lease);
ConstElementPtr command = CommandCreator::createLease6BulkApply(backlog);
ConstElementPtr arguments;

View File

@@ -202,8 +202,7 @@ public:
using HAService::config_;
using HAService::communication_state_;
using HAService::query_filter_;
using HAService::lease4_update_backlog_;
using HAService::lease6_update_backlog_;
using HAService::lease_update_backlog_;
};
/// @brief Pointer to the @c TestHAService.
@@ -906,7 +905,8 @@ public:
state->modifyPokeTime(-30);
// Create HA service and schedule lease updates.
service_.reset(new TestHAService(io_service_, network_state_, config_storage));
service_.reset(new TestHAService(io_service_, network_state_, config_storage,
HAServerType::DHCPv6));
service_->communication_state_ = state;
service_->transition(my_state.state_, HAService::NOP_EVT);
@@ -1087,7 +1087,7 @@ public:
EXPECT_FALSE(unpark_called);
// Let's make sure they have been queued.
EXPECT_EQ(2, service_->lease4_update_backlog_.size());
EXPECT_EQ(2, service_->lease_update_backlog_.size());
// Make partner available.
service_->communication_state_->poke();
@@ -1108,7 +1108,7 @@ public:
"192.2.3.4"));
// Backlog should be empty.
EXPECT_EQ(0, service_->lease4_update_backlog_.size());
EXPECT_EQ(0, service_->lease_update_backlog_.size());
}
/// @brief Tests that a DHCPv4 server trying to recover from the communication
@@ -1135,7 +1135,7 @@ public:
EXPECT_FALSE(unpark_called);
// Let's make sure they have been queued.
EXPECT_EQ(2, service_->lease4_update_backlog_.size());
EXPECT_EQ(2, service_->lease_update_backlog_.size());
// Make partner available.
service_->communication_state_->poke();
@@ -1158,7 +1158,7 @@ public:
EXPECT_FALSE(factory2_->getResponseCreator()->findRequest("lease4-update",
"192.1.2.3"));
// The backlog should be empty.
EXPECT_EQ(0, service_->lease4_update_backlog_.size());
EXPECT_EQ(0, service_->lease_update_backlog_.size());
}
/// @brief Tests scenarios when lease updates are not sent to the failover peer.
@@ -1411,7 +1411,7 @@ public:
EXPECT_FALSE(unpark_called);
// Let's make sure they have been queued.
EXPECT_EQ(2, service_->lease6_update_backlog_.size());
EXPECT_EQ(2, service_->lease_update_backlog_.size());
// Make partner available.
service_->communication_state_->poke();
@@ -1431,7 +1431,7 @@ public:
"2001:db8:1::efac"));
// Backlog should be empty.
EXPECT_EQ(0, service_->lease6_update_backlog_.size());
EXPECT_EQ(0, service_->lease_update_backlog_.size());
}
/// @brief Tests that a DHCPv6 server trying to recover from the communication
@@ -1458,7 +1458,7 @@ public:
EXPECT_FALSE(unpark_called);
// Let's make sure they have been queued.
EXPECT_EQ(2, service_->lease6_update_backlog_.size());
EXPECT_EQ(2, service_->lease_update_backlog_.size());
// Make partner available.
service_->communication_state_->poke();
@@ -1479,7 +1479,7 @@ public:
"2001:db8:1::efac"));
// Backlog should be empty.
EXPECT_EQ(0, service_->lease6_update_backlog_.size());
EXPECT_EQ(0, service_->lease_update_backlog_.size());
}
/// @brief Tests scenarios when lease updates are not sent to the failover peer.

View File

@@ -12,6 +12,7 @@
#include <dhcp/hwaddr.h>
#include <boost/make_shared.hpp>
#include <boost/pointer_cast.hpp>
#include <gtest/gtest.h>
using namespace isc::asiolink;
@@ -24,7 +25,7 @@ namespace {
// retrieved from the queue.
TEST(LeaseUpdateBacklogTest, pushAndPop) {
// Create the queue with limit of 5 lease updates.
Lease4UpdateBacklog backlog(5);
LeaseUpdateBacklog backlog(5);
// Add 5 lease updates.
for (auto i = 0; i < 5; ++i) {
@@ -33,7 +34,7 @@ TEST(LeaseUpdateBacklogTest, pushAndPop) {
HTYPE_ETHER);
Lease4Ptr lease = boost::make_shared<Lease4>(address, hwaddr, ClientIdPtr(), 60, 0, 1);
// Some lease updates have type "Add", some have type "Delete".
ASSERT_TRUE(backlog.push(i % 2 ? Lease4UpdateBacklog::ADD : Lease4UpdateBacklog::DELETE, lease));
ASSERT_TRUE(backlog.push(i % 2 ? LeaseUpdateBacklog::ADD : LeaseUpdateBacklog::DELETE, lease));
EXPECT_FALSE(backlog.wasOverflown());
}
@@ -42,19 +43,19 @@ TEST(LeaseUpdateBacklogTest, pushAndPop) {
HWAddrPtr hwaddr = boost::make_shared<HWAddr>(std::vector<uint8_t>(6, static_cast<uint8_t>(0xA)),
HTYPE_ETHER);
Lease4Ptr lease = boost::make_shared<Lease4>(address, hwaddr, ClientIdPtr(), 60, 0, 1);
ASSERT_FALSE(backlog.push(Lease4UpdateBacklog::ADD, lease));
ASSERT_FALSE(backlog.push(LeaseUpdateBacklog::ADD, lease));
EXPECT_TRUE(backlog.wasOverflown());
// Try to pop all lease updates.
Lease4UpdateBacklog::OpType op_type;
LeaseUpdateBacklog::OpType op_type;
for (auto i = 0; i < 5; ++i) {
auto lease = backlog.pop(op_type);
ASSERT_TRUE(lease);
ASSERT_EQ(i % 2 ? Lease4UpdateBacklog::ADD : Lease4UpdateBacklog::DELETE, op_type);
ASSERT_EQ(i % 2 ? LeaseUpdateBacklog::ADD : LeaseUpdateBacklog::DELETE, op_type);
}
// When trying to pop from an empty queue it should return null pointer.
lease = backlog.pop(op_type);
lease = boost::dynamic_pointer_cast<Lease4>(backlog.pop(op_type));
EXPECT_FALSE(lease);
EXPECT_TRUE(backlog.wasOverflown());
@@ -66,7 +67,7 @@ TEST(LeaseUpdateBacklogTest, pushAndPop) {
// This test verifies that all lease updates can be removed.
TEST(LeaseUpdateBacklogTest, clear) {
// Create the queue with limit of 5 lease updates.
Lease4UpdateBacklog backlog(5);
LeaseUpdateBacklog backlog(5);
// Add 5 lease updates.
for (auto i = 0; i < 3; ++i) {
@@ -74,7 +75,7 @@ TEST(LeaseUpdateBacklogTest, clear) {
HWAddrPtr hwaddr = boost::make_shared<HWAddr>(std::vector<uint8_t>(6, static_cast<uint8_t>(i)),
HTYPE_ETHER);
Lease4Ptr lease = boost::make_shared<Lease4>(address, hwaddr, ClientIdPtr(), 60, 0, 1);
ASSERT_TRUE(backlog.push(Lease4UpdateBacklog::ADD, lease));
ASSERT_TRUE(backlog.push(LeaseUpdateBacklog::ADD, lease));
}
// Make sure all lease updates have been added.