[5437] Optimized the number of queries for host reservations.

diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc
index 301b8af6f2caf11715408d5c03178fca9262f0ed..24795c429cc1c77d5464c96b3228a6e3ed28b06d 100644 (file)
--- a/src/lib/dhcpsrv/alloc_engine.cc
+++ b/src/lib/dhcpsrv/alloc_engine.cc
@@ -372,49 +372,6 @@ AllocEngine::AllocatorPtr AllocEngine::getAllocator(Lease::Type type) {
     return (alloc->second);
 }
 
-template<typename ContextType>
-void
-AllocEngine::findReservationInternal(ContextType& ctx,
-                                     const AllocEngine::HostGetFunc& host_get,
-                                     const bool ipv6_only) {
-    ctx.hosts_.clear();
-
-    auto subnet = ctx.subnet_;
-
-    // We can only search for the reservation if a subnet has been selected.
-    while (subnet) {
-
-        // Only makes sense to get reservations if the client has access
-        // to the class.
-        if (subnet->clientSupported(ctx.query_->getClasses())) {
-            // Iterate over configured identifiers in the order of preference
-            // and try to use each of them to search for the reservations.
-            BOOST_FOREACH(const IdentifierPair& id_pair, ctx.host_identifiers_) {
-                // Attempt to find a host using a specified identifier.
-                ConstHostPtr host = host_get(subnet->getID(), id_pair.first,
-                                             &id_pair.second[0], id_pair.second.size());
-                // If we found matching host for this subnet.
-                if (host && (!ipv6_only || host->hasIPv6Reservation())) {
-                    ctx.hosts_[subnet->getID()] = host;
-                    break;
-                }
-            }
-
-        } else {
-            LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE,
-                      ALLOC_ENGINE_RESERVATIONS_SKIPPED)
-                .arg(ctx.query_->getLabel())
-                .arg(subnet->toText());
-
-        }
-
-        // We need to get to the next subnet if this is a shared network. If it
-        // is not (a plain subnet), getNextSubnet will return NULL and we're
-        // done here.
-        subnet = subnet->getNextSubnet(ctx.subnet_, ctx.query_->getClasses());
-    }
-}
-
 } // end of namespace isc::dhcp
 } // end of namespace isc
 
@@ -536,9 +493,80 @@ AllocEngine::ClientContext6::currentHost() const {
 }
 
 void AllocEngine::findReservation(ClientContext6& ctx) {
-    findReservationInternal(ctx, boost::bind(&HostMgr::get6,
-                                             &HostMgr::instance(),
-                                             _1, _2, _3, _4));
+    ctx.hosts_.clear();
+
+    // If there is no subnet, there is nothing to do.
+    if (!ctx.subnet_) {
+        return;
+    }
+
+    auto subnet = ctx.subnet_;
+
+    std::map<SubnetID, ConstHostPtr> host_map;
+    SharedNetwork6Ptr network;
+    subnet->getSharedNetwork(network);
+
+    // If the subnet belongs to a shared network it is usually going to be
+    // more efficient to make a query for all reservations for a particular
+    // client rather than a query for each subnet within this shared network.
+    // The only case when it is going to be less efficient is when there are
+    // more host identifier types in use than subnets within a shared network.
+    const bool use_single_query = network &&
+        (network->getAllSubnets()->size() > ctx.host_identifiers_.size());
+
+    if (use_single_query) {
+        for (auto id_pair = ctx.host_identifiers_.begin();
+             id_pair != ctx.host_identifiers_.end();
+             ++id_pair) {
+            ConstHostCollection hosts = HostMgr::instance().getAll(id_pair->first,
+                                                                   &id_pair->second[0],
+                                                                   id_pair->second.size());
+            // Store the hosts in the temporary map, because some hosts may
+            // belong to subnets outside of the shared network. We'll need
+            // to eliminate them.
+            for (auto host = hosts.begin(); host != hosts.end(); ++host) {
+                if ((*host)->getIPv6SubnetID()) {
+                    host_map[(*host)->getIPv6SubnetID()] = *host;
+                }
+            }
+        }
+    }
+
+    // We can only search for the reservation if a subnet has been selected.
+    while (subnet) {
+
+        // Only makes sense to get reservations if the client has access
+        // to the class.
+        if (subnet->clientSupported(ctx.query_->getClasses())) {
+            // Iterate over configured identifiers in the order of preference
+            // and try to use each of them to search for the reservations.
+            BOOST_FOREACH(const IdentifierPair& id_pair, ctx.host_identifiers_) {
+                if (use_single_query) {
+                    if (host_map.count(subnet->getID()) > 0) {
+                        ctx.hosts_[subnet->getID()] = host_map[subnet->getID()];
+                    }
+
+                } else {
+                    // Attempt to find a host using a specified identifier.
+                    ConstHostPtr host = HostMgr::instance().get6(subnet->getID(),
+                                                                 id_pair.first,
+                                                                 &id_pair.second[0],
+                                                                 id_pair.second.size());
+                    // If we found matching host for this subnet.
+                    if (host) {
+                        ctx.hosts_[subnet->getID()] = host;
+                        break;
+                    }
+                }
+            }
+
+        }
+
+        // We need to get to the next subnet if this is a shared network. If it
+        // is not (a plain subnet), getNextSubnet will return NULL and we're
+        // done here.
+        subnet = subnet->getNextSubnet(ctx.subnet_, ctx.query_->getClasses());
+    }
 }
 
 Lease6Collection
