From: Marcin Siodelski Date: Fri, 19 Oct 2018 17:02:34 +0000 (+0200) Subject: [#78,!85] Use leaseX-get-page command instead of leaseX-get-all in HA sync. X-Git-Tag: 66-authoritative-flag-in-kea_base~14 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d557e4bcb724da983bc2f510cf452a7b80495e39;p=thirdparty%2Fkea.git [#78,!85] Use leaseX-get-page command instead of leaseX-get-all in HA sync. --- diff --git a/src/hooks/dhcp/high_availability/ha_service.cc b/src/hooks/dhcp/high_availability/ha_service.cc index 288f9d762a..f78ed9c89e 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); + 60, 1024); // If the leases synchronization was successful, let's transition // to the ready state. @@ -1158,13 +1158,15 @@ HAService::localEnable() { } void -HAService::asyncSyncLeases() { +HAService::asyncSyncLeases(const uint32_t limit) { PostRequestCallback null_action; - asyncSyncLeases(client_, null_action); + asyncSyncLeases(client_, LeasePtr(), limit, null_action); } void HAService::asyncSyncLeases(http::HttpClient& http_client, + const dhcp::LeasePtr& last_lease, + const uint32_t limit, const PostRequestCallback& post_sync_action) { HAConfig::PeerConfigPtr partner_config = config_->getFailoverPeerConfig(); @@ -1172,10 +1174,12 @@ HAService::asyncSyncLeases(http::HttpClient& http_client, PostHttpRequestJsonPtr request = boost::make_shared (HttpRequest::Method::HTTP_POST, "/", HttpVersion::HTTP_11()); if (server_type_ == HAServerType::DHCPv4) { - request->setBodyAsJson(CommandCreator::createLease4GetAll()); + request->setBodyAsJson(CommandCreator::createLease4GetPage( + boost::dynamic_pointer_cast(last_lease), limit)); } else { - request->setBodyAsJson(CommandCreator::createLease6GetAll()); + request->setBodyAsJson(CommandCreator::createLease6GetPage( + boost::dynamic_pointer_cast(last_lease), limit)); } request->finalize(); @@ -1185,11 +1189,15 @@ HAService::asyncSyncLeases(http::HttpClient& http_client, // Schedule asynchronous HTTP request. http_client.asyncSendRequest(partner_config->getUrl(), request, response, - [this, partner_config, post_sync_action] + [this, partner_config, post_sync_action, &http_client, limit] (const boost::system::error_code& ec, const HttpResponsePtr& response, const std::string& error_str) { + // Holds last lease received on the page of leases. If the last + // page was hit, this value remains null. + LeasePtr last_lease; + // There are three possible groups of errors during the heartneat. // One is the IO error causing issues in communication with the peer. // Another one is an HTTP parsing error. The last type of error is @@ -1225,6 +1233,13 @@ 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) { @@ -1288,6 +1303,10 @@ HAService::asyncSyncLeases(http::HttpClient& http_client, // partner as unavailable. if (!error_message.empty()) { communication_state_->setPartnerState("unavailable"); + + } else if (last_lease) { + asyncSyncLeases(http_client, last_lease, limit, post_sync_action); + return; } // Invoke post synchronization action if it was specified. @@ -1302,13 +1321,13 @@ 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); + int sync_status = synchronize(answer_message, server_name, max_period, 1024); return (createAnswer(sync_status, answer_message)); } int HAService::synchronize(std::string& status_message, const std::string& server_name, - const unsigned int max_period) { + const unsigned int max_period, const uint32_t page_limit) { IOService io_service; HttpClient client(io_service); @@ -1322,8 +1341,8 @@ HAService::synchronize(std::string& status_message, const std::string& server_na // If we have successfully disabled the DHCP service on the peer, // we can start fetching the leases. if (success) { - asyncSyncLeases(client, [&](const bool success, - const std::string& error_message) { + asyncSyncLeases(client, Lease4Ptr(), page_limit, + [&](const bool success, const std::string& error_message) { // If there was a fatal error while fetching the leases, let's // log an error message so as it can be included in the response // to the controlling client. diff --git a/src/hooks/dhcp/high_availability/ha_service.h b/src/hooks/dhcp/high_availability/ha_service.h index 58a2d6c83e..d9969c85cd 100644 --- a/src/hooks/dhcp/high_availability/ha_service.h +++ b/src/hooks/dhcp/high_availability/ha_service.h @@ -545,7 +545,9 @@ protected: /// remaining leases. /// /// This method variant uses default HTTP client for communication. - void asyncSyncLeases(); + /// + /// @param limit Limit of leases on the page. + void asyncSyncLeases(const uint32_t limit = 1024); /// @brief Asynchronously reads leases from a peer and updates local /// lease database using a provided client instance. @@ -563,10 +565,18 @@ protected: /// /// @param http_client reference to the client to be used to communicate /// with the other server. + /// @param lease Pointer to the last lease returned on the previous + /// page of leases. This lease is used to set the value of the "from" + /// parameter in the lease4-get-page and lease6-get-page commands. If this + /// command is sent to fetch the first page, the @c last_lease parameter + /// should be set to null. + /// @param limit Limit of leases on the page. /// @param post_sync_action pointer to the function to be executed when /// lease database synchronization is complete. If this is null, no /// post synchronization action is invoked. void asyncSyncLeases(http::HttpClient& http_client, + const dhcp::LeasePtr& last_lease, + const uint32_t limit, const PostRequestCallback& post_sync_action); public: @@ -605,11 +615,12 @@ protected: /// @param max_period maximum number of seconds to disable DHCP service /// of the peer. This value is used in dhcp-disable command issued to /// the peer before the lease4-get-all command. + /// @param page_limit Maximum size of a single page of leases to be returned. /// /// @return Synchronization result according to the status codes returned /// in responses to control commands. int synchronize(std::string& status_message, const std::string& server_name, - const unsigned int max_period); + const unsigned int max_period, const uint32_t page_limit); public: 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 6c54aaf4ce..ca8b6fffdc 100644 --- a/src/hooks/dhcp/high_availability/tests/ha_service_unittest.cc +++ b/src/hooks/dhcp/high_availability/tests/ha_service_unittest.cc @@ -786,8 +786,8 @@ public: ElementPtr response_arguments = Element::createMap(); response_arguments->set("leases", getTestLeases4AsJson()); - factory2_->getResponseCreator()->setArguments("lease4-get-all", response_arguments); - factory3_->getResponseCreator()->setArguments("lease4-get-all", response_arguments); + factory2_->getResponseCreator()->setArguments("lease4-get-page", response_arguments); + factory3_->getResponseCreator()->setArguments("lease4-get-page", response_arguments); // Start the servers. ASSERT_NO_THROW({ @@ -838,8 +838,8 @@ public: ElementPtr response_arguments = Element::createMap(); response_arguments->set("leases", getTestLeases6AsJson()); - factory2_->getResponseCreator()->setArguments("lease6-get-all", response_arguments); - factory3_->getResponseCreator()->setArguments("lease6-get-all", response_arguments); + factory2_->getResponseCreator()->setArguments("lease6-get-page", response_arguments); + factory3_->getResponseCreator()->setArguments("lease6-get-page", response_arguments); // Start the servers. ASSERT_NO_THROW({ @@ -1934,9 +1934,9 @@ TEST_F(HAServiceTest, processSynchronize4) { } // The following commands should have been sent to the server2: dhcp-disable, - // lease4-get-all and dhcp-enable. + // lease4-get-page and dhcp-enable. EXPECT_TRUE(factory2_->getResponseCreator()->findRequest("dhcp-disable","20")); - EXPECT_TRUE(factory2_->getResponseCreator()->findRequest("lease4-get-all","")); + EXPECT_TRUE(factory2_->getResponseCreator()->findRequest("lease4-get-page","")); EXPECT_TRUE(factory2_->getResponseCreator()->findRequest("dhcp-enable","")); } @@ -1958,15 +1958,15 @@ TEST_F(HAServiceTest, processSynchronizeDisableError) { // The server2 should only receive dhcp-disable commands. Remaining two should // not be sent. EXPECT_TRUE(factory2_->getResponseCreator()->findRequest("dhcp-disable","20")); - EXPECT_FALSE(factory2_->getResponseCreator()->findRequest("lease4-get-all","")); + EXPECT_FALSE(factory2_->getResponseCreator()->findRequest("lease4-get-page","")); EXPECT_FALSE(factory2_->getResponseCreator()->findRequest("dhcp-enable","")); } -// This test verifies that an error is reported when sending a lease4-get-all +// This test verifies that an error is reported when sending a lease4-get-page // command causes an error. TEST_F(HAServiceTest, processSynchronizeLease4GetAllError) { // Setup the server2 to return an error to dhcp-disable commands. - factory2_->getResponseCreator()->setControlResult("lease4-get-all", + factory2_->getResponseCreator()->setControlResult("lease4-get-page", CONTROL_RESULT_ERROR); // Run HAService::processSynchronize and gather a response. @@ -1980,7 +1980,7 @@ TEST_F(HAServiceTest, processSynchronizeLease4GetAllError) { // The server2 should receive all commands. The dhcp-disable was successful, so // the dhcp-enable command must be sent to re-enable the service after failure. EXPECT_TRUE(factory2_->getResponseCreator()->findRequest("dhcp-disable","20")); - EXPECT_TRUE(factory2_->getResponseCreator()->findRequest("lease4-get-all","")); + EXPECT_TRUE(factory2_->getResponseCreator()->findRequest("lease4-get-page","")); EXPECT_TRUE(factory2_->getResponseCreator()->findRequest("dhcp-enable","")); } @@ -2001,7 +2001,7 @@ TEST_F(HAServiceTest, processSynchronizeEnableError) { // The server2 should receive all commands. EXPECT_TRUE(factory2_->getResponseCreator()->findRequest("dhcp-disable","20")); - EXPECT_TRUE(factory2_->getResponseCreator()->findRequest("lease4-get-all","")); + EXPECT_TRUE(factory2_->getResponseCreator()->findRequest("lease4-get-page","")); EXPECT_TRUE(factory2_->getResponseCreator()->findRequest("dhcp-enable","")); } @@ -2027,9 +2027,9 @@ TEST_F(HAServiceTest, processSynchronize6) { } // The following commands should have been sent to the server2: dhcp-disable, - // lease6-get-all and dhcp-enable. + // lease6-get-page and dhcp-enable. EXPECT_TRUE(factory2_->getResponseCreator()->findRequest("dhcp-disable","20")); - EXPECT_TRUE(factory2_->getResponseCreator()->findRequest("lease6-get-all","")); + EXPECT_TRUE(factory2_->getResponseCreator()->findRequest("lease6-get-page","")); EXPECT_TRUE(factory2_->getResponseCreator()->findRequest("dhcp-enable","")); } @@ -2051,15 +2051,15 @@ TEST_F(HAServiceTest, processSynchronize6DisableError) { // The server2 should only receive dhcp-disable commands. Remaining two should // not be sent. EXPECT_TRUE(factory2_->getResponseCreator()->findRequest("dhcp-disable","20")); - EXPECT_FALSE(factory2_->getResponseCreator()->findRequest("lease6-get-all","")); + EXPECT_FALSE(factory2_->getResponseCreator()->findRequest("lease6-get-page","")); EXPECT_FALSE(factory2_->getResponseCreator()->findRequest("dhcp-enable","")); } -// This test verifies that an error is reported when sending a lease6-get-all +// This test verifies that an error is reported when sending a lease6-get-page // command causes an error. TEST_F(HAServiceTest, processSynchronizeLease6GetAllError) { // Setup the server2 to return an error to dhcp-disable commands. - factory2_->getResponseCreator()->setControlResult("lease6-get-all", + factory2_->getResponseCreator()->setControlResult("lease6-get-page", CONTROL_RESULT_ERROR); // Run HAService::processSynchronize and gather a response. @@ -2073,7 +2073,7 @@ TEST_F(HAServiceTest, processSynchronizeLease6GetAllError) { // The server2 should receive all commands. The dhcp-disable was successful, so // the dhcp-enable command must be sent to re-enable the service after failure. EXPECT_TRUE(factory2_->getResponseCreator()->findRequest("dhcp-disable","20")); - EXPECT_TRUE(factory2_->getResponseCreator()->findRequest("lease6-get-all","")); + EXPECT_TRUE(factory2_->getResponseCreator()->findRequest("lease6-get-page","")); EXPECT_TRUE(factory2_->getResponseCreator()->findRequest("dhcp-enable","")); } @@ -2094,7 +2094,7 @@ TEST_F(HAServiceTest, processSynchronize6EnableError) { // The server2 should receive all commands. EXPECT_TRUE(factory2_->getResponseCreator()->findRequest("dhcp-disable","20")); - EXPECT_TRUE(factory2_->getResponseCreator()->findRequest("lease6-get-all","")); + EXPECT_TRUE(factory2_->getResponseCreator()->findRequest("lease6-get-page","")); EXPECT_TRUE(factory2_->getResponseCreator()->findRequest("dhcp-enable","")); } @@ -2412,7 +2412,7 @@ public: /// @brief Enable response to commands required for leases synchronization. /// - /// Enables dhcp-disable, dhcp-enable and lease4-get-all commands. The last + /// Enables dhcp-disable, dhcp-enable and lease4-get-page commands. The last /// of them returns a bunch of test leases. void enableRespondLeaseFetching() { // Create IPv4 leases which will be fetched from the other server. @@ -2425,7 +2425,7 @@ public: ElementPtr response_arguments = Element::createMap(); response_arguments->set("leases", getLeasesAsJson(leases4)); - factory_->getResponseCreator()->setArguments("lease4-get-all", response_arguments); + factory_->getResponseCreator()->setArguments("lease4-get-page", response_arguments); } /// @brief Starts up the partner. @@ -3063,7 +3063,7 @@ TEST_F(HAServiceStateMachineTest, waitingSyncingReadyLoadBalancing) { // the partner's IO service in thread (in background). testSynchronousCommands([this, &partner]() { - // SYNCING state: the partner is up but it won't respond to the lease4-get-all + // SYNCING state: the partner is up but it won't respond to the lease4-get-page // command correctly. This should leave us in the SYNCING state until we finally // can synchronize. service_->runModel(HAService::NOP_EVT);