]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[#78,!85] Use leaseX-get-page command instead of leaseX-get-all in HA sync.
authorMarcin Siodelski <marcin@isc.org>
Fri, 19 Oct 2018 17:02:34 +0000 (19:02 +0200)
committerMarcin Siodelski <marcin@isc.org>
Mon, 5 Nov 2018 18:38:41 +0000 (13:38 -0500)
src/hooks/dhcp/high_availability/ha_service.cc
src/hooks/dhcp/high_availability/ha_service.h
src/hooks/dhcp/high_availability/tests/ha_service_unittest.cc

index 288f9d762ac03b3b1c6b668404933e063950d234..f78ed9c89ee9e3f76edf916e2a1433efc4304b05 100644 (file)
@@ -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<PostHttpRequestJson>
         (HttpRequest::Method::HTTP_POST, "/", HttpVersion::HTTP_11());
     if (server_type_ == HAServerType::DHCPv4) {
-        request->setBodyAsJson(CommandCreator::createLease4GetAll());
+        request->setBodyAsJson(CommandCreator::createLease4GetPage(
+            boost::dynamic_pointer_cast<Lease4>(last_lease), limit));
 
     } else {
-        request->setBodyAsJson(CommandCreator::createLease6GetAll());
+        request->setBodyAsJson(CommandCreator::createLease6GetPage(
+            boost::dynamic_pointer_cast<Lease6>(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<Lease>(*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.
index 58a2d6c83e8880565411413608d8d6940a68eecd..d9969c85cda468cf48bb5c981315ef38e29f56e2 100644 (file)
@@ -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:
 
index 6c54aaf4ce31b75cdbc34b576e8ab45a73987f53..ca8b6fffdc01a9d75ac28c710e9d18727d8e1f88 100644 (file)
@@ -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);