-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve performance when rendering the prefix hierarchy #6087
Comments
What would be the performance impacts, good or bad, (in terms of response-time to load prefix pages) of doing this? |
@tyler-8 there would be some degree of performance impact at write time, but it should greatly reduce the amount of time required to render the prefixes list. This proposal is still just a rough idea: further testing is needed to determine its viability. |
What implications are there for the REST API, if any? I presume new fields as stated in the OP, but if users have code around the existing fields, would their work be impacted? |
Status update: I ended up forgoing MPTT or any similar solution, and opted to simply cache the depth and child count of each prefix. It's not a super-elegant solution, but it works well and seems pretty robust. Load times for the full prefix hierarchy have been dramatically reduced. Plus, we can filter by depth and child count now! I still want to do some more testing, especially around the migration, but I expect this to debut in v2.11.5. |
I did a quick benchmark of the #6087 branch against
I made five requests for
Clearly, some pretty huge gains in performance. It's worth reiterating that I've hidden the utilization column from the prefixes tables for these tests, since that imposes additional overhead. (All other default columns were retained.) Still, with the utilization column included (though no child IP addresses defined), load times averaged around five seconds with #6087 in place. Migrating the roughly one million prefixes took about eight minutes on my development machine. A more realistic deployment with perhaps around 100 thousand prefixes should take less than a minute.
Obviously this is far from a scientific test, though I believe it illustrates the enormity of the performance improvements gained with this work. |
Nice work @jeremystretch ! Will the children be available in the API response as well? Or the other way around: will the parent prefix reference be exposed in the API? https://demo.netbox.dev/api/ipam/prefixes/?parent=123 ? https://demo.netbox.dev/api/ipam/prefixes/456/?include_children=true ? |
@sdktr this doesn't enforce any explicit relationship between parent and child prefixes. This would actually be infeasible given that we (sometimes) allow duplicate prefixes to coexist. We could extend the REST API serializers to annotate the depth and child count for each prefix with negligible performance impact, but that would be the extent of the changes (beyond the new filtering ability introduced). |
…dren Closes #6087: Cache prefix depth & children count
NetBox version
v2.10.8
Feature type
Change to existing functionality
Proposed functionality
Employ MPTT for constructing the prefix hierarchy. This entails automatically assigning each prefix to its parent (if any) and recalculating the hierarchy. NetBox currently employs MPTT for several models (e.g. nesting regions) but adopting it for prefixes will require the automatic assignment and re-assignment of parents as prefixes are added and removed.
Use case
MPTT allows us to more efficiently model the prefix hierarchy, and obviates the need to make verbose SQL annotations to count parent and child prefixes. However, it further consideration is needed regarding the automatic assignment of parent prefixes.
Database changes
Extend ipam.Prefix to an MPTT-enabled model by adding the
parent_id
,tree_id
,level
,lft
, andrght
fields.External dependencies
No response
The text was updated successfully, but these errors were encountered: