From 3f80f9439225bbcec76214a56a1a56afb86c064f Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Mon, 22 Oct 2018 10:11:24 +0200 Subject: [PATCH] [#78,!85] Sync page limit is now configurable. --- src/hooks/dhcp/high_availability/ha_config.cc | 6 +- src/hooks/dhcp/high_availability/ha_config.h | 18 ++ .../high_availability/ha_config_parser.cc | 5 + .../dhcp/high_availability/ha_service.cc | 33 ++-- src/hooks/dhcp/high_availability/ha_service.h | 4 +- .../tests/ha_config_unittest.cc | 3 + .../tests/ha_service_unittest.cc | 166 ++++++++++++++---- .../dhcp/high_availability/tests/ha_test.cc | 1 + 8 files changed, 186 insertions(+), 50 deletions(-) diff --git a/src/hooks/dhcp/high_availability/ha_config.cc b/src/hooks/dhcp/high_availability/ha_config.cc index b8b52c1f07..a91b448ece 100644 --- a/src/hooks/dhcp/high_availability/ha_config.cc +++ b/src/hooks/dhcp/high_availability/ha_config.cc @@ -139,9 +139,9 @@ HAConfig::StateMachineConfig::getStateConfig(const int state) { HAConfig::HAConfig() : this_server_name_(), ha_mode_(HOT_STANDBY), send_lease_updates_(true), - sync_leases_(true), sync_timeout_(60000), heartbeat_delay_(10000), - max_response_delay_(60000), max_ack_delay_(10000), max_unacked_clients_(10), - peers_(), state_machine_(new StateMachineConfig()) { + sync_leases_(true), sync_timeout_(60000), sync_page_limit_(10000), + heartbeat_delay_(10000), max_response_delay_(60000), max_ack_delay_(10000), + max_unacked_clients_(10), peers_(), state_machine_(new StateMachineConfig()) { } HAConfig::PeerConfigPtr diff --git a/src/hooks/dhcp/high_availability/ha_config.h b/src/hooks/dhcp/high_availability/ha_config.h index 51f6048115..ef48c8b671 100644 --- a/src/hooks/dhcp/high_availability/ha_config.h +++ b/src/hooks/dhcp/high_availability/ha_config.h @@ -348,6 +348,22 @@ public: sync_timeout_ = sync_timeout; } + /// @brief Returns page size for leases fetched from the partner + /// during database synchronization. + /// + /// @return Page limit. + uint32_t getSyncPageLimit() const { + return (sync_page_limit_); + } + + /// @brief Sets new page limit size for leases fetched from the partner + /// during database synchronization. + /// + /// @param sync_page_limit New page limit value. + void setSyncPageLimit(const uint32_t sync_page_limit) { + sync_page_limit_ = sync_page_limit; + } + /// @brief Returns heartbeat delay in milliseconds. /// /// This value indicates the delay in sending a heartbeat command after @@ -479,6 +495,8 @@ public: bool send_lease_updates_; ///< Send lease updates to partner? bool sync_leases_; ///< Synchronize databases on startup? uint32_t sync_timeout_; ///< Timeout for syncing lease database (ms) + uint32_t sync_page_limit_; ///< Page size limit while synchronizing + ///< leases. uint32_t heartbeat_delay_; ///< Heartbeat delay in milliseconds. uint32_t max_response_delay_; ///< Max delay in response to heartbeats. uint32_t max_ack_delay_; ///< Maximum DHCP message ack delay. diff --git a/src/hooks/dhcp/high_availability/ha_config_parser.cc b/src/hooks/dhcp/high_availability/ha_config_parser.cc index b7e41dccb8..465ede350f 100644 --- a/src/hooks/dhcp/high_availability/ha_config_parser.cc +++ b/src/hooks/dhcp/high_availability/ha_config_parser.cc @@ -23,6 +23,7 @@ const SimpleDefaults HA_CONFIG_DEFAULTS = { { "send-lease-updates", Element::boolean, "true" }, { "sync-leases", Element::boolean, "true" }, { "sync-timeout", Element::integer, "60000" }, + { "sync-page-limit", Element::integer, "10000" }, { "heartbeat-delay", Element::integer, "10000" }, { "max-response-delay", Element::integer, "60000" }, { "max-ack-delay", Element::integer, "10000" }, @@ -136,6 +137,10 @@ HAConfigParser::parseInternal(const HAConfigPtr& config_storage, uint32_t sync_timeout = getAndValidateInteger(c, "sync-timeout"); config_storage->setSyncTimeout(sync_timeout); + // Get 'sync-page-limit'. + uint32_t sync_page_limit = getAndValidateInteger(c, "sync-page-limit"); + config_storage->setSyncPageLimit(sync_page_limit); + // Get 'heartbeat-delay'. uint16_t heartbeat_delay = getAndValidateInteger(c, "heartbeat-delay"); config_storage->setHeartbeatDelay(heartbeat_delay); diff --git a/src/hooks/dhcp/high_availability/ha_service.cc b/src/hooks/dhcp/high_availability/ha_service.cc index f78ed9c89e..761f270842 100644 --- a/src/hooks/dhcp/high_availability/ha_service.cc +++ b/src/hooks/dhcp/high_availability/ha_service.cc @@ -359,7 +359,7 @@ HAService::syncingStateHandler() { std::string status_message; int sync_status = synchronize(status_message, config_->getFailoverPeerConfig()->getName(), - 60, 1024); + 60, config_->getSyncPageLimit()); // If the leases synchronization was successful, let's transition // to the ready state. @@ -1158,9 +1158,9 @@ HAService::localEnable() { } void -HAService::asyncSyncLeases(const uint32_t limit) { +HAService::asyncSyncLeases() { PostRequestCallback null_action; - asyncSyncLeases(client_, LeasePtr(), limit, null_action); + asyncSyncLeases(client_, LeasePtr(), config_->getSyncPageLimit(), null_action); } void @@ -1234,14 +1234,9 @@ HAService::asyncSyncLeases(http::HttpClient& http_client, // Iterate over the leases and update the database as appropriate. const auto& leases_element = leases->listValue(); - // If we haven't hit the last page. Set the last lease pointer so as - // it can be used as an input to the next leaseX-get-page command. - if (leases_element.size() >= limit) { - last_lease = boost::dynamic_pointer_cast(*leases_element.rbegin()); - } - for (auto l = leases_element.begin(); l != leases_element.end(); ++l) { try { + if (server_type_ == HAServerType::DHCPv4) { Lease4Ptr lease = Lease4::fromElement(*l); @@ -1262,6 +1257,13 @@ HAService::asyncSyncLeases(http::HttpClient& http_client, .arg(lease->subnet_id_); } + // If we're not on the last page and we're processing final lease on + // this page, let's record the lease as input to the next + // lease4-get-page command. + if ((leases_element.size() >= limit) && (l + 1 == leases_element.end())) { + last_lease = boost::dynamic_pointer_cast(lease); + } + } else { Lease6Ptr lease = Lease6::fromElement(*l); @@ -1282,6 +1284,14 @@ HAService::asyncSyncLeases(http::HttpClient& http_client, .arg(lease->addr_.toText()) .arg(lease->subnet_id_); } + + // If we're not on the last page and we're processing final lease on + // this page, let's record the lease as input to the next + // lease6-get-page command. + if ((leases_element.size() >= limit) && (l + 1 == leases_element.end())) { + last_lease = boost::dynamic_pointer_cast(lease); + } + } } catch (const std::exception& ex) { @@ -1305,6 +1315,8 @@ HAService::asyncSyncLeases(http::HttpClient& http_client, communication_state_->setPartnerState("unavailable"); } else if (last_lease) { + // This indicates that there are more leases to be fetched. + // Therefore, we have to send another leaseX-get-page command. asyncSyncLeases(http_client, last_lease, limit, post_sync_action); return; } @@ -1321,7 +1333,8 @@ ConstElementPtr HAService::processSynchronize(const std::string& server_name, const unsigned int max_period) { std::string answer_message; - int sync_status = synchronize(answer_message, server_name, max_period, 1024); + int sync_status = synchronize(answer_message, server_name, max_period, + config_->getSyncPageLimit()); return (createAnswer(sync_status, answer_message)); } diff --git a/src/hooks/dhcp/high_availability/ha_service.h b/src/hooks/dhcp/high_availability/ha_service.h index d9969c85cd..709c63967e 100644 --- a/src/hooks/dhcp/high_availability/ha_service.h +++ b/src/hooks/dhcp/high_availability/ha_service.h @@ -545,9 +545,7 @@ protected: /// remaining leases. /// /// This method variant uses default HTTP client for communication. - /// - /// @param limit Limit of leases on the page. - void asyncSyncLeases(const uint32_t limit = 1024); + void asyncSyncLeases(); /// @brief Asynchronously reads leases from a peer and updates local /// lease database using a provided client instance. diff --git a/src/hooks/dhcp/high_availability/tests/ha_config_unittest.cc b/src/hooks/dhcp/high_availability/tests/ha_config_unittest.cc index 37e1941fc2..c15c690151 100644 --- a/src/hooks/dhcp/high_availability/tests/ha_config_unittest.cc +++ b/src/hooks/dhcp/high_availability/tests/ha_config_unittest.cc @@ -68,6 +68,7 @@ TEST_F(HAConfigTest, configureLoadBalancing) { " \"send-lease-updates\": false," " \"sync-leases\": false," " \"sync-timeout\": 20000," + " \"sync-page-limit\": 3," " \"heartbeat-delay\": 8," " \"max-response-delay\": 11," " \"max-ack-delay\": 5," @@ -117,6 +118,7 @@ TEST_F(HAConfigTest, configureLoadBalancing) { EXPECT_FALSE(impl->getConfig()->amSendingLeaseUpdates()); EXPECT_FALSE(impl->getConfig()->amSyncingLeases()); EXPECT_EQ(20000, impl->getConfig()->getSyncTimeout()); + EXPECT_EQ(3, impl->getConfig()->getSyncPageLimit()); EXPECT_EQ(8, impl->getConfig()->getHeartbeatDelay()); EXPECT_EQ(11, impl->getConfig()->getMaxResponseDelay()); EXPECT_EQ(5, impl->getConfig()->getMaxAckDelay()); @@ -222,6 +224,7 @@ TEST_F(HAConfigTest, configureHotStandby) { EXPECT_TRUE(impl->getConfig()->amSendingLeaseUpdates()); EXPECT_TRUE(impl->getConfig()->amSyncingLeases()); EXPECT_EQ(60000, impl->getConfig()->getSyncTimeout()); + EXPECT_EQ(10000, impl->getConfig()->getSyncPageLimit()); EXPECT_EQ(10000, impl->getConfig()->getHeartbeatDelay()); EXPECT_EQ(10000, impl->getConfig()->getMaxAckDelay()); EXPECT_EQ(10, impl->getConfig()->getMaxUnackedClients()); diff --git a/src/hooks/dhcp/high_availability/tests/ha_service_unittest.cc b/src/hooks/dhcp/high_availability/tests/ha_service_unittest.cc index 95b660ce66..b57606c29e 100644 --- a/src/hooks/dhcp/high_availability/tests/ha_service_unittest.cc +++ b/src/hooks/dhcp/high_availability/tests/ha_service_unittest.cc @@ -209,7 +209,7 @@ public: TestHttpResponseCreator() : requests_(), control_result_(CONTROL_RESULT_SUCCESS), arguments_(), per_request_control_result_(), - per_request_arguments_() { + per_request_arguments_(), request_index_() { } /// @brief Removes all received requests. @@ -270,11 +270,23 @@ public: /// @brief Sets arguments to be included in the response to a particular /// command. /// + /// Some tests require that the returned arguments vary for the same + /// command sent multiple times. One example of such command is the + /// @c lease4-get-page command sent multiple times to fetch pages of + /// leases from the partner. This method facilitates such test scenario. + /// In order to set different arguments for consecutive requests this + /// method must be called multiple times. Each call results in adding + /// arguments to be returned when the command is issued multiple times. + /// /// @param command_name command name. /// @param arguments pointer to the arguments. void setArguments(const std::string& command_name, const ElementPtr& arguments) { - per_request_arguments_[command_name] = arguments; + per_request_arguments_[command_name].push_back(isc::data::copy(arguments)); + // Create request index for this command if it doesn't exist. + if (request_index_.count(command_name) == 0) { + request_index_[command_name] = 0; + } } /// @brief Create a new request. @@ -345,7 +357,17 @@ private: // Check if there are specific arguments to be returned for this // command. if (per_request_arguments_.count(command_name) > 0) { - arguments = per_request_arguments_[command_name]; + // For certain requests we may return different arguments for consecutive + // instances of the same command. The request_index_ tracks the current + // index of the arguments to be returned. + arguments = per_request_arguments_[command_name][request_index_[command_name]]; + + // If we have reached the last arguments do not increase the index. Simply + // continue returning the same arguments. + if (request_index_[command_name] + 1 < per_request_arguments_[command_name].size()) { + // Not the last arguments, increase the index. + ++request_index_[command_name]; + } } } } @@ -392,7 +414,10 @@ private: std::map per_request_control_result_; /// @brief Command specific response arguments. - std::map per_request_arguments_; + std::map > per_request_arguments_; + + /// @brief Index of the next request of the given type. + std::map request_index_; }; /// @brief Shared pointer to the @c TestHttpResponseCreator. @@ -509,9 +534,14 @@ public: generateTestLeases(leases4_); } - /// @brief Returns generated IPv4 leases in JSON format. - ConstElementPtr getTestLeases4AsJson() const { - return (getLeasesAsJson(leases4_)); + /// @brief Returns range of generated IPv4 leases in JSON format. + /// + /// @param first_index Index of the first lease to be returned. + /// @param last_index Index of the last lease to be returned. + ConstElementPtr getTestLeases4AsJson(const size_t first_index, + const size_t last_index) const { + return (getLeasesAsJson(std::vector(leases4_.begin() + first_index, + leases4_.begin() + last_index))); } /// @brief Generates IPv6 leases to be used by the tests. @@ -519,9 +549,82 @@ public: generateTestLeases(leases6_); } - /// @brief Returns generated IPv6 leases in JSON format. - ConstElementPtr getTestLeases6AsJson() const { - return (getLeasesAsJson(leases6_)); + /// @brief Returns range of generated IPv6 leases in JSON format. + /// + /// @param first_index Index of the first lease to be returned. + /// @param last_index Index of the last lease to be returned. + ConstElementPtr getTestLeases6AsJson(const size_t first_index, + const size_t last_index) const { + return (getLeasesAsJson(std::vector(leases6_.begin() + first_index, + leases6_.begin() + last_index))); + } + + /// @brief Configure server side to return IPv4 leases in 3-element + /// chunks. + /// + /// Leases are fetched in pages, so the lease4-get-page should be + /// sent multiple times. The server is configured to return leases + /// in 3-element chunks. Note that the HA configis set to ask for 3 + /// leases. + void createPagedSyncResponses4() { + ElementPtr response_arguments = Element::createMap(); + + // First, return leases with indexes from 0 to 2. + response_arguments->set("leases", getTestLeases4AsJson(0, 3)); + factory2_->getResponseCreator()->setArguments("lease4-get-page", response_arguments); + factory3_->getResponseCreator()->setArguments("lease4-get-page", response_arguments); + + // Next, return leases with indexes from 3 to 5. + response_arguments->set("leases", getTestLeases4AsJson(3, 6)); + factory2_->getResponseCreator()->setArguments("lease4-get-page", response_arguments); + factory3_->getResponseCreator()->setArguments("lease4-get-page", response_arguments); + + // Then, return leases with indexes from 6 to 8. + response_arguments->set("leases", getTestLeases4AsJson(6, 9)); + factory2_->getResponseCreator()->setArguments("lease4-get-page", response_arguments); + factory3_->getResponseCreator()->setArguments("lease4-get-page", response_arguments); + + // Finally, there is one lease with index 9 to be returned. + // When the server requests a page of 3 leases and gets 1 it + // means that the last page was returned. At this point, the + // server ends synchronization. + response_arguments->set("leases", getTestLeases4AsJson(9, 10)); + factory2_->getResponseCreator()->setArguments("lease4-get-page", response_arguments); + factory3_->getResponseCreator()->setArguments("lease4-get-page", response_arguments); + } + + /// @brief Configure server side to return IPv6 leases in 3-element + /// chunks. + /// + /// Leases are fetched in pages, so the lease6-get-page should be + /// sent multiple times. The server is configured to return leases + /// in 3-element chunks. Note that the HA configis set to ask for 3 + /// leases. + void createPagedSyncResponses6() { + ElementPtr response_arguments = Element::createMap(); + + // First, return leases with indexes from 0 to 2. + response_arguments->set("leases", getTestLeases6AsJson(0, 3)); + factory2_->getResponseCreator()->setArguments("lease6-get-page", response_arguments); + factory3_->getResponseCreator()->setArguments("lease6-get-page", response_arguments); + + // Next, return leases with indexes from 3 to 5. + response_arguments->set("leases", getTestLeases6AsJson(3, 6)); + factory2_->getResponseCreator()->setArguments("lease6-get-page", response_arguments); + factory3_->getResponseCreator()->setArguments("lease6-get-page", response_arguments); + + // Then, return leases with indexes from 6 to 8. + response_arguments->set("leases", getTestLeases6AsJson(6, 9)); + factory2_->getResponseCreator()->setArguments("lease6-get-page", response_arguments); + factory3_->getResponseCreator()->setArguments("lease6-get-page", response_arguments); + + // Finally, there is one lease with index 9 to be returned. + // When the server requests a page of 3 leases and gets 1 it + // means that the last page was returned. At this point, the + // server ends synchronization. + response_arguments->set("leases", getTestLeases6AsJson(9, 10)); + factory2_->getResponseCreator()->setArguments("lease6-get-page", response_arguments); + factory3_->getResponseCreator()->setArguments("lease6-get-page", response_arguments); } /// @brief Tests scenarios when lease updates are sent to a partner while @@ -780,14 +883,10 @@ public: // server 1. HAConfigPtr config_storage = createValidConfiguration(); - // Convert leases to the JSON format, the same as used by the lease_cmds - // hook library. Configure our test HTTP servers to return those - // leases in this format. - ElementPtr response_arguments = Element::createMap(); - response_arguments->set("leases", getTestLeases4AsJson()); - - factory2_->getResponseCreator()->setArguments("lease4-get-page", response_arguments); - factory3_->getResponseCreator()->setArguments("lease4-get-page", response_arguments); + // Leases are fetched in pages, so the lease4-get-page should be + // sent multiple times. The server is configured to return leases + // in 3-element chunks. + createPagedSyncResponses4(); // Start the servers. ASSERT_NO_THROW({ @@ -832,14 +931,10 @@ public: // server 1. HAConfigPtr config_storage = createValidConfiguration(); - // Convert leases to the JSON format, the same as used by the lease_cmds - // hook library. Configure our test HTTP servers to return those - // leases in this format. - ElementPtr response_arguments = Element::createMap(); - response_arguments->set("leases", getTestLeases6AsJson()); - - factory2_->getResponseCreator()->setArguments("lease6-get-page", response_arguments); - factory3_->getResponseCreator()->setArguments("lease6-get-page", response_arguments); + // Leases are fetched in pages, so the lease6-get-page should be + // sent multiple times. The server is configured to return leases + // in 3-element chunks. + createPagedSyncResponses6(); // Start the servers. ASSERT_NO_THROW({ @@ -1610,10 +1705,11 @@ TEST_F(HAServiceTest, asyncSyncLeases) { // hook library. Configure our test HTTP servers to return those // leases in this format. ElementPtr response_arguments = Element::createMap(); - response_arguments->set("leases", getTestLeases4AsJson()); - factory2_->getResponseCreator()->setArguments(response_arguments); - factory3_->getResponseCreator()->setArguments(response_arguments); + // Leases are fetched in pages, so the lease4-get-page should be + // sent multiple times. The server is configured to return leases + // in 3-element chunks. + createPagedSyncResponses4(); // Start the servers. ASSERT_NO_THROW({ @@ -1635,7 +1731,7 @@ TEST_F(HAServiceTest, asyncSyncLeases) { // Stop running the IO service if we see a lease in the lease // database which is expected to be inserted as a result of lease // syncing. - return (!LeaseMgrFactory::instance().getLeases4(SubnetID(4)).empty()); + return (!LeaseMgrFactory::instance().getLeases4(SubnetID(10)).empty()); })); // Check if all leases have been stored in the local database. @@ -1783,10 +1879,11 @@ TEST_F(HAServiceTest, asyncSyncLeases6) { // hook library. Configure our test HTTP servers to return those // leases in this format. ElementPtr response_arguments = Element::createMap(); - response_arguments->set("leases", getTestLeases6AsJson()); - factory2_->getResponseCreator()->setArguments(response_arguments); - factory3_->getResponseCreator()->setArguments(response_arguments); + // Leases are fetched in pages, so the lease4-get-page should be + // sent multiple times. We need to configure the server side to + // return leases in 3-element chunks. + createPagedSyncResponses6(); // Start the servers. ASSERT_NO_THROW({ @@ -1809,7 +1906,7 @@ TEST_F(HAServiceTest, asyncSyncLeases6) { // Stop running the IO service if we see a lease in the lease // database which is expected to be inserted as a result of lease // syncing. - return (!LeaseMgrFactory::instance().getLeases6(SubnetID(4)).empty()); + return (!LeaseMgrFactory::instance().getLeases6(SubnetID(10)).empty()); })); // Check if all leases have been stored in the local database. @@ -2502,6 +2599,7 @@ public: void startService(const HAConfigPtr& config, const HAServerType& server_type = HAServerType::DHCPv4) { config->setHeartbeatDelay(1); + config->setSyncPageLimit(1000); service_.reset(new TestHAService(io_service_, network_state_, config, server_type)); // Replace default communication state with custom state which exposes diff --git a/src/hooks/dhcp/high_availability/tests/ha_test.cc b/src/hooks/dhcp/high_availability/tests/ha_test.cc index bba8a33d3b..e3895dba90 100644 --- a/src/hooks/dhcp/high_availability/tests/ha_test.cc +++ b/src/hooks/dhcp/high_availability/tests/ha_test.cc @@ -151,6 +151,7 @@ HATest::createValidJsonConfiguration(const HAConfig::HAMode& ha_mode) const { " \"this-server-name\": \"server1\"," " \"mode\": " << (ha_mode == HAConfig::LOAD_BALANCING ? "\"load-balancing\"" : "\"hot-standby\"") << "," + " \"sync-page-limit\": 3," " \"heartbeat-delay\": 1000," " \"max-response-delay\": 1000," " \"max-ack-delay\": 10000," -- 2.47.2