Agency: Preload agency ancestors in the API to fix N+1 query

[Tschuppi81]

Agency: Preload agency ancestors in the API to fix N+1 query

Building the html link of each agency renders its full path, which walks up the parent chain. Only the direct parent was eager loaded, so every ancestor above it was loaded with its own query. Preload the ancestor closure in bulk instead.

TYPE: Performance
LINK: https://seantis-gmbh.sentry.io/issues/7426519446

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.50%. Comparing base (4711862) to head (ab4bf9d).
✅ All tests successful. No failed tests found.

Additional details and impacted files

Files with missing lines	Coverage Δ
src/onegov/agency/api.py	97.69% <100.00%> (+0.01%)	⬆️
src/onegov/api/__init__.py	100.00% <100.00%> (ø)
src/onegov/api/models.py	92.17% <100.00%> (+0.28%)	⬆️
src/onegov/core/orm/abstract/adjacency_list.py	97.45% <100.00%> (+0.17%)	⬆️
src/onegov/org/api.py	86.84% <100.00%> (+0.02%)	⬆️

... and 1 file with indirect coverage changes

---

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4711862...ab4bf9d. Read the comment docs.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Daverball]

We probably have the same issue for the /api/topics and /api/news endpoints. This is a problem with all models deriving from AdjacencyList. Perhaps it would be better to cache path on the model. We would just need to be careful, that the cache gets updated for all the descendant nodes when moving parent nodes around. But a simple observer on parent and name should get us there, just like we have an observer for maintaining alphabetic sorting of the siblings.

[Tschuppi81]

Correct, /api/topics was affected also but not /api/news as all news have the root element as parent. So no need to preload.

I didn't follow the observer path(s) as it seems quite complex to me (I could open a ticket for it to follow-up on this). Especially in case of moving an element or rename one the whole subtree's paths would need to be walked.

[Daverball]

I didn't follow the observer path(s) as it seems quite complex to me (I could open a ticket for it to follow-up on this). Especially in case of moving an element or rename one the whole subtree's paths would need to be walked.

You only need to touch the cache of your descendents when either your name or your parent changes, your observer is responsible for propagating the change to your descendants, it should be pretty straightforward, especially if we don't care about it being slow, since we shouldn't be changing multiple parents/names in one transaction.

The most straightforward, but slightly inefficient way would be putting one observer on name that updates your own path, by replacing the last path segment and a second observer on parent and parent.path, which should propagate the changes all the way through the descendants in this branch, which updates path by taking parent.path and appending your name to it. Which will trigger the observer on your descendants. Although I can't remember to what degree changes that were made by observers during the flush will be picked up and cause another cycle of observer triggers. But if it doesn't automatically propagate you can just manually do the propagation by calling the parent observer of your children in a loop.

Manually prefetching the descendants for some collections is a lot more complex and it only helps, if you remember to do it. Ideally in a collection context we should never be looking at the rest of the tree from individual nodes. Whenever we do, it makes more sense to just prefetch and cache the entire tree in a single query, like we already do for pages for the sidebar navigation.

[Tschuppi81]

So you prefer the observer path over the current preload_ancestors?

[Daverball]

Yes, since it will help us everywhere we're linking to these models, including places like search results and breadcrumbs.