[#1402] Improved comments in the code

... per review comments.

diff --git a/src/hooks/dhcp/high_availability/ha_config.h b/src/hooks/dhcp/high_availability/ha_config.h
index 113dcb07a8b27504e3325fd490ed600d7fa328e5..04f9765ca14a258e34f189004136e2859f180e2a 100644 (file)
--- a/src/hooks/dhcp/high_availability/ha_config.h
+++ b/src/hooks/dhcp/high_availability/ha_config.h
@@ -386,7 +386,7 @@ public:
     /// unsent in the communication-recovery state.
     ///
     /// If the server is in the communication-recovery state it is unable to
-    /// send lease updates to the partner. Instead it keeps lease updates
+    /// send lease updates to the partner. Instead it keeps lease updates,
     /// hoping to send them when the communication is resumed. This value
     /// designates a limit of how many such updates can be held. If this
     /// number is exceeded the server continues to respond to the clients
@@ -402,7 +402,7 @@ public:
     /// unsent in the communication-recovery state.
     ///
     /// If the server is in the communication-recovery state it is unable to
-    /// send lease updates to the partner. Instead it keeps lease updates
+    /// send lease updates to the partner. Instead it keeps lease updates,
     /// hoping to send them when the communication is resumed. This value
     /// designates a limit of how many such updates can be held. If this
     /// number is exceeded the server continues to respond to the clients

diff --git a/src/hooks/dhcp/high_availability/ha_service.cc b/src/hooks/dhcp/high_availability/ha_service.cc
index 619d18a4f64e76fe9a3b9b426f0fa4ea6bf82a4f..05e50b5b0740bb6949a072f244ad96b5405f2f65 100644 (file)
--- a/src/hooks/dhcp/high_availability/ha_service.cc
+++ b/src/hooks/dhcp/high_availability/ha_service.cc
@@ -82,7 +82,7 @@ HAService::defineEvents() {
     defineEvent(HA_MAINTENANCE_NOTIFY_EVT, "HA_MAINTENANCE_NOTIFY_EVT");
     defineEvent(HA_MAINTENANCE_START_EVT, "HA_MAINTENANCE_START_EVT");
     defineEvent(HA_MAINTENANCE_CANCEL_EVT, "HA_MAINTENANCE_CANCEL_EVT");
- }
+}
 
 void
 HAService::verifyEvents() {
@@ -986,6 +986,9 @@ HAService::asyncSendLeaseUpdates(const dhcp::Pkt4Ptr& query,
     for (auto p = peers_configs.begin(); p != peers_configs.end(); ++p) {
         HAConfig::PeerConfigPtr conf = p->second;
 
+        // Check if the lease updates should be queued. This is the case when the
+        // server is in the communication-recovery state. Queued lease updates may
+        // be sent when the communication is re-established.
         if (shouldQueueLeaseUpdates(conf)) {
             // Lease updates for deleted leases.
             for (auto l = deleted_leases->begin(); l != deleted_leases->end(); ++l) {
@@ -1044,6 +1047,9 @@ HAService::asyncSendLeaseUpdates(const dhcp::Pkt6Ptr& query,
     for (auto p = peers_configs.begin(); p != peers_configs.end(); ++p) {
         HAConfig::PeerConfigPtr conf = p->second;
 
+        // Check if the lease updates should be queued. This is the case when the
+        // server is in the communication-recovery state. Queued lease updates may
+        // 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);
@@ -2120,6 +2126,11 @@ HAService::asyncSendLeaseUpdatesFromBacklog(HttpClient& http_client,
                  }
              }
 
+             // Recursively send all outstanding lease updates or break when an
+             // error occurs. In DHCPv6, this is a single iteration because we use
+             // lease6-bulk-apply, which combines many lease updates in a single
+             // transaction. In the case of DHCPv4, each update is sent in its own
+             // transaction.
              if (error_message.empty()) {
                  asyncSendLeaseUpdatesFromBacklog(http_client, config, post_request_action);
              } else {

diff --git a/src/hooks/dhcp/high_availability/ha_service.h b/src/hooks/dhcp/high_availability/ha_service.h
index 2be62dff9cb8e49303dbe41afb3e3190f5a04705..887cf8892d9b1905915293b973b62c698fafa41c 100644 (file)
--- a/src/hooks/dhcp/high_availability/ha_service.h
+++ b/src/hooks/dhcp/high_availability/ha_service.h
@@ -141,9 +141,12 @@ public:
     /// tries to send them in bulk when it returns to the
     /// load-balancing state.
     ///
-    /// A server running in the hot-standby mode never enters this
-    /// state. In this mode, even a short communication failure may
-    /// cause the primary server to transition to the partner-down
+    /// Transitioning into this state is only enabled when delayed-updates-limit
+    /// is non-zero.
+    ///
+    /// A server running in the hot-standby mode is never allowed to
+    /// enter this state. In this mode, even a short communication failure
+    /// may cause the primary server to transition to the partner-down
     /// state. Consequently, two servers would be responding to
     /// DHCP queries, possibly allocating the same addresses to two
     /// different clients. This doesn't occur in load-balancing mode
@@ -813,6 +816,9 @@ protected:
     /// It instructs the server to disable the DHCP service on the HA peer,
     /// fetch all leases from the peer and update the local lease database.
     ///
+    /// This method creates its own instances of the HttpClient and IOService and
+    /// invokes IOService::run().
+    ///
     /// @param [out] status_message status message in textual form.
     /// @param server_name name of the server to fetch leases from.
     /// @param max_period maximum number of seconds to disable DHCP service
@@ -854,6 +860,9 @@ protected:
     /// operation in the load-balancing state. In order to prevent collisions
     /// between new allocations and oustanding updates this method is synchronous.
     ///
+    /// This method creates its own instances of the HttpClient and IOService and
+    /// invokes IOService::run().
+    ///
     /// @return boolean value indicating that the lease updates were delivered
     /// successfully (when true) or unsuccessfully (when false).
     bool sendLeaseUpdatesFromBacklog();
@@ -906,6 +915,9 @@ public:
     /// state and signal an error to the caller. If the partner is unavailable,
     /// this server will directly transition to the partner-down state.
     ///
+    /// This method creates its own instances of the HttpClient and IOService and
+    /// invokes IOService::run().
+    ///
     /// @return Pointer to the response to the ha-maintenance-start.
     data::ConstElementPtr processMaintenanceStart();