perf(backend): hand a template component the peer it carries - #10426
Conversation
Materializing a component of an object template named the peers it copies from the subtemplate by their id, although the level read had just brought those peers back as nodes. The kind check on the new object read the node again, and writing the object read it a second time for its branch: two node reads and an attribute read per component, for a node already in memory. The component is now handed the node itself, as it already was for the parent it hangs from. A device template with 65 interfaces naming the same transceiver model creates in 140 queries instead of 332, and what it reads no longer grows with the number of components. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
the
group/backend
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 4 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Performance optimization handing the in-hand peer node to template components instead of re-reading it by id, with fallback preserved and a test pinning the reduced query counts and successful peer write.
Re-trigger cubic
| @@ -0,0 +1 @@ | |||
| Creating objects from an object template no longer reads back the objects its components point at, such as the transceiver model every interface of a device template names, once for each component created. | |||
There was a problem hiding this comment.
This entry describes how it works rather than what a user gets. It is also not accurate as written. For cardinality-many relationships the peers are still read, because core/query/node.py around lines 216-222 calls get_peers(), which always goes to the database. I'd rewrite to something like: "Creating objects from an object template is now faster when the objects it creates share a related object, such as the transceiver model used by every interface of a device template." but it might be redundant with other fixes so far.
There was a problem hiding this comment.
Reworded to what a user gets, in 8c7a3db. You're right on cardinality-many: NodeCreateAllQuery.query_init still batches those peers through get_peers() → get_many() once per component, so only the peer-kind read is skipped there; the measured case and the test are cardinality-one. Kept it as its own fragment rather than folding into +faster-creates-from-object-templates, which lives on #10423's branch.
There was a problem hiding this comment.
Follow-up: 1161a68 closes the cardinality-many gap too — the create query now reads only the peers held by id (RelationshipManager.read_peers_not_in_hand()), 270 → 140 queries on the same 65-interface shape. Measurement is in the PR description.
| # The subtemplate was read with the peers its relationships name, so hand over the node | ||
| # where it is in hand: an id sends the checks on the new object, and the write itself, | ||
| # back to the database for a node already in memory. |
There was a problem hiding this comment.
I think "The peer is already read, so an id would send the checks and the write back to the database" would be enough as a comment here
| """A peer a subtemplate names is handed to the object created from it, not read again per object. | ||
|
|
||
| The subtemplates arrive with the peers their relationships name, so a component that carries | ||
| one costs what a component without one costs: its uniqueness check and its write. Naming the | ||
| peer by its id instead sent both the kind check on the new object and the write back to the | ||
| database for a node already in memory, once per component. | ||
| """ |
There was a problem hiding this comment.
The last two sentences describe the old behavior. Keeping the first line is enough.
| @@ -204,6 +204,13 @@ def get_concrete_peer_kind(self) -> str | None: | |||
|
|
|||
| return self._resolved_peer_kind | |||
There was a problem hiding this comment.
get_concrete_peer_kind repeats the check that now has a name, and the two are slightly different: this one tests if self._peer and ... while get_peer_in_hand tests is None. Same result only because Node is always truthy.
There was a problem hiding this comment.
Done in 8c7a3db — get_concrete_peer_kind and get_peer_kind (the same check five lines up) both go through get_peer_in_hand() now.
| assert set(node_reads.values()) == {4}, f"reading the subtemplates grew with the width of a level: {node_reads}" | ||
|
|
||
|
|
||
| async def test_a_peer_a_component_carries_is_not_read_back_for_every_component( |
There was a problem hiding this comment.
That is a good test 👍 . I put it on the base commit and ran it against the unfixed code. It seems to prove what it tries to.
73829b1 to
b36fd84
Compare
301e81a to
e3446ea
Compare
There was a problem hiding this comment.
No issues found across 4 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Performance fix: template components now receive the in-hand peer node instead of a bare id, eliminating redundant DB reads, with a visible id fallback and a test pinning reduced query counts and successful peer write.
Re-trigger cubic
- get_peer_kind and get_concrete_peer_kind reuse the check that now has a name instead of each spelling their own variant of it - shorten the hand-over comment in extract_peer_data and the docstring of the test that pins it - reword the changelog to what a user gets: cardinality-many peers are still batched through get_peers() once per component, so the previous wording overstated the change Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 4 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Internal performance fix: template components now receive the already-loaded peer node instead of re-reading it by id, with a fallback to the id when the peer isn't in hand, pinned by a test that verifies reduced query counts and that the peer is written. No schema, API, or policy change.
Re-trigger cubic
NodeCreateAllQuery batched every peer of a relationship of cardinality many through get_peers(), which reads whatever it is given: a component created from a template read back, once per component, the peer the template read had already brought back. The create query now asks the manager for the peers held only by id (read_peers_not_in_hand), so a peer handed over as a node is not read again. A device template with 65 interfaces each naming the same tag through a many-cardinality relationship: 270 -> 140 queries, node_list_get_info and node_list_get_attribute 68 -> 3 each. A component costs its uniqueness check and its write, as it already did for a peer of cardinality one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ae6f1d5 to
1161a68
Compare
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
read_peers_not_in_hand gates its batch on is_valid_uuid, the rule get_peer_id already applies: a peer named by a default filter value is left to Relationship.resolve(), which reads it the only way it can be read, instead of riding along in a get_many that cannot match it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Internal performance fix: template materialization hands already-loaded peer nodes to created components instead of re-reading them by id, with id fallback, extended to many-cardinality relationships via read_peers_not_in_hand(), pinned by tests that verify reduced query counts and that peers...
Re-trigger cubic
2 participants
Stacked on #10423 (
fac/template-level-reads-10419), which is the branch this work was measured against.What changed
Materializing a component of an object template named the peers it copies from the subtemplate by their id (
{"id": relationship.peer_id}), although the level read had just brought those peers back as nodes. The component is now handed the node itself — as it already was for the parent it hangs from.Relationship.get_peer_in_hand()returns the peer as the node it is, orNonewhen the relationship holds only its id, so the caller can tell the two apart without triggering a read.extract_peer_data()hands that node over. A peer the template read did not bring back still falls through to its id, unchanged.Why
A trace of an upsert creating a device with 66 components from a customer template showed 135
node_list_get_infoqueries. 128 of them were two reads of the same node, repeated once per component:RelationshipPeerKindConstraintread the peer's kind, because the copied id carried none.NodeCreateAllQuery.query_init→Relationship.get_create_data()→get_peer()read the whole node, for nothing butpeer.idandpeer.get_branch().In that trace the peer was one shared SFP node (
sfp_modelonNetworkPhysicalInterface), read 64 times over. The info queries were cheap (0.33 s), but the attribute read that follows each one averaged 106 ms — 6.8 s of a 15.1 s mutation.Measured
A device template with 65 interfaces, each naming the same 5-attribute peer:
node_list_get_infonode_list_get_attributeuniqueness_constraint_validationnode_create_allnode_list_get_relationshipA component carrying a peer now costs the 2 queries a component without one costs: its uniqueness check and its write. Node reads stay flat at 3, measured identical at 5, 20 and 65 interfaces.
Projected onto the traced mutation with its own per-query timings: 345 → 153 queries, ~7.2 s saved of 15.1 s. The per-component HFID uniqueness check (3.6 s, growing with the sibling count) becomes the next term.
Second commit: the same for peers of cardinality many
NodeCreateAllQuery.query_initbatched every peer of a many-cardinality relationship throughRelationshipManager.get_peers(), which reads whatever it is given — so a component naming its peers through such a relationship still read them back, once per component, even when they were handed over as nodes. The create query now asks for the peers held only by id (RelationshipManager.read_peers_not_in_hand()), in the same single call (1161a68).Same shape as above, the interface naming the tag through a relationship of cardinality many:
node_list_get_infonode_list_get_attributeuniqueness_constraint_validationnode_create_allnode_list_get_relationshipNode reads flat at 3, measured identical at 5, 20 and 65 interfaces.
test_a_peer_a_component_carries_is_not_read_back_for_every_componentnow runs once per cardinality; themanycase failed on the previous commit with node reads of 3 + N. Green: the suites below pluscomponent/graphql/resource_manager,test_mutation_relationship,mutations/test_profile_resource_pool_validationandqueries/test_resource_pool— 798 tests.Testing
test_a_peer_a_component_carries_is_not_read_back_for_every_componentpins 3 node reads / 3 attribute reads and 2 queries per component, and reloads the created objects from the database to assert the peer was actually written.component/templates,component/core/constraint_validators,component/core/profiles,component/core/node,component/graphql/test_mutation_{create,upsert},component/core/test_{node,relationship,relationship_manager,relationship_metadata}— 728 tests.mypyandruffclean on the touched files.🤖 Generated with Claude Code