fix: N+1 node queries in dhcp, api and websocket handlers#439
Open
xtrusia wants to merge 5 commits into
Open
Conversation
Collaborator
|
Check where you would like a Mattermost message to be sent to when CI completes and this PR is merged
|
xtrusia
force-pushed
the
fix/node-query-n-plus-one
branch
from
7d6b12c to
0d0e9ce
Compare
make_hosts_for_subnets iterated sip.interface_set.order_by("id") for
every allocated IP and then read each interface's node config, node
and parents -- several database queries per host, i.e. O(number of
hosts) round-trips (~18000 for a subnet with 6000 static IPs).
The whole configuration is regenerated on every change and runs in a
database thread, so this scales badly on large subnets.
Move the ordering into a Prefetch and load node_config/node and
parents alongside, so the query count stays constant regardless of
host count.
Signed-off-by: Seyeong Kim <seyeong.kim@canonical.com>
xtrusia
force-pushed
the
fix/node-query-n-plus-one
branch
from
0d0e9ce to
8fdaf87
Compare
IPAddressesHandler.read serialized each IP's interface_set, whose system_id/resource_uri call Interface.get_node() (node_config.node), so it ran a query per IP against maasserver_nodeconfig and maasserver_node. interface_set is a M2M, so those relations are not covered by a plain prefetch and must be named explicitly. Prefetch interface_set with the node selected in the same query so the node query count stays constant regardless of the number of IPs returned. Signed-off-by: Seyeong Kim <seyeong.kim@canonical.com>
Fabric.delete lists the still-connected interfaces via get_log_string(), which calls get_node() per interface. select_related node_config__node so building that error message does not query the node per interface. Signed-off-by: Seyeong Kim <seyeong.kim@canonical.com>
The vmcluster handler reads vm.machine (system_id/hostname) for each
tracked VM, querying the node per VM. select_related("machine") in
tracked_virtual_machines so the query count does not grow with the
number of VMs.
Signed-off-by: Seyeong Kim <seyeong.kim@canonical.com>
Pod.host calls hints.nodes.first(), which queried the node once per pod in the handler's list. Prefetch hints__nodes ordered by id in the handler queryset so first() is served from the cache. Signed-off-by: Seyeong Kim <seyeong.kim@canonical.com>
xtrusia
force-pushed
the
fix/node-query-n-plus-one
branch
from
8fdaf87 to
239efcc
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On large subnets several code paths run one or more node queries per object.
On a /19 subnet with about 6000 allocated IPs, one DHCP configuration rebuild runs about 18000 queries.
The ipaddresses API read has the same problem.
On one deployment at this scale, database statistics showed billions of reads on maasserver_nodeconfig and maasserver_node.
The same deployment had slow API responses and deploy timeouts.
The problem is worse on versions that also have LP:2130237.
That bug makes the region unresponsive during rack registration, so rack controllers time out and reconnect.
Reconnects trigger new DHCP configuration pushes.
On a large subnet every push runs the N+1 queries again on the same database thread pool.
Each commit fixes one path and adds a regression test for it.
Each path now loads the node together with the main query, so the query count stays constant when the object count grows.
The tests check that the query count does not grow with the number of objects.
The prefetches keep the related rows in memory and use an IN clause with one entry per IP, the normal prefetch_related tradeoff.
We plan to backport these fixes to the stable branches down to 3.4, where the same code paths run in production.