@@ -2732,9 +2760,80 @@ AllocEngine::allocateLease4(ClientContext4& ctx) {
 
 void
 AllocEngine::findReservation(ClientContext4& ctx) {
-    findReservationInternal(ctx, boost::bind(&HostMgr::get4,
-                                             &HostMgr::instance(),
-                                             _1, _2, _3, _4));
+    ctx.hosts_.clear();
+
+    // If there is no subnet, there is nothing to do.
+    if (!ctx.subnet_) {
+        return;
+    }
+
+    auto subnet = ctx.subnet_;
+
+    std::map<SubnetID, ConstHostPtr> host_map;
+    SharedNetwork4Ptr network;
+    subnet->getSharedNetwork(network);
+
+    // If the subnet belongs to a shared network it is usually going to be
+    // more efficient to make a query for all reservations for a particular
+    // client rather than a query for each subnet within this shared network.
+    // The only case when it is going to be less efficient is when there are
+    // more host identifier types in use than subnets within a shared network.
+    const bool use_single_query = network &&
+        (network->getAllSubnets()->size() > ctx.host_identifiers_.size());
+
+    if (use_single_query) {
+        for (auto id_pair = ctx.host_identifiers_.begin();
+             id_pair != ctx.host_identifiers_.end();
+             ++id_pair) {
+            ConstHostCollection hosts = HostMgr::instance().getAll(id_pair->first,
+                                                                   &id_pair->second[0],
+                                                                   id_pair->second.size());
+            // Store the hosts in the temporary map, because some hosts may
+            // belong to subnets outside of the shared network. We'll need
+            // to eliminate them.
+            for (auto host = hosts.begin(); host != hosts.end(); ++host) {
+                if ((*host)->getIPv4SubnetID() > 0) {
+                    host_map[(*host)->getIPv4SubnetID()] = *host;
+                }
+            }
+        }
+    }
+
+    // We can only search for the reservation if a subnet has been selected.
+    while (subnet) {
+
+        // Only makes sense to get reservations if the client has access
+        // to the class.
+        if (subnet->clientSupported(ctx.query_->getClasses())) {
+            // Iterate over configured identifiers in the order of preference
+            // and try to use each of them to search for the reservations.
+            BOOST_FOREACH(const IdentifierPair& id_pair, ctx.host_identifiers_) {
+                if (use_single_query) {
+                    if (host_map.count(subnet->getID()) > 0) {
+                        ctx.hosts_[subnet->getID()] = host_map[subnet->getID()];
+                        break;
+                    }
+
+                } else {
+                    // Attempt to find a host using a specified identifier.
+                    ConstHostPtr host = HostMgr::instance().get4(subnet->getID(),
+                                                                 id_pair.first,
+                                                                 &id_pair.second[0],
+                                                                 id_pair.second.size());
+                    // If we found matching host for this subnet.
+                    if (host) {
+                        ctx.hosts_[subnet->getID()] = host;
+                        break;
+                    }
+                }
+            }
+        }
+
+        // We need to get to the next subnet if this is a shared network. If it
+        // is not (a plain subnet), getNextSubnet will return NULL and we're
+        // done here.
+        subnet = subnet->getNextSubnet(ctx.subnet_, ctx.query_->getClasses());
+    }
 }
 
 Lease4Ptr

diff --git a/src/lib/dhcpsrv/alloc_engine.h b/src/lib/dhcpsrv/alloc_engine.h
index 7c3ee9882259d190d72f9f5779d245e6a0b9d953..cf89f669c084208408c6b3792e17909db8a8e6bc 100644 (file)
--- a/src/lib/dhcpsrv/alloc_engine.h
+++ b/src/lib/dhcpsrv/alloc_engine.h
@@ -746,28 +746,6 @@ public:
 
 private:
 
-    /// @brief Type of the function used by @ref findReservationInternal to
-    /// retrieve reservations by subnet identifier and host identifier.
-    typedef boost::function<ConstHostPtr(const SubnetID&,
-                                         const Host::IdentifierType&,
-                                         const uint8_t*, const size_t)> HostGetFunc;
-
-    /// @brief Common function for searching host reservations.
-    ///
-    /// This is a common function called by variants of @ref findReservation
-    /// functions.
-    ///
-    /// @param ctx Reference to a @ref ClientContext6 or @ref ClientContext4.
-    /// @param host_get Pointer to the @ref HostMgr functions to be used
-    /// to retrieve reservation by subnet identifier and host identifier.
-    /// @param ipv6_only Boolean value indicating if only IPv6 reservations
-    /// should be retrieved.
-    /// @tparam ContextType Either @ref ClientContext6 or @ref ClientContext4.
-    template<typename ContextType>
-    static void findReservationInternal(ContextType& ctx,
-                                        const HostGetFunc& host_get,
-                                        const bool ipv6_only = false);
-
     /// @brief creates a lease and inserts it in LeaseMgr if necessary
     ///
     /// Creates a lease based on specified parameters and tries to insert it

diff --git a/src/lib/dhcpsrv/alloc_engine_messages.mes b/src/lib/dhcpsrv/alloc_engine_messages.mes
index 27f633b00d2dbc7475a412bb8c0f233bb6eac757..fea8bac1182244ae02741cd52fc5da8ba447d60a 100644 (file)
--- a/src/lib/dhcpsrv/alloc_engine_messages.mes
+++ b/src/lib/dhcpsrv/alloc_engine_messages.mes
@@ -18,12 +18,6 @@ reclaimed has a corresponding DNS entry it needs to be removed.
 This message indicates that removal of the DNS entry has failed.
 Nevertheless the lease will be reclaimed.
 
-% ALLOC_ENGINE_RESERVATIONS_SKIPPED %1 not using reservations in subnet %2 because client's classes do not match
-This debug message is issued when the allocation engine will not use host
-reservations in a given subnet because this subnet is not allowed for
-the given client due to client classification. The first argument includes
-client identification information. The second argument identifies a subnet.
-
 % ALLOC_ENGINE_V4_ALLOC_ERROR %1: error during attempt to allocate an IPv4 address: %2
 An error occurred during an attempt to allocate an IPv4 address, the
 reason for the failure being contained in the message.  The server will