add Associations endpoints change delay timer
When multiple associations that point to the same interface are
created, each change (adding or removing one) leads to updating
"endpoints" property on dbus. This property update is time consuming
with many endpoints already present, because each update needs to send
the whole list of current elements plus/minus one. With a lot of
changes in short time it can cause the service to be unresponsive.
This change adds timer to delay updating dbus property. This way many
associations updates can be aggregated into single dbus property
update.
Tested: Ran on hardware with dbus sensor tester. 4000 created sensors
with interfaces are processed within 10 seconds. Time before the change
was above 2 minutes.
Signed-off-by: Kallas, Pawel <pawel.kallas@intel.com>
Change-Id: I1083c027ab12238249cffc67fb29a8ffef6baf83
diff --git a/src/associations.cpp b/src/associations.cpp
index 6dbc710..346cebd 100644
--- a/src/associations.cpp
+++ b/src/associations.cpp
@@ -1,11 +1,81 @@
#include "associations.hpp"
+#include <boost/asio/steady_timer.hpp>
#include <sdbusplus/exception.hpp>
#include <iostream>
#include <string>
-void removeAssociation(const std::string& sourcePath, const std::string& owner,
+void updateEndpointsOnDbus(sdbusplus::asio::object_server& objectServer,
+ const std::string& assocPath,
+ AssociationMaps& assocMaps)
+{
+ auto& iface = assocMaps.ifaces[assocPath];
+ auto& i = std::get<ifacePos>(iface);
+ auto& endpoints = std::get<endpointsPos>(iface);
+
+ // If the interface already exists, only need to update
+ // the property value, otherwise create it
+ if (i)
+ {
+ if (endpoints.empty())
+ {
+ objectServer.remove_interface(i);
+ i = nullptr;
+ }
+ else
+ {
+ i->set_property("endpoints", endpoints);
+ }
+ }
+ else
+ {
+ if (!endpoints.empty())
+ {
+ i = objectServer.add_interface(assocPath, xyzAssociationInterface);
+ i->register_property("endpoints", endpoints);
+ i->initialize();
+ }
+ }
+}
+
+void scheduleUpdateEndpointsOnDbus(boost::asio::io_context& io,
+ sdbusplus::asio::object_server& objectServer,
+ const std::string& assocPath,
+ AssociationMaps& assocMaps)
+{
+ static std::set<std::string> delayedUpdatePaths;
+
+ if (delayedUpdatePaths.contains(assocPath))
+ {
+ return;
+ }
+
+ auto& iface = assocMaps.ifaces[assocPath];
+ auto& endpoints = std::get<endpointsPos>(iface);
+
+ if (endpoints.size() > endpointsCountTimerThreshold)
+ {
+ delayedUpdatePaths.emplace(assocPath);
+ auto timer = std::make_shared<boost::asio::steady_timer>(
+ io, std::chrono::seconds(endpointUpdateDelaySeconds));
+ timer->async_wait([&objectServer, &assocMaps, timer,
+ assocPath](const boost::system::error_code& ec) {
+ if (!ec)
+ {
+ updateEndpointsOnDbus(objectServer, assocPath, assocMaps);
+ }
+ delayedUpdatePaths.erase(assocPath);
+ });
+ }
+ else
+ {
+ updateEndpointsOnDbus(objectServer, assocPath, assocMaps);
+ }
+}
+
+void removeAssociation(boost::asio::io_context& io,
+ const std::string& sourcePath, const std::string& owner,
sdbusplus::asio::object_server& server,
AssociationMaps& assocMaps)
{
@@ -35,7 +105,7 @@
for (const auto& [assocPath, endpointsToRemove] : assocs->second)
{
- removeAssociationEndpoints(server, assocPath, endpointsToRemove,
+ removeAssociationEndpoints(io, server, assocPath, endpointsToRemove,
assocMaps);
}
@@ -52,7 +122,8 @@
}
void removeAssociationEndpoints(
- sdbusplus::asio::object_server& objectServer, const std::string& assocPath,
+ boost::asio::io_context& io, sdbusplus::asio::object_server& objectServer,
+ const std::string& assocPath,
const boost::container::flat_set<std::string>& endpointsToRemove,
AssociationMaps& assocMaps)
{
@@ -75,22 +146,12 @@
}
}
- if (endpointsInDBus.empty())
- {
- objectServer.remove_interface(std::get<ifacePos>(assoc->second));
- std::get<ifacePos>(assoc->second) = nullptr;
- std::get<endpointsPos>(assoc->second).clear();
- }
- else
- {
- std::get<ifacePos>(assoc->second)
- ->set_property("endpoints", endpointsInDBus);
- }
+ scheduleUpdateEndpointsOnDbus(io, objectServer, assocPath, assocMaps);
}
void checkAssociationEndpointRemoves(
- const std::string& sourcePath, const std::string& owner,
- const AssociationPaths& newAssociations,
+ boost::asio::io_context& io, const std::string& sourcePath,
+ const std::string& owner, const AssociationPaths& newAssociations,
sdbusplus::asio::object_server& objectServer, AssociationMaps& assocMaps)
{
// Find the services that have associations on this path.
@@ -119,7 +180,7 @@
auto newEndpoints = newAssociations.find(originalAssocPath);
if (newEndpoints == newAssociations.end())
{
- removeAssociationEndpoints(objectServer, originalAssocPath,
+ removeAssociationEndpoints(io, objectServer, originalAssocPath,
originalEndpoints, assocMaps);
}
else
@@ -139,7 +200,7 @@
}
if (!toRemove.empty())
{
- removeAssociationEndpoints(objectServer, originalAssocPath,
+ removeAssociationEndpoints(io, objectServer, originalAssocPath,
toRemove, assocMaps);
}
}
@@ -147,12 +208,12 @@
}
void addEndpointsToAssocIfaces(
- sdbusplus::asio::object_server& objectServer, const std::string& assocPath,
+ boost::asio::io_context& io, sdbusplus::asio::object_server& objectServer,
+ const std::string& assocPath,
const boost::container::flat_set<std::string>& endpointPaths,
AssociationMaps& assocMaps)
{
auto& iface = assocMaps.ifaces[assocPath];
- auto& i = std::get<ifacePos>(iface);
auto& endpoints = std::get<endpointsPos>(iface);
// Only add new endpoints
@@ -163,22 +224,11 @@
endpoints.push_back(e);
}
}
-
- // If the interface already exists, only need to update
- // the property value, otherwise create it
- if (i)
- {
- i->set_property("endpoints", endpoints);
- }
- else
- {
- i = objectServer.add_interface(assocPath, xyzAssociationInterface);
- i->register_property("endpoints", endpoints);
- i->initialize();
- }
+ scheduleUpdateEndpointsOnDbus(io, objectServer, assocPath, assocMaps);
}
-void associationChanged(sdbusplus::asio::object_server& objectServer,
+void associationChanged(boost::asio::io_context& io,
+ sdbusplus::asio::object_server& objectServer,
const std::vector<Association>& associations,
const std::string& path, const std::string& owner,
const InterfaceMapType& interfaceMap,
@@ -218,12 +268,12 @@
}
for (const auto& object : objects)
{
- addEndpointsToAssocIfaces(objectServer, object.first, object.second,
+ addEndpointsToAssocIfaces(io, objectServer, object.first, object.second,
assocMaps);
}
// Check for endpoints being removed instead of added
- checkAssociationEndpointRemoves(path, owner, objects, objectServer,
+ checkAssociationEndpointRemoves(io, path, owner, objects, objectServer,
assocMaps);
if (!objects.empty())
@@ -313,7 +363,8 @@
}
}
-void addSingleAssociation(sdbusplus::asio::object_server& server,
+void addSingleAssociation(boost::asio::io_context& io,
+ sdbusplus::asio::object_server& server,
const std::string& assocPath,
const std::string& endpoint, const std::string& owner,
const std::string& ownerPath,
@@ -321,7 +372,7 @@
{
boost::container::flat_set<std::string> endpoints{endpoint};
- addEndpointsToAssocIfaces(server, assocPath, endpoints, assocMaps);
+ addEndpointsToAssocIfaces(io, server, assocPath, endpoints, assocMaps);
AssociationPaths objects;
boost::container::flat_set e{endpoint};
@@ -356,7 +407,8 @@
}
}
-void checkIfPendingAssociation(const std::string& objectPath,
+void checkIfPendingAssociation(boost::asio::io_context& io,
+ const std::string& objectPath,
const InterfaceMapType& interfaceMap,
AssociationMaps& assocMaps,
sdbusplus::asio::object_server& server)
@@ -400,13 +452,13 @@
try
{
- addSingleAssociation(server, assocPath, endpointPath, owner,
+ addSingleAssociation(io, server, assocPath, endpointPath, owner,
ownerPath, assocMaps);
// Now the reverse direction (still the same owner and ownerPath)
assocPath = endpointPath + '/' + std::get<reverseTypePos>(e);
endpointPath = objectPath;
- addSingleAssociation(server, assocPath, endpointPath, owner,
+ addSingleAssociation(io, server, assocPath, endpointPath, owner,
ownerPath, assocMaps);
}
catch (const sdbusplus::exception_t& e)
@@ -494,7 +546,8 @@
* @param[in,out] assocMaps - the association maps
* @param[in,out] server - sdbus system object
*/
-void removeAssociationIfacesEntry(const std::string& assocPath,
+void removeAssociationIfacesEntry(boost::asio::io_context& io,
+ const std::string& assocPath,
const std::string& endpointPath,
AssociationMaps& assocMaps,
sdbusplus::asio::object_server& server)
@@ -508,16 +561,7 @@
{
endpoints.erase(e);
- if (endpoints.empty())
- {
- server.remove_interface(std::get<ifacePos>(assoc->second));
- std::get<ifacePos>(assoc->second) = nullptr;
- }
- else
- {
- std::get<ifacePos>(assoc->second)
- ->set_property("endpoints", endpoints);
- }
+ scheduleUpdateEndpointsOnDbus(io, server, assocPath, assocMaps);
}
}
}
@@ -574,7 +618,8 @@
}
}
-void moveAssociationToPending(const std::string& endpointPath,
+void moveAssociationToPending(boost::asio::io_context& io,
+ const std::string& endpointPath,
AssociationMaps& assocMaps,
sdbusplus::asio::object_server& server)
{
@@ -596,9 +641,9 @@
reverseType, owner, assocMaps);
// Remove both sides of the association from assocMaps.ifaces
- removeAssociationIfacesEntry(forwardPath + '/' + forwardType,
+ removeAssociationIfacesEntry(io, forwardPath + '/' + forwardType,
reversePath, assocMaps, server);
- removeAssociationIfacesEntry(reversePath + '/' + reverseType,
+ removeAssociationIfacesEntry(io, reversePath + '/' + reverseType,
forwardPath, assocMaps, server);
// Remove both sides of the association from assocMaps.owners