suboptimal retrieval of host reservation in the case of db backends
in void AllocEngine::findReservation(ClientContext[4|6]& ctx)
void
AllocEngine::findReservation(ClientContext[4|6]& ctx) {
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;
SharedNetwork[4|6]Ptr network;
subnet->getSharedNetwork(network);
if (subnet->getHostReservationMode() == Network::HR_GLOBAL) {
ConstHostPtr ghost = findGlobalReservation(ctx);
if (ghost) {
ctx.hosts_[SUBNET_ID_GLOBAL] = ghost;
// @todo In theory, to support global as part of HR_ALL,
// we would just keep going, instead of returning.
return;
}
}
// 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.
// As it breaks RADIUS use of host caching this can be disabled by the
// host manager.
const bool use_single_query = network &&
!HostMgr::instance().getDisableSingleQuery() &&
(network->getAllSubnets()->size() > ctx.host_identifiers_.size());
if (use_single_query) {
for (auto id_pair : ctx.host_identifiers_) {
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)->getIPv[4|6]SubnetID()) {
host_map[(*host)->getIPv[4|6]SubnetID()] = *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 and host reservations are enabled.
if (subnet->clientSupported(ctx.query_->getClasses()) &&
(subnet->getHostReservationMode() != Network::HR_DISABLED)) {
// Iterate over configured identifiers in the order of preference
// and try to use each of them to search for the reservations.
for (auto 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().get[4|6](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());
}
}vs
void
AllocEngine::findReservation(ClientContext[4|6]& ctx) {
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;
SharedNetwork[4|6]Ptr network;
subnet->getSharedNetwork(network);
if (subnet->getHostReservationMode() == Network::HR_GLOBAL) {
ConstHostPtr ghost = findGlobalReservation(ctx);
if (ghost) {
ctx.hosts_[SUBNET_ID_GLOBAL] = ghost;
// @todo In theory, to support global as part of HR_ALL,
// we would just keep going, instead of returning.
return;
}
}
// 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.
// As it breaks RADIUS use of host caching this can be disabled by the
// host manager.
***
// Can not call getAll[4|6] because RADIUS does not support it.
// While the performance is acceptable for RADIUS and CfgHosts, doing
// network->getAllSubnets()->size() * ctx.host_identifiers_.size() for
// db backends is suboptimal, as getAll[4|6] is supported.
// This results in
// network->getAllSubnets()->size() * ctx.host_identifiers_.size()
// roundtrips vs network->getAllSubnets()->size() number of roundtrips
// Should this function be implemented as a Host virtual function?
***
const bool use_single_query = network &&
!HostMgr::instance().getDisableSingleQuery() &&
(network->getAllSubnets()->size() > ctx.host_identifiers_.size());
if (use_single_query) {
for (auto id_pair : ctx.host_identifiers_) {
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)->getIPv[4|6]SubnetID()) {
host_map[(*host)->getIPv[4|6]SubnetID()] = *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 and host reservations are enabled.
if (subnet->clientSupported(ctx.query_->getClasses()) &&
(subnet->getHostReservationMode() != Network::HR_DISABLED)) {
// Iterate over configured identifiers in the order of preference
// and try to use each of them to search for the reservations.
ConstHostCollection subnet_hosts;
if (!use_single_query) {
subnet_hosts = HostMgr::instance().getAll[4|6](subnet->getID());
}
for (auto 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 {
ConstHostPtr host;
for (auto current : subnet_hosts) {
if (current->getIdentifierType() != id_pair.first) {
continue;
}
if (current->getIdentifier() != id_pair.second) {
continue;
}
host = current;
break;
}
// 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());
}
}Things to note:
- the following comment is misleading:
// 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.
// As it breaks RADIUS use of host caching this can be disabled by the
// host manager.- the following check is wrong, because it uses the slow path for db backends when using global subnets:
const bool use_single_query = network &&
!HostMgr::instance().getDisableSingleQuery() &&
(network->getAllSubnets()->size() > ctx.host_identifiers_.size());because of the RADIUS limitation, there are still network->getAllSubnets()->size() * ctx.host_identifiers_.size() calls. This is optimal for CfgHosts and it is a limitation for RADIUS, but it is very bad on db backends as it does a lot of roundtrips (slower than actual in RAM filtering by calling getAll[4|6] and keeping hosts with matching identifiers).
Because there is no easy way to optimize this, maybe this function can be implemented as a virtual function in each backend: CfgHosts and Radius can use current implementation by calling get[4|6] for each subnet and for each identifier type, and mysql, postgresql and cassandra can call getAll[4|6] for each subnet and filter the identifiers in ram (effectively lowering the number of roundtrips).
It can also be implemented by adding a flag just like in case of RADIUS, 'getDisableSingleQuery', like 'getAllQueryEnabled', but , by using this approach, there is no way RADIUS can function with a db backend in an optimal way.
Edited by Razvan Becheriu