Skip to content

Conversation

@Tishka17
Copy link
Contributor

@Tishka17 Tishka17 commented Mar 6, 2025

Fixes: #18409

  • Do not do additional requests for CabledObjectModel.link_peers
  • Custom field for arrays containing generic foreign keys
  • Use custom field instead of CablePath._get_path method
  • Enable prefetching for interface _path and cable

@Tishka17
Copy link
Contributor Author

Tishka17 commented Mar 7, 2025

I see that I've missed termination.link is not None and other path processing logic, I'm going to fix it

@Tishka17
Copy link
Contributor Author

Tishka17 commented Mar 7, 2025

I remove loaded data cache invalidation, so the current logic of updating path continues working

@bctiemann bctiemann changed the base branch from develop to main March 11, 2025 21:59
@bctiemann
Copy link
Contributor

@Tishka17 I've been testing this and the performance gains do look good. I'm seeing similar query and timing numbers to yours in fetching interfaces/peers/path_objects etc, and the data looks to be the same as before. A couple of questions though, mostly just around organization:

  1. Now that NetBox 4.2 is on Django 5.1, does that change your earlier approach to GenericPrefetch as that originally had to be backported into Django 4.2 as of your comments 3 weeks ago? Can the change be simplified to use anything that is now part of Django itself?
  2. I see that you put GenericArrayForeignKey into utilities/generics/field.py; would utilities/fields.py not be an appropriate place for it? It already has the conceptually similar RestrictedGenericForeignKey class.

Thanks for your patience!

@Tishka17
Copy link
Contributor Author

  1. In current version I am using GenericPrefetch from Django 5.x. Backport is needed only for older versions of netbox, but I guess we are not going to updated them.

  2. Ok, I will move files.

@bctiemann bctiemann merged commit b1e7d7c into netbox-community:main Mar 12, 2025
3 checks passed
@Tishka17 Tishka17 deleted the fix/generic_prefetch_4.2 branch March 12, 2025 22:58
jnovinger added a commit that referenced this pull request Mar 25, 2025
`GenericArrayForeignKey`, which was added in #18826, advertised itself
as a many_to_many relation, which is somewhat inaccurate and triggered
DRF's introspection of many_to_many relations which looks for a
`remote_field.model` property chain on the relation. Since that doesn't
exist, DRF threw an AttributeError resulting in a 500 error.

This changes `GenericArrayForeignKey` to advertise itself to DRF as a
one_to_many relation, which is more accurate, and does not trigger the
property access in DRF that was causing the problem.

This also adds a regression test for both FrontPort and RearPort "path"
API view.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 12, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Poor performance in cable trace API calls: N+1 problem

2 participants