From 9dd8ba5700f6ef8070258dc7505d676ce19da726 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Kr=C3=B6ger?= Date: Mon, 14 Sep 2026 15:09:22 +0000 Subject: [PATCH 1/5] perf(serverdb): defer intern_ip in materializer prefetches prefetch_related() issues its own query built from Server._base_manager, so defer("server__intern_ip") on the outer queryset never reached it - that spelling only takes effect for select_related() traversals. Every prefetched Server therefore parsed intern_ip through netfields and ipaddress, which is pure Python and accounted for ~10% query time of profiled queries, even though intern_ip is not a real attribute and never appears in a restrict clause. Pass an explicit Prefetch queryset at the two sites where the prefetched Server is only used as a dict key. The reverse branch keeps the full row on purpose: sa.server is stored as an attribute value there, so it can escape into the results and reach _get_servers_to_join(), which reads intern_ip - deferring it would cost a query per object. This can be reverted once we removed intern_ip fully. --- .../serverdb/query_materializer.py | 45 +++++++++++++------ 1 file changed, 31 insertions(+), 14 deletions(-) diff --git a/packages/serveradmin/serveradmin/serverdb/query_materializer.py b/packages/serveradmin/serveradmin/serverdb/query_materializer.py index 6ad76a3b..e9ebf150 100644 --- a/packages/serveradmin/serveradmin/serverdb/query_materializer.py +++ b/packages/serveradmin/serveradmin/serverdb/query_materializer.py @@ -11,6 +11,9 @@ import logging from ipaddress import IPv4Address, IPv6Address + +from django.db.models import Prefetch + from adminapi.dataset import DatasetObject from serveradmin.serverdb.models import ( Servertype, @@ -24,6 +27,27 @@ logger = logging.getLogger(__package__) +def _server_prefetch(): + """Prefetch "server" without loading the expensive intern_ip column + + prefetch_related() issues its own query, built from Server._base_manager + (see ForwardManyToOneDescriptor.get_prefetch_querysets), so a + defer("server__intern_ip") on the outer queryset never reaches it - that + spelling only takes effect for select_related() traversals. Passing an + explicit queryset is the only way to actually defer the column here. + + It is worth deferring because netfields runs every inet value through + ipaddress.ip_interface(), which is pure Python and showed up as roughly a + quarter of the profile of a large query - even though intern_ip is not a + real attribute and is never part of a restrict clause. + + Only intern_ip is deferred. servertype_id is a plain varchar with no + converter, so deferring it saves nothing while risking a query per object + for anything that reads it. + """ + return Prefetch('server', queryset=Server._base_manager.defer('intern_ip')) + + class QueryMaterializer: def __init__(self, servers, joined_attributes, order_by_attributes=[]): self._servers = servers @@ -145,11 +169,12 @@ def _add_attributes(self, servers_by_type): value_id__in=self._server_attributes.keys(), attribute_id__in=reversed_attributes.keys(), ) + # Unlike the other branches, sa.server is stored as an + # attribute *value* here, so it can escape into the + # results and reach _get_servers_to_join(), which reads + # intern_ip. Deferring it would cost a query per object, + # so this one deliberately keeps the whole row. .prefetch_related("server") - .defer( - "server__intern_ip", - "server__servertype", - ) ): self._add_attribute_value( sa.value, @@ -164,11 +189,7 @@ def _add_attributes(self, servers_by_type): server__in=self._server_attributes.keys(), attribute__in=attributes, ) - .prefetch_related("server") - .defer( - "server__intern_ip", - "server__servertype", - ) + .prefetch_related(_server_prefetch()) ): self._add_attribute_value( sa.server, @@ -285,11 +306,7 @@ def _add_related_attribute(self, attribute, servertype_attribute, servers_by_typ server__hostname__in=servers_by_related.keys(), attribute=attribute, ) - .prefetch_related("server") - .defer( - "server__intern_ip", - "server__servertype", - ) + .prefetch_related(_server_prefetch()) ): for target in servers_by_related[sa.server]: self._add_attribute_value(target, attribute, sa.get_value()) From dc12eaccc3b731933c468e499746cea72451b24c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Kr=C3=B6ger?= Date: Mon, 14 Sep 2026 15:28:41 +0000 Subject: [PATCH 2/5] perf(serverdb): fetch supernet nets once instead of per row _add_supernet_attribute() read its join through Server.objects.raw(), which builds one model instance per result row. The join yields a row per (host, net) pair, so a large query created tens of thousands of Server objects - each parsing net.intern_ip through netfields - to represent far fewer distinct networks. Select only the ids, read them through a plain cursor, and fetch the distinct nets once. This trades two extra round trips for fewer model instantiations and the same number of inet parses which pays out especially on queries with many results. The nets are still fetched whole: they are stored as attribute values and can reach a nested QueryMaterializer via _get_servers_to_join(), which reads intern_ip. host_attr_name used to ride along on each per-row Server object and was read back off the previously stored supernet for the conflict warning. One net object is now shared by every host inside it, so it is tracked separately, keyed by host id. The two inet_address_family columns the query selected were never read by anything and are gone. --- .../serverdb/query_materializer.py | 55 ++++++++++++------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/packages/serveradmin/serveradmin/serverdb/query_materializer.py b/packages/serveradmin/serveradmin/serverdb/query_materializer.py index e9ebf150..2a3e5e51 100644 --- a/packages/serveradmin/serveradmin/serverdb/query_materializer.py +++ b/packages/serveradmin/serveradmin/serverdb/query_materializer.py @@ -12,6 +12,7 @@ from ipaddress import IPv4Address, IPv6Address +from django.db import connection from django.db.models import Prefetch from adminapi.dataset import DatasetObject @@ -225,14 +226,16 @@ def _add_supernet_attribute(self, attribute: Attribute, servers_in): the host's IP address or prefix fits within the net's IP address or prefix. """ + # Only ids are selected. This join yields one row per (host, net) + # pair - tens of thousands of them on a large query - while the + # distinct nets they point at number far fewer. Reading the net rows + # through Server.objects.raw() therefore built one model instance per + # row, each parsing net.intern_ip through netfields, rather than one + # per distinct net. The two inet_address_family columns this used to + # select were never read at all. q = f""" SELECT - net.server_id, - net.hostname, - net.intern_ip, - net.servertype_id, - net_attr.inet_address_family, - host_attr.inet_address_family, + net.server_id AS net_server_id, host.server_id AS host_server_id, host_attr.attribute_id AS host_attr_name FROM server AS host @@ -254,9 +257,8 @@ def _add_supernet_attribute(self, attribute: Attribute, servers_in): servers_by_id = {s.server_id: s for s in servers_in} - supernets = Server.objects.raw( - q, - { + with connection.cursor() as cursor: + cursor.execute(q, { "target_servertypes": list( attribute.target_servertype.values_list( 'servertype_id', flat=True @@ -264,25 +266,38 @@ def _add_supernet_attribute(self, attribute: Attribute, servers_in): ), "address_family": attribute.inet_address_family, "hosts": list(servers_by_id.keys()), - }, - translations={ - "net.server_id": "server_id", - "net.hostname": "hostname", - "net.intern_ip": "intern_ip", - "net.servertype_id": "servertype_id", - }, - ) - for cur_supernet in supernets: - cur_server = servers_by_id[cur_supernet.host_server_id] + }) + rows = cursor.fetchall() + + # The nets are fetched whole, intern_ip included: they are stored as + # attribute values and can reach a nested QueryMaterializer through + # _get_servers_to_join(), which reads it. + supernets = { + net.server_id: net + for net in Server.objects.filter( + server_id__in={r[0] for r in rows} + ) + } + + # Which host attribute produced the supernet currently stored for a + # host. This used to be read back off the supernet object itself, + # which is no longer possible now that one net object is shared by + # every host sitting in it. + via_attribute_ids = {} + for net_server_id, host_server_id, host_attr_name in rows: + cur_server = servers_by_id[host_server_id] + cur_supernet = supernets[net_server_id] prev_supernet = self._server_attributes.get(cur_server, {}).get(attribute) if prev_supernet and prev_supernet != cur_supernet: # TODO: Raise an exception once all data is cleaned up and conflicting # AF-unaware attributes are removed. logger.warning( f"Conflicting supernet {attribute} for {cur_server.hostname}: " - f"{prev_supernet.host_attr_name}->{prev_supernet} vs {cur_supernet.host_attr_name}->{cur_supernet}" + f"{via_attribute_ids.get(host_server_id)}->{prev_supernet} vs " + f"{host_attr_name}->{cur_supernet}" ) self._server_attributes[cur_server][attribute] = cur_supernet + via_attribute_ids[host_server_id] = host_attr_name def _add_related_attribute(self, attribute, servertype_attribute, servers_by_type): related_via_attribute = servertype_attribute.related_via_attribute From df664e294743228fecae771210ffbd24e3c23ee3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Kr=C3=B6ger?= Date: Mon, 14 Sep 2026 15:36:52 +0000 Subject: [PATCH 3/5] perf(serverdb): key materializer attribute map by server_id _server_attributes was keyed by Server instances. Model.__hash__() is a Python-level call that goes through _is_pk_set() and the pk property, and the map is looked up once per server per attribute - 1174077 times on a large query. Plain ints hash in C. The key change pays for itself twice over: with an int key the generic branch of _add_attributes() can take sa.server_id straight off the attribute row, so its prefetch_related() of Server disappears - and with it the huge IN ((1),(2),...) lists Django was compiling for those prefetches, which alone were about a fifth of the request. That branch covers string, number, boolean, inet, macaddr, date and datetime, so it is the bulk of the rows. The Server objects it used to fetch were only ever dict keys. The reverse branch now keys off sa.value_id instead of resolving the FK, and _add_supernet_attribute() uses the host_server_id its own SQL already returns. In a query with hundreds and thousands of results this way the function calls can be halved and runtime could be reduced by circa 40%. Three places still key by Server on purpose: servers_by_related in _add_related_attribute(), whose keys feed a hostname filter; _get_join_results(); and the reverse branch, which stores sa.server as an attribute value rather than using it as a key. All three are small next to the main map. --- .../serverdb/query_materializer.py | 46 +++++++++++-------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/packages/serveradmin/serveradmin/serverdb/query_materializer.py b/packages/serveradmin/serveradmin/serverdb/query_materializer.py index 2a3e5e51..5a669790 100644 --- a/packages/serveradmin/serveradmin/serverdb/query_materializer.py +++ b/packages/serveradmin/serveradmin/serverdb/query_materializer.py @@ -60,10 +60,14 @@ def __init__(self, servers, joined_attributes, order_by_attributes=[]): for servertype in Servertype.objects.all() } + # Keyed by server_id, not by the Server instance. Model.__hash__() + # is a Python-level call that goes through _is_pk_set() and the pk + # property, and this map is looked up once per server per attribute - + # over a million times on a large query. Plain ints hash in C. self._server_attributes = {} servers_by_type = {} for server in self._servers: - self._server_attributes[server] = { + self._server_attributes[server.server_id] = { Attribute.specials["object_id"]: server.server_id, Attribute.specials["hostname"]: server.hostname, Attribute.specials["intern_ip"]: server.intern_ip, @@ -138,7 +142,7 @@ def _initialize_attributes(self, servers_by_type): init = attribute.initializer() for servertype_id in servertype_ids: for server in servers_by_type[servertype_id]: - self._server_attributes[server][attribute] = init() + self._server_attributes[server.server_id][attribute] = init() def _add_attributes(self, servers_by_type): """Add the attributes to the results""" @@ -178,7 +182,7 @@ def _add_attributes(self, servers_by_type): .prefetch_related("server") ): self._add_attribute_value( - sa.value, + sa.value_id, reversed_attributes[sa.attribute_id], sa.server, ) @@ -187,13 +191,12 @@ def _add_attributes(self, servers_by_type): for sa in ( ServerAttribute.get_model(key) .objects.filter( - server__in=self._server_attributes.keys(), + server_id__in=self._server_attributes.keys(), attribute__in=attributes, ) - .prefetch_related(_server_prefetch()) ): self._add_attribute_value( - sa.server, + sa.server_id, attribute_lookup[sa.attribute_id], sa.get_value(), ) @@ -213,7 +216,7 @@ def _add_domain_attribute(self, attribute, servers): } for server in servers: - self._server_attributes[server][attribute] = domain_lookup.get( + self._server_attributes[server.server_id][attribute] = domain_lookup.get( server.hostname.split(".", 1)[-1] ) @@ -285,18 +288,20 @@ def _add_supernet_attribute(self, attribute: Attribute, servers_in): # every host sitting in it. via_attribute_ids = {} for net_server_id, host_server_id, host_attr_name in rows: - cur_server = servers_by_id[host_server_id] cur_supernet = supernets[net_server_id] - prev_supernet = self._server_attributes.get(cur_server, {}).get(attribute) + prev_supernet = ( + self._server_attributes.get(host_server_id, {}).get(attribute) + ) if prev_supernet and prev_supernet != cur_supernet: # TODO: Raise an exception once all data is cleaned up and conflicting # AF-unaware attributes are removed. logger.warning( - f"Conflicting supernet {attribute} for {cur_server.hostname}: " + f"Conflicting supernet {attribute} for " + f"{servers_by_id[host_server_id].hostname}: " f"{via_attribute_ids.get(host_server_id)}->{prev_supernet} vs " f"{host_attr_name}->{cur_supernet}" ) - self._server_attributes[cur_server][attribute] = cur_supernet + self._server_attributes[host_server_id][attribute] = cur_supernet via_attribute_ids[host_server_id] = host_attr_name def _add_related_attribute(self, attribute, servertype_attribute, servers_by_type): @@ -305,7 +310,7 @@ def _add_related_attribute(self, attribute, servertype_attribute, servers_by_typ # First, index the related servers for fast access later servers_by_related = {} for target in servers_by_type[servertype_attribute.servertype_id]: - attributes = self._server_attributes[target] + attributes = self._server_attributes[target.server_id] if related_via_attribute in attributes: if related_via_attribute.multi: for source in attributes[related_via_attribute]: @@ -324,19 +329,21 @@ def _add_related_attribute(self, attribute, servertype_attribute, servers_by_typ .prefetch_related(_server_prefetch()) ): for target in servers_by_related[sa.server]: - self._add_attribute_value(target, attribute, sa.get_value()) + self._add_attribute_value( + target.server_id, attribute, sa.get_value() + ) - def _add_attribute_value(self, server, attribute, value): + def _add_attribute_value(self, server_id, attribute, value): if attribute.multi: try: - self._server_attributes[server][attribute].add(value) + self._server_attributes[server_id][attribute].add(value) except KeyError: # If the attribute is removed from the servertype but # left on the servers, this error would occur. It is not # really expected, but we don't want to crash either. pass else: - self._server_attributes[server][attribute] = value + self._server_attributes[server_id][attribute] = value def _get_order_by_attribute(self, server, attribute): """Return a tuple to sort items by the key @@ -347,9 +354,10 @@ def _get_order_by_attribute(self, server, attribute): mind that some datatypes are not sortable with each other, some not even with None, so we have to so something in here. """ - if attribute not in self._server_attributes[server]: + server_attributes = self._server_attributes[server.server_id] + if attribute not in server_attributes: return 1, None - value = self._server_attributes[server][attribute] + value = server_attributes[attribute] if value is None: return -1, None if attribute.multi: @@ -358,7 +366,7 @@ def _get_order_by_attribute(self, server, attribute): def _get_attributes(self, server, join_results): # NOQA: C901 servertype = self._servertype_lookup[server.servertype_id] - server_attributes = self._server_attributes[server] + server_attributes = self._server_attributes[server.server_id] for attribute, value in server_attributes.items(): if attribute not in self._joined_attributes: continue From 8bf93d815fb836d3fec0934d2c96030378031135 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Kr=C3=B6ger?= Date: Fri, 18 Sep 2026 08:03:55 +0000 Subject: [PATCH 4/5] perf(serverdb): hash Attribute by attribute_id directly The query materializer keys its per-server attribute dicts by Attribute instances, so every lookup goes through Model.__hash__(). That is a Python-level method which first calls _is_pk_set() and then reads the pk property before hashing - three calls for what is, for this model, just hash(self.attribute_id), since attribute_id is the primary key. Those dicts are hit once per server per attribute: Hundreds of thousands of times on a queries with thousand objects. The override returns the identical value and skips the two extra calls. Equality stays Django's, so hash and eq remain consistent. The only behavioral difference is that an unsaved Attribute without an attribute_id becomes hashable, which Django forbids on purpose. Here attribute_id is a natural key assigned before save, so the case does not arise. I currently think this is safe to do but if it makes any problems feel free to revert this commit is isolated from the other changes. --- packages/serveradmin/serveradmin/serverdb/models.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/serveradmin/serveradmin/serverdb/models.py b/packages/serveradmin/serveradmin/serverdb/models.py index ff227ea6..7ff989fa 100644 --- a/packages/serveradmin/serveradmin/serverdb/models.py +++ b/packages/serveradmin/serveradmin/serverdb/models.py @@ -338,6 +338,18 @@ class Meta: def __str__(self): return self.attribute_id + def __hash__(self): + # Same value Model.__hash__() produces - the pk *is* attribute_id - + # without its _is_pk_set() call and pk property lookup. Attribute + # instances key the per-server attribute dicts in the query + # materializer, which are hit once per server per attribute: close + # to a million hashes on a large query, where the two extra Python + # calls were most of the cost. The one behavioural difference is + # that an unsaved Attribute without an attribute_id becomes hashable, + # which Django deliberately forbids; attribute_id is a natural key + # assigned before save, so that case does not arise here. + return hash(self.attribute_id) + def initializer(self): if self.multi: return set From b41cb5eb4c9dd11c84467af3b70c2bf82370562e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Kr=C3=B6ger?= Date: Fri, 18 Sep 2026 08:18:26 +0000 Subject: [PATCH 5/5] perf(serverdb): read stored attribute rows as tuples The generic branch of QueryMaterializer._add_attributes() built a model instance for every attribute row - over a hundred thousand on a large query - only to read server_id, attribute_id and value once each. An instance costs Model.__init__() plus the pre_init and post_init signals; that machinery was about 15% runtime on profiled requests. Read the rows with values_list() instead. The per-type conversion get_value() applied is now also available as a classmethod, convert_db_value(), so the knowledge stays on the models: number keeps its Decimal-to-int/float rule there and get_value() delegates to it, boolean returns True as before since its table has no value column, and relation raises NotImplementedError because its column is a server_id that cannot become a Server without a query - the materializer resolves those targets with one bulk query instead of a prefetch per chunk. The branch moves into its own method, _add_stored_attributes(), since the two type checks would otherwise push _add_attributes() past the project's cyclomatic complexity limit. It clears the relation manager's default prefetch of "value", which would run against the tuples and fail, and is redundant given the bulk fetch anyway. Inet values still pass through the netfields converter; values_list() applies field converters like the model path does. I was unsure whatever it is worth to make this change the LLM suggested but found the way with the classmethod acceptable in readability vs performance trade-off. --- .../serveradmin/serverdb/models.py | 40 +++++++++-- .../serverdb/query_materializer.py | 66 +++++++++++++++---- 2 files changed, 88 insertions(+), 18 deletions(-) diff --git a/packages/serveradmin/serveradmin/serverdb/models.py b/packages/serveradmin/serveradmin/serverdb/models.py index 7ff989fa..882e2731 100644 --- a/packages/serveradmin/serveradmin/serverdb/models.py +++ b/packages/serveradmin/serveradmin/serverdb/models.py @@ -585,6 +585,21 @@ def __str__(self): def get_value(self): return self.value + @classmethod + def convert_db_value(cls, value): + """Turn a raw "value" column into what get_value() would return + + This exists for performance. The query materializer reads attribute + rows with values_list() instead of as model instances: building an + instance costs Model.__init__() plus the init signals for every row, + and a large query reads over a hundred thousand rows only to look at + server_id, attribute_id and value once each. Tuples have no + get_value() to call, so the per-type conversion it applies is kept + here, on the model, where it belongs. Subclasses that override + get_value() must override this to match. + """ + return value + def save_value(self, value): # Normally, there shouldn't be any transformation necessary. self.value = value @@ -673,6 +688,16 @@ class Meta: unique_together = [["server", "attribute", "value"]] indexes = [models.Index(fields=["attribute", "value"])] + @classmethod + def convert_db_value(cls, value): + # The column holds a server_id. Turning it into the Server that + # get_value() returns needs a query, which per row would be exactly + # the cost this method exists to avoid. Callers reading relation + # rows as tuples resolve the targets in bulk instead. + raise NotImplementedError( + "relation targets must be resolved in bulk, not per value" + ) + def save_value(self, value): try: target_server = Server.objects.get(hostname=value) @@ -708,6 +733,11 @@ class Meta: def get_value(self): return True + @classmethod + def convert_db_value(cls, value): + # There is no value column; the row's existence is the value. + return True + def save_value(self, value): if value: self.save() @@ -731,11 +761,11 @@ class Meta: indexes = [models.Index(fields=["attribute", "value"])] def get_value(self): - return ( - int(self.value) - if self.value.as_tuple().exponent == 0 - else float(self.value) - ) + return self.convert_db_value(self.value) + + @classmethod + def convert_db_value(cls, value): + return int(value) if value.as_tuple().exponent == 0 else float(value) class ServerInetAttribute(ServerAttribute): diff --git a/packages/serveradmin/serveradmin/serverdb/query_materializer.py b/packages/serveradmin/serveradmin/serverdb/query_materializer.py index 5a669790..aec9d68c 100644 --- a/packages/serveradmin/serveradmin/serverdb/query_materializer.py +++ b/packages/serveradmin/serveradmin/serverdb/query_materializer.py @@ -187,19 +187,59 @@ def _add_attributes(self, servers_by_type): sa.server, ) else: - attribute_lookup = {a.attribute_id: a for a in attributes} - for sa in ( - ServerAttribute.get_model(key) - .objects.filter( - server_id__in=self._server_attributes.keys(), - attribute__in=attributes, - ) - ): - self._add_attribute_value( - sa.server_id, - attribute_lookup[sa.attribute_id], - sa.get_value(), - ) + self._add_stored_attributes(key, attributes) + + def _add_stored_attributes(self, key, attributes): + """Add the values of one attribute type kept in its own value table + + The rows are read as tuples, not model instances. An instance costs + Model.__init__() and the init signals for every row - over a hundred + thousand of them on a large query - only to read server_id, + attribute_id and value once each. The conversion get_value() would + have applied lives on the models as convert_db_value(), so the + tuples end up holding the same values an instance would have given. + """ + attribute_lookup = {a.attribute_id: a for a in attributes} + model = ServerAttribute.get_model(key) + # ServerRelationAttribute's manager prefetches "value" by default. + # That would run against the tuples and fail, and the targets are + # fetched in bulk below anyway. + rows = model.objects.filter( + server_id__in=self._server_attributes.keys(), + attribute__in=attributes, + ).prefetch_related(None) + + if key == "boolean": + # No value column: a row's existence is the value. + rows = ( + (server_id, attribute_id, None) + for server_id, attribute_id in rows.values_list( + "server_id", "attribute_id" + ) + ) + else: + rows = rows.values_list("server_id", "attribute_id", "value") + + if key == "relation": + # The value column holds a server_id. Resolve the targets with + # one query rather than one per row. They are stored as + # attribute values, so they are fetched whole: + # _get_servers_to_join() may read their intern_ip. + rows = list(rows) + targets = { + server.server_id: server + for server in Server.objects.filter( + server_id__in={value for _, _, value in rows} + ) + } + convert = targets.__getitem__ + else: + convert = model.convert_db_value + + for server_id, attribute_id, value in rows: + self._add_attribute_value( + server_id, attribute_lookup[attribute_id], convert(value) + ) def _add_related_attributes(self, servers_by_type): for attribute, sa in self._related_servertype_attributes: