Skip to content
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

Slow API calls: N+1 problem #18409

Open
Tishka17 opened this issue Jan 16, 2025 · 8 comments
Open

Slow API calls: N+1 problem #18409

Tishka17 opened this issue Jan 16, 2025 · 8 comments
Labels
severity: low Does not significantly disrupt application functionality, or a workaround is available status: revisions needed This issue requires additional information to be actionable type: bug A confirmed report of unexpected behavior in the application

Comments

@Tishka17
Copy link

Tishka17 commented Jan 16, 2025

Deployment Type

Self-hosted

Triage priority

N/A

NetBox Version

v3.7, v4.2

Python Version

3.11

Steps to Reproduce

  1. Fill enough data for devices with cables and connections.
  2. Do API requests to load device, interfaces, ip-addresses like
    http://localhost:8000/api/dcim/interfaces/?device_id=3971&limit=100&offset=0

Expected Behavior

Data is loaded quite fast, <0.5 seconds. Number of db queries pretty small up to 10-20 per HTTP-request

Observed Behavior

Some requests are pretty slow. Up to 3 seconds.
130 sql queries are produced on 3.7 and 160 db queries on 4.2

CabledObjectModel.link_peers has N+1 problem: https://github.com/netbox-community/netbox/blob/v3.7.8/netbox/dcim/models/device_components.py#L192
CablePath._get_path has N+1 problem as well: https://github.com/netbox-community/netbox/blob/v3.7.8/netbox/dcim/models/cables.py#L746

I tried to comment out these methods and got only 20 db queries on the same http request

@Tishka17 Tishka17 added status: needs triage This issue is awaiting triage by a maintainer type: bug A confirmed report of unexpected behavior in the application labels Jan 16, 2025
@Tishka17 Tishka17 changed the title N+1 problem in API calls Slow API calls: N+1 problem Jan 16, 2025
@bctiemann bctiemann added status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation severity: low Does not significantly disrupt application functionality, or a workaround is available and removed status: needs triage This issue is awaiting triage by a maintainer labels Jan 16, 2025
@bctiemann
Copy link
Contributor

It would be ideal if each of these identified points of improvement could be opened as a separate bug, so they can be individually fixed (i.e. if one is easier than another).

@jeremystretch
Copy link
Member

Data is loaded quite fast, <0.5 seconds. Number of db queries pretty small up to 10-20 per HTTP-request

I think we need something more explicit here. Asserting that the application should perform faster is not sufficient justification for a bug report. @Tishka17 please extend your post above to cite the specific optimizations you believe are missing.

@jeremystretch jeremystretch added status: revisions needed This issue requires additional information to be actionable and removed status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation labels Jan 17, 2025
@Tishka17
Copy link
Author

@jeremystretch sorry, I thought the "N+1 problem" words with links to corresponding functions are enough. What else can I add? For me it is a performance issue, but according to a code - it's just a technical bug with improper related data loading

@jeremystretch
Copy link
Member

What else can I add?

Please detail the specific changes you are proposing.

@Tishka17
Copy link
Author

I am proposing to fix code in a way, that it has no N+1 problem or lazy loading. I am not Django developer, so I am not familiar how it's technics like prefetch_related can be used in this specific case

@n-rodriguez
Copy link

@Tishka17
Copy link
Author

Tishka17 commented Jan 20, 2025

The problem is that some fields are calculated are not just generated based on model data, but explicitly do requests to a database.

I've tried to fix link_peers replacing .exclude query with filtering already loaded self.cable.terminations but it references then GenericForeignKey and we need its children in response..., so I did not succceded.

- peers = self.cable.terminations.exclude(cable_end=self.cable_end).prefetch_related('termination')
- return [peer.termination for peer in peers]
+ return [peer.termination for peer in self.cable.terminations if peer.cable_end!=self.cable_end]

@jsenecal
Copy link
Contributor

To answer to that specific example (prefetch w/ generic foreignkeys) there was an upstream issue to try to mitigate this (
https://code.djangoproject.com/ticket/33651) which has been implemented in Django 4.0.

Not sure if the netbox codebase uses that everywhere, but it is implemented in https://github.com/netbox-community/netbox/blob/main/netbox/utilities/fields.py#L74

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
severity: low Does not significantly disrupt application functionality, or a workaround is available status: revisions needed This issue requires additional information to be actionable type: bug A confirmed report of unexpected behavior in the application
Projects
None yet
Development

No branches or pull requests

5 participants